diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs new file mode 100644 index 0000000000..07eba0bed5 --- /dev/null +++ b/compiler/can/src/abilities.rs @@ -0,0 +1,64 @@ +use roc_collections::all::{MutMap, MutSet}; +use roc_module::symbol::Symbol; +use roc_types::types::Type; + +use crate::annotation::HasClause; + +/// Stores information about an ability member definition, including the parent ability, the +/// defining type, and what type variables need to be instantiated with instances of the ability. +#[derive(Debug)] +struct AbilityMemberData { + #[allow(unused)] + parent_ability: Symbol, + #[allow(unused)] + signature: Type, + #[allow(unused)] + bound_has_clauses: Vec, +} + +/// Stores information about what abilities exist in a scope, what it means to implement an +/// ability, and what types implement them. +// TODO(abilities): this should probably go on the Scope, I don't put it there for now because we +// are only dealing with inter-module abilities for now. +#[derive(Default, Debug)] +pub struct AbilitiesStore { + /// Maps an ability to the members defining it. + #[allow(unused)] + members_of_ability: MutMap>, + + /// Information about all members composing abilities. + #[allow(unused)] + ability_members: MutMap, + + /// Tuples of (type, member) specifying that `type` declares an implementation of an ability + /// member `member`. + #[allow(unused)] + declared_implementations: MutSet<(Symbol, Symbol)>, +} + +impl AbilitiesStore { + pub fn register_ability( + &mut self, + ability: Symbol, + members: Vec<(Symbol, Type, Vec)>, + ) { + let mut members_vec = Vec::with_capacity(members.len()); + for (member, signature, bound_has_clauses) in members.into_iter() { + members_vec.push(member); + self.ability_members.insert( + member, + AbilityMemberData { + parent_ability: ability, + signature, + bound_has_clauses, + }, + ); + } + self.members_of_ability.insert(ability, members_vec); + } + + pub fn register_implementation(&mut self, implementing_type: Symbol, ability_member: Symbol) { + self.declared_implementations + .insert((implementing_type, ability_member)); + } +} diff --git a/compiler/can/src/annotation.rs b/compiler/can/src/annotation.rs index b44641f57a..7cb0fce345 100644 --- a/compiler/can/src/annotation.rs +++ b/compiler/can/src/annotation.rs @@ -1,10 +1,9 @@ use crate::env::Env; use crate::scope::Scope; use roc_collections::all::{ImMap, MutMap, MutSet, SendMap}; -use roc_error_macros::todo_abilities; use roc_module::ident::{Ident, Lowercase, TagName}; use roc_module::symbol::{IdentIds, ModuleId, Symbol}; -use roc_parse::ast::{AssignedField, Pattern, Tag, TypeAnnotation, TypeHeader}; +use roc_parse::ast::{AssignedField, ExtractSpaces, Pattern, Tag, TypeAnnotation, TypeHeader}; use roc_region::all::{Loc, Region}; use roc_types::subs::{VarStore, Variable}; use roc_types::types::{ @@ -104,6 +103,10 @@ impl IntroducedVariables { .find(|nv| nv.variable == var) .map(|nv| &nv.name) } + + pub fn named_var_by_name(&self, name: &Lowercase) -> Option<&NamedVariable> { + self.named.iter().find(|nv| &nv.name == name) + } } fn malformed(env: &mut Env, region: Region, name: &str) { @@ -143,6 +146,78 @@ pub fn canonicalize_annotation( } } +#[derive(Clone, Debug)] +pub struct HasClause { + pub var_name: Lowercase, + pub var: Variable, + pub ability: Symbol, +} + +pub fn canonicalize_annotation_with_possible_clauses( + env: &mut Env, + scope: &mut Scope, + annotation: &TypeAnnotation, + region: Region, + var_store: &mut VarStore, + abilities_in_scope: &[Symbol], +) -> (Annotation, Vec>) { + let mut introduced_variables = IntroducedVariables::default(); + let mut references = MutSet::default(); + let mut aliases = SendMap::default(); + + let (annotation, region, clauses) = match annotation { + TypeAnnotation::Where(annotation, clauses) => { + let mut can_clauses = Vec::with_capacity(clauses.len()); + for clause in clauses.iter() { + match canonicalize_has_clause( + env, + scope, + var_store, + &mut introduced_variables, + clause, + abilities_in_scope, + &mut references, + ) { + Ok(result) => can_clauses.push(Loc::at(clause.region, result)), + Err(err_type) => { + return ( + Annotation { + typ: err_type, + introduced_variables, + references, + aliases, + }, + can_clauses, + ) + } + }; + } + (&annotation.value, annotation.region, can_clauses) + } + annot => (annot, region, vec![]), + }; + + let typ = can_annotation_help( + env, + annotation, + region, + scope, + var_store, + &mut introduced_variables, + &mut aliases, + &mut references, + ); + + let annot = Annotation { + typ, + introduced_variables, + references, + aliases, + }; + + (annot, clauses) +} + fn make_apply_symbol( env: &mut Env, region: Region, @@ -271,7 +346,13 @@ pub fn find_type_def_symbols( SpaceBefore(inner, _) | SpaceAfter(inner, _) => { stack.push(inner); } - Where(..) => todo_abilities!(), + Where(annotation, clauses) => { + stack.push(&annotation.value); + + for has_clause in clauses.iter() { + stack.push(&has_clause.value.ability.value); + } + } Inferred | Wildcard | Malformed(_) => {} } } @@ -685,7 +766,17 @@ fn can_annotation_help( Type::Variable(var) } - Where(..) => todo_abilities!(), + Where(_annotation, clauses) => { + debug_assert!(!clauses.is_empty()); + + // Has clauses are allowed only on the top level of an ability member signature (for + // now), which we handle elsewhere. + env.problem(roc_problem::can::Problem::IllegalHasClause { + region: Region::across_all(clauses.iter().map(|clause| &clause.region)), + }); + + return Type::Erroneous(Problem::CanonicalizationProblem); + } Malformed(string) => { malformed(env, region, string); @@ -698,6 +789,71 @@ fn can_annotation_help( } } +fn canonicalize_has_clause( + env: &mut Env, + scope: &mut Scope, + var_store: &mut VarStore, + introduced_variables: &mut IntroducedVariables, + clause: &Loc>, + abilities_in_scope: &[Symbol], + references: &mut MutSet, +) -> Result { + let Loc { + region, + value: roc_parse::ast::HasClause { var, ability }, + } = clause; + let region = *region; + + let var_name = var.extract_spaces().item; + debug_assert!( + var_name.starts_with(char::is_lowercase), + "Vars should have been parsed as lowercase" + ); + let var_name = Lowercase::from(var_name); + + let ability = match ability.value { + TypeAnnotation::Apply(module_name, ident, _type_arguments) => { + let symbol = make_apply_symbol(env, ability.region, scope, module_name, ident)?; + if !abilities_in_scope.contains(&symbol) { + let region = ability.region; + env.problem(roc_problem::can::Problem::HasClauseIsNotAbility { region }); + return Err(Type::Erroneous(Problem::HasClauseIsNotAbility(region))); + } + symbol + } + _ => { + let region = ability.region; + env.problem(roc_problem::can::Problem::HasClauseIsNotAbility { region }); + return Err(Type::Erroneous(Problem::HasClauseIsNotAbility(region))); + } + }; + + references.insert(ability); + + if let Some(shadowing) = introduced_variables.named_var_by_name(&var_name) { + let var_name_ident = var_name.to_string().into(); + let shadow = Loc::at(region, var_name_ident); + env.problem(roc_problem::can::Problem::ShadowingInAnnotation { + original_region: shadowing.first_seen, + shadow: shadow.clone(), + }); + return Err(Type::Erroneous(Problem::Shadowed( + shadowing.first_seen, + shadow, + ))); + } + + let var = var_store.fresh(); + + introduced_variables.insert_named(var_name.clone(), Loc::at(region, var)); + + Ok(HasClause { + var_name, + var, + ability, + }) +} + #[allow(clippy::too_many_arguments)] fn can_extension_type<'a>( env: &mut Env, diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 9dc3de33d8..a319302906 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1,4 +1,6 @@ +use crate::abilities::AbilitiesStore; use crate::annotation::canonicalize_annotation; +use crate::annotation::canonicalize_annotation_with_possible_clauses; use crate::annotation::IntroducedVariables; use crate::env::Env; use crate::expr::ClosureData; @@ -10,10 +12,11 @@ use crate::scope::create_alias; use crate::scope::Scope; use roc_collections::all::ImSet; use roc_collections::all::{default_hasher, ImEntry, ImMap, MutMap, MutSet, SendMap}; -use roc_error_macros::todo_abilities; use roc_module::ident::Lowercase; use roc_module::symbol::Symbol; use roc_parse::ast; +use roc_parse::ast::AbilityMember; +use roc_parse::ast::ExtractSpaces; use roc_parse::ast::TypeHeader; use roc_parse::pattern::PatternType; use roc_problem::can::{CycleEntry, Problem, RuntimeError}; @@ -86,10 +89,19 @@ enum PendingTypeDef<'a> { kind: AliasKind, }, + Ability { + name: Loc, + members: &'a [ast::AbilityMember<'a>], + }, + /// An invalid alias, that is ignored in the rest of the pipeline /// e.g. a shadowed alias, or a definition like `MyAlias 1 : Int` /// with an incorrect pattern InvalidAlias { kind: AliasKind }, + + /// An invalid ability, that is ignored in the rest of the pipeline. + /// E.g. a shadowed ability, or with a bad definition. + InvalidAbility, } // See github.com/rtfeldman/roc/issues/800 for discussion of the large_enum_variant check. @@ -239,9 +251,19 @@ pub fn canonicalize_defs<'a>( env.home.register_debug_idents(&env.ident_ids); } - let mut aliases = SendMap::default(); + enum TypeDef<'a> { + AliasLike( + Loc, + Vec>, + &'a Loc>, + AliasKind, + ), + Ability(Loc, &'a [AbilityMember<'a>]), + } + + let mut type_defs = MutMap::default(); + let mut abilities_in_scope = Vec::new(); - let mut alias_defs = MutMap::default(); let mut referenced_type_symbols = MutMap::default(); for pending_def in pending_type_defs.into_iter() { @@ -260,93 +282,137 @@ pub fn canonicalize_defs<'a>( referenced_type_symbols.insert(name.value, referenced_symbols); - alias_defs.insert(name.value, (name, vars, ann, kind)); + type_defs.insert(name.value, TypeDef::AliasLike(name, vars, ann, kind)); + } + PendingTypeDef::Ability { name, members } => { + let mut referenced_symbols = Vec::with_capacity(2); + + for member in members.iter() { + // Add the referenced type symbols of each member function. We need to make + // sure those are processed first before we resolve the whole ability + // definition. + referenced_symbols.extend(crate::annotation::find_type_def_symbols( + env.home, + &mut env.ident_ids, + &member.typ.value, + )); + } + + referenced_type_symbols.insert(name.value, referenced_symbols); + type_defs.insert(name.value, TypeDef::Ability(name, members)); + abilities_in_scope.push(name.value); + } + PendingTypeDef::InvalidAlias { .. } | PendingTypeDef::InvalidAbility { .. } => { /* ignore */ } - PendingTypeDef::InvalidAlias { .. } => { /* ignore */ } } } let sorted = sort_type_defs_before_introduction(referenced_type_symbols); + let mut aliases = SendMap::default(); + let mut abilities = MutMap::default(); for type_name in sorted { - let (name, vars, ann, kind) = alias_defs.remove(&type_name).unwrap(); + match type_defs.remove(&type_name).unwrap() { + TypeDef::AliasLike(name, vars, ann, kind) => { + let symbol = name.value; + let can_ann = + canonicalize_annotation(env, &mut scope, &ann.value, ann.region, var_store); - let symbol = name.value; - let can_ann = canonicalize_annotation(env, &mut scope, &ann.value, ann.region, var_store); + // Does this alias reference any abilities? For now, we don't permit that. + let ability_references = can_ann + .references + .iter() + .filter_map(|&ty_ref| abilities_in_scope.iter().find(|&&name| name == ty_ref)) + .collect::>(); - // Record all the annotation's references in output.references.lookups - for symbol in can_ann.references { - output.references.type_lookups.insert(symbol); - output.references.referenced_type_defs.insert(symbol); - } - - let mut can_vars: Vec> = Vec::with_capacity(vars.len()); - let mut is_phantom = false; - - let mut named = can_ann.introduced_variables.named; - for loc_lowercase in vars.iter() { - match named.iter().position(|nv| nv.name == loc_lowercase.value) { - Some(index) => { - // This is a valid lowercase rigid var for the type def. - let named_variable = named.swap_remove(index); - - can_vars.push(Loc { - value: (named_variable.name, named_variable.variable), - region: loc_lowercase.region, + if let Some(one_ability_ref) = ability_references.first() { + env.problem(Problem::AliasUsesAbility { + loc_name: name, + ability: **one_ability_ref, }); } - None => { - is_phantom = true; - env.problems.push(Problem::PhantomTypeArgument { + // Record all the annotation's references in output.references.lookups + for symbol in can_ann.references { + output.references.type_lookups.insert(symbol); + output.references.referenced_type_defs.insert(symbol); + } + + let mut can_vars: Vec> = Vec::with_capacity(vars.len()); + let mut is_phantom = false; + + let mut named = can_ann.introduced_variables.named; + for loc_lowercase in vars.iter() { + match named.iter().position(|nv| nv.name == loc_lowercase.value) { + Some(index) => { + // This is a valid lowercase rigid var for the type def. + let named_variable = named.swap_remove(index); + + can_vars.push(Loc { + value: (named_variable.name, named_variable.variable), + region: loc_lowercase.region, + }); + } + None => { + is_phantom = true; + + env.problems.push(Problem::PhantomTypeArgument { + typ: symbol, + variable_region: loc_lowercase.region, + variable_name: loc_lowercase.value.clone(), + }); + } + } + } + + if is_phantom { + // Bail out + continue; + } + + let IntroducedVariables { + wildcards, + inferred, + .. + } = can_ann.introduced_variables; + let num_unbound = named.len() + wildcards.len() + inferred.len(); + if num_unbound > 0 { + let one_occurrence = named + .iter() + .map(|nv| Loc::at(nv.first_seen, nv.variable)) + .chain(wildcards) + .chain(inferred) + .next() + .unwrap() + .region; + + env.problems.push(Problem::UnboundTypeVariable { typ: symbol, - variable_region: loc_lowercase.region, - variable_name: loc_lowercase.value.clone(), + num_unbound, + one_occurrence, + kind, }); + + // Bail out + continue; } + + let alias = create_alias( + symbol, + name.region, + can_vars.clone(), + can_ann.typ.clone(), + kind, + ); + aliases.insert(symbol, alias.clone()); + } + + TypeDef::Ability(name, members) => { + // For now we enforce that aliases cannot reference abilities, so let's wait to + // resolve ability definitions until aliases are resolved and in scope below. + abilities.insert(name.value, (name, members)); } } - - if is_phantom { - // Bail out - continue; - } - - let IntroducedVariables { - wildcards, - inferred, - .. - } = can_ann.introduced_variables; - let num_unbound = named.len() + wildcards.len() + inferred.len(); - if num_unbound > 0 { - let one_occurrence = named - .iter() - .map(|nv| Loc::at(nv.first_seen, nv.variable)) - .chain(wildcards) - .chain(inferred) - .next() - .unwrap() - .region; - - env.problems.push(Problem::UnboundTypeVariable { - typ: symbol, - num_unbound, - one_occurrence, - kind, - }); - - // Bail out - continue; - } - - let alias = create_alias( - symbol, - name.region, - can_vars.clone(), - can_ann.typ.clone(), - kind, - ); - aliases.insert(symbol, alias.clone()); } // Now that we know the alias dependency graph, we can try to insert recursion variables @@ -362,6 +428,94 @@ pub fn canonicalize_defs<'a>( ); } + // Now we can go through and resolve all pending abilities, to add them to scope. + let mut abilities_store = AbilitiesStore::default(); + for (loc_ability_name, members) in abilities.into_values() { + let mut can_members = Vec::with_capacity(members.len()); + + for member in members { + let (member_annot, clauses) = canonicalize_annotation_with_possible_clauses( + env, + &mut scope, + &member.typ.value, + member.typ.region, + var_store, + &abilities_in_scope, + ); + + // Record all the annotation's references in output.references.lookups + for symbol in member_annot.references { + output.references.type_lookups.insert(symbol); + output.references.referenced_type_defs.insert(symbol); + } + + let member_name = member.name.extract_spaces().item; + + let member_sym = match scope.introduce( + member_name.into(), + &env.exposed_ident_ids, + &mut env.ident_ids, + member.name.region, + ) { + Ok(sym) => sym, + Err((original_region, shadow, _new_symbol)) => { + env.problem(roc_problem::can::Problem::ShadowingInAnnotation { + original_region, + shadow, + }); + // Pretend the member isn't a part of the ability + continue; + } + }; + + // What variables in the annotation are bound to the parent ability, and what variables + // are bound to some other ability? + let (variables_bound_to_ability, variables_bound_to_other_abilities): (Vec<_>, Vec<_>) = + clauses + .into_iter() + .partition(|has_clause| has_clause.value.ability == loc_ability_name.value); + + let mut bad_has_clauses = false; + + if variables_bound_to_ability.is_empty() { + // There are no variables bound to the parent ability - then this member doesn't + // need to be a part of the ability. + env.problem(Problem::AbilityMemberMissingHasClause { + member: member_sym, + ability: loc_ability_name.value, + region: member.name.region, + }); + bad_has_clauses = true; + } + + if !variables_bound_to_other_abilities.is_empty() { + // Disallow variables bound to other abilities, for now. + for bad_clause in variables_bound_to_other_abilities.iter() { + env.problem(Problem::AbilityMemberBindsExternalAbility { + member: member_sym, + ability: loc_ability_name.value, + region: bad_clause.region, + }); + } + bad_has_clauses = true; + } + + if bad_has_clauses { + // Pretend the member isn't a part of the ability + continue; + } + + let has_clauses = variables_bound_to_ability + .into_iter() + .map(|clause| clause.value) + .collect(); + can_members.push((member_sym, member_annot.typ, has_clauses)); + } + + // Store what symbols a type must define implementations for to have this ability. + abilities_store.register_ability(loc_ability_name.value, can_members); + } + // Now that we have the scope completely assembled, and shadowing resolved, // we're ready to canonicalize any body exprs. @@ -1551,7 +1705,46 @@ fn to_pending_type_def<'a>( } } - Ability { .. } => todo_abilities!(), + Ability { + header: TypeHeader { name, vars }, + members, + loc_has: _, + } => { + let name = match scope.introduce_without_shadow_symbol( + name.value.into(), + &env.exposed_ident_ids, + &mut env.ident_ids, + name.region, + ) { + Ok(symbol) => Loc::at(name.region, symbol), + Err((original_region, shadowed_symbol)) => { + env.problem(Problem::ShadowingInAnnotation { + original_region, + shadow: shadowed_symbol, + }); + return Some((Output::default(), PendingTypeDef::InvalidAbility)); + } + }; + + if !vars.is_empty() { + // Disallow ability type arguments, at least for now. + let variables_region = Region::across_all(vars.iter().map(|v| &v.region)); + + env.problem(Problem::AbilityHasTypeVariables { + name: name.value, + variables_region, + }); + return Some((Output::default(), PendingTypeDef::InvalidAbility)); + } + + let pending_ability = PendingTypeDef::Ability { + name, + // We'll handle adding the member symbols later on when we do all value defs. + members, + }; + + Some((Output::default(), pending_ability)) + } } } diff --git a/compiler/can/src/lib.rs b/compiler/can/src/lib.rs index fd813a423f..f22d1a1fe6 100644 --- a/compiler/can/src/lib.rs +++ b/compiler/can/src/lib.rs @@ -1,6 +1,7 @@ #![warn(clippy::dbg_macro)] // See github.com/rtfeldman/roc/issues/800 for discussion of the large_enum_variant check. #![allow(clippy::large_enum_variant)] +pub mod abilities; pub mod annotation; pub mod builtins; pub mod constraint; diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index 34e6369fec..9a6b9219ee 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -19,6 +19,9 @@ pub struct Scope { /// The type aliases currently in scope pub aliases: SendMap, + /// The abilities currently in scope, and their implementors. + pub abilities: SendMap, + /// The current module being processed. This will be used to turn /// unqualified idents into Symbols. home: ModuleId, @@ -62,6 +65,8 @@ impl Scope { idents: Symbol::default_in_scope(), symbols: SendMap::default(), aliases, + // TODO(abilities): default abilities in scope + abilities: SendMap::default(), } } @@ -176,6 +181,11 @@ impl Scope { /// /// Returns Err if this would shadow an existing ident, including the /// Symbol and Region of the ident we already had in scope under that name. + /// + /// If this ident shadows an existing one, a new ident is allocated for the shadow. This is + /// done so that all identifiers have unique symbols, which is important in particular when + /// we generate code for value identifiers. + /// If this behavior is undesirable, use [`Self::introduce_without_shadow_symbol`]. pub fn introduce( &mut self, ident: Ident, @@ -198,25 +208,53 @@ impl Scope { Err((original_region, shadow, symbol)) } - None => { - // If this IdentId was already added previously - // when the value was exposed in the module header, - // use that existing IdentId. Otherwise, create a fresh one. - let ident_id = match exposed_ident_ids.get_id(&ident) { - Some(ident_id) => ident_id, - None => all_ident_ids.add(ident.clone()), - }; - - let symbol = Symbol::new(self.home, ident_id); - - self.symbols.insert(symbol, region); - self.idents.insert(ident, (symbol, region)); - - Ok(symbol) - } + None => Ok(self.commit_introduction(ident, exposed_ident_ids, all_ident_ids, region)), } } + /// Like [Self::introduce], but does not introduce a new symbol for the shadowing symbol. + pub fn introduce_without_shadow_symbol( + &mut self, + ident: Ident, + exposed_ident_ids: &IdentIds, + all_ident_ids: &mut IdentIds, + region: Region, + ) -> Result)> { + match self.idents.get(&ident) { + Some(&(_, original_region)) => { + let shadow = Loc { + value: ident.clone(), + region, + }; + Err((original_region, shadow)) + } + None => Ok(self.commit_introduction(ident, exposed_ident_ids, all_ident_ids, region)), + } + } + + fn commit_introduction( + &mut self, + ident: Ident, + exposed_ident_ids: &IdentIds, + all_ident_ids: &mut IdentIds, + region: Region, + ) -> Symbol { + // If this IdentId was already added previously + // when the value was exposed in the module header, + // use that existing IdentId. Otherwise, create a fresh one. + let ident_id = match exposed_ident_ids.get_id(&ident) { + Some(ident_id) => ident_id, + None => all_ident_ids.add(ident.clone()), + }; + + let symbol = Symbol::new(self.home, ident_id); + + self.symbols.insert(symbol, region); + self.idents.insert(ident, (symbol, region)); + + symbol + } + /// Ignore an identifier. /// /// Used for record guards like { x: Just _ } diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 97f81ed416..1c6256bdfe 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -866,24 +866,26 @@ fn parse_defs_end<'a>( } Ok((_, loc_pattern, state)) => { // First let's check whether this is an ability definition. - if let Loc { - value: - Pattern::Apply( - loc_name @ Loc { - value: Pattern::GlobalTag(name), - .. - }, - args, - ), - .. - } = loc_pattern + let opt_tag_and_args: Option<(&str, Region, &[Loc])> = match loc_pattern.value { + Pattern::Apply( + Loc { + value: Pattern::GlobalTag(name), + region, + }, + args, + ) => Some((name, *region, args)), + Pattern::GlobalTag(name) => Some((name, loc_pattern.region, &[])), + _ => None, + }; + + if let Some((name, name_region, args)) = opt_tag_and_args { if let Ok((_, loc_has, state)) = loc_has_parser(min_indent).parse(arena, state.clone()) { let (_, loc_def, state) = finish_parsing_ability_def( start_column, - Loc::at(loc_name.region, name), + Loc::at(name_region, name), args, loc_has, arena, diff --git a/compiler/parse/tests/snapshots/pass/ability_two_in_a_row.expr.result-ast b/compiler/parse/tests/snapshots/pass/ability_two_in_a_row.expr.result-ast index 4bd0831c5c..cba0cace4b 100644 --- a/compiler/parse/tests/snapshots/pass/ability_two_in_a_row.expr.result-ast +++ b/compiler/parse/tests/snapshots/pass/ability_two_in_a_row.expr.result-ast @@ -33,61 +33,59 @@ Defs( }, ], ), - }, - ], - }, - ), - ], - @35-71 SpaceBefore( - Defs( - [ - @35-68 Type( - Ability { - header: TypeHeader { - name: @35-38 "Ab2", - vars: [], - }, - loc_has: @39-42 Has, - members: [ - AbilityMember { - name: @43-46 "ab2", - typ: @54-68 Where( - @54-56 Function( - [ - @49-50 BoundVariable( - "a", - ), - ], - @54-56 Record { - fields: [], - ext: None, - }, - ), - [ - @59-68 HasClause { - var: @59-60 "a", - ability: @65-68 Apply( - "", - "Ab2", - [], - ), - }, - ], + [ + @24-33 HasClause { + var: @24-25 "a", + ability: @30-33 Apply( + "", + "Ab1", + [], ), }, ], - }, - ), + ), + }, ], - @70-71 SpaceBefore( - Num( - "1", - ), - [ - Newline, - Newline, - ], - ), + }, + @35-68 Ability { + header: TypeHeader { + name: @35-38 "Ab2", + vars: [], + }, + loc_has: @39-42 Has, + members: [ + AbilityMember { + name: @43-46 "ab2", + typ: @54-68 Where( + @54-56 Function( + [ + @49-50 BoundVariable( + "a", + ), + ], + @54-56 Record { + fields: [], + ext: None, + }, + ), + [ + @59-68 HasClause { + var: @59-60 "a", + ability: @65-68 Apply( + "", + "Ab2", + [], + ), + }, + ], + ), + }, + ], + }, + ], + @70-71 SpaceBefore( + Num( + "1", ), [ Newline, diff --git a/compiler/problem/src/can.rs b/compiler/problem/src/can.rs index daa20c0778..56756a8508 100644 --- a/compiler/problem/src/can.rs +++ b/compiler/problem/src/can.rs @@ -95,6 +95,30 @@ pub enum Problem { region: Region, kind: ExtensionTypeKind, }, + AbilityHasTypeVariables { + name: Symbol, + variables_region: Region, + }, + HasClauseIsNotAbility { + region: Region, + }, + IllegalHasClause { + region: Region, + }, + AbilityMemberMissingHasClause { + member: Symbol, + ability: Symbol, + region: Region, + }, + AbilityMemberBindsExternalAbility { + member: Symbol, + ability: Symbol, + region: Region, + }, + AliasUsesAbility { + loc_name: Loc, + ability: Symbol, + }, } #[derive(Clone, Debug, PartialEq)] diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index bd8b68199e..6c55d062fa 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -1856,6 +1856,7 @@ pub enum Problem { }, InvalidModule, SolvedTypeError, + HasClauseIsNotAbility(Region), } #[derive(PartialEq, Eq, Debug, Clone)] diff --git a/reporting/src/error/canonicalize.rs b/reporting/src/error/canonicalize.rs index 83e35f6ade..24a27f5e9c 100644 --- a/reporting/src/error/canonicalize.rs +++ b/reporting/src/error/canonicalize.rs @@ -38,6 +38,12 @@ const OPAQUE_DECLARED_OUTSIDE_SCOPE: &str = "OPAQUE TYPE DECLARED OUTSIDE SCOPE" const OPAQUE_NOT_APPLIED: &str = "OPAQUE TYPE NOT APPLIED"; const OPAQUE_OVER_APPLIED: &str = "OPAQUE TYPE APPLIED TO TOO MANY ARGS"; const INVALID_EXTENSION_TYPE: &str = "INVALID_EXTENSION_TYPE"; +const ABILITY_HAS_TYPE_VARIABLES: &str = "ABILITY HAS TYPE VARIABLES"; +const HAS_CLAUSE_IS_NOT_AN_ABILITY: &str = "HAS CLAUSE IS NOT AN ABILITY"; +const ALIAS_USES_ABILITY: &str = "ALIAS USES ABILITY"; +const ILLEGAL_HAS_CLAUSE: &str = "ILLEGAL HAS CLAUSE"; +const ABILITY_MEMBER_MISSING_HAS_CLAUSE: &str = "ABILITY MEMBER MISSING HAS CLAUSE"; +const ABILITY_MEMBER_HAS_EXTRANEOUS_HAS_CLAUSE: &str = "ABILITY MEMBER HAS EXTRANEOUS HAS CLAUSE"; pub fn can_problem<'b>( alloc: &'b RocDocAllocator<'b>, @@ -562,6 +568,134 @@ pub fn can_problem<'b>( title = INVALID_EXTENSION_TYPE.to_string(); severity = Severity::RuntimeError; } + + Problem::AbilityHasTypeVariables { + name, + variables_region, + } => { + doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The definition of the "), + alloc.symbol_unqualified(name), + alloc.reflow(" ability includes type variables:"), + ]), + alloc.region(lines.convert_region(variables_region)), + alloc.reflow( + "Abilities cannot depend on type variables, but their member values can!", + ), + ]); + title = ABILITY_HAS_TYPE_VARIABLES.to_string(); + severity = Severity::RuntimeError; + } + + Problem::HasClauseIsNotAbility { + region: clause_region, + } => { + doc = alloc.stack(vec![ + alloc.reflow(r#"The type referenced in this "has" clause is not an ability:"#), + alloc.region(lines.convert_region(clause_region)), + ]); + title = HAS_CLAUSE_IS_NOT_AN_ABILITY.to_string(); + severity = Severity::RuntimeError; + } + + Problem::AliasUsesAbility { + loc_name: Loc { + region, + value: name, + }, + ability, + } => { + doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The definition of the "), + alloc.symbol_unqualified(name), + alloc.reflow(" aliases references the ability "), + alloc.symbol_unqualified(ability), + alloc.reflow(":"), + ]), + alloc.region(lines.convert_region(region)), + ]); + title = ALIAS_USES_ABILITY.to_string(); + severity = Severity::RuntimeError; + } + + Problem::IllegalHasClause { region } => { + doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("A "), + alloc.keyword("has"), + alloc.reflow(" clause is not allowed here:"), + ]), + alloc.region(lines.convert_region(region)), + alloc.concat(vec![ + alloc.keyword("has"), + alloc.reflow(" clauses can only be specified on the top-level type annotation of an ability member."), + ]), + ]); + title = ILLEGAL_HAS_CLAUSE.to_string(); + severity = Severity::RuntimeError; + } + + Problem::AbilityMemberMissingHasClause { + member, + ability, + region, + } => { + doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The definition of the ability member "), + alloc.symbol_unqualified(member), + alloc.reflow(" does not include a "), + alloc.keyword("has"), + alloc.reflow(" clause binding a type variable to the ability "), + alloc.symbol_unqualified(ability), + alloc.reflow(":"), + ]), + alloc.region(lines.convert_region(region)), + alloc.concat(vec![ + alloc.reflow("Ability members must include a "), + alloc.keyword("has"), + alloc.reflow(" clause binding a type variable to an ability, like"), + ]), + alloc.type_block(alloc.concat(vec![ + alloc.type_variable("a".into()), + alloc.space(), + alloc.keyword("has"), + alloc.space(), + alloc.symbol_unqualified(ability), + ])), + alloc.concat(vec![alloc.reflow( + "Otherwise, the function does not need to be part of the ability!", + )]), + ]); + title = ABILITY_MEMBER_MISSING_HAS_CLAUSE.to_string(); + severity = Severity::RuntimeError; + } + + Problem::AbilityMemberBindsExternalAbility { + member, + ability, + region, + } => { + doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The definition of the ability member "), + alloc.symbol_unqualified(member), + alloc.reflow(" includes a has clause binding an ability it is not a part of:"), + ]), + alloc.region(lines.convert_region(region)), + alloc.reflow("Currently, ability members can only bind variables to the ability they are a part of."), + alloc.concat(vec![ + alloc.hint(""), + alloc.reflow("Did you mean to bind the "), + alloc.symbol_unqualified(ability), + alloc.reflow(" ability instead?"), + ]), + ]); + title = ABILITY_MEMBER_HAS_EXTRANEOUS_HAS_CLAUSE.to_string(); + severity = Severity::RuntimeError; + } }; Report { diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index d16764eaa2..73b427947a 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -8896,4 +8896,333 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn bad_type_parameter_in_ability() { + report_problem_as( + indoc!( + r#" + Hash a b c has + hash : a -> U64 | a has Hash + + 1 + "# + ), + indoc!( + r#" + ── ABILITY HAS TYPE VARIABLES ────────────────────────────────────────────────── + + The definition of the `Hash` ability includes type variables: + + 1│ Hash a b c has + ^^^^^ + + Abilities cannot depend on type variables, but their member values + can! + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `Hash` is not used anywhere in your code. + + 1│ Hash a b c has + ^^^^ + + If you didn't intend on using `Hash` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn alias_in_has_clause() { + report_problem_as( + indoc!( + r#" + Hash has hash : a, b -> U64 | a has Hash, b has Bool + + 1 + "# + ), + indoc!( + r#" + ── HAS CLAUSE IS NOT AN ABILITY ──────────────────────────────────────────────── + + The type referenced in this "has" clause is not an ability: + + 1│ Hash has hash : a, b -> U64 | a has Hash, b has Bool + ^^^^ + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `hash` is not used anywhere in your code. + + 1│ Hash has hash : a, b -> U64 | a has Hash, b has Bool + ^^^^ + + If you didn't intend on using `hash` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn shadowed_type_variable_in_has_clause() { + report_problem_as( + indoc!( + r#" + Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 + + 1 + "# + ), + indoc!( + r#" + ── DUPLICATE NAME ────────────────────────────────────────────────────────────── + + The `a` name is first defined here: + + 1│ Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 + ^^^^^^^^^ + + But then it's defined a second time here: + + 1│ Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 + ^^^^^^^^^ + + Since these variables have the same name, it's easy to use the wrong + one on accident. Give one of them a new name. + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `ab1` is not used anywhere in your code. + + 1│ Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 + ^^^ + + If you didn't intend on using `ab1` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn alias_using_ability() { + report_problem_as( + indoc!( + r#" + Ability has ab : a -> {} | a has Ability + + Alias : Ability + + a : Alias + a + "# + ), + indoc!( + r#" + ── ALIAS USES ABILITY ────────────────────────────────────────────────────────── + + The definition of the `Alias` aliases references the ability `Ability`: + + 3│ Alias : Ability + ^^^^^ + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `ab` is not used anywhere in your code. + + 1│ Ability has ab : a -> {} | a has Ability + ^^ + + If you didn't intend on using `ab` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn ability_shadows_ability() { + report_problem_as( + indoc!( + r#" + Ability has ab : a -> U64 | a has Ability + + Ability has ab1 : a -> U64 | a has Ability + + 1 + "# + ), + indoc!( + r#" + ── DUPLICATE NAME ────────────────────────────────────────────────────────────── + + The `Ability` name is first defined here: + + 1│ Ability has ab : a -> U64 | a has Ability + ^^^^^^^ + + But then it's defined a second time here: + + 3│ Ability has ab1 : a -> U64 | a has Ability + ^^^^^^^ + + Since these variables have the same name, it's easy to use the wrong + one on accident. Give one of them a new name. + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `ab` is not used anywhere in your code. + + 1│ Ability has ab : a -> U64 | a has Ability + ^^ + + If you didn't intend on using `ab` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn ability_member_does_not_bind_ability() { + report_problem_as( + indoc!( + r#" + Ability has ab : {} -> {} + + 1 + "# + ), + indoc!( + r#" + ── ABILITY MEMBER MISSING HAS CLAUSE ─────────────────────────────────────────── + + The definition of the ability member `ab` does not include a `has` clause + binding a type variable to the ability `Ability`: + + 1│ Ability has ab : {} -> {} + ^^ + + Ability members must include a `has` clause binding a type variable to + an ability, like + + a has Ability + + Otherwise, the function does not need to be part of the ability! + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `Ability` is not used anywhere in your code. + + 1│ Ability has ab : {} -> {} + ^^^^^^^ + + If you didn't intend on using `Ability` then remove it so future readers + of your code don't wonder why it is there. + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `ab` is not used anywhere in your code. + + 1│ Ability has ab : {} -> {} + ^^ + + If you didn't intend on using `ab` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn ability_member_binds_extra_ability() { + report_problem_as( + indoc!( + r#" + Eq has eq : a, a -> Bool | a has Eq + Hash has hash : a, b -> U64 | a has Eq, b has Hash + + 1 + "# + ), + indoc!( + r#" + ── ABILITY MEMBER HAS EXTRANEOUS HAS CLAUSE ──────────────────────────────────── + + The definition of the ability member `hash` includes a has clause + binding an ability it is not a part of: + + 2│ Hash has hash : a, b -> U64 | a has Eq, b has Hash + ^^^^^^^^ + + Currently, ability members can only bind variables to the ability they + are a part of. + + Hint: Did you mean to bind the `Hash` ability instead? + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `eq` is not used anywhere in your code. + + 1│ Eq has eq : a, a -> Bool | a has Eq + ^^ + + If you didn't intend on using `eq` then remove it so future readers of + your code don't wonder why it is there. + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `hash` is not used anywhere in your code. + + 2│ Hash has hash : a, b -> U64 | a has Eq, b has Hash + ^^^^ + + If you didn't intend on using `hash` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } + + #[test] + fn has_clause_outside_of_ability() { + report_problem_as( + indoc!( + r#" + Hash has hash : a -> U64 | a has Hash + + f : a -> U64 | a has Hash + + f + "# + ), + indoc!( + r#" + ── ILLEGAL HAS CLAUSE ────────────────────────────────────────────────────────── + + A `has` clause is not allowed here: + + 3│ f : a -> U64 | a has Hash + ^^^^^^^^^^ + + `has` clauses can only be specified on the top-level type annotation of + an ability member. + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `hash` is not used anywhere in your code. + + 1│ Hash has hash : a -> U64 | a has Hash + ^^^^ + + If you didn't intend on using `hash` then remove it so future readers of + your code don't wonder why it is there. + "# + ), + ) + } }