From 7536d48a2295f3bda0d4d09d72ba507e7ed16f03 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 11 Apr 2022 16:12:01 -0400 Subject: [PATCH 01/21] Remove unused function --- compiler/can/src/annotation.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/compiler/can/src/annotation.rs b/compiler/can/src/annotation.rs index c8af0d3e0a..52e4f34103 100644 --- a/compiler/can/src/annotation.rs +++ b/compiler/can/src/annotation.rs @@ -98,13 +98,6 @@ impl IntroducedVariables { .map(|nv| &nv.variable) } - pub fn name_by_var(&self, var: Variable) -> Option<&Lowercase> { - self.named - .iter() - .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) } From 913d97cab117392d92fd070e75eae79a7395abb8 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 15:36:36 -0400 Subject: [PATCH 02/21] Add needed imports --- Cargo.lock | 6 ++++++ compiler/load/Cargo.toml | 2 ++ compiler/solve/Cargo.toml | 2 ++ compiler/unify/Cargo.toml | 3 +++ reporting/Cargo.toml | 2 ++ 5 files changed, 15 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 27c0e7bf0f..dc9e364812 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3703,6 +3703,7 @@ dependencies = [ "roc_constrain", "roc_load_internal", "roc_module", + "roc_reporting", "roc_target", "roc_types", ] @@ -3893,6 +3894,7 @@ dependencies = [ "roc_collections", "roc_constrain", "roc_exhaustive", + "roc_load", "roc_module", "roc_mono", "roc_parse", @@ -3902,6 +3904,7 @@ dependencies = [ "roc_target", "roc_test_utils", "roc_types", + "tempfile", "ven_pretty", ] @@ -3916,11 +3919,13 @@ dependencies = [ "roc_builtins", "roc_can", "roc_collections", + "roc_error_macros", "roc_load", "roc_module", "roc_parse", "roc_problem", "roc_region", + "roc_reporting", "roc_solve", "roc_target", "roc_types", @@ -3968,6 +3973,7 @@ version = "0.1.0" dependencies = [ "bitflags", "roc_collections", + "roc_error_macros", "roc_module", "roc_types", ] diff --git a/compiler/load/Cargo.toml b/compiler/load/Cargo.toml index 4f7632fb60..f3d1d4c30f 100644 --- a/compiler/load/Cargo.toml +++ b/compiler/load/Cargo.toml @@ -13,10 +13,12 @@ roc_constrain= { path = "../constrain" } roc_types = { path = "../types" } roc_module = { path = "../module" } roc_collections = { path = "../collections" } +roc_reporting = { path = "../../reporting" } [build-dependencies] roc_load_internal = { path = "../load_internal" } roc_builtins = { path = "../builtins" } roc_module = { path = "../module" } +roc_reporting = { path = "../../reporting" } roc_target = { path = "../roc_target" } bumpalo = { version = "3.8.0", features = ["collections"] } diff --git a/compiler/solve/Cargo.toml b/compiler/solve/Cargo.toml index ebb9e4a615..b78faa8421 100644 --- a/compiler/solve/Cargo.toml +++ b/compiler/solve/Cargo.toml @@ -7,6 +7,7 @@ edition = "2018" [dependencies] roc_collections = { path = "../collections" } +roc_error_macros = { path = "../../error_macros" } roc_region = { path = "../region" } roc_module = { path = "../module" } roc_types = { path = "../types" } @@ -22,6 +23,7 @@ roc_problem = { path = "../problem" } roc_parse = { path = "../parse" } roc_solve = { path = "../solve" } roc_target = { path = "../roc_target" } +roc_reporting = { path = "../../reporting" } pretty_assertions = "1.0.0" indoc = "1.0.3" tempfile = "3.2.0" diff --git a/compiler/unify/Cargo.toml b/compiler/unify/Cargo.toml index e15626603c..d8622ed928 100644 --- a/compiler/unify/Cargo.toml +++ b/compiler/unify/Cargo.toml @@ -11,6 +11,9 @@ bitflags = "1.3.2" [dependencies.roc_collections] path = "../collections" +[dependencies.roc_error_macros] +path = "../../error_macros" + [dependencies.roc_module] path = "../module" diff --git a/reporting/Cargo.toml b/reporting/Cargo.toml index f2f9bf1c57..63e16ad7c3 100644 --- a/reporting/Cargo.toml +++ b/reporting/Cargo.toml @@ -23,9 +23,11 @@ bumpalo = { version = "3.8.0", features = ["collections"] } [dev-dependencies] roc_constrain = { path = "../compiler/constrain" } roc_builtins = { path = "../compiler/builtins" } +roc_load = { path = "../compiler/load" } roc_problem = { path = "../compiler/problem" } roc_parse = { path = "../compiler/parse" } roc_target = { path = "../compiler/roc_target" } roc_test_utils = { path = "../test_utils" } pretty_assertions = "1.0.0" indoc = "1.0.3" +tempfile = "3.2.0" From 15a040ec870cb6aa7473f5d9ba5e72a2ab34f29f Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 15:37:16 -0400 Subject: [PATCH 03/21] Basic type inference and solving for abilities Note that is still pretty limited. We only permit opaque types to implement abilities, abilities cannot have type arguments, and also no other functions may depend on abilities --- compiler/can/src/abilities.rs | 101 +++++-- compiler/can/src/annotation.rs | 156 ++++++---- compiler/can/src/def.rs | 55 ++-- compiler/can/src/expr.rs | 1 + compiler/can/src/module.rs | 14 + compiler/can/src/pattern.rs | 5 +- compiler/can/src/scope.rs | 14 +- compiler/constrain/src/module.rs | 19 +- compiler/constrain/src/pattern.rs | 20 +- compiler/load/build.rs | 1 + compiler/load/src/lib.rs | 13 +- compiler/load_internal/src/file.rs | 110 ++++++- compiler/mono/src/layout.rs | 4 + compiler/mono/src/layout_soa.rs | 18 +- compiler/solve/src/module.rs | 16 +- compiler/solve/src/solve.rs | 179 +++++++++-- compiler/solve/tests/solve_expr.rs | 154 +++++++++- compiler/types/src/pretty_print.rs | 136 ++++++--- compiler/types/src/subs.rs | 151 +++++++++- compiler/types/src/types.rs | 31 +- compiler/unify/src/unify.rs | 465 +++++++++++++++++++++-------- reporting/src/error/type.rs | 175 ++++++++--- reporting/src/report.rs | 19 ++ reporting/tests/helpers/mod.rs | 12 +- reporting/tests/test_reporting.rs | 416 ++++++++++++++++++++------ 25 files changed, 1808 insertions(+), 477 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index 5998f9da8e..71be29a706 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -1,55 +1,62 @@ use roc_collections::all::{MutMap, MutSet}; use roc_module::symbol::Symbol; -use roc_types::types::Type; - -use crate::annotation::HasClause; +use roc_region::all::Region; +use roc_types::{subs::Variable, types::Type}; /// 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, +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AbilityMemberData { + pub parent_ability: Symbol, + pub signature_var: Variable, + pub signature: Type, + pub region: Region, } /// 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)] +#[derive(Default, Debug, Clone, PartialEq, Eq)] pub struct AbilitiesStore { /// Maps an ability to the members defining it. - #[allow(unused)] members_of_ability: MutMap>, /// Information about all members composing abilities. ability_members: MutMap, + /// Map of symbols that specialize an ability member to the root ability symbol name. + /// For example, for the program + /// Hash has hash : a -> U64 | a has Hash + /// ^^^^ gets the symbol "#hash" + /// hash = \@Id n -> n + /// ^^^^ gets the symbol "#hash1" + /// + /// We keep the mapping #hash1->#hash + specialization_to_root: MutMap, + /// Tuples of (type, member) specifying that `type` declares an implementation of an ability /// member `member`. - #[allow(unused)] - declared_implementations: MutSet<(Symbol, Symbol)>, + declared_specializations: MutSet<(Symbol, Symbol)>, } impl AbilitiesStore { + /// Records the definition of an ability, including its members. pub fn register_ability( &mut self, ability: Symbol, - members: Vec<(Symbol, Type, Vec)>, + members: Vec<(Symbol, Region, Variable, Type)>, ) { let mut members_vec = Vec::with_capacity(members.len()); - for (member, signature, bound_has_clauses) in members.into_iter() { + for (member, region, signature_var, signature) in members.into_iter() { members_vec.push(member); let old_member = self.ability_members.insert( member, AbilityMemberData { parent_ability: ability, + signature_var, signature, - bound_has_clauses, + region, }, ); debug_assert!(old_member.is_none(), "Replacing existing member definition"); @@ -61,14 +68,64 @@ impl AbilitiesStore { ); } - pub fn register_implementation(&mut self, implementing_type: Symbol, ability_member: Symbol) { - let old_impl = self - .declared_implementations + /// Records a specialization of `ability_member` with specialized type `implementing_type`. + pub fn register_specialization_for_type( + &mut self, + implementing_type: Symbol, + ability_member: Symbol, + ) { + let is_new_insert = self + .declared_specializations .insert((implementing_type, ability_member)); - debug_assert!(!old_impl, "Replacing existing implementation"); + debug_assert!(is_new_insert, "Replacing existing implementation"); } + /// Checks if `name` is a root ability member symbol name. + /// Note that this will return `false` for specializations of an ability member, which have + /// different symbols from the root. pub fn is_ability_member_name(&self, name: Symbol) -> bool { self.ability_members.contains_key(&name) } + + /// Returns information about all known ability members and their root symbols. + pub fn root_ability_members(&self) -> &MutMap { + &self.ability_members + } + + /// Records that the symbol `specializing_symbol` claims to specialize `ability_member`; for + /// example the symbol of `hash : Id -> U64` specializing `hash : a -> U64 | a has Hash`. + pub fn register_specializing_symbol( + &mut self, + specializing_symbol: Symbol, + ability_member: Symbol, + ) { + self.specialization_to_root + .insert(specializing_symbol, ability_member); + } + + /// Returns whether a symbol is declared to specialize an ability member. + pub fn is_specialization_name(&self, symbol: Symbol) -> bool { + self.specialization_to_root.contains_key(&symbol) + } + + /// Finds the symbol name and ability member definition for a symbol specializing the ability + /// member, if it specializes any. + /// For example, suppose `hash : Id -> U64` has symbol #hash1 and specializes + /// `hash : a -> U64 | a has Hash` with symbol #hash. Calling this with #hash1 would retrieve + /// the ability member data for #hash. + pub fn root_name_and_def( + &self, + specializing_symbol: Symbol, + ) -> Option<(Symbol, &AbilityMemberData)> { + let root_symbol = self.specialization_to_root.get(&specializing_symbol)?; + debug_assert!(self.ability_members.contains_key(&root_symbol)); + let root_data = self.ability_members.get(&root_symbol).unwrap(); + Some((*root_symbol, root_data)) + } + + /// Returns pairs of (type, ability member) specifying that "ability member" has a + /// specialization with type "type". + pub fn get_known_specializations(&self) -> &MutSet<(Symbol, Symbol)> { + &self.declared_specializations + } } diff --git a/compiler/can/src/annotation.rs b/compiler/can/src/annotation.rs index 52e4f34103..ddc5292dcd 100644 --- a/compiler/can/src/annotation.rs +++ b/compiler/can/src/annotation.rs @@ -19,6 +19,22 @@ pub struct Annotation { pub aliases: SendMap, } +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum NamedOrAbleVariable<'a> { + Named(&'a NamedVariable), + Able(&'a AbleVariable), +} + +impl<'a> NamedOrAbleVariable<'a> { + pub fn first_seen(&self) -> Region { + match self { + NamedOrAbleVariable::Named(nv) => nv.first_seen, + NamedOrAbleVariable::Able(av) => av.first_seen, + } + } +} + +/// A named type variable, not bound to an ability. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct NamedVariable { pub variable: Variable, @@ -27,21 +43,40 @@ pub struct NamedVariable { pub first_seen: Region, } +/// A type variable bound to an ability, like "a has Hash". +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub struct AbleVariable { + pub variable: Variable, + pub name: Lowercase, + pub ability: Symbol, + // NB: there may be multiple occurrences of a variable + pub first_seen: Region, +} + #[derive(Clone, Debug, PartialEq, Default)] pub struct IntroducedVariables { pub wildcards: Vec>, pub lambda_sets: Vec, pub inferred: Vec>, pub named: Vec, + pub able: Vec, pub host_exposed_aliases: MutMap, } impl IntroducedVariables { + #[inline(always)] + fn debug_assert_not_already_present(&self, var: Variable) { + debug_assert!((self.wildcards.iter().map(|v| &v.value)) + .chain(self.lambda_sets.iter()) + .chain(self.inferred.iter().map(|v| &v.value)) + .chain(self.named.iter().map(|nv| &nv.variable)) + .chain(self.able.iter().map(|av| &av.variable)) + .chain(self.host_exposed_aliases.values()) + .all(|&v| v != var)); + } + pub fn insert_named(&mut self, name: Lowercase, var: Loc) { - debug_assert!(!self - .named - .iter() - .any(|nv| nv.name == name || nv.variable == var.value)); + self.debug_assert_not_already_present(var.value); let named_variable = NamedVariable { name, @@ -52,19 +87,36 @@ impl IntroducedVariables { self.named.push(named_variable); } + pub fn insert_able(&mut self, name: Lowercase, var: Loc, ability: Symbol) { + self.debug_assert_not_already_present(var.value); + + let able_variable = AbleVariable { + name, + ability, + variable: var.value, + first_seen: var.region, + }; + + self.able.push(able_variable); + } + pub fn insert_wildcard(&mut self, var: Loc) { + self.debug_assert_not_already_present(var.value); self.wildcards.push(var); } pub fn insert_inferred(&mut self, var: Loc) { + self.debug_assert_not_already_present(var.value); self.inferred.push(var); } fn insert_lambda_set(&mut self, var: Variable) { + self.debug_assert_not_already_present(var); self.lambda_sets.push(var); } pub fn insert_host_exposed_alias(&mut self, symbol: Symbol, var: Variable) { + self.debug_assert_not_already_present(var); self.host_exposed_aliases.insert(symbol, var); } @@ -78,6 +130,10 @@ impl IntroducedVariables { self.named.extend(other.named.iter().cloned()); self.named.sort(); self.named.dedup(); + + self.able.extend(other.able.iter().cloned()); + self.able.sort(); + self.able.dedup(); } pub fn union_owned(&mut self, other: Self) { @@ -91,15 +147,26 @@ impl IntroducedVariables { self.named.dedup(); } - pub fn var_by_name(&self, name: &Lowercase) -> Option<&Variable> { - self.named - .iter() - .find(|nv| &nv.name == name) - .map(|nv| &nv.variable) + pub fn var_by_name(&self, name: &Lowercase) -> Option { + (self.named.iter().map(|nv| (&nv.name, nv.variable))) + .chain(self.able.iter().map(|av| (&av.name, av.variable))) + .find(|(cand, _)| cand == &name) + .map(|(_, var)| var) } - pub fn named_var_by_name(&self, name: &Lowercase) -> Option<&NamedVariable> { - self.named.iter().find(|nv| &nv.name == name) + pub fn named_var_by_name(&self, name: &Lowercase) -> Option { + if let Some(nav) = self + .named + .iter() + .find(|nv| &nv.name == name) + .map(|nv| NamedOrAbleVariable::Named(nv)) + { + return Some(nav); + } + self.able + .iter() + .find(|av| &av.name == name) + .map(|av| NamedOrAbleVariable::Able(av)) } } @@ -140,13 +207,6 @@ 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, @@ -154,16 +214,17 @@ pub fn canonicalize_annotation_with_possible_clauses( region: Region, var_store: &mut VarStore, abilities_in_scope: &[Symbol], -) -> (Annotation, Vec>) { +) -> Annotation { let mut introduced_variables = IntroducedVariables::default(); let mut references = MutSet::default(); let mut aliases = SendMap::default(); - let (annotation, region, clauses) = match annotation { + let (annotation, region) = match annotation { TypeAnnotation::Where(annotation, clauses) => { - let mut can_clauses = Vec::with_capacity(clauses.len()); + // Add each "has" clause. The association of a variable to an ability will be saved on + // `introduced_variables`, which we'll process later. for clause in clauses.iter() { - match canonicalize_has_clause( + let opt_err = canonicalize_has_clause( env, scope, var_store, @@ -171,24 +232,19 @@ pub fn canonicalize_annotation_with_possible_clauses( 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, - ) - } - }; + ); + if let Err(err_type) = opt_err { + return Annotation { + typ: err_type, + introduced_variables, + references, + aliases, + }; + } } - (&annotation.value, annotation.region, can_clauses) + (&annotation.value, annotation.region) } - annot => (annot, region, vec![]), + annot => (annot, region), }; let typ = can_annotation_help( @@ -209,7 +265,7 @@ pub fn canonicalize_annotation_with_possible_clauses( aliases, }; - (annot, clauses) + annot } fn make_apply_symbol( @@ -495,7 +551,7 @@ fn can_annotation_help( let name = Lowercase::from(*v); match introduced_variables.var_by_name(&name) { - Some(var) => Type::Variable(*var), + Some(var) => Type::Variable(var), None => { let var = var_store.fresh(); @@ -559,8 +615,8 @@ fn can_annotation_help( let var_name = Lowercase::from(var); if let Some(var) = introduced_variables.var_by_name(&var_name) { - vars.push((var_name.clone(), Type::Variable(*var))); - lowercase_vars.push(Loc::at(loc_var.region, (var_name, *var))); + vars.push((var_name.clone(), Type::Variable(var))); + lowercase_vars.push(Loc::at(loc_var.region, (var_name, var))); } else { let var = var_store.fresh(); @@ -792,7 +848,7 @@ fn canonicalize_has_clause( clause: &Loc>, abilities_in_scope: &[Symbol], references: &mut MutSet, -) -> Result { +) -> Result<(), Type> { let Loc { region, value: roc_parse::ast::HasClause { var, ability }, @@ -829,25 +885,21 @@ fn canonicalize_has_clause( let var_name_ident = var_name.to_string().into(); let shadow = Loc::at(region, var_name_ident); env.problem(roc_problem::can::Problem::Shadowing { - original_region: shadowing.first_seen, + original_region: shadowing.first_seen(), shadow: shadow.clone(), kind: ShadowKind::Variable, }); return Err(Type::Erroneous(Problem::Shadowed( - shadowing.first_seen, + shadowing.first_seen(), shadow, ))); } let var = var_store.fresh(); - introduced_variables.insert_named(var_name.clone(), Loc::at(region, var)); + introduced_variables.insert_able(var_name.clone(), Loc::at(region, var), ability); - Ok(HasClause { - var_name, - var, - ability, - }) + Ok(()) } #[allow(clippy::too_many_arguments)] @@ -1098,7 +1150,7 @@ fn can_assigned_fields<'a>( let field_name = Lowercase::from(loc_field_name.value); let field_type = { if let Some(var) = introduced_variables.var_by_name(&field_name) { - Type::Variable(*var) + Type::Variable(var) } else { let field_var = var_store.fresh(); introduced_variables.insert_named( diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 460b8a5efc..75d17270af 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1,4 +1,3 @@ -use crate::abilities::AbilitiesStore; use crate::annotation::canonicalize_annotation; use crate::annotation::canonicalize_annotation_with_possible_clauses; use crate::annotation::IntroducedVariables; @@ -430,12 +429,11 @@ 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( + let member_annot = canonicalize_annotation_with_possible_clauses( env, &mut scope, &member.typ.value, @@ -450,13 +448,14 @@ pub fn canonicalize_defs<'a>( output.references.referenced_type_defs.insert(symbol); } + let name_region = member.name.region; 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, + name_region, ) { Ok(sym) => sym, Err((original_region, shadow, _new_symbol)) => { @@ -473,9 +472,11 @@ pub fn canonicalize_defs<'a>( // 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); + member_annot + .introduced_variables + .able + .iter() + .partition(|av| av.ability == loc_ability_name.value); let mut bad_has_clauses = false; @@ -485,18 +486,18 @@ pub fn canonicalize_defs<'a>( env.problem(Problem::AbilityMemberMissingHasClause { member: member_sym, ability: loc_ability_name.value, - region: member.name.region, + region: 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() { + for bad_variable in variables_bound_to_other_abilities.iter() { env.problem(Problem::AbilityMemberBindsExternalAbility { member: member_sym, ability: loc_ability_name.value, - region: bad_clause.region, + region: bad_variable.first_seen, }); } bad_has_clauses = true; @@ -507,17 +508,22 @@ pub fn canonicalize_defs<'a>( 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)); + // The introduced variables are good; add them to the output. + output + .introduced_variables + .union(&member_annot.introduced_variables); + + can_members.push((member_sym, name_region, var_store.fresh(), member_annot.typ)); } // Store what symbols a type must define implementations for to have this ability. - abilities_store.register_ability(loc_ability_name.value, can_members); + scope + .abilities_store + .register_ability(loc_ability_name.value, can_members); } + dbg!(&scope.abilities_store, pattern_type); + // Now that we have the scope completely assembled, and shadowing resolved, // we're ready to canonicalize any body exprs. @@ -526,14 +532,7 @@ pub fn canonicalize_defs<'a>( // once we've finished assembling the entire scope. let mut pending_value_defs = Vec::with_capacity(value_defs.len()); for loc_def in value_defs.into_iter() { - match to_pending_value_def( - env, - var_store, - loc_def.value, - &mut scope, - &abilities_store, - pattern_type, - ) { + match to_pending_value_def(env, var_store, loc_def.value, &mut scope, pattern_type) { None => { /* skip */ } Some((new_output, pending_def)) => { // store the top-level defs, used to ensure that closures won't capture them @@ -1561,7 +1560,9 @@ pub fn can_defs_with_return<'a>( // Now that we've collected all the references, check to see if any of the new idents // we defined went unused by the return expression. If any were unused, report it. for (symbol, region) in symbols_introduced { - if !output.references.has_value_lookup(symbol) && !output.references.has_type_lookup(symbol) + if !output.references.has_value_lookup(symbol) + && !output.references.has_type_lookup(symbol) + && !scope.abilities_store.is_specialization_name(symbol) { env.problem(Problem::UnusedDef(symbol, region)); } @@ -1772,7 +1773,6 @@ fn to_pending_value_def<'a>( var_store: &mut VarStore, def: &'a ast::ValueDef<'a>, scope: &mut Scope, - abilities_store: &AbilitiesStore, pattern_type: PatternType, ) -> Option<(Output, PendingValueDef<'a>)> { use ast::ValueDef::*; @@ -1784,7 +1784,6 @@ fn to_pending_value_def<'a>( env, var_store, scope, - abilities_store, pattern_type, &loc_pattern.value, loc_pattern.region, @@ -1801,7 +1800,6 @@ fn to_pending_value_def<'a>( env, var_store, scope, - abilities_store, pattern_type, &loc_pattern.value, loc_pattern.region, @@ -1832,7 +1830,6 @@ fn to_pending_value_def<'a>( env, var_store, scope, - abilities_store, pattern_type, &body_pattern.value, body_pattern.region, diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index b021dc2192..d9262ceab1 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -1083,6 +1083,7 @@ fn canonicalize_when_branch<'a>( && !branch_output.references.has_value_lookup(symbol) && !branch_output.references.has_type_lookup(symbol) && !original_scope.contains_symbol(symbol) + && !scope.abilities_store.is_specialization_name(symbol) { env.problem(Problem::UnusedDef(symbol, *region)); } diff --git a/compiler/can/src/module.rs b/compiler/can/src/module.rs index c0b01b8888..01e6664d14 100644 --- a/compiler/can/src/module.rs +++ b/compiler/can/src/module.rs @@ -1,3 +1,4 @@ +use crate::abilities::AbilitiesStore; use crate::def::{canonicalize_defs, sort_can_defs, Declaration, Def}; use crate::effect_module::HostedGeneratedFunctions; use crate::env::Env; @@ -28,11 +29,13 @@ pub struct Module { /// all aliases. `bool` indicates whether it is exposed pub aliases: MutMap, pub rigid_variables: RigidVariables, + pub abilities_store: AbilitiesStore, } #[derive(Debug, Default)] pub struct RigidVariables { pub named: MutMap, + pub able: MutMap, pub wildcards: MutSet, } @@ -250,6 +253,7 @@ pub fn canonicalize_module_defs<'a>( if !output.references.has_value_lookup(symbol) && !output.references.has_type_lookup(symbol) && !exposed_symbols.contains(&symbol) + && !scope.abilities_store.is_specialization_name(symbol) { env.problem(Problem::UnusedDef(symbol, region)); } @@ -259,6 +263,12 @@ pub fn canonicalize_module_defs<'a>( rigid_variables.named.insert(named.variable, named.name); } + for able in output.introduced_variables.able { + rigid_variables + .able + .insert(able.variable, (able.name, able.ability)); + } + for var in output.introduced_variables.wildcards { rigid_variables.wildcards.insert(var.value); } @@ -444,6 +454,10 @@ pub fn canonicalize_module_defs<'a>( aliases.insert(symbol, alias); } + for member in scope.abilities_store.root_ability_members().keys() { + exposed_but_not_defined.remove(member); + } + // By this point, all exposed symbols should have been removed from // exposed_symbols and added to exposed_vars_by_symbol. If any were // not, that means they were declared as exposed but there was diff --git a/compiler/can/src/pattern.rs b/compiler/can/src/pattern.rs index 251c889a0c..9df8cd7312 100644 --- a/compiler/can/src/pattern.rs +++ b/compiler/can/src/pattern.rs @@ -1,4 +1,3 @@ -use crate::abilities::AbilitiesStore; use crate::annotation::freshen_opaque_def; use crate::env::Env; use crate::expr::{canonicalize_expr, unescape_char, Expr, IntValue, Output}; @@ -157,7 +156,6 @@ pub fn canonicalize_def_header_pattern<'a>( env: &mut Env<'a>, var_store: &mut VarStore, scope: &mut Scope, - abilities_store: &AbilitiesStore, pattern_type: PatternType, pattern: &ast::Pattern<'a>, region: Region, @@ -168,11 +166,10 @@ pub fn canonicalize_def_header_pattern<'a>( match pattern { // Identifiers that shadow ability members may appear (and may only appear) at the header of a def. Identifier(name) => match scope.introduce_or_shadow_ability_member( - (*name).into(), + dbg!((*name).into()), &env.exposed_ident_ids, &mut env.ident_ids, region, - abilities_store, ) { Ok((symbol, shadowing_ability_member)) => { output.references.bound_symbols.insert(symbol); diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index 179f15bdf0..f08ba453a8 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -22,7 +22,7 @@ pub struct Scope { pub aliases: SendMap, /// The abilities currently in scope, and their implementors. - pub abilities: SendMap, + pub abilities_store: AbilitiesStore, /// The current module being processed. This will be used to turn /// unqualified idents into Symbols. @@ -68,7 +68,7 @@ impl Scope { symbols: SendMap::default(), aliases, // TODO(abilities): default abilities in scope - abilities: SendMap::default(), + abilities_store: AbilitiesStore::default(), } } @@ -247,16 +247,20 @@ impl Scope { exposed_ident_ids: &IdentIds, all_ident_ids: &mut IdentIds, region: Region, - abilities_store: &AbilitiesStore, ) -> Result<(Symbol, Option), (Region, Loc, Symbol)> { - match self.idents.get(&ident) { + match dbg!(self.idents.get(&ident)) { Some(&(original_symbol, original_region)) => { let shadow_ident_id = all_ident_ids.add(ident.clone()); let shadow_symbol = Symbol::new(self.home, shadow_ident_id); self.symbols.insert(shadow_symbol, region); - if abilities_store.is_ability_member_name(original_symbol) { + if self + .abilities_store + .is_ability_member_name(dbg!(original_symbol)) + { + self.abilities_store + .register_specializing_symbol(shadow_symbol, original_symbol); // Add a symbol for the shadow, but don't re-associate the member name. Ok((shadow_symbol, Some(original_symbol))) } else { diff --git a/compiler/constrain/src/module.rs b/compiler/constrain/src/module.rs index e9b622cbee..566b15cd88 100644 --- a/compiler/constrain/src/module.rs +++ b/compiler/constrain/src/module.rs @@ -1,4 +1,5 @@ use roc_builtins::std::StdLib; +use roc_can::abilities::AbilitiesStore; use roc_can::constraint::{Constraint, Constraints}; use roc_can::def::Declaration; use roc_collections::all::MutMap; @@ -100,10 +101,26 @@ pub enum ExposedModuleTypes { pub fn constrain_module( constraints: &mut Constraints, + abilities_store: &AbilitiesStore, declarations: &[Declaration], home: ModuleId, ) -> Constraint { - crate::expr::constrain_decls(constraints, home, declarations) + let mut constraint = crate::expr::constrain_decls(constraints, home, declarations); + + for (member_name, member_data) in abilities_store.root_ability_members().iter() { + constraint = constraints.let_constraint( + [], + [], + [(*member_name, Loc::at_zero(member_data.signature.clone()))], + Constraint::True, + constraint, + ); + } + + // The module constraint should always save the environment at the end. + debug_assert!(constraints.contains_save_the_environment(&constraint)); + + constraint } #[derive(Debug, Clone)] diff --git a/compiler/constrain/src/pattern.rs b/compiler/constrain/src/pattern.rs index a66dcb03a5..2672602b7b 100644 --- a/compiler/constrain/src/pattern.rs +++ b/compiler/constrain/src/pattern.rs @@ -188,9 +188,23 @@ pub fn constrain_pattern( // Erroneous patterns don't add any constraints. } - Identifier(symbol) | Shadowed(_, _, symbol) - // TODO(abilities): handle linking the member def to the specialization ident - | AbilityMemberSpecialization { + Identifier(symbol) | Shadowed(_, _, symbol) => { + if could_be_a_tag_union(expected.get_type_ref()) { + state + .constraints + .push(constraints.is_open_type(expected.get_type_ref().clone())); + } + + state.headers.insert( + *symbol, + Loc { + region, + value: expected.get_type(), + }, + ); + } + + AbilityMemberSpecialization { ident: symbol, specializes: _, } => { diff --git a/compiler/load/build.rs b/compiler/load/build.rs index dc7b09cca9..3484b412d1 100644 --- a/compiler/load/build.rs +++ b/compiler/load/build.rs @@ -36,6 +36,7 @@ fn write_subs_for_module(module_id: ModuleId, filename: &str) { &src_dir, Default::default(), target_info, + roc_reporting::report::RenderTarget::ColorTerminal, ); let module = res_module.unwrap(); diff --git a/compiler/load/src/lib.rs b/compiler/load/src/lib.rs index 3b05a22bdf..b867228cca 100644 --- a/compiler/load/src/lib.rs +++ b/compiler/load/src/lib.rs @@ -2,6 +2,7 @@ use bumpalo::Bump; use roc_collections::all::MutMap; use roc_constrain::module::ExposedByModule; use roc_module::symbol::{ModuleId, Symbol}; +use roc_reporting::report::RenderTarget; use roc_target::TargetInfo; use roc_types::subs::{Subs, Variable}; use std::path::{Path, PathBuf}; @@ -18,6 +19,7 @@ fn load<'a>( exposed_types: ExposedByModule, goal_phase: Phase, target_info: TargetInfo, + render: RenderTarget, ) -> Result, LoadingProblem<'a>> { let cached_subs = read_cached_subs(); @@ -29,6 +31,7 @@ fn load<'a>( goal_phase, target_info, cached_subs, + render, ) } @@ -39,6 +42,7 @@ pub fn load_and_monomorphize_from_str<'a>( src_dir: &Path, exposed_types: ExposedByModule, target_info: TargetInfo, + render: RenderTarget, ) -> Result, LoadingProblem<'a>> { use LoadResult::*; @@ -51,6 +55,7 @@ pub fn load_and_monomorphize_from_str<'a>( exposed_types, Phase::MakeSpecializations, target_info, + render, )? { Monomorphized(module) => Ok(module), TypeChecked(_) => unreachable!(""), @@ -63,10 +68,11 @@ pub fn load_and_monomorphize<'a>( src_dir: &Path, exposed_types: ExposedByModule, target_info: TargetInfo, + render: RenderTarget, ) -> Result, LoadingProblem<'a>> { use LoadResult::*; - let load_start = LoadStart::from_path(arena, filename)?; + let load_start = LoadStart::from_path(arena, filename, render)?; match load( arena, @@ -75,6 +81,7 @@ pub fn load_and_monomorphize<'a>( exposed_types, Phase::MakeSpecializations, target_info, + render, )? { Monomorphized(module) => Ok(module), TypeChecked(_) => unreachable!(""), @@ -87,10 +94,11 @@ pub fn load_and_typecheck<'a>( src_dir: &Path, exposed_types: ExposedByModule, target_info: TargetInfo, + render: RenderTarget, ) -> Result> { use LoadResult::*; - let load_start = LoadStart::from_path(arena, filename)?; + let load_start = LoadStart::from_path(arena, filename, render)?; match load( arena, @@ -99,6 +107,7 @@ pub fn load_and_typecheck<'a>( exposed_types, Phase::SolveTypes, target_info, + render, )? { Monomorphized(_) => unreachable!(""), TypeChecked(module) => Ok(module), diff --git a/compiler/load_internal/src/file.rs b/compiler/load_internal/src/file.rs index 27f8cb8c76..ae1a70d631 100644 --- a/compiler/load_internal/src/file.rs +++ b/compiler/load_internal/src/file.rs @@ -5,6 +5,7 @@ use crossbeam::deque::{Injector, Stealer, Worker}; use crossbeam::thread; use parking_lot::Mutex; use roc_builtins::std::borrow_stdlib; +use roc_can::abilities::AbilitiesStore; use roc_can::constraint::{Constraint as ConstraintSoa, Constraints}; use roc_can::def::Declaration; use roc_can::module::{canonicalize_module_defs, Module}; @@ -31,6 +32,7 @@ use roc_parse::ident::UppercaseIdent; use roc_parse::module::module_defs; use roc_parse::parser::{FileError, Parser, SyntaxError}; use roc_region::all::{LineInfo, Loc, Region}; +use roc_reporting::report::RenderTarget; use roc_solve::module::SolvedModule; use roc_solve::solve; use roc_target::TargetInfo; @@ -347,6 +349,7 @@ pub struct LoadedModule { pub sources: MutMap)>, pub timings: MutMap, pub documentation: MutMap, + pub abilities_store: AbilitiesStore, } impl LoadedModule { @@ -508,6 +511,7 @@ enum Msg<'a> { decls: Vec, dep_idents: MutMap, module_timing: ModuleTiming, + abilities_store: AbilitiesStore, }, FinishedAllTypeChecking { solved_subs: Solved, @@ -515,6 +519,7 @@ enum Msg<'a> { exposed_aliases_by_symbol: MutMap, dep_idents: MutMap, documentation: MutMap, + abilities_store: AbilitiesStore, }, FoundSpecializations { module_id: ModuleId, @@ -604,6 +609,8 @@ struct State<'a> { // (Granted, this has not been attempted or measured!) pub layout_caches: std::vec::Vec>, + pub render: RenderTarget, + // cached subs (used for builtin modules, could include packages in the future too) cached_subs: CachedSubs, } @@ -619,6 +626,7 @@ impl<'a> State<'a> { arc_modules: Arc>>, ident_ids_by_module: Arc>>, cached_subs: MutMap)>, + render: RenderTarget, ) -> Self { let arc_shorthands = Arc::new(Mutex::new(MutMap::default())); @@ -643,6 +651,7 @@ impl<'a> State<'a> { timings: MutMap::default(), layout_caches: std::vec::Vec::with_capacity(num_cpus::get()), cached_subs: Arc::new(Mutex::new(cached_subs)), + render, } } } @@ -824,6 +833,7 @@ pub fn load_and_typecheck_str<'a>( src_dir: &Path, exposed_types: ExposedByModule, target_info: TargetInfo, + render: RenderTarget, ) -> Result> { use LoadResult::*; @@ -841,12 +851,19 @@ pub fn load_and_typecheck_str<'a>( Phase::SolveTypes, target_info, cached_subs, + render, )? { Monomorphized(_) => unreachable!(""), TypeChecked(module) => Ok(module), } } +#[derive(Clone, Copy)] +pub enum PrintTarget { + ColorTerminal, + Generic, +} + pub struct LoadStart<'a> { arc_modules: Arc>>, ident_ids_by_module: Arc>>, @@ -855,7 +872,11 @@ pub struct LoadStart<'a> { } impl<'a> LoadStart<'a> { - pub fn from_path(arena: &'a Bump, filename: PathBuf) -> Result> { + pub fn from_path( + arena: &'a Bump, + filename: PathBuf, + render: RenderTarget, + ) -> Result> { let arc_modules = Arc::new(Mutex::new(PackageModuleIds::default())); let root_exposed_ident_ids = IdentIds::exposed_builtins(0); let ident_ids_by_module = Arc::new(Mutex::new(root_exposed_ident_ids)); @@ -887,7 +908,12 @@ impl<'a> LoadStart<'a> { // if parsing failed, this module did not add any identifiers let root_exposed_ident_ids = IdentIds::exposed_builtins(0); - let buf = to_parse_problem_report(problem, module_ids, root_exposed_ident_ids); + let buf = to_parse_problem_report( + problem, + module_ids, + root_exposed_ident_ids, + render, + ); return Err(LoadingProblem::FormattedReport(buf)); } Err(LoadingProblem::FileProblem { filename, error }) => { @@ -995,6 +1021,7 @@ pub fn load<'a>( goal_phase: Phase, target_info: TargetInfo, cached_subs: MutMap)>, + render: RenderTarget, ) -> Result, LoadingProblem<'a>> { // When compiling to wasm, we cannot spawn extra threads // so we have a single-threaded implementation @@ -1007,6 +1034,7 @@ pub fn load<'a>( goal_phase, target_info, cached_subs, + render, ) } else { load_multi_threaded( @@ -1017,6 +1045,7 @@ pub fn load<'a>( goal_phase, target_info, cached_subs, + render, ) } } @@ -1031,12 +1060,14 @@ fn load_single_threaded<'a>( goal_phase: Phase, target_info: TargetInfo, cached_subs: MutMap)>, + render: RenderTarget, ) -> Result, LoadingProblem<'a>> { let LoadStart { arc_modules, ident_ids_by_module, root_id, root_msg, + .. } = load_start; let (msg_tx, msg_rx) = bounded(1024); @@ -1053,6 +1084,7 @@ fn load_single_threaded<'a>( arc_modules, ident_ids_by_module, cached_subs, + render, ); // We'll add tasks to this, and then worker threads will take tasks from it. @@ -1115,6 +1147,7 @@ fn state_thread_step<'a>( exposed_aliases_by_symbol, dep_idents, documentation, + abilities_store, } => { // We're done! There should be no more messages pending. debug_assert!(msg_rx.is_empty()); @@ -1131,6 +1164,7 @@ fn state_thread_step<'a>( exposed_vars_by_symbol, dep_idents, documentation, + abilities_store, ); Ok(ControlFlow::Break(LoadResult::TypeChecked(typechecked))) @@ -1153,8 +1187,12 @@ fn state_thread_step<'a>( Msg::FailedToParse(problem) => { let module_ids = (*state.arc_modules).lock().clone().into_module_ids(); - let buf = - to_parse_problem_report(problem, module_ids, state.constrained_ident_ids); + let buf = to_parse_problem_report( + problem, + module_ids, + state.constrained_ident_ids, + state.render, + ); Err(LoadingProblem::FormattedReport(buf)) } msg => { @@ -1164,6 +1202,8 @@ fn state_thread_step<'a>( let constrained_ident_ids = state.constrained_ident_ids.clone(); let arc_modules = state.arc_modules.clone(); + let render = state.render; + let res_state = update( state, msg, @@ -1185,8 +1225,12 @@ fn state_thread_step<'a>( .into_inner() .into_module_ids(); - let buf = - to_parse_problem_report(problem, module_ids, constrained_ident_ids); + let buf = to_parse_problem_report( + problem, + module_ids, + constrained_ident_ids, + render, + ); Err(LoadingProblem::FormattedReport(buf)) } Err(e) => Err(e), @@ -1210,12 +1254,14 @@ fn load_multi_threaded<'a>( goal_phase: Phase, target_info: TargetInfo, cached_subs: MutMap)>, + render: RenderTarget, ) -> Result, LoadingProblem<'a>> { let LoadStart { arc_modules, ident_ids_by_module, root_id, root_msg, + .. } = load_start; let mut state = State::new( @@ -1226,6 +1272,7 @@ fn load_multi_threaded<'a>( arc_modules, ident_ids_by_module, cached_subs, + render, ); let (msg_tx, msg_rx) = bounded(1024); @@ -1746,6 +1793,7 @@ fn update<'a>( decls, dep_idents, mut module_timing, + abilities_store, } => { log!("solved types for {:?}", module_id); module_timing.end_time = SystemTime::now(); @@ -1798,6 +1846,7 @@ fn update<'a>( exposed_aliases_by_symbol: solved_module.aliases, dep_idents, documentation, + abilities_store, }) .map_err(|_| LoadingProblem::MsgChannelDied)?; @@ -2126,6 +2175,7 @@ fn finish( exposed_vars_by_symbol: Vec<(Symbol, Variable)>, dep_idents: MutMap, documentation: MutMap, + abilities_store: AbilitiesStore, ) -> LoadedModule { let module_ids = Arc::try_unwrap(state.arc_modules) .unwrap_or_else(|_| panic!("There were still outstanding Arc references to module_ids")) @@ -2160,6 +2210,7 @@ fn finish( sources, timings: state.timings, documentation, + abilities_store, } } @@ -3102,6 +3153,10 @@ fn add_imports( rigid_vars.extend(copied_import.rigid); rigid_vars.extend(copied_import.flex); + // Rigid vars bound to abilities are also treated like rigids. + rigid_vars.extend(copied_import.rigid_able); + rigid_vars.extend(copied_import.flex_able); + import_variables.extend(copied_import.registered); def_types.push(( @@ -3126,11 +3181,17 @@ fn run_solve_solve( constraint: ConstraintSoa, mut var_store: VarStore, module: Module, -) -> (Solved, Vec<(Symbol, Variable)>, Vec) { +) -> ( + Solved, + Vec<(Symbol, Variable)>, + Vec, + AbilitiesStore, +) { let Module { exposed_symbols, aliases, rigid_variables, + abilities_store, .. } = module; @@ -3155,12 +3216,13 @@ fn run_solve_solve( solve_aliases.insert(*name, alias.clone()); } - let (solved_subs, solved_env, problems) = roc_solve::module::run_solve( + let (solved_subs, solved_env, problems, abilities_store) = roc_solve::module::run_solve( &constraints, actual_constraint, rigid_variables, subs, solve_aliases, + abilities_store, ); let solved_subs = if true { @@ -3179,7 +3241,12 @@ fn run_solve_solve( .filter(|(k, _)| exposed_symbols.contains(k)) .collect(); - (solved_subs, exposed_vars_by_symbol, problems) + ( + solved_subs, + exposed_vars_by_symbol, + problems, + abilities_store, + ) } #[allow(clippy::too_many_arguments)] @@ -3203,7 +3270,7 @@ fn run_solve<'a>( // TODO remove when we write builtins in roc let aliases = module.aliases.clone(); - let (solved_subs, exposed_vars_by_symbol, problems) = { + let (solved_subs, exposed_vars_by_symbol, problems, abilities_store) = { if module_id.is_builtin() { match cached_subs.lock().remove(&module_id) { None => { @@ -3217,9 +3284,13 @@ fn run_solve<'a>( module, ) } - Some((subs, exposed_vars_by_symbol)) => { - (Solved(subs), exposed_vars_by_symbol.to_vec(), vec![]) - } + Some((subs, exposed_vars_by_symbol)) => ( + Solved(subs), + exposed_vars_by_symbol.to_vec(), + vec![], + // TODO(abilities) replace when we have abilities for builtins + AbilitiesStore::default(), + ), } } else { run_solve_solve( @@ -3258,6 +3329,7 @@ fn run_solve<'a>( dep_idents, solved_module, module_timing, + abilities_store, } } @@ -3385,8 +3457,12 @@ fn canonicalize_and_constrain<'a>( let mut constraints = Constraints::new(); - let constraint = - constrain_module(&mut constraints, &module_output.declarations, module_id); + let constraint = constrain_module( + &mut constraints, + &module_output.scope.abilities_store, + &module_output.declarations, + module_id, + ); let after = roc_types::types::get_type_clone_count(); @@ -3426,6 +3502,7 @@ fn canonicalize_and_constrain<'a>( referenced_types: module_output.referenced_types, aliases, rigid_variables: module_output.rigid_variables, + abilities_store: module_output.scope.abilities_store, }; let constrained_module = ConstrainedModule { @@ -4068,6 +4145,7 @@ fn to_parse_problem_report<'a>( problem: FileError<'a, SyntaxError<'a>>, mut module_ids: ModuleIds, all_ident_ids: MutMap, + render: RenderTarget, ) -> String { use roc_reporting::report::{parse_problem, RocDocAllocator, DEFAULT_PALETTE}; @@ -4102,7 +4180,7 @@ fn to_parse_problem_report<'a>( let mut buf = String::new(); let palette = DEFAULT_PALETTE; - report.render_color_terminal(&mut buf, &alloc, &palette); + report.render(render, &mut buf, &alloc, &palette); buf } diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 07cf05ea5e..2b7548c3bb 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -3,6 +3,7 @@ use bumpalo::collections::Vec; use bumpalo::Bump; use roc_builtins::bitcode::{FloatWidth, IntWidth}; use roc_collections::all::{default_hasher, MutMap}; +use roc_error_macros::todo_abilities; use roc_module::ident::{Lowercase, TagName}; use roc_module::symbol::{Interns, Symbol}; use roc_problem::can::RuntimeError; @@ -72,6 +73,7 @@ impl<'a> RawFunctionLayout<'a> { use roc_types::subs::Content::*; match content { FlexVar(_) | RigidVar(_) => Err(LayoutProblem::UnresolvedTypeVar(var)), + FlexAbleVar(_, _) | RigidAbleVar(_, _) => todo_abilities!("Not reachable yet"), RecursionVar { structure, .. } => { let structure_content = env.subs.get_content_without_compacting(structure); Self::new_help(env, structure, *structure_content) @@ -952,6 +954,7 @@ impl<'a> Layout<'a> { use roc_types::subs::Content::*; match content { FlexVar(_) | RigidVar(_) => Err(LayoutProblem::UnresolvedTypeVar(var)), + FlexAbleVar(_, _) | RigidAbleVar(_, _) => todo_abilities!("Not reachable yet"), RecursionVar { structure, .. } => { let structure_content = env.subs.get_content_without_compacting(structure); Self::new_help(env, structure, *structure_content) @@ -2657,6 +2660,7 @@ fn layout_from_num_content<'a>( // (e.g. for (5 + 5) assume both 5s are 64-bit integers.) Ok(Layout::default_integer()) } + FlexAbleVar(_, _) | RigidAbleVar(_, _) => todo_abilities!("Not reachable yet"), Structure(Apply(symbol, args)) => match *symbol { // Ints Symbol::NUM_NAT => Ok(Layout::usize(target_info)), diff --git a/compiler/mono/src/layout_soa.rs b/compiler/mono/src/layout_soa.rs index b4f9bd1a72..ed813d0c0a 100644 --- a/compiler/mono/src/layout_soa.rs +++ b/compiler/mono/src/layout_soa.rs @@ -137,8 +137,10 @@ impl FunctionLayout { use LayoutError::*; match content { - Content::FlexVar(_) => Err(UnresolvedVariable(var)), - Content::RigidVar(_) => Err(UnresolvedVariable(var)), + Content::FlexVar(_) + | Content::RigidVar(_) + | Content::FlexAbleVar(_, _) + | Content::RigidAbleVar(_, _) => Err(UnresolvedVariable(var)), Content::RecursionVar { .. } => Err(TypeError(())), Content::Structure(flat_type) => Self::from_flat_type(layouts, subs, flat_type), Content::Alias(_, _, actual, _) => Self::from_var_help(layouts, subs, *actual), @@ -243,8 +245,10 @@ impl LambdaSet { use LayoutError::*; match content { - Content::FlexVar(_) => Err(UnresolvedVariable(var)), - Content::RigidVar(_) => Err(UnresolvedVariable(var)), + Content::FlexVar(_) + | Content::RigidVar(_) + | Content::FlexAbleVar(_, _) + | Content::RigidAbleVar(_, _) => Err(UnresolvedVariable(var)), Content::RecursionVar { .. } => { unreachable!("lambda sets cannot currently be recursive") } @@ -627,8 +631,10 @@ impl Layout { use LayoutError::*; match content { - Content::FlexVar(_) => Err(UnresolvedVariable(var)), - Content::RigidVar(_) => Err(UnresolvedVariable(var)), + Content::FlexVar(_) + | Content::RigidVar(_) + | Content::FlexAbleVar(_, _) + | Content::RigidAbleVar(_, _) => Err(UnresolvedVariable(var)), Content::RecursionVar { structure, opt_name: _, diff --git a/compiler/solve/src/module.rs b/compiler/solve/src/module.rs index c6197065a8..c0f5f7ef94 100644 --- a/compiler/solve/src/module.rs +++ b/compiler/solve/src/module.rs @@ -1,4 +1,5 @@ use crate::solve::{self, Aliases}; +use roc_can::abilities::AbilitiesStore; use roc_can::constraint::{Constraint as ConstraintSoa, Constraints}; use roc_can::module::RigidVariables; use roc_collections::all::MutMap; @@ -32,13 +33,23 @@ pub fn run_solve( rigid_variables: RigidVariables, mut subs: Subs, mut aliases: Aliases, -) -> (Solved, solve::Env, Vec) { + mut abilities_store: AbilitiesStore, +) -> ( + Solved, + solve::Env, + Vec, + AbilitiesStore, +) { let env = solve::Env::default(); for (var, name) in rigid_variables.named { subs.rigid_var(var, name); } + for (var, (name, ability)) in rigid_variables.able { + subs.rigid_able_var(var, name, ability); + } + for var in rigid_variables.wildcards { subs.rigid_var(var, "*".into()); } @@ -55,9 +66,10 @@ pub fn run_solve( subs, &mut aliases, &constraint, + &mut abilities_store, ); - (solved_subs, solved_env, problems) + (solved_subs, solved_env, problems, abilities_store) } pub fn exposed_types_storage_subs( diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index e546730f70..8399b8cba2 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1,4 +1,5 @@ use bumpalo::Bump; +use roc_can::abilities::AbilitiesStore; use roc_can::constraint::Constraint::{self, *}; use roc_can::constraint::{Constraints, LetConstraint}; use roc_can::expected::{Expected, PExpected}; @@ -14,9 +15,9 @@ use roc_types::subs::{ use roc_types::types::Type::{self, *}; use roc_types::types::{ gather_fields_unsorted_iter, AliasCommon, AliasKind, Category, ErrorType, PatternCategory, - TypeExtension, + Reason, TypeExtension, }; -use roc_unify::unify::{unify, Mode, Unified::*}; +use roc_unify::unify::{unify, Mode, MustImplementAbility, Unified::*}; // Type checking system adapted from Elm by Evan Czaplicki, BSD-3-Clause Licensed // https://github.com/elm/compiler @@ -515,8 +516,17 @@ pub fn run( mut subs: Subs, aliases: &mut Aliases, constraint: &Constraint, + abilities_store: &mut AbilitiesStore, ) -> (Solved, Env) { - let env = run_in_place(constraints, env, problems, &mut subs, aliases, constraint); + let env = run_in_place( + constraints, + env, + problems, + &mut subs, + aliases, + constraint, + abilities_store, + ); (Solved(subs), env) } @@ -529,6 +539,7 @@ pub fn run_in_place( subs: &mut Subs, aliases: &mut Aliases, constraint: &Constraint, + abilities_store: &mut AbilitiesStore, ) -> Env { let mut pools = Pools::default(); @@ -540,6 +551,8 @@ pub fn run_in_place( let arena = Bump::new(); + let mut deferred_must_implement_abilities = Vec::new(); + let state = solve( &arena, constraints, @@ -551,8 +564,12 @@ pub fn run_in_place( aliases, subs, constraint, + abilities_store, + &mut deferred_must_implement_abilities, ); + // TODO run through and check that all abilities are properly implemented + state.env } @@ -604,6 +621,8 @@ fn solve( aliases: &mut Aliases, subs: &mut Subs, constraint: &Constraint, + abilities_store: &mut AbilitiesStore, + deferred_must_implement_abilities: &mut Vec, ) -> State { let initial = Work::Constraint { env, @@ -752,6 +771,18 @@ fn solve( let mut new_env = env.clone(); for (symbol, loc_var) in local_def_vars.iter() { + check_ability_specialization( + subs, + &new_env, + pools, + rank, + abilities_store, + problems, + deferred_must_implement_abilities, + *symbol, + *loc_var, + ); + new_env.insert_symbol_var_if_vacant(*symbol, loc_var.value); } @@ -796,12 +827,15 @@ fn solve( let expected = type_to_var(subs, rank, pools, aliases, expectation.get_type_ref()); match unify(subs, actual, expected, Mode::EQ) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impls) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadExpr( @@ -838,12 +872,15 @@ fn solve( let target = *target; match unify(subs, actual, target, Mode::EQ) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { introduce(subs, rank, pools, &vars); state } - Failure(vars, _actual_type, _expected_type) => { + Failure(vars, _actual_type, _expected_type, _bad_impls) => { introduce(subs, rank, pools, &vars); // ERROR NOT REPORTED @@ -890,13 +927,16 @@ fn solve( type_to_var(subs, rank, pools, aliases, expectation.get_type_ref()); match unify(subs, actual, expected, Mode::EQ) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impls) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadExpr( @@ -954,12 +994,15 @@ fn solve( }; match unify(subs, actual, expected, mode) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impls) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadPattern( @@ -1123,12 +1166,15 @@ fn solve( let includes = type_to_var(subs, rank, pools, aliases, &tag_ty); match unify(subs, actual, includes, Mode::PRESENT) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_to_include_type) => { + Failure(vars, actual_type, expected_to_include_type, _bad_impls) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadPattern( @@ -1156,6 +1202,92 @@ fn solve( state } +/// If a symbol claims to specialize an ability member, check that its solved type in fact +/// does specialize the ability, and record the specialization. +fn check_ability_specialization( + subs: &mut Subs, + env: &Env, + pools: &mut Pools, + rank: Rank, + abilities_store: &mut AbilitiesStore, + problems: &mut Vec, + deferred_must_implement_abilities: &mut Vec, + symbol: Symbol, + symbol_loc_var: Loc, +) { + // If the symbol specializes an ability member, we need to make sure that the + // inferred type for the specialization actually aligns with the expected + // implementation. + if let Some((root_symbol, root_data)) = abilities_store.root_name_and_def(symbol) { + let root_signature_var = env + .get_var_by_symbol(&root_symbol) + .expect("Ability should be registered in env by now!"); + + // Check if they unify - if they don't, that's a type error! + let snapshot = subs.snapshot(); + let unified = unify(subs, symbol_loc_var.value, root_signature_var, Mode::EQ); + + match unified { + Success { + vars: _, + must_implement_ability, + } => { + // We don't actually care about storing the unified type since the checked type for the + // specialization is what we want for code generation. + subs.rollback_to(snapshot); + + if must_implement_ability.is_empty() { + // This is an error.. but what kind? + } + + // First, figure out and register for what type does this symbol specialize + // the ability member. + let mut ability_implementations_for_specialization = must_implement_ability + .iter() + .filter(|mia| mia.ability == root_data.parent_ability) + .collect::>(); + ability_implementations_for_specialization.dedup(); + debug_assert_eq!( + ability_implementations_for_specialization.len(), 1, + "If there's more than one, the definition is ambiguous - this should be an error" + ); + + let specialization_type = ability_implementations_for_specialization[0].typ; + abilities_store.register_specialization_for_type(specialization_type, root_symbol); + + // Store the checks for what abilities must be implemented to be checked after the + // whole module is complete. + deferred_must_implement_abilities.extend(must_implement_ability); + } + Failure(vars, actual_type, expected_type, unimplemented_abilities) => { + subs.commit_snapshot(snapshot); + introduce(subs, rank, pools, &vars); + + let reason = Reason::InvalidAbilityMemberSpecialization { + member_name: root_symbol, + def_region: root_data.region, + unimplemented_abilities, + }; + + let problem = TypeError::BadExpr( + symbol_loc_var.region, + Category::AbilityMemberSpecialization(root_symbol), + actual_type, + Expected::ForReason(reason, expected_type, symbol_loc_var.region), + ); + + problems.push(problem); + } + BadType(vars, problem) => { + subs.commit_snapshot(snapshot); + introduce(subs, rank, pools, &vars); + + problems.push(TypeError::BadType(problem)); + } + } + } +} + #[derive(Debug)] enum LocalDefVarsVec { Stack(arrayvec::ArrayVec), @@ -1288,7 +1420,7 @@ impl RegisterVariable { use RegisterVariable::*; match typ { - Variable(var) => Direct(*var), + Type::Variable(var) => Direct(*var), EmptyRec => Direct(Variable::EMPTY_RECORD), EmptyTagUnion => Direct(Variable::EMPTY_TAG_UNION), Type::DelayedAlias(AliasCommon { symbol, .. }) => { @@ -2180,7 +2312,7 @@ fn adjust_rank_content( use roc_types::subs::FlatType::*; match content { - FlexVar(_) | RigidVar(_) | Error => group_rank, + FlexVar(_) | RigidVar(_) | FlexAbleVar(_, _) | RigidAbleVar(_, _) | Error => group_rank, RecursionVar { .. } => group_rank, @@ -2396,7 +2528,14 @@ fn instantiate_rigids_help(subs: &mut Subs, max_rank: Rank, initial: Variable) { desc.mark = Mark::NONE; desc.copy = OptVariable::NONE; } - FlexVar(_) | Error => (), + &RigidAbleVar(name, ability) => { + // Same as `RigidVar` above + desc.content = FlexAbleVar(Some(name), ability); + desc.rank = max_rank; + desc.mark = Mark::NONE; + desc.copy = OptVariable::NONE; + } + FlexVar(_) | FlexAbleVar(_, _) | Error => (), RecursionVar { structure, .. } => { stack.push(*structure); @@ -2684,7 +2823,7 @@ fn deep_copy_var_help( copy } - FlexVar(_) | Error => copy, + FlexVar(_) | FlexAbleVar(_, _) | Error => copy, RecursionVar { opt_name, @@ -2709,6 +2848,12 @@ fn deep_copy_var_help( copy } + RigidAbleVar(name, ability) => { + subs.set(copy, make_descriptor(FlexAbleVar(Some(name), ability))); + + copy + } + Alias(symbol, arguments, real_type_var, kind) => { let new_variables = SubsSlice::reserve_into_subs(subs, arguments.all_variables_len as _); diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 7e3e2e8215..6a0205be68 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -10,20 +10,12 @@ mod helpers; #[cfg(test)] mod solve_expr { use crate::helpers::with_larger_debug_stack; + use roc_load::LoadedModule; use roc_types::pretty_print::{content_to_string, name_all_type_vars}; // HELPERS - fn infer_eq_help( - src: &str, - ) -> Result< - ( - Vec, - Vec, - String, - ), - std::io::Error, - > { + fn run_load_and_infer(src: &str) -> Result { use bumpalo::Bump; use std::fs::File; use std::io::Write; @@ -58,6 +50,7 @@ mod solve_expr { dir.path(), exposed_types, roc_target::TargetInfo::default_x86_64(), + roc_reporting::report::RenderTarget::Generic, ); dir.close()?; @@ -66,8 +59,19 @@ mod solve_expr { }; let loaded = loaded.expect("failed to load module"); + Ok(loaded) + } - use roc_load::LoadedModule; + fn infer_eq_help( + src: &str, + ) -> Result< + ( + Vec, + Vec, + String, + ), + std::io::Error, + > { let LoadedModule { module_id: home, mut can_problems, @@ -76,7 +80,7 @@ mod solve_expr { mut solved, exposed_to_host, .. - } = loaded; + } = run_load_and_infer(src)?; let mut can_problems = can_problems.remove(&home).unwrap_or_default(); let type_problems = type_problems.remove(&home).unwrap_or_default(); @@ -155,6 +159,51 @@ mod solve_expr { assert_eq!(actual, expected.to_string()); } + fn check_inferred_abilities<'a, I>(src: &'a str, expected_specializations: I) + where + I: IntoIterator, + { + let LoadedModule { + module_id: home, + mut can_problems, + mut type_problems, + interns, + abilities_store, + .. + } = run_load_and_infer(src).unwrap(); + + let can_problems = can_problems.remove(&home).unwrap_or_default(); + let type_problems = type_problems.remove(&home).unwrap_or_default(); + + assert_eq!(can_problems, Vec::new(), "Canonicalization problems: "); + + if !type_problems.is_empty() { + eprintln!("{:?}", type_problems); + panic!(); + } + + let known_specializations = abilities_store.get_known_specializations(); + use std::collections::HashSet; + let pretty_specializations = known_specializations + .into_iter() + .map(|(typ, member)| { + ( + typ.ident_str(&interns).as_str(), + member.ident_str(&interns).as_str(), + ) + }) + .collect::>(); + + for expected_spec in expected_specializations.into_iter() { + assert!( + pretty_specializations.contains(&expected_spec), + "{:#?} not in {:#?}", + expected_spec, + pretty_specializations, + ); + } + } + #[test] fn int_literal() { infer_eq("5", "Num *"); @@ -5710,4 +5759,85 @@ mod solve_expr { "a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, aa, bb -> { a : a, aa : aa, b : b, bb : bb, c : c, d : d, e : e, f : f, g : g, h : h, i : i, j : j, k : k, l : l, m : m, n : n, o : o, p : p, q : q, r : r, s : s, t : t, u : u, v : v, w : w, x : x, y : y, z : z }", ) } + + #[test] + fn exposed_ability_name() { + infer_eq_without_problem( + indoc!( + r#" + app "test" provides [ hash ] to "./platform" + + Hash has hash : a -> U64 | a has Hash + "# + ), + "a -> U64 | a has Hash", + ) + } + + #[test] + fn single_ability_single_member_specializations() { + check_inferred_abilities( + indoc!( + r#" + app "test" provides [ hash ] to "./platform" + + Hash has hash : a -> U64 | a has Hash + + Id := U64 + + hash = \$Id n -> n + "# + ), + [("Id", "hash")], + ) + } + + #[test] + fn single_ability_multiple_members_specializations() { + check_inferred_abilities( + indoc!( + r#" + app "test" provides [ hash, hash32 ] to "./platform" + + Hash has + hash : a -> U64 | a has Hash + hash32 : a -> U32 | a has Hash + + Id := U64 + + hash = \$Id n -> n + hash32 = \$Id n -> Num.toU32 n + "# + ), + [("Id", "hash"), ("Id", "hash32")], + ) + } + + #[test] + fn multiple_abilities_multiple_members_specializations() { + check_inferred_abilities( + indoc!( + r#" + app "test" provides [ hash, hash32, eq, le ] to "./platform" + + Hash has + hash : a -> U64 | a has Hash + hash32 : a -> U32 | a has Hash + + Ord has + eq : a, a -> Bool | a has Ord + le : a, a -> Bool | a has Ord + + Id := U64 + + hash = \$Id n -> n + hash32 = \$Id n -> Num.toU32 n + + eq = \$Id m, $Id n -> m == n + le = \$Id m, $Id n -> m < n + "# + ), + [("Id", "hash"), ("Id", "hash32"), ("Id", "eq"), ("Id", "le")], + ) + } } diff --git a/compiler/types/src/pretty_print.rs b/compiler/types/src/pretty_print.rs index ce6b338282..e4e8456fe3 100644 --- a/compiler/types/src/pretty_print.rs +++ b/compiler/types/src/pretty_print.rs @@ -116,7 +116,7 @@ fn find_names_needed( } match &subs.get_content_without_compacting(variable).clone() { - RecursionVar { opt_name: None, .. } | FlexVar(None) => { + RecursionVar { opt_name: None, .. } | FlexVar(None) | FlexAbleVar(None, _) => { let root = subs.get_root_key_without_compacting(variable); // If this var is *not* its own root, then the @@ -139,7 +139,8 @@ fn find_names_needed( opt_name: Some(name_index), .. } - | FlexVar(Some(name_index)) => { + | FlexVar(Some(name_index)) + | FlexAbleVar(Some(name_index), _) => { // This root already has a name. Nothing more to do here! // User-defined names are already taken. @@ -147,7 +148,7 @@ fn find_names_needed( let name = subs.field_names[name_index.index as usize].clone(); names_taken.insert(name); } - RigidVar(name_index) => { + RigidVar(name_index) | RigidAbleVar(name_index, _) => { // User-defined names are already taken. // We must not accidentally generate names that collide with them! let name = subs.field_names[name_index.index as usize].clone(); @@ -289,6 +290,11 @@ fn set_root_name(root: Variable, name: Lowercase, subs: &mut Subs) { } } +#[derive(Default)] +struct Context<'a> { + able_variables: Vec<(&'a str, Symbol)>, +} + pub fn content_to_string( content: &Content, subs: &Subs, @@ -297,8 +303,16 @@ pub fn content_to_string( ) -> String { let mut buf = String::new(); let env = Env { home, interns }; + let mut ctx = Context::default(); - write_content(&env, content, subs, &mut buf, Parens::Unnecessary); + write_content(&env, &mut ctx, content, subs, &mut buf, Parens::Unnecessary); + + for (i, (var, ability)) in ctx.able_variables.into_iter().enumerate() { + buf.push_str(if i == 0 { " | " } else { ", " }); + buf.push_str(var); + buf.push_str(" has "); + write_symbol(&env, ability, &mut buf); + } buf } @@ -314,7 +328,14 @@ pub fn get_single_arg<'a>(subs: &'a Subs, args: &'a AliasVariables) -> &'a Conte subs.get_content_without_compacting(arg_var) } -fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, parens: Parens) { +fn write_content<'a>( + env: &Env, + ctx: &mut Context<'a>, + content: &Content, + subs: &'a Subs, + buf: &mut String, + parens: Parens, +) { use crate::subs::Content::*; match content { @@ -327,6 +348,18 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa let name = &subs.field_names[name_index.index as usize]; buf.push_str(name.as_str()) } + FlexAbleVar(opt_name_index, ability) => { + let name = opt_name_index + .map(|name_index| subs.field_names[name_index.index as usize].as_str()) + .unwrap_or(WILDCARD); + ctx.able_variables.push((name, *ability)); + buf.push_str(name); + } + RigidAbleVar(name_index, ability) => { + let name = subs.field_names[name_index.index as usize].as_str(); + ctx.able_variables.push((name, *ability)); + buf.push_str(name); + } RecursionVar { opt_name, .. } => match opt_name { Some(name_index) => { let name = &subs.field_names[name_index.index as usize]; @@ -334,7 +367,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa } None => buf.push_str(WILDCARD), }, - Structure(flat_type) => write_flat_type(env, flat_type, subs, buf, parens), + Structure(flat_type) => write_flat_type(env, ctx, flat_type, subs, buf, parens), Alias(symbol, args, _actual, _kind) => { let write_parens = parens == Parens::InTypeParam && !args.is_empty(); @@ -346,6 +379,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa Symbol::NUM_INTEGER => { write_integer( env, + ctx, get_single_arg(subs, &args), subs, buf, @@ -357,13 +391,13 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa _ => write_parens!(write_parens, buf, { buf.push_str("Num "); - write_content(env, content, subs, buf, parens); + write_content(env, ctx, content, subs, buf, parens); }), }, _ => write_parens!(write_parens, buf, { buf.push_str("Num "); - write_content(env, content, subs, buf, parens); + write_content(env, ctx, content, subs, buf, parens); }), } } @@ -371,7 +405,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa Symbol::NUM_INT => { let content = get_single_arg(subs, args); - write_integer(env, content, subs, buf, parens, write_parens) + write_integer(env, ctx, content, subs, buf, parens, write_parens) } Symbol::NUM_FLOAT => { @@ -390,7 +424,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa Alias(Symbol::NUM_DECIMAL, _, _, _) => buf.push_str("Dec"), _ => write_parens!(write_parens, buf, { buf.push_str("Float "); - write_content(env, content, subs, buf, parens); + write_content(env, ctx, content, subs, buf, parens); }), } } @@ -403,6 +437,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa buf.push(' '); write_content( env, + ctx, subs.get_content_without_compacting(var), subs, buf, @@ -414,7 +449,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa if false { buf.push_str("[[ but really "); let content = subs.get_content_without_compacting(*_actual); - write_content(env, content, subs, buf, parens); + write_content(env, ctx, content, subs, buf, parens); buf.push_str("]]"); } }), @@ -422,6 +457,7 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa } RangedNumber(typ, _range_vars) => write_content( env, + ctx, subs.get_content_without_compacting(*typ), subs, buf, @@ -431,10 +467,11 @@ fn write_content(env: &Env, content: &Content, subs: &Subs, buf: &mut String, pa } } -fn write_integer( +fn write_integer<'a>( env: &Env, + ctx: &mut Context<'a>, content: &Content, - subs: &Subs, + subs: &'a Subs, buf: &mut String, parens: Parens, write_parens: bool, @@ -454,7 +491,7 @@ fn write_integer( )* actual => { buf.push_str("Int "); - write_content(env, actual, subs, buf, parens); + write_content(env, ctx, actual, subs, buf, parens); } } ) @@ -497,6 +534,7 @@ impl<'a> ExtContent<'a> { fn write_ext_content<'a>( env: &Env, + ctx: &mut Context<'a>, subs: &'a Subs, buf: &mut String, ext_content: ExtContent<'a>, @@ -508,12 +546,13 @@ fn write_ext_content<'a>( // // e.g. the "*" at the end of `{ x: I64 }*` // or the "r" at the end of `{ x: I64 }r` - write_content(env, content, subs, buf, parens) + write_content(env, ctx, content, subs, buf, parens) } } fn write_sorted_tags2<'a>( env: &Env, + ctx: &mut Context<'a>, subs: &'a Subs, buf: &mut String, tags: &UnionTags, @@ -546,6 +585,7 @@ fn write_sorted_tags2<'a>( buf.push(' '); write_content( env, + ctx, subs.get_content_without_compacting(*var), subs, buf, @@ -559,6 +599,7 @@ fn write_sorted_tags2<'a>( fn write_sorted_tags<'a>( env: &Env, + ctx: &mut Context<'a>, subs: &'a Subs, buf: &mut String, tags: &MutMap>, @@ -603,6 +644,7 @@ fn write_sorted_tags<'a>( buf.push(' '); write_content( env, + ctx, subs.get_content_without_compacting(*var), subs, buf, @@ -614,18 +656,37 @@ fn write_sorted_tags<'a>( ExtContent::from_var(subs, ext_var) } -fn write_flat_type(env: &Env, flat_type: &FlatType, subs: &Subs, buf: &mut String, parens: Parens) { +fn write_flat_type<'a>( + env: &Env, + ctx: &mut Context<'a>, + flat_type: &FlatType, + subs: &'a Subs, + buf: &mut String, + parens: Parens, +) { use crate::subs::FlatType::*; match flat_type { - Apply(symbol, args) => { - write_apply(env, *symbol, subs.get_subs_slice(*args), subs, buf, parens) - } + Apply(symbol, args) => write_apply( + env, + ctx, + *symbol, + subs.get_subs_slice(*args), + subs, + buf, + parens, + ), EmptyRecord => buf.push_str(EMPTY_RECORD), EmptyTagUnion => buf.push_str(EMPTY_TAG_UNION), - Func(args, _closure, ret) => { - write_fn(env, subs.get_subs_slice(*args), *ret, subs, buf, parens) - } + Func(args, _closure, ret) => write_fn( + env, + ctx, + subs.get_subs_slice(*args), + *ret, + subs, + buf, + parens, + ), Record(fields, ext_var) => { use crate::types::{gather_fields, RecordStructure}; @@ -664,6 +725,7 @@ fn write_flat_type(env: &Env, flat_type: &FlatType, subs: &Subs, buf: &mut Strin write_content( env, + ctx, subs.get_content_without_compacting(var), subs, buf, @@ -684,18 +746,18 @@ fn write_flat_type(env: &Env, flat_type: &FlatType, subs: &Subs, buf: &mut Strin // // e.g. the "*" at the end of `{ x: I64 }*` // or the "r" at the end of `{ x: I64 }r` - write_content(env, content, subs, buf, parens) + write_content(env, ctx, content, subs, buf, parens) } } } TagUnion(tags, ext_var) => { buf.push_str("[ "); - let ext_content = write_sorted_tags2(env, subs, buf, tags, *ext_var); + let ext_content = write_sorted_tags2(env, ctx, subs, buf, tags, *ext_var); buf.push_str(" ]"); - write_ext_content(env, subs, buf, ext_content, parens) + write_ext_content(env, ctx, subs, buf, ext_content, parens) } FunctionOrTagUnion(tag_name, _, ext_var) => { @@ -703,25 +765,26 @@ fn write_flat_type(env: &Env, flat_type: &FlatType, subs: &Subs, buf: &mut Strin let mut tags: MutMap = MutMap::default(); tags.insert(subs[*tag_name].clone(), vec![]); - let ext_content = write_sorted_tags(env, subs, buf, &tags, *ext_var); + let ext_content = write_sorted_tags(env, ctx, subs, buf, &tags, *ext_var); buf.push_str(" ]"); - write_ext_content(env, subs, buf, ext_content, parens) + write_ext_content(env, ctx, subs, buf, ext_content, parens) } RecursiveTagUnion(rec_var, tags, ext_var) => { buf.push_str("[ "); - let ext_content = write_sorted_tags2(env, subs, buf, tags, *ext_var); + let ext_content = write_sorted_tags2(env, ctx, subs, buf, tags, *ext_var); buf.push_str(" ]"); - write_ext_content(env, subs, buf, ext_content, parens); + write_ext_content(env, ctx, subs, buf, ext_content, parens); buf.push_str(" as "); write_content( env, + ctx, subs.get_content_without_compacting(*rec_var), subs, buf, @@ -777,11 +840,12 @@ pub fn chase_ext_tag_union<'a>( } } -fn write_apply( +fn write_apply<'a>( env: &Env, + ctx: &mut Context<'a>, symbol: Symbol, args: &[Variable], - subs: &Subs, + subs: &'a Subs, buf: &mut String, parens: Parens, ) { @@ -805,7 +869,7 @@ fn write_apply( buf.push('('); } - write_content(env, content, subs, &mut arg_param, Parens::InTypeParam); + write_content(env, ctx, content, subs, &mut arg_param, Parens::InTypeParam); buf.push_str("Num "); buf.push_str(&arg_param); @@ -838,6 +902,7 @@ fn write_apply( buf.push(' '); write_content( env, + ctx, subs.get_content_without_compacting(*arg), subs, buf, @@ -852,11 +917,12 @@ fn write_apply( } } -fn write_fn( +fn write_fn<'a>( env: &Env, + ctx: &mut Context<'a>, args: &[Variable], ret: Variable, - subs: &Subs, + subs: &'a Subs, buf: &mut String, parens: Parens, ) { @@ -876,6 +942,7 @@ fn write_fn( write_content( env, + ctx, subs.get_content_without_compacting(*arg), subs, buf, @@ -886,6 +953,7 @@ fn write_fn( buf.push_str(" -> "); write_content( env, + ctx, subs.get_content_without_compacting(ret), subs, buf, diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 36f0ac811d..8d10405afd 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -754,7 +754,9 @@ impl<'a> fmt::Debug for SubsFmtContent<'a> { fn subs_fmt_content(this: &Content, subs: &Subs, f: &mut fmt::Formatter) -> fmt::Result { match this { Content::FlexVar(name) => write!(f, "Flex({:?})", name), + Content::FlexAbleVar(name, symbol) => write!(f, "FlexAble({:?}, {:?})", name, symbol), Content::RigidVar(name) => write!(f, "Rigid({:?})", name), + Content::RigidAbleVar(name, symbol) => write!(f, "RigidAble({:?}, {:?})", name, symbol), Content::RecursionVar { structure, opt_name, @@ -794,7 +796,19 @@ fn subs_fmt_flat_type(this: &FlatType, subs: &Subs, f: &mut fmt::Formatter) -> f } FlatType::Func(arguments, lambda_set, result) => { let slice = subs.get_subs_slice(*arguments); - write!(f, "Func({:?}, {:?}, {:?})", slice, lambda_set, result) + write!(f, "Func([")?; + for var in slice { + let content = subs.get_content_without_compacting(*var); + write!(f, "<{:?}>{:?},", *var, SubsFmtContent(content, subs))?; + } + let result_content = subs.get_content_without_compacting(*result); + write!( + f, + "], {:?}, <{:?}>{:?})", + lambda_set, + *result, + SubsFmtContent(result_content, subs) + ) } FlatType::Record(fields, ext) => { write!(f, "{{ ")?; @@ -1737,6 +1751,14 @@ impl Subs { self.set(var, desc); } + pub fn rigid_able_var(&mut self, var: Variable, name: Lowercase, ability: Symbol) { + let name_index = SubsIndex::push_new(&mut self.field_names, name); + let content = Content::RigidAbleVar(name_index, ability); + let desc = Descriptor::from(content); + + self.set(var, desc); + } + /// Unions two keys without the possibility of failure. pub fn union(&mut self, left: Variable, right: Variable, desc: Descriptor) { let l_root = self.utable.inlined_get_root_key(left); @@ -2118,6 +2140,12 @@ pub enum Content { FlexVar(Option>), /// name given in a user-written annotation RigidVar(SubsIndex), + /// Like a [Self::FlexVar], but is also bound to an ability. + /// This can only happen when unified with a [Self::RigidAbleVar]. + FlexAbleVar(Option>, Symbol), + /// Like a [Self::RigidVar], but is also bound to an ability. + /// For example, "a has Hash". + RigidAbleVar(SubsIndex, Symbol), /// name given to a recursion variable RecursionVar { structure: Variable, @@ -2838,7 +2866,12 @@ fn occurs( Err((root_var, vec![])) } else { match subs.get_content_without_compacting(root_var) { - FlexVar(_) | RigidVar(_) | RecursionVar { .. } | Error => Ok(()), + FlexVar(_) + | RigidVar(_) + | FlexAbleVar(_, _) + | RigidAbleVar(_, _) + | RecursionVar { .. } + | Error => Ok(()), Structure(flat_type) => { let mut new_seen = seen.to_owned(); @@ -2966,7 +2999,12 @@ fn explicit_substitute( to } else { match subs.get(in_var).content { - FlexVar(_) | RigidVar(_) | RecursionVar { .. } | Error => in_var, + FlexVar(_) + | RigidVar(_) + | FlexAbleVar(_, _) + | RigidAbleVar(_, _) + | RecursionVar { .. } + | Error => in_var, Structure(flat_type) => { match flat_type { @@ -3134,9 +3172,9 @@ fn get_var_names( subs.set_mark(var, Mark::GET_VAR_NAMES); match desc.content { - Error | FlexVar(None) => taken_names, + Error | FlexVar(None) | FlexAbleVar(None, _) => taken_names, - FlexVar(Some(name_index)) => add_name( + FlexVar(Some(name_index)) | FlexAbleVar(Some(name_index), _) => add_name( subs, 0, name_index, @@ -3163,7 +3201,9 @@ fn get_var_names( None => taken_names, }, - RigidVar(name_index) => add_name(subs, 0, name_index, var, RigidVar, taken_names), + RigidVar(name_index) | RigidAbleVar(name_index, _) => { + add_name(subs, 0, name_index, var, RigidVar, taken_names) + } Alias(_, args, _, _) => args.into_iter().fold(taken_names, |answer, arg_var| { get_var_names(subs, subs[arg_var], answer) @@ -3329,11 +3369,6 @@ fn content_to_err_type( match content { Structure(flat_type) => flat_type_to_err_type(subs, state, flat_type), - FlexVar(Some(name_index)) => { - let name = subs.field_names[name_index.index as usize].clone(); - ErrorType::FlexVar(name) - } - FlexVar(opt_name) => { let name = match opt_name { Some(name_index) => subs.field_names[name_index.index as usize].clone(), @@ -3356,6 +3391,28 @@ fn content_to_err_type( ErrorType::RigidVar(name) } + FlexAbleVar(opt_name, ability) => { + let name = match opt_name { + Some(name_index) => subs.field_names[name_index.index as usize].clone(), + None => { + // set the name so when this variable occurs elsewhere in the type it gets the same name + let name = get_fresh_var_name(state); + let name_index = SubsIndex::push_new(&mut subs.field_names, name.clone()); + + subs.set_content(var, FlexVar(Some(name_index))); + + name + } + }; + + ErrorType::FlexAbleVar(name, ability) + } + + RigidAbleVar(name_index, ability) => { + let name = subs.field_names[name_index.index as usize].clone(); + ErrorType::RigidAbleVar(name, ability) + } + RecursionVar { opt_name, .. } => { let name = match opt_name { Some(name_index) => subs.field_names[name_index.index as usize].clone(), @@ -3628,7 +3685,7 @@ fn restore_help(subs: &mut Subs, initial: Variable) { use FlatType::*; match &desc.content { - FlexVar(_) | RigidVar(_) | Error => (), + FlexVar(_) | RigidVar(_) | FlexAbleVar(_, _) | RigidAbleVar(_, _) | Error => (), RecursionVar { structure, .. } => { stack.push(*structure); @@ -3857,6 +3914,8 @@ impl StorageSubs { match content { FlexVar(opt_name) => FlexVar(*opt_name), RigidVar(name) => RigidVar(*name), + FlexAbleVar(opt_name, ability) => FlexAbleVar(*opt_name, *ability), + RigidAbleVar(name, ability) => RigidAbleVar(*name, *ability), RecursionVar { structure, opt_name, @@ -4253,6 +4312,29 @@ fn deep_copy_var_to_help(env: &mut DeepCopyVarToEnv<'_>, var: Variable) -> Varia copy } + FlexAbleVar(opt_name_index, ability) => { + let new_name_index = opt_name_index.map(|name_index| { + let name = env.source.field_names[name_index.index as usize].clone(); + SubsIndex::push_new(&mut env.target.field_names, name) + }); + + let content = FlexAbleVar(new_name_index, ability); + env.target.set_content(copy, content); + + copy + } + + RigidAbleVar(name_index, ability) => { + let name = env.source.field_names[name_index.index as usize].clone(); + let new_name_index = SubsIndex::push_new(&mut env.target.field_names, name); + env.target.set( + copy, + make_descriptor(FlexAbleVar(Some(new_name_index), ability)), + ); + + copy + } + Alias(symbol, arguments, real_type_var, kind) => { let new_variables = SubsSlice::reserve_into_subs(env.target, arguments.all_variables_len as _); @@ -4312,6 +4394,8 @@ pub struct CopiedImport { pub variable: Variable, pub flex: Vec, pub rigid: Vec, + pub flex_able: Vec, + pub rigid_able: Vec, pub translations: Vec<(Variable, Variable)>, pub registered: Vec, } @@ -4322,6 +4406,8 @@ struct CopyImportEnv<'a> { target: &'a mut Subs, flex: Vec, rigid: Vec, + flex_able: Vec, + rigid_able: Vec, translations: Vec<(Variable, Variable)>, registered: Vec, } @@ -4343,6 +4429,8 @@ pub fn copy_import_to( target, flex: Vec::new(), rigid: Vec::new(), + flex_able: Vec::new(), + rigid_able: Vec::new(), translations: Vec::new(), registered: Vec::new(), }; @@ -4354,6 +4442,8 @@ pub fn copy_import_to( source, flex, rigid, + flex_able, + rigid_able, translations, registered, target: _, @@ -4376,6 +4466,8 @@ pub fn copy_import_to( variable: copy, flex, rigid, + flex_able, + rigid_able, translations, registered, } @@ -4393,7 +4485,10 @@ pub fn copy_import_to( /// standard variables fn is_registered(content: &Content) -> bool { match content { - Content::FlexVar(_) | Content::RigidVar(_) => false, + Content::FlexVar(_) + | Content::RigidVar(_) + | Content::FlexAbleVar(..) + | Content::RigidAbleVar(..) => false, Content::Structure(FlatType::EmptyRecord | FlatType::EmptyTagUnion) => false, Content::Structure(_) @@ -4631,6 +4726,20 @@ fn copy_import_to_help(env: &mut CopyImportEnv<'_>, max_rank: Rank, var: Variabl copy } + FlexAbleVar(opt_name_index, ability) => { + if let Some(name_index) = opt_name_index { + let name = env.source.field_names[name_index.index as usize].clone(); + let new_name_index = SubsIndex::push_new(&mut env.target.field_names, name); + + let content = FlexAbleVar(Some(new_name_index), ability); + env.target.set_content(copy, content); + } + + env.flex_able.push(copy); + + copy + } + Error => { // Open question: should this return Error, or a Flex var? @@ -4653,6 +4762,20 @@ fn copy_import_to_help(env: &mut CopyImportEnv<'_>, max_rank: Rank, var: Variabl copy } + RigidAbleVar(name_index, ability) => { + let name = env.source.field_names[name_index.index as usize].clone(); + let new_name_index = SubsIndex::push_new(&mut env.target.field_names, name); + + env.target + .set(copy, make_descriptor(RigidAbleVar(new_name_index, ability))); + + env.rigid_able.push(copy); + + env.translations.push((var, copy)); + + copy + } + RecursionVar { opt_name, structure, @@ -4746,7 +4869,7 @@ where use Content::*; use FlatType::*; match content { - FlexVar(_) | RigidVar(_) => {} + FlexVar(_) | RigidVar(_) | FlexAbleVar(_, _) | RigidAbleVar(_, _) => {} RecursionVar { structure, opt_name: _, diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index eca1b3bc66..846ef8817d 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -1744,6 +1744,11 @@ pub enum Reason { RecordUpdateKeys(Symbol, SendMap), RecordDefaultField(Lowercase), NumericLiteralSuffix, + InvalidAbilityMemberSpecialization { + member_name: Symbol, + def_region: Region, + unimplemented_abilities: DoesNotImplementAbility, + }, } #[derive(PartialEq, Debug, Clone)] @@ -1783,6 +1788,8 @@ pub enum Category { Accessor(Lowercase), Access(Lowercase), DefaultValue(Lowercase), // for setting optional fields + + AbilityMemberSpecialization(Symbol), } #[derive(Debug, Clone, PartialEq, Eq)] @@ -1867,14 +1874,19 @@ pub enum Mismatch { InconsistentWhenBranches, CanonicalizationProblem, TypeNotInRange, + DoesNotImplementAbiity(Variable, Symbol), } +pub type DoesNotImplementAbility = Vec<(ErrorType, Symbol)>; + #[derive(PartialEq, Eq, Clone, Hash)] pub enum ErrorType { Infinite, Type(Symbol, Vec), FlexVar(Lowercase), RigidVar(Lowercase), + FlexAbleVar(Lowercase, Symbol), + RigidAbleVar(Lowercase, Symbol), Record(SendMap>, TypeExt), TagUnion(SendMap>, TypeExt), RecursiveTagUnion(Box, SendMap>, TypeExt), @@ -1905,10 +1917,7 @@ impl ErrorType { match self { Infinite => {} Type(_, ts) => ts.iter().for_each(|t| t.add_names(taken)), - FlexVar(v) => { - taken.insert(v.clone()); - } - RigidVar(v) => { + FlexVar(v) | RigidVar(v) | FlexAbleVar(v, _) | RigidAbleVar(v, _) => { taken.insert(v.clone()); } Record(fields, ext) => { @@ -2087,8 +2096,18 @@ fn write_debug_error_type_help(error_type: ErrorType, buf: &mut String, parens: match error_type { Infinite => buf.push('∞'), Error => buf.push('?'), - FlexVar(name) => buf.push_str(name.as_str()), - RigidVar(name) => buf.push_str(name.as_str()), + FlexVar(name) | RigidVar(name) => buf.push_str(name.as_str()), + FlexAbleVar(name, symbol) | RigidAbleVar(name, symbol) => { + let write_parens = parens == Parens::InTypeParam; + if write_parens { + buf.push('('); + } + buf.push_str(name.as_str()); + buf.push_str(&format!(" has {:?}", symbol)); + if write_parens { + buf.push(')'); + } + } Type(symbol, arguments) => { let write_parens = parens == Parens::InTypeParam && !arguments.is_empty(); diff --git a/compiler/unify/src/unify.rs b/compiler/unify/src/unify.rs index 82a96045b5..a586cdaed7 100644 --- a/compiler/unify/src/unify.rs +++ b/compiler/unify/src/unify.rs @@ -1,4 +1,5 @@ use bitflags::bitflags; +use roc_error_macros::todo_abilities; use roc_module::ident::{Lowercase, TagName}; use roc_module::symbol::Symbol; use roc_types::subs::Content::{self, *}; @@ -6,7 +7,7 @@ use roc_types::subs::{ AliasVariables, Descriptor, ErrorTypeContext, FlatType, GetSubsSlice, Mark, OptVariable, RecordFields, Subs, SubsIndex, SubsSlice, UnionTags, Variable, VariableSubsSlice, }; -use roc_types::types::{AliasKind, ErrorType, Mismatch, RecordField}; +use roc_types::types::{AliasKind, DoesNotImplementAbility, ErrorType, Mismatch, RecordField}; macro_rules! mismatch { () => {{ @@ -19,7 +20,10 @@ macro_rules! mismatch { ); } - vec![Mismatch::TypeMismatch] + Outcome { + mismatches: vec![Mismatch::TypeMismatch], + ..Outcome::default() + } }}; ($msg:expr) => {{ if cfg!(debug_assertions) && std::env::var("ROC_PRINT_MISMATCHES").is_ok() { @@ -34,7 +38,10 @@ macro_rules! mismatch { } - vec![Mismatch::TypeMismatch] + Outcome { + mismatches: vec![Mismatch::TypeMismatch], + ..Outcome::default() + } }}; ($msg:expr,) => {{ mismatch!($msg) @@ -51,8 +58,28 @@ macro_rules! mismatch { println!(""); } - vec![Mismatch::TypeMismatch] + Outcome { + mismatches: vec![Mismatch::TypeMismatch], + ..Outcome::default() + } }}; + (%not_able, $var:expr, $ability:expr, $msg:expr, $($arg:tt)*) => {{ + if cfg!(debug_assertions) && std::env::var("ROC_PRINT_MISMATCHES").is_ok() { + println!( + "Mismatch in {} Line {} Column {}", + file!(), + line!(), + column!() + ); + println!($msg, $($arg)*); + println!(""); + } + + Outcome { + mismatches: vec![Mismatch::TypeMismatch, Mismatch::DoesNotImplementAbiity($var, $ability)], + ..Outcome::default() + } + }} } type Pool = Vec; @@ -105,20 +132,52 @@ pub struct Context { #[derive(Debug)] pub enum Unified { - Success(Pool), - Failure(Pool, ErrorType, ErrorType), + Success { + vars: Pool, + must_implement_ability: Vec, + }, + Failure(Pool, ErrorType, ErrorType, DoesNotImplementAbility), BadType(Pool, roc_types::types::Problem), } -type Outcome = Vec; +/// Specifies that `type` must implement the ability `ability`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MustImplementAbility { + // This only points to opaque type names currently. + // TODO(abilities) support structural types in general + pub typ: Symbol, + pub ability: Symbol, +} + +#[derive(Debug, Default)] +pub struct Outcome { + mismatches: Vec, + /// We defer these checks until the end of a solving phase. + /// NOTE: this vector is almost always empty! + must_implement_ability: Vec, +} + +impl Outcome { + fn union(&mut self, other: Self) { + self.mismatches.extend(other.mismatches); + self.must_implement_ability + .extend(other.must_implement_ability); + } +} #[inline(always)] pub fn unify(subs: &mut Subs, var1: Variable, var2: Variable, mode: Mode) -> Unified { let mut vars = Vec::new(); - let mismatches = unify_pool(subs, &mut vars, var1, var2, mode); + let Outcome { + mismatches, + must_implement_ability, + } = unify_pool(subs, &mut vars, var1, var2, mode); if mismatches.is_empty() { - Unified::Success(vars) + Unified::Success { + vars, + must_implement_ability, + } } else { let error_context = if mismatches.contains(&Mismatch::TypeNotInRange) { ErrorTypeContext::ExpandRanges @@ -136,7 +195,19 @@ pub fn unify(subs: &mut Subs, var1: Variable, var2: Variable, mode: Mode) -> Uni if !problems.is_empty() { Unified::BadType(vars, problems.remove(0)) } else { - Unified::Failure(vars, type1, type2) + let do_not_implement_ability = mismatches + .into_iter() + .filter_map(|mismatch| match mismatch { + Mismatch::DoesNotImplementAbiity(var, ab) => { + let (err_type, _new_problems) = + subs.var_to_error_type_contextual(var, error_context); + Some((err_type, ab)) + } + _ => None, + }) + .collect(); + + Unified::Failure(vars, type1, type2, do_not_implement_ability) } } } @@ -150,7 +221,7 @@ pub fn unify_pool( mode: Mode, ) -> Outcome { if subs.equivalent(var1, var2) { - Vec::new() + Outcome::default() } else { let ctx = Context { first: var1, @@ -191,7 +262,14 @@ fn unify_context(subs: &mut Subs, pool: &mut Pool, ctx: Context) -> Outcome { ); } match &ctx.first_desc.content { - FlexVar(opt_name) => unify_flex(subs, &ctx, opt_name, &ctx.second_desc.content), + FlexVar(opt_name) => unify_flex(subs, &ctx, opt_name, None, &ctx.second_desc.content), + FlexAbleVar(opt_name, ability) => unify_flex( + subs, + &ctx, + opt_name, + Some(*ability), + &ctx.second_desc.content, + ), RecursionVar { opt_name, structure, @@ -203,7 +281,10 @@ fn unify_context(subs: &mut Subs, pool: &mut Pool, ctx: Context) -> Outcome { *structure, &ctx.second_desc.content, ), - RigidVar(name) => unify_rigid(subs, &ctx, name, &ctx.second_desc.content), + RigidVar(name) => unify_rigid(subs, &ctx, name, None, &ctx.second_desc.content), + RigidAbleVar(name, ability) => { + unify_rigid(subs, &ctx, name, Some(*ability), &ctx.second_desc.content) + } Structure(flat_type) => { unify_structure(subs, pool, &ctx, flat_type, &ctx.second_desc.content) } @@ -238,7 +319,7 @@ fn unify_ranged_number( } &RangedNumber(other_real_var, other_range_vars) => { let outcome = unify_pool(subs, pool, real_var, other_real_var, ctx.mode); - if outcome.is_empty() { + if outcome.mismatches.is_empty() { check_valid_range(subs, pool, ctx.first, other_range_vars, ctx.mode) } else { outcome @@ -246,9 +327,12 @@ fn unify_ranged_number( // TODO: We should probably check that "range_vars" and "other_range_vars" intersect } Error => merge(subs, ctx, Error), + FlexAbleVar(..) | RigidAbleVar(..) => { + todo_abilities!("I don't think this can be reached yet") + } }; - if !outcome.is_empty() { + if !outcome.mismatches.is_empty() { return outcome; } @@ -269,11 +353,11 @@ fn check_valid_range( let snapshot = subs.snapshot(); let old_pool = pool.clone(); let outcome = unify_pool(subs, pool, var, possible_var, mode | Mode::RIGID_AS_FLEX); - if outcome.is_empty() { + if outcome.mismatches.is_empty() { // Okay, we matched some type in the range. subs.rollback_to(snapshot); *pool = old_pool; - return vec![]; + return Outcome::default(); } else if it.peek().is_some() { // We failed to match something in the range, but there are still things we can try. subs.rollback_to(snapshot); @@ -283,7 +367,10 @@ fn check_valid_range( } } - return vec![Mismatch::TypeNotInRange]; + Outcome { + mismatches: vec![Mismatch::TypeNotInRange], + ..Outcome::default() + } } #[inline(always)] @@ -310,13 +397,19 @@ fn unify_alias( unify_pool(subs, pool, real_var, *structure, ctx.mode) } RigidVar(_) => unify_pool(subs, pool, real_var, ctx.second, ctx.mode), + RigidAbleVar (_, ability) | FlexAbleVar(_, ability) if kind == AliasKind::Opaque && args.is_empty() => { + // Opaque type wins + let mut outcome = merge(subs, ctx, Alias(symbol, args, real_var, kind)); + outcome.must_implement_ability.push(MustImplementAbility { typ: symbol, ability: *ability }); + outcome + } Alias(other_symbol, other_args, other_real_var, _) // Opaques types are only equal if the opaque symbols are equal! if !either_is_opaque || symbol == *other_symbol => { if symbol == *other_symbol { if args.len() == other_args.len() { - let mut problems = Vec::new(); + let mut outcome = Outcome::default(); let it = args .all_variables() .into_iter() @@ -327,23 +420,23 @@ fn unify_alias( for (l, r) in it { let l_var = subs[l]; let r_var = subs[r]; - problems.extend(unify_pool(subs, pool, l_var, r_var, ctx.mode)); + outcome.union(unify_pool(subs, pool, l_var, r_var, ctx.mode)); } - if problems.is_empty() { - problems.extend(merge(subs, ctx, *other_content)); + if outcome.mismatches.is_empty() { + outcome.union(merge(subs, ctx, *other_content)); } let args_unification_had_changes = !subs.vars_since_snapshot(&args_unification_snapshot).is_empty(); subs.commit_snapshot(args_unification_snapshot); - if !args.is_empty() && args_unification_had_changes && problems.is_empty() { + if !args.is_empty() && args_unification_had_changes && outcome.mismatches.is_empty() { // We need to unify the real vars because unification of type variables // may have made them larger, which then needs to be reflected in the `real_var`. - problems.extend(unify_pool(subs, pool, real_var, *other_real_var, ctx.mode)); + outcome.union(unify_pool(subs, pool, real_var, *other_real_var, ctx.mode)); } - problems + outcome } else { dbg!(args.len(), other_args.len()); mismatch!("{:?}", symbol) @@ -355,7 +448,7 @@ fn unify_alias( Structure(_) if !either_is_opaque => unify_pool(subs, pool, real_var, ctx.second, ctx.mode), RangedNumber(other_real_var, other_range_vars) if !either_is_opaque => { let outcome = unify_pool(subs, pool, real_var, *other_real_var, ctx.mode); - if outcome.is_empty() { + if outcome.mismatches.is_empty() { check_valid_range(subs, pool, real_var, *other_range_vars, ctx.mode) } else { outcome @@ -448,13 +541,31 @@ fn unify_structure( }, RangedNumber(other_real_var, other_range_vars) => { let outcome = unify_pool(subs, pool, ctx.first, *other_real_var, ctx.mode); - if outcome.is_empty() { + if outcome.mismatches.is_empty() { check_valid_range(subs, pool, ctx.first, *other_range_vars, ctx.mode) } else { outcome } } Error => merge(subs, ctx, Error), + + FlexAbleVar(_, ability) => { + // TODO(abilities) support structural types in ability bounds + mismatch!( + %not_able, ctx.first, *ability, + "trying to unify {:?} with FlexAble {:?}", + &flat_type, + &other + ) + } + RigidAbleVar(_, ability) => { + mismatch!( + %not_able, ctx.first, *ability, + "trying to unify {:?} with RigidAble {:?}", + &flat_type, + &other + ) + } } } @@ -474,29 +585,29 @@ fn unify_record( if separate.only_in_1.is_empty() { if separate.only_in_2.is_empty() { // these variable will be the empty record, but we must still unify them - let ext_problems = unify_pool(subs, pool, ext1, ext2, ctx.mode); + let ext_outcome = unify_pool(subs, pool, ext1, ext2, ctx.mode); - if !ext_problems.is_empty() { - return ext_problems; + if !ext_outcome.mismatches.is_empty() { + return ext_outcome; } - let mut field_problems = + let mut field_outcome = unify_shared_fields(subs, pool, ctx, shared_fields, OtherFields::None, ext1); - field_problems.extend(ext_problems); + field_outcome.union(ext_outcome); - field_problems + field_outcome } else { let only_in_2 = RecordFields::insert_into_subs(subs, separate.only_in_2); let flat_type = FlatType::Record(only_in_2, ext2); let sub_record = fresh(subs, pool, ctx, Structure(flat_type)); - let ext_problems = unify_pool(subs, pool, ext1, sub_record, ctx.mode); + let ext_outcome = unify_pool(subs, pool, ext1, sub_record, ctx.mode); - if !ext_problems.is_empty() { - return ext_problems; + if !ext_outcome.mismatches.is_empty() { + return ext_outcome; } - let mut field_problems = unify_shared_fields( + let mut field_outcome = unify_shared_fields( subs, pool, ctx, @@ -505,21 +616,21 @@ fn unify_record( sub_record, ); - field_problems.extend(ext_problems); + field_outcome.union(ext_outcome); - field_problems + field_outcome } } else if separate.only_in_2.is_empty() { let only_in_1 = RecordFields::insert_into_subs(subs, separate.only_in_1); let flat_type = FlatType::Record(only_in_1, ext1); let sub_record = fresh(subs, pool, ctx, Structure(flat_type)); - let ext_problems = unify_pool(subs, pool, sub_record, ext2, ctx.mode); + let ext_outcome = unify_pool(subs, pool, sub_record, ext2, ctx.mode); - if !ext_problems.is_empty() { - return ext_problems; + if !ext_outcome.mismatches.is_empty() { + return ext_outcome; } - let mut field_problems = unify_shared_fields( + let mut field_outcome = unify_shared_fields( subs, pool, ctx, @@ -528,9 +639,9 @@ fn unify_record( sub_record, ); - field_problems.extend(ext_problems); + field_outcome.union(ext_outcome); - field_problems + field_outcome } else { let only_in_1 = RecordFields::insert_into_subs(subs, separate.only_in_1); let only_in_2 = RecordFields::insert_into_subs(subs, separate.only_in_2); @@ -544,24 +655,26 @@ fn unify_record( let sub1 = fresh(subs, pool, ctx, Structure(flat_type1)); let sub2 = fresh(subs, pool, ctx, Structure(flat_type2)); - let rec1_problems = unify_pool(subs, pool, ext1, sub2, ctx.mode); - if !rec1_problems.is_empty() { - return rec1_problems; + let rec1_outcome = unify_pool(subs, pool, ext1, sub2, ctx.mode); + if !rec1_outcome.mismatches.is_empty() { + return rec1_outcome; } - let rec2_problems = unify_pool(subs, pool, sub1, ext2, ctx.mode); - if !rec2_problems.is_empty() { - return rec2_problems; + let rec2_outcome = unify_pool(subs, pool, sub1, ext2, ctx.mode); + if !rec2_outcome.mismatches.is_empty() { + return rec2_outcome; } - let mut field_problems = + let mut field_outcome = unify_shared_fields(subs, pool, ctx, shared_fields, other_fields, ext); - field_problems.reserve(rec1_problems.len() + rec2_problems.len()); - field_problems.extend(rec1_problems); - field_problems.extend(rec2_problems); + field_outcome + .mismatches + .reserve(rec1_outcome.mismatches.len() + rec2_outcome.mismatches.len()); + field_outcome.union(rec1_outcome); + field_outcome.union(rec2_outcome); - field_problems + field_outcome } } @@ -584,7 +697,7 @@ fn unify_shared_fields( let num_shared_fields = shared_fields.len(); for (name, (actual, expected)) in shared_fields { - let local_problems = unify_pool( + let local_outcome = unify_pool( subs, pool, actual.into_inner(), @@ -592,7 +705,7 @@ fn unify_shared_fields( ctx.mode, ); - if local_problems.is_empty() { + if local_outcome.mismatches.is_empty() { use RecordField::*; // Unification of optional fields @@ -856,18 +969,18 @@ fn unify_tag_union_new( if separate.only_in_1.is_empty() { if separate.only_in_2.is_empty() { - let ext_problems = if ctx.mode.is_eq() { + let ext_outcome = if ctx.mode.is_eq() { unify_pool(subs, pool, ext1, ext2, ctx.mode) } else { // In a presence context, we don't care about ext2 being equal to ext1 - vec![] + Outcome::default() }; - if !ext_problems.is_empty() { - return ext_problems; + if !ext_outcome.mismatches.is_empty() { + return ext_outcome; } - let mut tag_problems = unify_shared_tags_new( + let mut shared_tags_outcome = unify_shared_tags_new( subs, pool, ctx, @@ -877,20 +990,20 @@ fn unify_tag_union_new( recursion_var, ); - tag_problems.extend(ext_problems); + shared_tags_outcome.union(ext_outcome); - tag_problems + shared_tags_outcome } else { let unique_tags2 = UnionTags::insert_slices_into_subs(subs, separate.only_in_2); let flat_type = FlatType::TagUnion(unique_tags2, ext2); let sub_record = fresh(subs, pool, ctx, Structure(flat_type)); - let ext_problems = unify_pool(subs, pool, ext1, sub_record, ctx.mode); + let ext_outcome = unify_pool(subs, pool, ext1, sub_record, ctx.mode); - if !ext_problems.is_empty() { - return ext_problems; + if !ext_outcome.mismatches.is_empty() { + return ext_outcome; } - let mut tag_problems = unify_shared_tags_new( + let mut shared_tags_outcome = unify_shared_tags_new( subs, pool, ctx, @@ -900,9 +1013,9 @@ fn unify_tag_union_new( recursion_var, ); - tag_problems.extend(ext_problems); + shared_tags_outcome.union(ext_outcome); - tag_problems + shared_tags_outcome } } else if separate.only_in_2.is_empty() { let unique_tags1 = UnionTags::insert_slices_into_subs(subs, separate.only_in_1); @@ -911,10 +1024,10 @@ fn unify_tag_union_new( // In a presence context, we don't care about ext2 being equal to tags1 if ctx.mode.is_eq() { - let ext_problems = unify_pool(subs, pool, sub_record, ext2, ctx.mode); + let ext_outcome = unify_pool(subs, pool, sub_record, ext2, ctx.mode); - if !ext_problems.is_empty() { - return ext_problems; + if !ext_outcome.mismatches.is_empty() { + return ext_outcome; } } @@ -961,17 +1074,17 @@ fn unify_tag_union_new( let snapshot = subs.snapshot(); - let ext1_problems = unify_pool(subs, pool, ext1, sub2, ctx.mode); - if !ext1_problems.is_empty() { + let ext1_outcome = unify_pool(subs, pool, ext1, sub2, ctx.mode); + if !ext1_outcome.mismatches.is_empty() { subs.rollback_to(snapshot); - return ext1_problems; + return ext1_outcome; } if ctx.mode.is_eq() { - let ext2_problems = unify_pool(subs, pool, sub1, ext2, ctx.mode); - if !ext2_problems.is_empty() { + let ext2_outcome = unify_pool(subs, pool, sub1, ext2, ctx.mode); + if !ext2_outcome.mismatches.is_empty() { subs.rollback_to(snapshot); - return ext2_problems; + return ext2_outcome; } } @@ -1063,17 +1176,17 @@ fn unify_shared_tags_new( maybe_mark_tag_union_recursive(subs, actual); maybe_mark_tag_union_recursive(subs, expected); - let mut problems = Vec::new(); + let mut outcome = Outcome::default(); - problems.extend(unify_pool(subs, pool, actual, expected, ctx.mode)); + outcome.union(unify_pool(subs, pool, actual, expected, ctx.mode)); // clearly, this is very suspicious: these variables have just been unified. And yet, // not doing this leads to stack overflows if let Rec::Right(_) = recursion_var { - if problems.is_empty() { + if outcome.mismatches.is_empty() { matching_vars.push(expected); } - } else if problems.is_empty() { + } else if outcome.mismatches.is_empty() { matching_vars.push(actual); } } @@ -1215,39 +1328,43 @@ fn unify_flat_type( debug_assert!(is_recursion_var(subs, *rec2)); let rec = Rec::Both(*rec1, *rec2); - let mut problems = + let mut outcome = unify_tag_union_new(subs, pool, ctx, *tags1, *ext1, *tags2, *ext2, rec); - problems.extend(unify_pool(subs, pool, *rec1, *rec2, ctx.mode)); + outcome.union(unify_pool(subs, pool, *rec1, *rec2, ctx.mode)); - problems + outcome } (Apply(l_symbol, l_args), Apply(r_symbol, r_args)) if l_symbol == r_symbol => { - let problems = unify_zip_slices(subs, pool, *l_args, *r_args); + let mut outcome = unify_zip_slices(subs, pool, *l_args, *r_args); - if problems.is_empty() { - merge(subs, ctx, Structure(Apply(*r_symbol, *r_args))) - } else { - problems + if outcome.mismatches.is_empty() { + outcome.union(merge(subs, ctx, Structure(Apply(*r_symbol, *r_args)))); } + + outcome } (Func(l_args, l_closure, l_ret), Func(r_args, r_closure, r_ret)) if l_args.len() == r_args.len() => { - let arg_problems = unify_zip_slices(subs, pool, *l_args, *r_args); - let ret_problems = unify_pool(subs, pool, *l_ret, *r_ret, ctx.mode); - let closure_problems = unify_pool(subs, pool, *l_closure, *r_closure, ctx.mode); + let arg_outcome = unify_zip_slices(subs, pool, *l_args, *r_args); + let ret_outcome = unify_pool(subs, pool, *l_ret, *r_ret, ctx.mode); + let closure_outcome = unify_pool(subs, pool, *l_closure, *r_closure, ctx.mode); - if arg_problems.is_empty() && closure_problems.is_empty() && ret_problems.is_empty() { - merge(subs, ctx, Structure(Func(*r_args, *r_closure, *r_ret))) - } else { - let mut problems = ret_problems; + let mut outcome = ret_outcome; - problems.extend(closure_problems); - problems.extend(arg_problems); + outcome.union(closure_outcome); + outcome.union(arg_outcome); - problems + if outcome.mismatches.is_empty() { + outcome.union(merge( + subs, + ctx, + Structure(Func(*r_args, *r_closure, *r_ret)), + )); } + + outcome } (FunctionOrTagUnion(tag_name, tag_symbol, ext), Func(args, closure, ret)) => { unify_function_or_tag_union_and_func( @@ -1282,12 +1399,12 @@ fn unify_flat_type( let tag_name_2_ref = &subs[*tag_name_2]; if tag_name_1_ref == tag_name_2_ref { - let problems = unify_pool(subs, pool, *ext1, *ext2, ctx.mode); - if problems.is_empty() { + let outcome = unify_pool(subs, pool, *ext1, *ext2, ctx.mode); + if outcome.mismatches.is_empty() { let content = *subs.get_content_without_compacting(ctx.second); merge(subs, ctx, content) } else { - problems + outcome } } else { let tags1 = UnionTags::from_tag_name_index(*tag_name_1); @@ -1343,7 +1460,7 @@ fn unify_zip_slices( left: SubsSlice, right: SubsSlice, ) -> Outcome { - let mut problems = Vec::new(); + let mut outcome = Outcome::default(); let it = left.into_iter().zip(right.into_iter()); @@ -1351,10 +1468,10 @@ fn unify_zip_slices( let l_var = subs[l_index]; let r_var = subs[r_index]; - problems.extend(unify_pool(subs, pool, l_var, r_var, Mode::EQ)); + outcome.union(unify_pool(subs, pool, l_var, r_var, Mode::EQ)); } - problems + outcome } #[inline(always)] @@ -1362,6 +1479,7 @@ fn unify_rigid( subs: &mut Subs, ctx: &Context, name: &SubsIndex, + opt_able_bound: Option, other: &Content, ) -> Outcome { match other { @@ -1369,16 +1487,76 @@ fn unify_rigid( // If the other is flex, rigid wins! merge(subs, ctx, RigidVar(*name)) } - RigidVar(_) | RecursionVar { .. } | Structure(_) | Alias(_, _, _, _) | RangedNumber(..) => { - if !ctx.mode.contains(Mode::RIGID_AS_FLEX) { - // Type mismatch! Rigid can only unify with flex, even if the - // rigid names are the same. - mismatch!("Rigid {:?} with {:?}", ctx.first, &other) - } else { - // We are treating rigid vars as flex vars; admit this - merge(subs, ctx, *other) + FlexAbleVar(_, other_ability) => { + match opt_able_bound { + Some(ability) => { + if ability == *other_ability { + // The ability bounds are the same, so rigid wins! + merge(subs, ctx, RigidAbleVar(*name, ability)) + } else { + // Mismatch for now. + // TODO check ability hierarchies. + mismatch!( + %not_able, ctx.second, ability, + "RigidAble {:?} with ability {:?} not compatible with ability {:?}", + ctx.first, + ability, + other_ability + ) + } + } + None => { + // Mismatch - Rigid can unify with FlexAble only when the Rigid has an ability + // bound as well, otherwise the user failed to correctly annotate the bound. + mismatch!( + %not_able, ctx.first, *other_ability, + "Rigid {:?} with FlexAble {:?}", ctx.first, other + ) + } } } + + RigidVar(_) | RecursionVar { .. } | Structure(_) | Alias(_, _, _, _) | RangedNumber(..) + if ctx.mode.contains(Mode::RIGID_AS_FLEX) => + { + // Usually rigids can only unify with flex, but the mode indicates we are treating + // rigid vars as flex, so admit this. + match (opt_able_bound, other) { + (None, other) => merge(subs, ctx, *other), + (Some(ability), Alias(opaque_name, vars, _real_var, AliasKind::Opaque)) + if vars.is_empty() => + { + let mut output = merge(subs, ctx, *other); + let must_implement_ability = MustImplementAbility { + typ: *opaque_name, + ability, + }; + output.must_implement_ability.push(must_implement_ability); + output + } + (Some(ability), other) => { + // For now, only allow opaque types with no type variables to implement abilities. + mismatch!( + %not_able, ctx.second, ability, + "RigidAble {:?} with non-opaque or opaque with type variables {:?}", + ctx.first, + &other + ) + } + } + } + + RigidVar(_) + | RigidAbleVar(..) + | RecursionVar { .. } + | Structure(_) + | Alias(..) + | RangedNumber(..) => { + // Type mismatch! Rigid can only unify with flex, even if the + // rigid names are the same. + mismatch!("Rigid {:?} with {:?}", ctx.first, &other) + } + Error => { // Error propagates. merge(subs, ctx, Error) @@ -1391,16 +1569,49 @@ fn unify_flex( subs: &mut Subs, ctx: &Context, opt_name: &Option>, + opt_able_bound: Option, other: &Content, ) -> Outcome { match other { FlexVar(None) => { // If both are flex, and only left has a name, keep the name around. - merge(subs, ctx, FlexVar(*opt_name)) + match opt_able_bound { + Some(ability) => merge(subs, ctx, FlexAbleVar(*opt_name, ability)), + None => merge(subs, ctx, FlexVar(*opt_name)), + } + } + + FlexAbleVar(opt_other_name, other_ability) => { + // Prefer the right's name when possible. + let opt_name = (opt_other_name).or(*opt_name); + + match opt_able_bound { + Some(ability) => { + if ability == *other_ability { + // The ability bounds are the same! Keep the name around if it exists. + merge(subs, ctx, FlexAbleVar(opt_name, ability)) + } else { + // Ability names differ; mismatch for now. + // TODO check ability hierarchies. + mismatch!( + %not_able, ctx.second, ability, + "FlexAble {:?} with ability {:?} not compatible with ability {:?}", + ctx.first, + ability, + other_ability + ) + } + } + None => { + // Right has an ability bound, but left might have the name. Combine them. + merge(subs, ctx, FlexAbleVar(opt_name, *other_ability)) + } + } } FlexVar(Some(_)) | RigidVar(_) + | RigidAbleVar(_, _) | RecursionVar { .. } | Structure(_) | Alias(_, _, _, _) @@ -1446,7 +1657,13 @@ fn unify_recursion( // unify the structure variable with this Structure unify_pool(subs, pool, structure, ctx.second, ctx.mode) } - RigidVar(_) => mismatch!("RecursionVar {:?} with rigid {:?}", ctx.first, &other), + RigidVar(_) => { + mismatch!("RecursionVar {:?} with rigid {:?}", ctx.first, &other) + } + + FlexAbleVar(..) | RigidAbleVar(..) => { + mismatch!("RecursionVar {:?} with able var {:?}", ctx.first, &other) + } FlexVar(_) => merge( subs, @@ -1492,7 +1709,7 @@ pub fn merge(subs: &mut Subs, ctx: &Context, content: Content) -> Outcome { subs.union(ctx.first, ctx.second, desc); - Vec::new() + Outcome::default() } fn register(subs: &mut Subs, desc: Descriptor, pool: &mut Pool) -> Variable { @@ -1543,7 +1760,7 @@ fn unify_function_or_tag_union_and_func( let new_tag_union_var = fresh(subs, pool, ctx, content); - let mut problems = if left { + let mut outcome = if left { unify_pool(subs, pool, new_tag_union_var, function_return, ctx.mode) } else { unify_pool(subs, pool, function_return, new_tag_union_var, ctx.mode) @@ -1567,16 +1784,16 @@ fn unify_function_or_tag_union_and_func( pool, ); - let closure_problems = if left { + let closure_outcome = if left { unify_pool(subs, pool, tag_lambda_set, function_lambda_set, ctx.mode) } else { unify_pool(subs, pool, function_lambda_set, tag_lambda_set, ctx.mode) }; - problems.extend(closure_problems); + outcome.union(closure_outcome); } - if problems.is_empty() { + if outcome.mismatches.is_empty() { let desc = if left { subs.get(ctx.second) } else { @@ -1586,5 +1803,5 @@ fn unify_function_or_tag_union_and_func( subs.union(ctx.first, ctx.second, desc); } - problems + outcome } diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 21ec179d6f..848c510651 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -950,6 +950,62 @@ fn to_expr_report<'b>( None, ), + Reason::InvalidAbilityMemberSpecialization { + member_name, + def_region: _, + unimplemented_abilities, + } => { + let problem = alloc.concat(vec![ + alloc.reflow("Something is off with this specialization of "), + alloc.symbol_unqualified(member_name), + alloc.reflow(":"), + ]); + let this_is = alloc.reflow("This value is"); + let instead_of = alloc.concat(vec![ + alloc.reflow("But the type annotation on "), + alloc.symbol_unqualified(member_name), + alloc.reflow(" says it must match:"), + ]); + + let hint = if unimplemented_abilities.is_empty() { + None + } else { + let mut stack = Vec::with_capacity(unimplemented_abilities.len()); + for (err_type, ability) in unimplemented_abilities.into_iter() { + stack.push(alloc.concat(vec![ + to_doc(alloc, Parens::Unnecessary, err_type).0, + alloc.reflow(" does not implement "), + alloc.symbol_unqualified(ability), + ])); + } + + let hint = alloc.stack(vec![ + alloc.concat(vec![ + alloc.note(""), + alloc.reflow("Some types in this specialization don't implement the abilities they are expected to. I found the following missing implementations:"), + ]), + alloc.type_block(alloc.stack(stack)), + ]); + + Some(hint) + }; + + report_mismatch( + alloc, + lines, + filename, + &category, + found, + expected_type, + region, + Some(expr_region), + problem, + this_is, + instead_of, + hint, + ) + } + Reason::LowLevelOpArg { op, arg_index } => { panic!( "Compiler bug: argument #{} to low-level operation {:?} was the wrong type!", @@ -1281,6 +1337,10 @@ fn format_category<'b>( alloc.concat(vec![this_is, alloc.text(" a default field")]), alloc.text(" of type:"), ), + AbilityMemberSpecialization(_ability_member) => ( + alloc.concat(vec![this_is, alloc.text(" a declared specialization")]), + alloc.text(" of type:"), + ), } } @@ -1526,7 +1586,7 @@ fn to_circular_report<'b>( You will see ∞ for parts of the type that repeat \ something already printed out infinitely.", ), - alloc.type_block(to_doc(alloc, Parens::Unnecessary, overall_type)), + alloc.type_block(to_doc(alloc, Parens::Unnecessary, overall_type).0), ]), ]) }, @@ -1683,6 +1743,7 @@ pub struct Diff { left: T, right: T, status: Status, + // TODO: include able variables } fn ext_to_doc<'b>(alloc: &'b RocDocAllocator<'b>, ext: TypeExt) -> Option> { @@ -1694,10 +1755,30 @@ fn ext_to_doc<'b>(alloc: &'b RocDocAllocator<'b>, ext: TypeExt) -> Option; + +#[derive(Default)] +struct Context { + able_variables: AbleVariables, +} + pub fn to_doc<'b>( alloc: &'b RocDocAllocator<'b>, parens: Parens, tipe: ErrorType, +) -> (RocDocBuilder<'b>, AbleVariables) { + let mut ctx = Context::default(); + + let doc = to_doc_help(&mut ctx, alloc, parens, dbg!(tipe)); + + (doc, ctx.able_variables) +} + +fn to_doc_help<'b>( + ctx: &mut Context, + alloc: &'b RocDocAllocator<'b>, + parens: Parens, + tipe: ErrorType, ) -> RocDocBuilder<'b> { use ErrorType::*; @@ -1706,22 +1787,32 @@ pub fn to_doc<'b>( alloc, parens, args.into_iter() - .map(|arg| to_doc(alloc, Parens::InFn, arg)) + .map(|arg| to_doc_help(ctx, alloc, Parens::InFn, arg)) .collect(), - to_doc(alloc, Parens::InFn, *ret), + to_doc_help(ctx, alloc, Parens::InFn, *ret), ), Infinite => alloc.text("∞"), Error => alloc.text("?"), - FlexVar(lowercase) => alloc.type_variable(lowercase), - RigidVar(lowercase) => alloc.type_variable(lowercase), + FlexVar(lowercase) | RigidVar(lowercase) => alloc.type_variable(lowercase), + FlexAbleVar(lowercase, ability) | RigidAbleVar(lowercase, ability) => { + // TODO we should be putting able variables on the toplevel of the type, not here + ctx.able_variables.push((lowercase.clone(), ability)); + alloc.concat(vec![ + alloc.type_variable(lowercase), + alloc.space(), + alloc.keyword("has"), + alloc.space(), + alloc.symbol_foreign_qualified(ability), + ]) + } Type(symbol, args) => report_text::apply( alloc, parens, alloc.symbol_foreign_qualified(symbol), args.into_iter() - .map(|arg| to_doc(alloc, Parens::InTypeParam, arg)) + .map(|arg| to_doc_help(ctx, alloc, Parens::InTypeParam, arg)) .collect(), ), @@ -1730,7 +1821,7 @@ pub fn to_doc<'b>( parens, alloc.symbol_foreign_qualified(symbol), args.into_iter() - .map(|arg| to_doc(alloc, Parens::InTypeParam, arg)) + .map(|arg| to_doc_help(ctx, alloc, Parens::InTypeParam, arg)) .collect(), ), @@ -1746,15 +1837,24 @@ pub fn to_doc<'b>( ( alloc.string(k.as_str().to_string()), match value { - RecordField::Optional(v) => { - RecordField::Optional(to_doc(alloc, Parens::Unnecessary, v)) - } - RecordField::Required(v) => { - RecordField::Required(to_doc(alloc, Parens::Unnecessary, v)) - } - RecordField::Demanded(v) => { - RecordField::Demanded(to_doc(alloc, Parens::Unnecessary, v)) - } + RecordField::Optional(v) => RecordField::Optional(to_doc_help( + ctx, + alloc, + Parens::Unnecessary, + v, + )), + RecordField::Required(v) => RecordField::Required(to_doc_help( + ctx, + alloc, + Parens::Unnecessary, + v, + )), + RecordField::Demanded(v) => RecordField::Demanded(to_doc_help( + ctx, + alloc, + Parens::Unnecessary, + v, + )), }, ) }) @@ -1770,7 +1870,7 @@ pub fn to_doc<'b>( ( name, args.into_iter() - .map(|arg| to_doc(alloc, Parens::InTypeParam, arg)) + .map(|arg| to_doc_help(ctx, alloc, Parens::InTypeParam, arg)) .collect::>(), ) }) @@ -1793,7 +1893,7 @@ pub fn to_doc<'b>( ( name, args.into_iter() - .map(|arg| to_doc(alloc, Parens::InTypeParam, arg)) + .map(|arg| to_doc_help(ctx, alloc, Parens::InTypeParam, arg)) .collect::>(), ) }) @@ -1802,7 +1902,7 @@ pub fn to_doc<'b>( report_text::recursive_tag_union( alloc, - to_doc(alloc, Parens::Unnecessary, *rec_var), + to_doc_help(ctx, alloc, Parens::Unnecessary, *rec_var), tags.into_iter() .map(|(k, v)| (alloc.tag_name(k), v)) .collect(), @@ -1811,10 +1911,10 @@ pub fn to_doc<'b>( } Range(typ, range_types) => { - let typ = to_doc(alloc, parens, *typ); + let typ = to_doc_help(ctx, alloc, parens, *typ); let range_types = range_types .into_iter() - .map(|arg| to_doc(alloc, Parens::Unnecessary, arg)) + .map(|arg| to_doc_help(ctx, alloc, Parens::Unnecessary, arg)) .collect(); report_text::range(alloc, typ, range_types) } @@ -1826,7 +1926,7 @@ fn same<'b>( parens: Parens, tipe: ErrorType, ) -> Diff> { - let doc = to_doc(alloc, parens, tipe); + let doc = to_doc(alloc, parens, tipe).0; Diff { left: doc.clone(), @@ -1870,8 +1970,8 @@ fn to_diff<'b>( status, } } else { - let left = to_doc(alloc, Parens::InFn, type1); - let right = to_doc(alloc, Parens::InFn, type2); + let left = to_doc(alloc, Parens::InFn, type1).0; + let right = to_doc(alloc, Parens::InFn, type2).0; Diff { left, @@ -1928,8 +2028,8 @@ fn to_diff<'b>( } (Alias(_, _, _, AliasKind::Opaque), _) | (_, Alias(_, _, _, AliasKind::Opaque)) => { - let left = to_doc(alloc, Parens::InFn, type1); - let right = to_doc(alloc, Parens::InFn, type2); + let left = to_doc(alloc, Parens::InFn, type1).0; + let right = to_doc(alloc, Parens::InFn, type2).0; Diff { left, @@ -1961,8 +2061,8 @@ fn to_diff<'b>( (RecursiveTagUnion(_rec1, _tags1, _ext1), RecursiveTagUnion(_rec2, _tags2, _ext2)) => { // TODO do a better job here - let left = to_doc(alloc, Parens::Unnecessary, type1); - let right = to_doc(alloc, Parens::Unnecessary, type2); + let left = to_doc(alloc, Parens::Unnecessary, type1).0; + let right = to_doc(alloc, Parens::Unnecessary, type2).0; Diff { left, @@ -1973,8 +2073,8 @@ fn to_diff<'b>( pair => { // We hit none of the specific cases where we give more detailed information - let left = to_doc(alloc, parens, type1); - let right = to_doc(alloc, parens, type2); + let left = to_doc(alloc, parens, type1).0; + let right = to_doc(alloc, parens, type2).0; let is_int = |t: &ErrorType| match t { ErrorType::Type(Symbol::NUM_INT, _) => true, @@ -2135,7 +2235,7 @@ fn diff_record<'b>( ( field.clone(), alloc.string(field.as_str().to_string()), - tipe.map(|t| to_doc(alloc, Parens::Unnecessary, t.clone())), + tipe.map(|t| to_doc(alloc, Parens::Unnecessary, t.clone()).0), ) }; let shared_keys = fields1 @@ -2261,7 +2361,7 @@ fn diff_tag_union<'b>( alloc.tag_name(field.clone()), // TODO add spaces between args args.iter() - .map(|arg| to_doc(alloc, Parens::InTypeParam, arg.clone())) + .map(|arg| to_doc(alloc, Parens::InTypeParam, arg.clone()).0) .collect(), ) }; @@ -2518,7 +2618,7 @@ mod report_text { let entry_to_doc = |(name, tipe): (Lowercase, RecordField)| { ( alloc.string(name.as_str().to_string()), - to_doc(alloc, Parens::Unnecessary, tipe.into_inner()), + to_doc(alloc, Parens::Unnecessary, tipe.into_inner()).0, ) }; @@ -2863,7 +2963,14 @@ fn type_problem_to_pretty<'b>( match tipe { Infinite | Error | FlexVar(_) => alloc.nil(), - RigidVar(y) => bad_double_rigid(x, y), + FlexAbleVar(_, ability) => bad_rigid_var( + x, + alloc.concat(vec![ + alloc.reflow("an instance of the ability "), + alloc.symbol_unqualified(ability), + ]), + ), + RigidVar(y) | RigidAbleVar(y, _) => bad_double_rigid(x, y), Function(_, _, _) => bad_rigid_var(x, alloc.reflow("a function value")), Record(_, _) => bad_rigid_var(x, alloc.reflow("a record value")), TagUnion(_, _) | RecursiveTagUnion(_, _, _) => { diff --git a/reporting/src/report.rs b/reporting/src/report.rs index 4dc7c46d3e..fb6de7f29b 100644 --- a/reporting/src/report.rs +++ b/reporting/src/report.rs @@ -72,6 +72,12 @@ pub enum Severity { Warning, } +#[derive(Clone, Copy, Debug)] +pub enum RenderTarget { + ColorTerminal, + Generic, +} + /// A textual report. pub struct Report<'b> { pub title: String, @@ -81,6 +87,19 @@ pub struct Report<'b> { } impl<'b> Report<'b> { + pub fn render( + self, + target: RenderTarget, + buf: &'b mut String, + alloc: &'b RocDocAllocator<'b>, + palette: &'b Palette, + ) { + match target { + RenderTarget::Generic => self.render_ci(buf, alloc), + RenderTarget::ColorTerminal => self.render_color_terminal(buf, alloc, palette), + } + } + /// Render to CI console output, where no colors are available. pub fn render_ci(self, buf: &'b mut String, alloc: &'b RocDocAllocator<'b>) { let err_msg = ""; diff --git a/reporting/tests/helpers/mod.rs b/reporting/tests/helpers/mod.rs index eda767542e..34aa95bf8f 100644 --- a/reporting/tests/helpers/mod.rs +++ b/reporting/tests/helpers/mod.rs @@ -1,6 +1,7 @@ extern crate bumpalo; use self::bumpalo::Bump; +use roc_can::abilities::AbilitiesStore; use roc_can::constraint::{Constraint, Constraints}; use roc_can::env::Env; use roc_can::expected::Expected; @@ -31,10 +32,19 @@ pub fn infer_expr( constraints: &Constraints, constraint: &Constraint, aliases: &mut Aliases, + abilities_store: &mut AbilitiesStore, expr_var: Variable, ) -> (Content, Subs) { let env = solve::Env::default(); - let (solved, _) = solve::run(constraints, &env, problems, subs, aliases, constraint); + let (solved, _) = solve::run( + constraints, + &env, + problems, + subs, + aliases, + constraint, + abilities_store, + ); let content = *solved.inner().get_content_without_compacting(expr_var); diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 65525012f1..675a259de0 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -8,17 +8,20 @@ mod helpers; #[cfg(test)] mod test_reporting { - use crate::helpers::test_home; - use crate::helpers::{can_expr, infer_expr, CanExprOut, ParseErrOut}; + use crate::helpers::{can_expr, infer_expr, test_home, CanExprOut, ParseErrOut}; use bumpalo::Bump; use indoc::indoc; + use roc_can::abilities::AbilitiesStore; + use roc_can::def::Declaration; + use roc_can::pattern::Pattern; + use roc_load::{self, LoadedModule, LoadingProblem}; use roc_module::symbol::{Interns, ModuleId}; use roc_mono::ir::{Procs, Stmt, UpdateModeIds}; use roc_mono::layout::LayoutCache; use roc_region::all::LineInfo; use roc_reporting::report::{ - can_problem, mono_problem, parse_problem, type_problem, Report, Severity, ANSI_STYLE_CODES, - DEFAULT_PALETTE, + can_problem, mono_problem, parse_problem, type_problem, RenderTarget, Report, Severity, + ANSI_STYLE_CODES, DEFAULT_PALETTE, }; use roc_reporting::report::{RocDocAllocator, RocDocBuilder}; use roc_solve::solve; @@ -43,6 +46,214 @@ mod test_reporting { } } + fn promote_expr_to_module(src: &str) -> String { + let mut buffer = + String::from("app \"test\" provides [ main ] to \"./platform\"\n\nmain =\n"); + + for line in src.lines() { + // indent the body! + buffer.push_str(" "); + buffer.push_str(line); + buffer.push('\n'); + } + + buffer + } + + fn run_load_and_infer<'a>( + arena: &'a Bump, + src: &'a str, + ) -> (String, Result>) { + use std::fs::File; + use std::io::Write; + use tempfile::tempdir; + + let module_src = if src.starts_with("app") { + // this is already a module + src.to_string() + } else { + // this is an expression, promote it to a module + promote_expr_to_module(src) + }; + + let exposed_types = Default::default(); + let loaded = { + let dir = tempdir().unwrap(); + let filename = PathBuf::from("Test.roc"); + let file_path = dir.path().join(filename); + let full_file_path = file_path.clone(); + let mut file = File::create(file_path).unwrap(); + writeln!(file, "{}", module_src).unwrap(); + let result = roc_load::load_and_typecheck( + arena, + full_file_path, + dir.path(), + exposed_types, + roc_target::TargetInfo::default_x86_64(), + RenderTarget::Generic, + ); + drop(file); + + dir.close().unwrap(); + + result + }; + + (module_src, loaded) + } + + fn infer_expr_help_new<'a>( + arena: &'a Bump, + expr_src: &'a str, + ) -> Result< + ( + String, + Vec, + Vec, + Vec, + ModuleId, + Interns, + ), + LoadingProblem<'a>, + > { + let (module_src, result) = run_load_and_infer(arena, expr_src); + let LoadedModule { + module_id: home, + mut can_problems, + mut type_problems, + interns, + mut solved, + exposed_to_host, + mut declarations_by_id, + .. + } = result?; + + let can_problems = can_problems.remove(&home).unwrap_or_default(); + let type_problems = type_problems.remove(&home).unwrap_or_default(); + + let subs = solved.inner_mut(); + + for var in exposed_to_host.values() { + name_all_type_vars(*var, subs); + } + + let mut mono_problems = Vec::new(); + + // MONO + + if type_problems.is_empty() && can_problems.is_empty() { + let arena = Bump::new(); + + assert!(exposed_to_host.len() == 1); + let (sym, _var) = exposed_to_host.into_iter().next().unwrap(); + + let home_decls = declarations_by_id.remove(&home).unwrap(); + let (loc_expr, var) = home_decls + .into_iter() + .find_map(|decl| match decl { + Declaration::Declare(def) => match def.loc_pattern.value { + Pattern::Identifier(s) if s == sym => Some((def.loc_expr, def.expr_var)), + _ => None, + }, + _ => None, + }) + .expect("No expression to monomorphize found!"); + + // Compile and add all the Procs before adding main + let mut procs = Procs::new_in(&arena); + let mut ident_ids = interns.all_ident_ids.get(&home).unwrap().clone(); + let mut update_mode_ids = UpdateModeIds::new(); + + // Populate Procs and Subs, and get the low-level Expr from the canonical Expr + let target_info = roc_target::TargetInfo::default_x86_64(); + let mut layout_cache = LayoutCache::new(target_info); + let mut mono_env = roc_mono::ir::Env { + arena: &arena, + subs, + problems: &mut mono_problems, + home, + ident_ids: &mut ident_ids, + update_mode_ids: &mut update_mode_ids, + target_info, + // call_specialization_counter=0 is reserved + call_specialization_counter: 1, + }; + let _mono_expr = Stmt::new( + &mut mono_env, + loc_expr.value, + var, + &mut procs, + &mut layout_cache, + ); + } + + Ok(( + module_src, + type_problems, + can_problems, + mono_problems, + home, + interns, + )) + } + + fn list_reports_new(arena: &Bump, src: &str, finalize_render: F) -> String + where + F: FnOnce(RocDocBuilder<'_>, &mut String), + { + use ven_pretty::DocAllocator; + + let filename = filename_from_string(r"\code\proj\Main.roc"); + + let mut buf = String::new(); + + match infer_expr_help_new(arena, src) { + Err(LoadingProblem::FormattedReport(fail)) => fail, + Ok((module_src, type_problems, can_problems, mono_problems, home, interns)) => { + let lines = LineInfo::new(&module_src); + let src_lines: Vec<&str> = module_src.split('\n').collect(); + let mut reports = Vec::new(); + + let alloc = RocDocAllocator::new(&src_lines, home, &interns); + + for problem in can_problems { + let report = can_problem(&alloc, &lines, filename.clone(), problem.clone()); + reports.push(report); + } + + for problem in type_problems { + if let Some(report) = + type_problem(&alloc, &lines, filename.clone(), problem.clone()) + { + reports.push(report); + } + } + + for problem in mono_problems { + let report = mono_problem(&alloc, &lines, filename.clone(), problem.clone()); + reports.push(report); + } + + let has_reports = !reports.is_empty(); + + let doc = alloc + .stack(reports.into_iter().map(|v| v.pretty(&alloc))) + .append(if has_reports { + alloc.line() + } else { + alloc.nil() + }); + + finalize_render(doc, &mut buf); + buf + } + Err(other) => { + assert!(false, "failed to load: {:?}", other); + unreachable!() + } + } + } + fn infer_expr_help<'a>( arena: &'a Bump, expr_src: &'a str, @@ -85,12 +296,14 @@ mod test_reporting { } let mut unify_problems = Vec::new(); + let mut abilities_store = AbilitiesStore::default(); let (_content, mut subs) = infer_expr( subs, &mut unify_problems, &constraints, &constraint, &mut solve_aliases, + &mut abilities_store, var, ); @@ -298,6 +511,27 @@ mod test_reporting { assert_eq!(readable, expected_rendering); } + fn new_report_problem_as(src: &str, expected_rendering: &str) { + let arena = Bump::new(); + + let finalize_render = |doc: RocDocBuilder<'_>, buf: &mut String| { + doc.1 + .render_raw(70, &mut roc_reporting::report::CiWrite::new(buf)) + .expect("list_reports") + }; + + let buf = list_reports_new(&arena, src, finalize_render); + + // convenient to copy-paste the generated message + if true && buf != expected_rendering { + for line in buf.split('\n') { + println!(" {}", line); + } + } + + assert_multiline_str_eq!(expected_rendering, buf.as_str()); + } + fn human_readable(str: &str) -> String { str.replace(ANSI_STYLE_CODES.red, "") .replace(ANSI_STYLE_CODES.white, "") @@ -8682,7 +8916,7 @@ I need all branches in an `if` to have the same type! #[test] fn ability_demands_not_indented_with_first() { - report_problem_as( + new_report_problem_as( indoc!( r#" Eq has @@ -8699,19 +8933,18 @@ I need all branches in an `if` to have the same type! I was partway through parsing an ability definition, but I got stuck here: - 2│ eq : a, a -> U64 | a has Eq - 3│ neq : a, a -> U64 | a has Eq - ^ + 5│ eq : a, a -> U64 | a has Eq + 6│ neq : a, a -> U64 | a has Eq + ^ - I suspect this line is indented too much (by 4 spaces) - "# + I suspect this line is indented too much (by 4 spaces)"# ), ) } #[test] fn ability_demand_value_has_args() { - report_problem_as( + new_report_problem_as( indoc!( r#" Eq has @@ -8727,12 +8960,11 @@ I need all branches in an `if` to have the same type! I was partway through parsing an ability definition, but I got stuck here: - 2│ eq b c : a, a -> U64 | a has Eq - ^ + 5│ eq b c : a, a -> U64 | a has Eq + ^ I was expecting to see a : annotating the signature of this value - next. - "# + next."# ), ) } @@ -8898,8 +9130,8 @@ I need all branches in an `if` to have the same type! } #[test] - fn bad_type_parameter_in_ability() { - report_problem_as( + fn ability_bad_type_parameter() { + new_report_problem_as( indoc!( r#" Hash a b c has @@ -8914,8 +9146,8 @@ I need all branches in an `if` to have the same type! The definition of the `Hash` ability includes type variables: - 1│ Hash a b c has - ^^^^^ + 4│ Hash a b c has + ^^^^^ Abilities cannot depend on type variables, but their member values can! @@ -8924,8 +9156,8 @@ I need all branches in an `if` to have the same type! `Hash` is not used anywhere in your code. - 1│ Hash a b c has - ^^^^ + 4│ 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. @@ -8936,12 +9168,12 @@ I need all branches in an `if` to have the same type! #[test] fn alias_in_has_clause() { - report_problem_as( + new_report_problem_as( indoc!( r#" - Hash has hash : a, b -> U64 | a has Hash, b has Bool + app "test" provides [ hash ] to "./platform" - 1 + Hash has hash : a, b -> U64 | a has Hash, b has Bool "# ), indoc!( @@ -8950,18 +9182,8 @@ I need all branches in an `if` to have the same type! The type referenced in this "has" clause is not an ability: - 1│ Hash has hash : a, b -> U64 | a has Hash, b has Bool + 3│ 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. "# ), ) @@ -8969,12 +9191,12 @@ I need all branches in an `if` to have the same type! #[test] fn shadowed_type_variable_in_has_clause() { - report_problem_as( + new_report_problem_as( indoc!( r#" - Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 + app "test" provides [ ab1 ] to "./platform" - 1 + Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 "# ), indoc!( @@ -8983,26 +9205,16 @@ I need all branches in an `if` to have the same type! The `a` name is first defined here: - 1│ Ab1 has ab1 : a -> {} | a has Ab1, a has Ab1 + 3│ 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 + 3│ 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. "# ), ) @@ -9010,7 +9222,7 @@ I need all branches in an `if` to have the same type! #[test] fn alias_using_ability() { - report_problem_as( + new_report_problem_as( indoc!( r#" Ability has ab : a -> {} | a has Ability @@ -9027,8 +9239,8 @@ I need all branches in an `if` to have the same type! The definition of the `Alias` aliases references the ability `Ability`: - 3│ Alias : Ability - ^^^^^ + 6│ Alias : Ability + ^^^^^ Abilities are not types, but you can add an ability constraint to a type variable `a` by writing @@ -9041,8 +9253,8 @@ I need all branches in an `if` to have the same type! `ab` is not used anywhere in your code. - 1│ Ability has ab : a -> {} | a has Ability - ^^ + 4│ 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. @@ -9053,7 +9265,7 @@ I need all branches in an `if` to have the same type! #[test] fn ability_shadows_ability() { - report_problem_as( + new_report_problem_as( indoc!( r#" Ability has ab : a -> U64 | a has Ability @@ -9069,13 +9281,13 @@ I need all branches in an `if` to have the same type! The `Ability` name is first defined here: - 1│ Ability has ab : a -> U64 | a has Ability - ^^^^^^^ + 4│ 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 - ^^^^^^^ + 6│ Ability has ab1 : a -> U64 | a has Ability + ^^^^^^^ Since these abilities have the same name, it's easy to use the wrong one on accident. Give one of them a new name. @@ -9084,8 +9296,8 @@ I need all branches in an `if` to have the same type! `ab` is not used anywhere in your code. - 1│ Ability has ab : a -> U64 | a has Ability - ^^ + 4│ 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. @@ -9096,12 +9308,12 @@ I need all branches in an `if` to have the same type! #[test] fn ability_member_does_not_bind_ability() { - report_problem_as( + new_report_problem_as( indoc!( r#" - Ability has ab : {} -> {} + app "test" provides [ ] to "./platform" - 1 + Ability has ab : {} -> {} "# ), indoc!( @@ -9111,7 +9323,7 @@ I need all branches in an `if` to have the same type! 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 : {} -> {} + 3│ Ability has ab : {} -> {} ^^ Ability members must include a `has` clause binding a type variable to @@ -9125,7 +9337,7 @@ I need all branches in an `if` to have the same type! `Ability` is not used anywhere in your code. - 1│ Ability has ab : {} -> {} + 3│ Ability has ab : {} -> {} ^^^^^^^ If you didn't intend on using `Ability` then remove it so future readers @@ -9135,7 +9347,7 @@ I need all branches in an `if` to have the same type! `ab` is not used anywhere in your code. - 1│ Ability has ab : {} -> {} + 3│ Ability has ab : {} -> {} ^^ If you didn't intend on using `ab` then remove it so future readers of @@ -9147,13 +9359,13 @@ I need all branches in an `if` to have the same type! #[test] fn ability_member_binds_extra_ability() { - report_problem_as( + new_report_problem_as( indoc!( r#" + app "test" provides [ eq ] to "./platform" + Eq has eq : a, a -> Bool | a has Eq Hash has hash : a, b -> U64 | a has Eq, b has Hash - - 1 "# ), indoc!( @@ -9163,7 +9375,7 @@ I need all branches in an `if` to have the same type! 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 + 4│ Hash has hash : a, b -> U64 | a has Eq, b has Hash ^^^^^^^^ Currently, ability members can only bind variables to the ability they @@ -9173,19 +9385,9 @@ I need all branches in an `if` to have the same type! ── 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 + 4│ 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 @@ -9197,14 +9399,14 @@ I need all branches in an `if` to have the same type! #[test] fn has_clause_outside_of_ability() { - report_problem_as( + new_report_problem_as( indoc!( r#" + app "test" provides [ hash, f ] to "./platform" + Hash has hash : a -> U64 | a has Hash f : a -> U64 | a has Hash - - f "# ), indoc!( @@ -9213,21 +9415,49 @@ I need all branches in an `if` to have the same type! A `has` clause is not allowed here: - 3│ f : a -> U64 | a has Hash + 5│ f : a -> U64 | a has Hash ^^^^^^^^^^ `has` clauses can only be specified on the top-level type annotation of an ability member. + "# + ), + ) + } - ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + #[test] + fn ability_specialization_with_non_implementing_type() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ hash ] to "./platform" - `hash` is not used anywhere in your code. + Hash has hash : a -> U64 | a has Hash - 1│ Hash has hash : a -> U64 | a has Hash - ^^^^ + hash = \{} -> 0u64 + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── - If you didn't intend on using `hash` then remove it so future readers of - your code don't wonder why it is there. + Something is off with this specialization of `hash`: + + 5│ hash = \{} -> 0u64 + ^^^^ + + This value is a declared specialization of type: + + {}a -> U64 + + But the type annotation on `hash` says it must match: + + a has Hash -> U64 + + Note: Some types in this specialization don't implement the abilities + they are expected to. I found the following missing implementations: + + {}a does not implement Hash "# ), ) From ce7c61eb09aac7fa54c128938238d97e37146a1b Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 15:45:42 -0400 Subject: [PATCH 04/21] Propogate render target forward --- Cargo.lock | 2 ++ ast/Cargo.toml | 1 + ast/src/module.rs | 1 + ast/src/solve_type.rs | 51 +++++++++++++++++++++++++++++++++---------- cli/src/build.rs | 5 +++++ docs/Cargo.toml | 1 + docs/src/lib.rs | 1 + repl_eval/src/gen.rs | 1 + 8 files changed, 51 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc9e364812..d2babf3e7b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3351,6 +3351,7 @@ dependencies = [ "roc_parse", "roc_problem", "roc_region", + "roc_reporting", "roc_target", "roc_types", "roc_unify", @@ -3519,6 +3520,7 @@ dependencies = [ "roc_module", "roc_parse", "roc_region", + "roc_reporting", "roc_target", "roc_types", "snafu", diff --git a/ast/Cargo.toml b/ast/Cargo.toml index 86d203cee4..d1384b96ee 100644 --- a/ast/Cargo.toml +++ b/ast/Cargo.toml @@ -19,6 +19,7 @@ roc_unify = { path = "../compiler/unify"} roc_load = { path = "../compiler/load" } roc_target = { path = "../compiler/roc_target" } roc_error_macros = { path = "../error_macros" } +roc_reporting = { path = "../reporting" } arrayvec = "0.7.2" bumpalo = { version = "3.8.0", features = ["collections"] } page_size = "0.4.2" diff --git a/ast/src/module.rs b/ast/src/module.rs index 55b324b35d..769df5d162 100644 --- a/ast/src/module.rs +++ b/ast/src/module.rs @@ -19,6 +19,7 @@ pub fn load_module(src_file: &Path) -> LoadedModule { }), subs_by_module, TargetInfo::default_x86_64(), + roc_reporting::report::RenderTarget::ColorTerminal, ); match loaded { diff --git a/ast/src/solve_type.rs b/ast/src/solve_type.rs index dc2e53f3a8..f15b128827 100644 --- a/ast/src/solve_type.rs +++ b/ast/src/solve_type.rs @@ -227,12 +227,16 @@ fn solve<'a>( ); match unify(subs, actual, expected, Mode::EQ) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { + // TODO(abilities) record deferred ability checks introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impl) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadExpr( @@ -267,7 +271,7 @@ fn solve<'a>( // // state // } - // Failure(vars, _actual_type, _expected_type) => { + // Failure(vars, _actual_type, _expected_type, _bad_impl) => { // introduce(subs, rank, pools, &vars); // // // ERROR NOT REPORTED @@ -320,13 +324,17 @@ fn solve<'a>( ); match unify(subs, actual, expected, Mode::EQ) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { + // TODO(abilities) record deferred ability checks introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impl) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadExpr( @@ -391,12 +399,16 @@ fn solve<'a>( // TODO(ayazhafiz): presence constraints for Expr2/Type2 match unify(subs, actual, expected, Mode::EQ) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { + // TODO(abilities) record deferred ability checks introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impl) => { introduce(subs, rank, pools, &vars); let problem = TypeError::BadPattern( @@ -699,12 +711,16 @@ fn solve<'a>( let includes = type_to_var(arena, mempool, subs, rank, pools, cached_aliases, &tag_ty); match unify(subs, actual, includes, Mode::PRESENT) { - Success(vars) => { + Success { + vars, + must_implement_ability: _, + } => { + // TODO(abilities) record deferred ability checks introduce(subs, rank, pools, &vars); state } - Failure(vars, actual_type, expected_type) => { + Failure(vars, actual_type, expected_type, _bad_impl) => { introduce(subs, rank, pools, &vars); // TODO: do we need a better error type here? @@ -1281,7 +1297,7 @@ fn adjust_rank_content( use roc_types::subs::FlatType::*; match content { - FlexVar(_) | RigidVar(_) | Error => group_rank, + FlexVar(_) | RigidVar(_) | FlexAbleVar(..) | RigidAbleVar(..) | Error => group_rank, RecursionVar { .. } => group_rank, @@ -1536,7 +1552,7 @@ fn instantiate_rigids_help( }; } - FlexVar(_) | Error => {} + FlexVar(_) | FlexAbleVar(_, _) | Error => {} RecursionVar { structure, .. } => { instantiate_rigids_help(subs, max_rank, pools, structure); @@ -1547,6 +1563,11 @@ fn instantiate_rigids_help( subs.set(copy, make_descriptor(FlexVar(Some(name)))); } + RigidAbleVar(name, ability) => { + // what it's all about: convert the rigid var into a flex var + subs.set(copy, make_descriptor(FlexAbleVar(Some(name), ability))); + } + Alias(_, args, real_type_var, _) => { for var_index in args.all_variables() { let var = subs[var_index]; @@ -1772,7 +1793,7 @@ fn deep_copy_var_help( copy } - FlexVar(_) | Error => copy, + FlexVar(_) | FlexAbleVar(_, _) | Error => copy, RecursionVar { opt_name, @@ -1797,6 +1818,12 @@ fn deep_copy_var_help( copy } + RigidAbleVar(name, ability) => { + subs.set(copy, make_descriptor(FlexAbleVar(Some(name), ability))); + + copy + } + Alias(symbol, mut args, real_type_var, kind) => { let mut new_args = Vec::with_capacity(args.all_variables().len()); diff --git a/cli/src/build.rs b/cli/src/build.rs index ab3d7d0c42..c39b55f043 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -6,6 +6,7 @@ use roc_build::{ use roc_builtins::bitcode; use roc_load::LoadingProblem; use roc_mono::ir::OptLevel; +use roc_reporting::report::RenderTarget; use roc_target::TargetInfo; use std::path::PathBuf; use std::time::{Duration, SystemTime}; @@ -68,6 +69,8 @@ pub fn build_file<'a>( src_dir.as_path(), subs_by_module, target_info, + // TODO: expose this from CLI? + RenderTarget::ColorTerminal, )?; use target_lexicon::Architecture; @@ -374,6 +377,8 @@ pub fn check_file( src_dir.as_path(), subs_by_module, target_info, + // TODO: expose this from CLI? + RenderTarget::ColorTerminal, )?; let buf = &mut String::with_capacity(1024); diff --git a/docs/Cargo.toml b/docs/Cargo.toml index d377031ac9..1fe3c231e2 100644 --- a/docs/Cargo.toml +++ b/docs/Cargo.toml @@ -21,6 +21,7 @@ roc_parse = { path = "../compiler/parse" } roc_target = { path = "../compiler/roc_target" } roc_collections = { path = "../compiler/collections" } roc_highlight = { path = "../highlight"} +roc_reporting = { path = "../reporting"} bumpalo = { version = "3.8.0", features = ["collections"] } snafu = { version = "0.6.10", features = ["backtraces"] } peg = "0.8.0" diff --git a/docs/src/lib.rs b/docs/src/lib.rs index 7a4a05f0c9..d09134d98d 100644 --- a/docs/src/lib.rs +++ b/docs/src/lib.rs @@ -424,6 +424,7 @@ pub fn load_modules_for_files(filenames: Vec) -> Vec { src_dir.as_path(), Default::default(), roc_target::TargetInfo::default_x86_64(), // This is just type-checking for docs, so "target" doesn't matter + roc_reporting::report::RenderTarget::ColorTerminal, ) { Ok(loaded) => modules.push(loaded), Err(LoadingProblem::FormattedReport(report)) => { diff --git a/repl_eval/src/gen.rs b/repl_eval/src/gen.rs index a5e1fa6dbd..4e8f698c29 100644 --- a/repl_eval/src/gen.rs +++ b/repl_eval/src/gen.rs @@ -60,6 +60,7 @@ pub fn compile_to_mono<'a>( src_dir, exposed_types, target_info, + roc_reporting::report::RenderTarget::ColorTerminal, ); let mut loaded = match loaded { From 64b559073d90be0cc79752f26c7f8b9e55a562d2 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 15:48:26 -0400 Subject: [PATCH 05/21] Clippy --- compiler/can/src/abilities.rs | 4 ++-- compiler/can/src/annotation.rs | 12 +++++------- compiler/load_internal/src/file.rs | 2 ++ compiler/solve/src/solve.rs | 1 + 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index 71be29a706..16c90273e2 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -118,8 +118,8 @@ impl AbilitiesStore { specializing_symbol: Symbol, ) -> Option<(Symbol, &AbilityMemberData)> { let root_symbol = self.specialization_to_root.get(&specializing_symbol)?; - debug_assert!(self.ability_members.contains_key(&root_symbol)); - let root_data = self.ability_members.get(&root_symbol).unwrap(); + debug_assert!(self.ability_members.contains_key(root_symbol)); + let root_data = self.ability_members.get(root_symbol).unwrap(); Some((*root_symbol, root_data)) } diff --git a/compiler/can/src/annotation.rs b/compiler/can/src/annotation.rs index ddc5292dcd..c5d1ea3f23 100644 --- a/compiler/can/src/annotation.rs +++ b/compiler/can/src/annotation.rs @@ -159,14 +159,14 @@ impl IntroducedVariables { .named .iter() .find(|nv| &nv.name == name) - .map(|nv| NamedOrAbleVariable::Named(nv)) + .map(NamedOrAbleVariable::Named) { return Some(nav); } self.able .iter() .find(|av| &av.name == name) - .map(|av| NamedOrAbleVariable::Able(av)) + .map(NamedOrAbleVariable::Able) } } @@ -258,14 +258,12 @@ pub fn canonicalize_annotation_with_possible_clauses( &mut references, ); - let annot = Annotation { + Annotation { typ, introduced_variables, references, aliases, - }; - - annot + } } fn make_apply_symbol( @@ -897,7 +895,7 @@ fn canonicalize_has_clause( let var = var_store.fresh(); - introduced_variables.insert_able(var_name.clone(), Loc::at(region, var), ability); + introduced_variables.insert_able(var_name, Loc::at(region, var), ability); Ok(()) } diff --git a/compiler/load_internal/src/file.rs b/compiler/load_internal/src/file.rs index ae1a70d631..5585d32da3 100644 --- a/compiler/load_internal/src/file.rs +++ b/compiler/load_internal/src/file.rs @@ -618,6 +618,7 @@ struct State<'a> { type CachedSubs = Arc)>>>; impl<'a> State<'a> { + #[allow(clippy::too_many_arguments)] fn new( root_id: ModuleId, target_info: TargetInfo, @@ -3174,6 +3175,7 @@ fn add_imports( import_variables } +#[allow(clippy::complexity)] fn run_solve_solve( imported_builtins: Vec, exposed_for_module: ExposedForModule, diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 8399b8cba2..12adce566f 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1204,6 +1204,7 @@ fn solve( /// If a symbol claims to specialize an ability member, check that its solved type in fact /// does specialize the ability, and record the specialization. +#[allow(clippy::too_many_arguments)] fn check_ability_specialization( subs: &mut Subs, env: &Env, From cbbbb8c8554ddd98b046bb4ebc3b57cbcd9629ac Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 16:32:50 -0400 Subject: [PATCH 06/21] Remove stray `dbg`s --- compiler/can/src/def.rs | 2 -- compiler/can/src/pattern.rs | 2 +- compiler/can/src/scope.rs | 7 ++----- reporting/src/error/type.rs | 2 +- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 75d17270af..514d43d8e6 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -522,8 +522,6 @@ pub fn canonicalize_defs<'a>( .register_ability(loc_ability_name.value, can_members); } - dbg!(&scope.abilities_store, pattern_type); - // Now that we have the scope completely assembled, and shadowing resolved, // we're ready to canonicalize any body exprs. diff --git a/compiler/can/src/pattern.rs b/compiler/can/src/pattern.rs index 9df8cd7312..e620f41f00 100644 --- a/compiler/can/src/pattern.rs +++ b/compiler/can/src/pattern.rs @@ -166,7 +166,7 @@ pub fn canonicalize_def_header_pattern<'a>( match pattern { // Identifiers that shadow ability members may appear (and may only appear) at the header of a def. Identifier(name) => match scope.introduce_or_shadow_ability_member( - dbg!((*name).into()), + (*name).into(), &env.exposed_ident_ids, &mut env.ident_ids, region, diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index f08ba453a8..f4cb75e1be 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -248,17 +248,14 @@ impl Scope { all_ident_ids: &mut IdentIds, region: Region, ) -> Result<(Symbol, Option), (Region, Loc, Symbol)> { - match dbg!(self.idents.get(&ident)) { + match self.idents.get(&ident) { Some(&(original_symbol, original_region)) => { let shadow_ident_id = all_ident_ids.add(ident.clone()); let shadow_symbol = Symbol::new(self.home, shadow_ident_id); self.symbols.insert(shadow_symbol, region); - if self - .abilities_store - .is_ability_member_name(dbg!(original_symbol)) - { + if self.abilities_store.is_ability_member_name(original_symbol) { self.abilities_store .register_specializing_symbol(shadow_symbol, original_symbol); // Add a symbol for the shadow, but don't re-associate the member name. diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 848c510651..823f87c47d 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -1769,7 +1769,7 @@ pub fn to_doc<'b>( ) -> (RocDocBuilder<'b>, AbleVariables) { let mut ctx = Context::default(); - let doc = to_doc_help(&mut ctx, alloc, parens, dbg!(tipe)); + let doc = to_doc_help(&mut ctx, alloc, parens, tipe); (doc, ctx.able_variables) } From 865c1f15d7d56d245eaef7014165fcb1dfd3de7a Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 16:42:51 -0400 Subject: [PATCH 07/21] Fix test compile errors, and simply load_internal tests --- Cargo.lock | 2 +- compiler/load_internal/Cargo.toml | 1 - compiler/load_internal/tests/test_load.rs | 24 ++++++++++------------ compiler/test_gen/src/helpers/llvm.rs | 2 ++ compiler/test_mono/Cargo.toml | 1 + compiler/test_mono/src/tests.rs | 1 + reporting/tests/test_reporting.rs | 25 +++++++++++++++++++++++ 7 files changed, 41 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d2babf3e7b..71b9a3e355 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3737,7 +3737,6 @@ dependencies = [ "roc_target", "roc_types", "roc_unify", - "strip-ansi-escapes", "tempfile", "ven_pretty", ] @@ -4526,6 +4525,7 @@ dependencies = [ "roc_load", "roc_module", "roc_mono", + "roc_reporting", "roc_target", "test_mono_macros", ] diff --git a/compiler/load_internal/Cargo.toml b/compiler/load_internal/Cargo.toml index 5381fc94b9..3d876623f5 100644 --- a/compiler/load_internal/Cargo.toml +++ b/compiler/load_internal/Cargo.toml @@ -33,4 +33,3 @@ tempfile = "3.2.0" pretty_assertions = "1.0.0" maplit = "1.0.2" indoc = "1.0.3" -strip-ansi-escapes = "0.1.1" diff --git a/compiler/load_internal/tests/test_load.rs b/compiler/load_internal/tests/test_load.rs index 8f0223d483..9cc4a46721 100644 --- a/compiler/load_internal/tests/test_load.rs +++ b/compiler/load_internal/tests/test_load.rs @@ -25,6 +25,7 @@ mod test_load { use roc_problem::can::Problem; use roc_region::all::LineInfo; use roc_reporting::report::can_problem; + use roc_reporting::report::RenderTarget; use roc_reporting::report::RocDocAllocator; use roc_target::TargetInfo; use roc_types::pretty_print::{content_to_string, name_all_type_vars}; @@ -41,7 +42,7 @@ mod test_load { ) -> Result> { use LoadResult::*; - let load_start = LoadStart::from_path(arena, filename)?; + let load_start = LoadStart::from_path(arena, filename, RenderTarget::Generic)?; match roc_load_internal::file::load( arena, @@ -51,6 +52,7 @@ mod test_load { Phase::SolveTypes, target_info, Default::default(), // these tests will re-compile the builtins + RenderTarget::Generic, )? { Monomorphized(_) => unreachable!(""), TypeChecked(module) => Ok(module), @@ -88,8 +90,6 @@ mod test_load { } fn multiple_modules(files: Vec<(&str, &str)>) -> Result { - use roc_load_internal::file::LoadingProblem; - let arena = Bump::new(); let arena = &arena; @@ -589,18 +589,18 @@ mod test_load { report, indoc!( " - \u{1b}[36m── UNFINISHED LIST ─────────────────────────────────────────────────────────────\u{1b}[0m + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── - I cannot find the end of this list: + I cannot find the end of this list: - \u{1b}[36m3\u{1b}[0m\u{1b}[36m│\u{1b}[0m \u{1b}[37mmain = [\u{1b}[0m - \u{1b}[31m^\u{1b}[0m + 3│ main = [ + ^ - You could change it to something like \u{1b}[33m[ 1, 2, 3 ]\u{1b}[0m or even just \u{1b}[33m[]\u{1b}[0m. - Anything where there is an open and a close square bracket, and where - the elements of the list are separated by commas. + You could change it to something like [ 1, 2, 3 ] or even just []. + Anything where there is an open and a close square bracket, and where + the elements of the list are separated by commas. - \u{1b}[4mNote\u{1b}[0m: I may be confused by indentation" + Note: I may be confused by indentation" ) ), Ok(_) => unreachable!("we expect failure here"), @@ -769,8 +769,6 @@ mod test_load { ]; let err = multiple_modules(modules).unwrap_err(); - let err = strip_ansi_escapes::strip(err).unwrap(); - let err = String::from_utf8(err).unwrap(); assert_eq!( err, indoc!( diff --git a/compiler/test_gen/src/helpers/llvm.rs b/compiler/test_gen/src/helpers/llvm.rs index f6660f977b..03bc6b7a1e 100644 --- a/compiler/test_gen/src/helpers/llvm.rs +++ b/compiler/test_gen/src/helpers/llvm.rs @@ -7,6 +7,7 @@ use roc_collections::all::MutSet; use roc_gen_llvm::llvm::externs::add_default_roc_externs; use roc_mono::ir::OptLevel; use roc_region::all::LineInfo; +use roc_reporting::report::RenderTarget; use target_lexicon::Triple; fn promote_expr_to_module(src: &str) -> String { @@ -57,6 +58,7 @@ fn create_llvm_module<'a>( src_dir, Default::default(), target_info, + RenderTarget::ColorTerminal, ); let mut loaded = match loaded { diff --git a/compiler/test_mono/Cargo.toml b/compiler/test_mono/Cargo.toml index 2f56668cb1..9f336dbe73 100644 --- a/compiler/test_mono/Cargo.toml +++ b/compiler/test_mono/Cargo.toml @@ -17,6 +17,7 @@ roc_load = { path = "../load" } roc_can = { path = "../can" } roc_mono = { path = "../mono" } roc_target = { path = "../roc_target" } +roc_reporting = { path = "../../reporting" } test_mono_macros = { path = "../test_mono_macros" } pretty_assertions = "1.0.0" bumpalo = { version = "3.8.0", features = ["collections"] } diff --git a/compiler/test_mono/src/tests.rs b/compiler/test_mono/src/tests.rs index 777a790d52..74e17f9a15 100644 --- a/compiler/test_mono/src/tests.rs +++ b/compiler/test_mono/src/tests.rs @@ -101,6 +101,7 @@ fn compiles_to_ir(test_name: &str, src: &str) { src_dir, Default::default(), TARGET_INFO, + roc_reporting::report::RenderTarget::Generic, ); let mut loaded = match loaded { diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 675a259de0..84e0509b5d 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9462,4 +9462,29 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn ability_specialization_is_incomplete() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ eq, le ] to "./platform" + + Eq has + eq : a, a -> Bool | a has Eq + le : a, a -> Bool | a has Eq + + Id := U64 + + eq = \$Id m, $Id n -> m == n + + le = \$Id m, $Id n -> m < n + "# + ), + indoc!( + r#" + "# + ), + ) + } } From a73c8a85c21907c0ceae1b11f5a0d5457528711b Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 17:10:44 -0400 Subject: [PATCH 08/21] Remove stray test for now --- reporting/tests/test_reporting.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 84e0509b5d..675a259de0 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9462,29 +9462,4 @@ I need all branches in an `if` to have the same type! ), ) } - - #[test] - fn ability_specialization_is_incomplete() { - new_report_problem_as( - indoc!( - r#" - app "test" provides [ eq, le ] to "./platform" - - Eq has - eq : a, a -> Bool | a has Eq - le : a, a -> Bool | a has Eq - - Id := U64 - - eq = \$Id m, $Id n -> m == n - - le = \$Id m, $Id n -> m < n - "# - ), - indoc!( - r#" - "# - ), - ) - } } From 5171e3964d85a9b9f6e0a47cd7e611dd0d7710ab Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 17:19:27 -0400 Subject: [PATCH 09/21] Lift able variables to the top of a reported error type --- reporting/src/error/type.rs | 141 ++++++++++++++++++++++++------ reporting/tests/test_reporting.rs | 2 +- 2 files changed, 113 insertions(+), 30 deletions(-) diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 823f87c47d..737ea2be40 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -1687,10 +1687,12 @@ fn to_comparison<'b>( expected: ErrorType, ) -> Comparison<'b> { let diff = to_diff(alloc, Parens::Unnecessary, actual, expected); + let actual = type_with_able_vars(alloc, diff.left, diff.left_able); + let expected = type_with_able_vars(alloc, diff.right, diff.right_able); Comparison { - actual: alloc.type_block(diff.left), - expected: alloc.type_block(diff.right), + actual: alloc.type_block(actual), + expected: alloc.type_block(expected), problems: match diff.status { Status::Similar => vec![], Status::Different(problems) => problems, @@ -1743,7 +1745,9 @@ pub struct Diff { left: T, right: T, status: Status, - // TODO: include able variables + // idea: lift "able" type variables so they are shown at the top of a type. + left_able: AbleVariables, + right_able: AbleVariables, } fn ext_to_doc<'b>(alloc: &'b RocDocAllocator<'b>, ext: TypeExt) -> Option> { @@ -1798,13 +1802,7 @@ fn to_doc_help<'b>( FlexAbleVar(lowercase, ability) | RigidAbleVar(lowercase, ability) => { // TODO we should be putting able variables on the toplevel of the type, not here ctx.able_variables.push((lowercase.clone(), ability)); - alloc.concat(vec![ - alloc.type_variable(lowercase), - alloc.space(), - alloc.keyword("has"), - alloc.space(), - alloc.symbol_foreign_qualified(ability), - ]) + alloc.type_variable(lowercase) } Type(symbol, args) => report_text::apply( @@ -1926,15 +1924,42 @@ fn same<'b>( parens: Parens, tipe: ErrorType, ) -> Diff> { - let doc = to_doc(alloc, parens, tipe).0; + let (doc, able) = to_doc(alloc, parens, tipe); Diff { left: doc.clone(), right: doc, status: Status::Similar, + left_able: able.clone(), + right_able: able, } } +fn type_with_able_vars<'b>( + alloc: &'b RocDocAllocator<'b>, + typ: RocDocBuilder<'b>, + able: AbleVariables, +) -> RocDocBuilder<'b> { + if able.is_empty() { + // fast path: taken the vast majority of the time + return typ; + } + + let mut doc = Vec::with_capacity(1 + 6 * able.len()); + doc.push(typ); + + for (i, (var, ability)) in able.into_iter().enumerate() { + doc.push(alloc.string(if i == 0 { " | " } else { ", " }.to_string())); + doc.push(alloc.type_variable(var)); + doc.push(alloc.space()); + doc.push(alloc.keyword("has")); + doc.push(alloc.space()); + doc.push(alloc.symbol_foreign_qualified(ability)); + } + + alloc.concat(doc) +} + fn to_diff<'b>( alloc: &'b RocDocAllocator<'b>, parens: Parens, @@ -1963,15 +1988,21 @@ fn to_diff<'b>( let left = report_text::function(alloc, parens, arg_diff.left, ret_diff.left); let right = report_text::function(alloc, parens, arg_diff.right, ret_diff.right); + let mut left_able = arg_diff.left_able; + left_able.extend(ret_diff.left_able); + let mut right_able = arg_diff.right_able; + right_able.extend(ret_diff.right_able); Diff { left, right, status, + left_able, + right_able, } } else { - let left = to_doc(alloc, Parens::InFn, type1).0; - let right = to_doc(alloc, Parens::InFn, type2).0; + let (left, left_able) = to_doc(alloc, Parens::InFn, type1); + let (right, right_able) = to_doc(alloc, Parens::InFn, type2); Diff { left, @@ -1980,6 +2011,8 @@ fn to_diff<'b>( args1.len(), args2.len(), )]), + left_able, + right_able, } } } @@ -2002,6 +2035,8 @@ fn to_diff<'b>( left, right, status: args_diff.status, + left_able: args_diff.left_able, + right_able: args_diff.right_able, } } @@ -2024,17 +2059,21 @@ fn to_diff<'b>( left, right, status: args_diff.status, + left_able: args_diff.left_able, + right_able: args_diff.right_able, } } (Alias(_, _, _, AliasKind::Opaque), _) | (_, Alias(_, _, _, AliasKind::Opaque)) => { - let left = to_doc(alloc, Parens::InFn, type1).0; - let right = to_doc(alloc, Parens::InFn, type2).0; + let (left, left_able) = to_doc(alloc, Parens::InFn, type1); + let (right, right_able) = to_doc(alloc, Parens::InFn, type2); Diff { left, right, status: Status::Different(vec![Problem::OpaqueComparedToNonOpaque]), + left_able, + right_able, } } @@ -2061,20 +2100,22 @@ fn to_diff<'b>( (RecursiveTagUnion(_rec1, _tags1, _ext1), RecursiveTagUnion(_rec2, _tags2, _ext2)) => { // TODO do a better job here - let left = to_doc(alloc, Parens::Unnecessary, type1).0; - let right = to_doc(alloc, Parens::Unnecessary, type2).0; + let (left, left_able) = to_doc(alloc, Parens::Unnecessary, type1); + let (right, right_able) = to_doc(alloc, Parens::Unnecessary, type2); Diff { left, right, status: Status::Similar, + left_able, + right_able, } } pair => { // We hit none of the specific cases where we give more detailed information - let left = to_doc(alloc, parens, type1).0; - let right = to_doc(alloc, parens, type2).0; + let (left, left_able) = to_doc(alloc, parens, type1); + let (right, right_able) = to_doc(alloc, parens, type2); let is_int = |t: &ErrorType| match t { ErrorType::Type(Symbol::NUM_INT, _) => true, @@ -2130,6 +2171,8 @@ fn to_diff<'b>( left, right, status: Status::Different(problems), + left_able, + right_able, } } } @@ -2149,6 +2192,8 @@ where // TODO use ExactSizeIterator to pre-allocate here let mut left = Vec::new(); let mut right = Vec::new(); + let mut left_able = Vec::new(); + let mut right_able = Vec::new(); for (arg1, arg2) in args1.into_iter().zip(args2.into_iter()) { let diff = to_diff(alloc, parens, arg1, arg2); @@ -2156,12 +2201,16 @@ where left.push(diff.left); right.push(diff.right); status.merge(diff.status); + left_able.extend(diff.left_able); + right_able.extend(diff.right_able); } Diff { left, right, status, + left_able, + right_able, } } @@ -2228,6 +2277,8 @@ fn diff_record<'b>( _ => diff.status, } }, + left_able: diff.left_able, + right_able: diff.right_able, } }; @@ -2293,12 +2344,16 @@ fn diff_record<'b>( left: vec![], right: vec![], status: Status::Similar, + left_able: vec![], + right_able: vec![], }; for diff in both { fields_diff.left.push(diff.left); fields_diff.right.push(diff.right); fields_diff.status.merge(diff.status); + fields_diff.left_able.extend(diff.left_able); + fields_diff.right_able.extend(diff.right_able); } if !all_fields_shared { @@ -2336,6 +2391,8 @@ fn diff_record<'b>( left: doc1, right: doc2, status: fields_diff.status, + left_able: fields_diff.left_able, + right_able: fields_diff.right_able, } } @@ -2353,16 +2410,26 @@ fn diff_tag_union<'b>( left: (field.clone(), alloc.tag_name(field.clone()), diff.left), right: (field.clone(), alloc.tag_name(field), diff.right), status: diff.status, + left_able: diff.left_able, + right_able: diff.right_able, } }; - let to_unknown_docs = |(field, args): (&TagName, &Vec)| { + let to_unknown_docs = |(field, args): (&TagName, &Vec)| -> ( + TagName, + RocDocBuilder<'b>, + Vec>, + AbleVariables, + ) { + let (args, able): (_, Vec) = + // TODO add spaces between args + args.iter() + .map(|arg| to_doc(alloc, Parens::InTypeParam, arg.clone())) + .unzip(); ( field.clone(), alloc.tag_name(field.clone()), - // TODO add spaces between args - args.iter() - .map(|arg| to_doc(alloc, Parens::InTypeParam, arg.clone()).0) - .collect(), + args, + able.into_iter().flatten().collect(), ) }; let shared_keys = fields1 @@ -2380,7 +2447,7 @@ fn diff_tag_union<'b>( let status = match (ext_has_fixed_fields(&ext1), ext_has_fixed_fields(&ext2)) { (true, true) => match left.peek() { - Some((f, _, _)) => Status::Different(vec![Problem::TagTypo( + Some((f, _, _, _)) => Status::Different(vec![Problem::TagTypo( f.clone(), fields2.keys().cloned().collect(), )]), @@ -2397,14 +2464,14 @@ fn diff_tag_union<'b>( } }, (false, true) => match left.peek() { - Some((f, _, _)) => Status::Different(vec![Problem::TagTypo( + Some((f, _, _, _)) => Status::Different(vec![Problem::TagTypo( f.clone(), fields2.keys().cloned().collect(), )]), None => Status::Similar, }, (true, false) => match right.peek() { - Some((f, _, _)) => Status::Different(vec![Problem::TagTypo( + Some((f, _, _, _)) => Status::Different(vec![Problem::TagTypo( f.clone(), fields1.keys().cloned().collect(), )]), @@ -2419,17 +2486,27 @@ fn diff_tag_union<'b>( left: vec![], right: vec![], status: Status::Similar, + left_able: vec![], + right_able: vec![], }; for diff in both { fields_diff.left.push(diff.left); fields_diff.right.push(diff.right); fields_diff.status.merge(diff.status); + fields_diff.left_able.extend(diff.left_able); + fields_diff.right_able.extend(diff.right_able); } if !all_fields_shared { - fields_diff.left.extend(left); - fields_diff.right.extend(right); + for (tag, tag_doc, args, able) in left { + fields_diff.left.push((tag, tag_doc, args)); + fields_diff.left_able.extend(able); + } + for (tag, tag_doc, args, able) in right { + fields_diff.right.push((tag, tag_doc, args)); + fields_diff.right_able.extend(able); + } fields_diff.status.merge(Status::Different(vec![])); } @@ -2456,6 +2533,8 @@ fn diff_tag_union<'b>( left: doc1, right: doc2, status: fields_diff.status, + left_able: fields_diff.left_able, + right_able: fields_diff.right_able, } } @@ -2473,12 +2552,16 @@ fn ext_to_diff<'b>( left: ext_doc_1, right: ext_doc_2, status, + left_able: vec![], + right_able: vec![], }, Status::Different(_) => Diff { // NOTE elm colors these differently at this point left: ext_doc_1, right: ext_doc_2, status, + left_able: vec![], + right_able: vec![], }, } } diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 675a259de0..8a1fe1d0b9 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9452,7 +9452,7 @@ I need all branches in an `if` to have the same type! But the type annotation on `hash` says it must match: - a has Hash -> U64 + a -> U64 | a has Hash Note: Some types in this specialization don't implement the abilities they are expected to. I found the following missing implementations: From 462f443956733b4a9770f6aced49197a11b1969e Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 17:23:03 -0400 Subject: [PATCH 10/21] Comment --- compiler/solve/src/solve.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 12adce566f..b0042086c4 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1224,7 +1224,8 @@ fn check_ability_specialization( .get_var_by_symbol(&root_symbol) .expect("Ability should be registered in env by now!"); - // Check if they unify - if they don't, that's a type error! + // Check if they unify - if they don't, then the claimed specialization isn't really one, + // and that's a type error! let snapshot = subs.snapshot(); let unified = unify(subs, symbol_loc_var.value, root_signature_var, Mode::EQ); From b86bf94d9279f0f6b8e15530f4b2a008363727cc Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 17:35:00 -0400 Subject: [PATCH 11/21] Dev, wasm test bugs --- compiler/test_gen/src/helpers/dev.rs | 1 + compiler/test_gen/src/helpers/wasm.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/compiler/test_gen/src/helpers/dev.rs b/compiler/test_gen/src/helpers/dev.rs index 2adcaadb90..0ecd1bb37c 100644 --- a/compiler/test_gen/src/helpers/dev.rs +++ b/compiler/test_gen/src/helpers/dev.rs @@ -54,6 +54,7 @@ pub fn helper( src_dir, Default::default(), roc_target::TargetInfo::default_x86_64(), + roc_reporting::report::RenderTarget::ColorTerminal, ); let mut loaded = loaded.expect("failed to load module"); diff --git a/compiler/test_gen/src/helpers/wasm.rs b/compiler/test_gen/src/helpers/wasm.rs index 6460cb1139..0c59df3cf5 100644 --- a/compiler/test_gen/src/helpers/wasm.rs +++ b/compiler/test_gen/src/helpers/wasm.rs @@ -91,6 +91,7 @@ fn compile_roc_to_wasm_bytes<'a, T: Wasm32Result>( src_dir, Default::default(), roc_target::TargetInfo::default_wasm32(), + roc_reporting::report::RenderTarget::ColorTerminal, ); let loaded = loaded.expect("failed to load module"); From 127d435ff2b25e89968a8cff4ecedab7c7531588 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 18:24:10 -0400 Subject: [PATCH 12/21] Report incomplete ability implementations --- compiler/can/src/abilities.rs | 48 +++++++++++++++++++++++------- compiler/solve/src/solve.rs | 49 +++++++++++++++++++++++++++++-- reporting/src/error/type.rs | 47 +++++++++++++++++++++++++++++ reporting/tests/test_reporting.rs | 39 ++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 13 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index 16c90273e2..453cded77f 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -1,4 +1,4 @@ -use roc_collections::all::{MutMap, MutSet}; +use roc_collections::all::MutMap; use roc_module::symbol::Symbol; use roc_region::all::Region; use roc_types::{subs::Variable, types::Type}; @@ -13,6 +13,13 @@ pub struct AbilityMemberData { pub region: Region, } +/// A particular specialization of an ability member. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MemberSpecialization { + pub symbol: Symbol, + pub region: Region, +} + /// 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 @@ -35,9 +42,9 @@ pub struct AbilitiesStore { /// We keep the mapping #hash1->#hash specialization_to_root: MutMap, - /// Tuples of (type, member) specifying that `type` declares an implementation of an ability - /// member `member`. - declared_specializations: MutSet<(Symbol, Symbol)>, + /// Maps a tuple (member, type) specifying that `type` declares an implementation of an ability + /// member `member`, to the exact symbol that implements the ability. + declared_specializations: MutMap<(Symbol, Symbol), MemberSpecialization>, } impl AbilitiesStore { @@ -69,15 +76,18 @@ impl AbilitiesStore { } /// Records a specialization of `ability_member` with specialized type `implementing_type`. + /// Entries via this function are considered a source of truth. It must be ensured that a + /// specialization is validated before being registered here. pub fn register_specialization_for_type( &mut self, - implementing_type: Symbol, ability_member: Symbol, + implementing_type: Symbol, + specialization: MemberSpecialization, ) { - let is_new_insert = self + let old_spec = self .declared_specializations - .insert((implementing_type, ability_member)); - debug_assert!(is_new_insert, "Replacing existing implementation"); + .insert((ability_member, implementing_type), specialization); + debug_assert!(old_spec.is_none(), "Replacing existing specialization"); } /// Checks if `name` is a root ability member symbol name. @@ -123,9 +133,27 @@ impl AbilitiesStore { Some((*root_symbol, root_data)) } + /// Finds the ability member definition for a member name. + pub fn member_def(&self, member: Symbol) -> Option<&AbilityMemberData> { + self.ability_members.get(&member) + } + + /// Returns an iterator over pairs (ability member, type) specifying that + /// "ability member" has a specialization with type "type". + pub fn get_known_specializations<'lifetime_for_copied>( + &'lifetime_for_copied self, + ) -> impl Iterator + 'lifetime_for_copied { + self.declared_specializations.keys().copied() + } + + /// Retrieves the specialization of `member` for `typ`, if it exists. + pub fn get_specialization(&self, member: Symbol, typ: Symbol) -> Option { + self.declared_specializations.get(&(member, typ)).copied() + } + /// Returns pairs of (type, ability member) specifying that "ability member" has a /// specialization with type "type". - pub fn get_known_specializations(&self) -> &MutSet<(Symbol, Symbol)> { - &self.declared_specializations + pub fn members_of_ability(&self, ability: Symbol) -> Option<&[Symbol]> { + self.members_of_ability.get(&ability).map(|v| v.as_ref()) } } diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index b0042086c4..b5e83160f9 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1,5 +1,5 @@ use bumpalo::Bump; -use roc_can::abilities::AbilitiesStore; +use roc_can::abilities::{AbilitiesStore, MemberSpecialization}; use roc_can::constraint::Constraint::{self, *}; use roc_can::constraint::{Constraints, LetConstraint}; use roc_can::expected::{Expected, PExpected}; @@ -76,6 +76,13 @@ pub enum TypeError { CircularType(Region, Symbol, ErrorType), BadType(roc_types::types::Problem), UnexposedLookup(Symbol), + IncompleteAbilityImplementation { + // TODO(abilities): have general types here, not just opaques + typ: Symbol, + ability: Symbol, + specialized_members: Vec>, + missing_members: Vec>, + }, } use roc_types::types::Alias; @@ -568,7 +575,34 @@ pub fn run_in_place( &mut deferred_must_implement_abilities, ); - // TODO run through and check that all abilities are properly implemented + // Now that the module has been solved, we can run through and check all + // types claimed to implement abilities. + deferred_must_implement_abilities.dedup(); + for MustImplementAbility { typ, ability } in deferred_must_implement_abilities.into_iter() { + let members_of_ability = abilities_store.members_of_ability(ability).unwrap(); + let mut specialized_members = Vec::with_capacity(members_of_ability.len()); + let mut missing_members = Vec::with_capacity(members_of_ability.len()); + for &member in members_of_ability { + match abilities_store.get_specialization(member, typ) { + None => { + let root_data = abilities_store.member_def(member).unwrap(); + missing_members.push(Loc::at(root_data.region, member)); + } + Some(specialization) => { + specialized_members.push(Loc::at(specialization.region, member)); + } + } + } + + if !missing_members.is_empty() { + problems.push(TypeError::IncompleteAbilityImplementation { + typ, + ability, + specialized_members, + missing_members, + }); + } + } state.env } @@ -1254,8 +1288,17 @@ fn check_ability_specialization( "If there's more than one, the definition is ambiguous - this should be an error" ); + // This is a valid specialization! Record it. let specialization_type = ability_implementations_for_specialization[0].typ; - abilities_store.register_specialization_for_type(specialization_type, root_symbol); + let specialization = MemberSpecialization { + symbol, + region: symbol_loc_var.region, + }; + abilities_store.register_specialization_for_type( + root_symbol, + specialization_type, + specialization, + ); // Store the checks for what abilities must be implemented to be checked after the // whole module is complete. diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 737ea2be40..c98d125e3d 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -118,6 +118,53 @@ pub fn type_problem<'b>( other => panic!("unhandled bad type: {:?}", other), } } + IncompleteAbilityImplementation { + typ, + ability, + specialized_members, + missing_members, + } => { + let title = "INCOMPLETE ABILITY IMPLEMENTATION".to_string(); + + let mut stack = vec![alloc.concat(vec![ + alloc.reflow("The type "), + alloc.symbol_unqualified(typ), + alloc.reflow(" does not fully implement the ability "), + alloc.symbol_unqualified(ability), + alloc.reflow(". The following specializations are missing:"), + ])]; + + for member in missing_members.into_iter() { + stack.push(alloc.concat(vec![ + alloc.reflow("A specialization for "), + alloc.symbol_unqualified(member.value), + alloc.reflow(", which is defined here:"), + ])); + stack.push(alloc.region(lines.convert_region(member.region))); + } + + debug_assert!(!specialized_members.is_empty()); + + stack.push(alloc.concat(vec![ + alloc.note(""), + alloc.symbol_unqualified(typ), + alloc.reflow(" specializes the following members of "), + alloc.symbol_unqualified(ability), + alloc.reflow(":"), + ])); + + for spec in specialized_members { + stack.push(alloc.concat(vec![ + alloc.symbol_unqualified(spec.value), + alloc.reflow(", specialized here:"), + ])); + stack.push(alloc.region(lines.convert_region(spec.region))); + } + + let doc = alloc.stack(stack); + + report(title, doc, filename) + } } } diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 8a1fe1d0b9..2b285c334f 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9462,4 +9462,43 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn ability_specialization_is_incomplete() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ eq, le ] to "./platform" + + Eq has + eq : a, a -> Bool | a has Eq + le : a, a -> Bool | a has Eq + + Id := U64 + + eq = \$Id m, $Id n -> m == n + "# + ), + indoc!( + r#" + ── INCOMPLETE ABILITY IMPLEMENTATION ─────────────────────────────────────────── + + The type `Id` does not fully implement the ability `Eq`. The following + specializations are missing: + + A specialization for `le`, which is defined here: + + 5│ le : a, a -> Bool | a has Eq + ^^ + + Note: `Id` specializes the following members of `Eq`: + + `eq`, specialized here: + + 9│ eq = \$Id m, $Id n -> m == n + ^^ + "# + ), + ) + } } From 16d00608246b605ac30b3185128aceec26465b54 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 18:41:20 -0400 Subject: [PATCH 13/21] Report errors for type errors in specializations outside of ability impl --- compiler/solve/src/solve.rs | 16 +++++++------- reporting/tests/test_reporting.rs | 35 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index b5e83160f9..596b511af0 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -806,6 +806,7 @@ fn solve( let mut new_env = env.clone(); for (symbol, loc_var) in local_def_vars.iter() { check_ability_specialization( + arena, subs, &new_env, pools, @@ -1240,6 +1241,7 @@ fn solve( /// does specialize the ability, and record the specialization. #[allow(clippy::too_many_arguments)] fn check_ability_specialization( + arena: &Bump, subs: &mut Subs, env: &Env, pools: &mut Pools, @@ -1260,7 +1262,13 @@ fn check_ability_specialization( // Check if they unify - if they don't, then the claimed specialization isn't really one, // and that's a type error! - let snapshot = subs.snapshot(); + // This also fixes any latent type variables that need to be specialized to exactly what + // the ability signature expects. + + // We need to freshly instantiate the root signature so that all unifications are reflected + // in the specialization type, but not the original signature type. + let root_signature_var = + deep_copy_var_in(subs, Rank::toplevel(), pools, root_signature_var, arena); let unified = unify(subs, symbol_loc_var.value, root_signature_var, Mode::EQ); match unified { @@ -1268,10 +1276,6 @@ fn check_ability_specialization( vars: _, must_implement_ability, } => { - // We don't actually care about storing the unified type since the checked type for the - // specialization is what we want for code generation. - subs.rollback_to(snapshot); - if must_implement_ability.is_empty() { // This is an error.. but what kind? } @@ -1305,7 +1309,6 @@ fn check_ability_specialization( deferred_must_implement_abilities.extend(must_implement_ability); } Failure(vars, actual_type, expected_type, unimplemented_abilities) => { - subs.commit_snapshot(snapshot); introduce(subs, rank, pools, &vars); let reason = Reason::InvalidAbilityMemberSpecialization { @@ -1324,7 +1327,6 @@ fn check_ability_specialization( problems.push(problem); } BadType(vars, problem) => { - subs.commit_snapshot(snapshot); introduce(subs, rank, pools, &vars); problems.push(TypeError::BadType(problem)); diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 2b285c334f..e579b03db0 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9463,6 +9463,41 @@ I need all branches in an `if` to have the same type! ) } + #[test] + fn ability_specialization_does_not_match_type() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ hash ] to "./platform" + + Hash has hash : a -> U64 | a has Hash + + Id := U32 + + hash = \$Id n -> n + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + + Something is off with this specialization of `hash`: + + 7│ hash = \$Id n -> n + ^^^^ + + This value is a declared specialization of type: + + Id -> U32 + + But the type annotation on `hash` says it must match: + + Id -> U64 + "# + ), + ) + } + #[test] fn ability_specialization_is_incomplete() { new_report_problem_as( From d94556d807625947c742989dee3c71be6a73e26f Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 18:52:49 -0400 Subject: [PATCH 14/21] Report overly general specializations --- compiler/solve/src/solve.rs | 40 +++++++++++++++++++++++++++--- compiler/types/src/types.rs | 4 +++ reporting/src/error/type.rs | 40 ++++++++++++++++++++++++++++++ reporting/tests/test_reporting.rs | 41 +++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 3 deletions(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 596b511af0..4a57c10378 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1269,16 +1269,48 @@ fn check_ability_specialization( // in the specialization type, but not the original signature type. let root_signature_var = deep_copy_var_in(subs, Rank::toplevel(), pools, root_signature_var, arena); + let snapshot = subs.snapshot(); let unified = unify(subs, symbol_loc_var.value, root_signature_var, Mode::EQ); match unified { Success { vars: _, must_implement_ability, + } if must_implement_ability.is_empty() => { + // This can happen when every ability constriant on a type variable went + // through only another type variable. That means this def is not specialized + // for one type - for now, we won't admit this. + + // Rollback the snapshot so we unlink the root signature with the specialization, + // so we can have two separate error types. + subs.rollback_to(snapshot); + + let (expected_type, _problems) = subs.var_to_error_type(root_signature_var); + let (actual_type, _problems) = subs.var_to_error_type(symbol_loc_var.value); + + let reason = Reason::GeneralizedAbilityMemberSpecialization { + member_name: root_symbol, + def_region: root_data.region, + }; + + let problem = TypeError::BadExpr( + symbol_loc_var.region, + Category::AbilityMemberSpecialization(root_symbol), + actual_type, + Expected::ForReason(reason, expected_type, symbol_loc_var.region), + ); + + problems.push(problem); + + return; + } + + Success { + vars, + must_implement_ability, } => { - if must_implement_ability.is_empty() { - // This is an error.. but what kind? - } + subs.commit_snapshot(snapshot); + introduce(subs, rank, pools, &vars); // First, figure out and register for what type does this symbol specialize // the ability member. @@ -1309,6 +1341,7 @@ fn check_ability_specialization( deferred_must_implement_abilities.extend(must_implement_ability); } Failure(vars, actual_type, expected_type, unimplemented_abilities) => { + subs.commit_snapshot(snapshot); introduce(subs, rank, pools, &vars); let reason = Reason::InvalidAbilityMemberSpecialization { @@ -1327,6 +1360,7 @@ fn check_ability_specialization( problems.push(problem); } BadType(vars, problem) => { + subs.commit_snapshot(snapshot); introduce(subs, rank, pools, &vars); problems.push(TypeError::BadType(problem)); diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 846ef8817d..8f5f68636e 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -1749,6 +1749,10 @@ pub enum Reason { def_region: Region, unimplemented_abilities: DoesNotImplementAbility, }, + GeneralizedAbilityMemberSpecialization { + member_name: Symbol, + def_region: Region, + }, } #[derive(PartialEq, Debug, Clone)] diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index c98d125e3d..b1d6334d77 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -1053,6 +1053,46 @@ fn to_expr_report<'b>( ) } + Reason::GeneralizedAbilityMemberSpecialization { + member_name, + def_region: _, + } => { + let problem = alloc.concat(vec![ + alloc.reflow("This specialization of "), + alloc.symbol_unqualified(member_name), + alloc.reflow(" is overly general:"), + ]); + let this_is = alloc.reflow("This value is"); + let instead_of = alloc.concat(vec![ + alloc.reflow("But the type annotation on "), + alloc.symbol_unqualified(member_name), + alloc.reflow(" says it must match:"), + ]); + + let note = alloc.stack(vec![ + alloc.concat(vec![ + alloc.note(""), + alloc.reflow("The specialized type is too general, and does not provide a concrete type where a type variable is bound to an ability."), + ]), + alloc.reflow("Specializations can only be made for concrete types. If you have a generic implementation for this value, perhaps you don't need an ability?"), + ]); + + report_mismatch( + alloc, + lines, + filename, + &category, + found, + expected_type, + region, + Some(expr_region), + problem, + this_is, + instead_of, + Some(note), + ) + } + Reason::LowLevelOpArg { op, arg_index } => { panic!( "Compiler bug: argument #{} to low-level operation {:?} was the wrong type!", diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index e579b03db0..d544e1cb64 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9536,4 +9536,45 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn ability_specialization_overly_generalized() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ hash ] to "./platform" + + Hash has + hash : a -> U64 | a has Hash + + hash = \_ -> 0u64 + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + + This specialization of `hash` is overly general: + + 6│ hash = \_ -> 0u64 + ^^^^ + + This value is a declared specialization of type: + + a -> U64 + + But the type annotation on `hash` says it must match: + + a -> U64 | a has Hash + + Note: The specialized type is too general, and does not provide a + concrete type where a type variable is bound to an ability. + + Specializations can only be made for concrete types. If you have a + generic implementation for this value, perhaps you don't need an + ability? + "# + ), + ) + } } From cf1a6691dda7b8540ba4340bf93f87d082e7f72b Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 18:54:12 -0400 Subject: [PATCH 15/21] Fix solve tests --- compiler/solve/tests/solve_expr.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 6a0205be68..fd2f854633 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -5788,7 +5788,7 @@ mod solve_expr { hash = \$Id n -> n "# ), - [("Id", "hash")], + [("hash", "Id")], ) } @@ -5809,7 +5809,7 @@ mod solve_expr { hash32 = \$Id n -> Num.toU32 n "# ), - [("Id", "hash"), ("Id", "hash32")], + [("hash", "Id"), ("hash32", "Id")], ) } @@ -5837,7 +5837,7 @@ mod solve_expr { le = \$Id m, $Id n -> m < n "# ), - [("Id", "hash"), ("Id", "hash32"), ("Id", "eq"), ("Id", "le")], + [("hash", "Id"), ("hash32", "Id"), ("eq", "Id"), ("le", "Id")], ) } } From 67b5ab7fe7ff015a889fce2b1e386f252e7bebf8 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 19:01:36 -0400 Subject: [PATCH 16/21] Add test for when specialization types conflict --- compiler/solve/src/solve.rs | 6 ++--- reporting/tests/test_reporting.rs | 42 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 4a57c10378..7179caac90 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1319,10 +1319,8 @@ fn check_ability_specialization( .filter(|mia| mia.ability == root_data.parent_ability) .collect::>(); ability_implementations_for_specialization.dedup(); - debug_assert_eq!( - ability_implementations_for_specialization.len(), 1, - "If there's more than one, the definition is ambiguous - this should be an error" - ); + + debug_assert!(ability_implementations_for_specialization.len() == 1, "Multiple variables bound to an ability - this is ambiguous and should have been caught in canonicalization"); // This is a valid specialization! Record it. let specialization_type = ability_implementations_for_specialization[0].typ; diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index d544e1cb64..a8d1b8a32a 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9577,4 +9577,46 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn ability_specialization_conflicting_specialization_types() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ eq ] to "./platform" + + Eq has + eq : a, a -> Bool | a has Eq + + You := {} + AndI := {} + + eq = \$You {}, $AndI {} -> False + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + + Something is off with this specialization of `eq`: + + 9│ eq = \$You {}, $AndI {} -> False + ^^ + + This value is a declared specialization of type: + + You, AndI -> [ False, True ] + + But the type annotation on `eq` says it must match: + + You, You -> Bool + + Tip: Type comparisons between an opaque type are only ever equal if + both types are the same opaque type. Did you mean to create an opaque + type by wrapping it? If I have an opaque type Age := U32 I can create + an instance of this opaque type by doing @Age 23. + "# + ), + ) + } } From 0336b6ad27c996d779f8b07acc7c34ba7dc04132 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 12 Apr 2022 19:02:58 -0400 Subject: [PATCH 17/21] Clippy --- compiler/can/src/abilities.rs | 4 +--- compiler/solve/src/solve.rs | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index 453cded77f..558f73441d 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -140,9 +140,7 @@ impl AbilitiesStore { /// Returns an iterator over pairs (ability member, type) specifying that /// "ability member" has a specialization with type "type". - pub fn get_known_specializations<'lifetime_for_copied>( - &'lifetime_for_copied self, - ) -> impl Iterator + 'lifetime_for_copied { + pub fn get_known_specializations(&self) -> impl Iterator + '_ { self.declared_specializations.keys().copied() } diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 7179caac90..aab17659f2 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1301,8 +1301,6 @@ fn check_ability_specialization( ); problems.push(problem); - - return; } Success { From 03d4d81f8d4521e08ed1f4d451d33dceb319f5ce Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 13 Apr 2022 08:41:45 -0400 Subject: [PATCH 18/21] Remove unnecessary condition --- reporting/tests/test_reporting.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index a8d1b8a32a..140926a53a 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -523,7 +523,7 @@ mod test_reporting { let buf = list_reports_new(&arena, src, finalize_render); // convenient to copy-paste the generated message - if true && buf != expected_rendering { + if buf != expected_rendering { for line in buf.split('\n') { println!(" {}", line); } From a1c1dc1a9f8759a0a007be821afc5f9c3b5bef07 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 13 Apr 2022 08:49:27 -0400 Subject: [PATCH 19/21] Ask rustc to please always inline --- compiler/solve/src/solve.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index aab17659f2..649c1e28ed 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1240,6 +1240,8 @@ fn solve( /// If a symbol claims to specialize an ability member, check that its solved type in fact /// does specialize the ability, and record the specialization. #[allow(clippy::too_many_arguments)] +// Aggressive but necessary - there aren't many usages. +#[inline(always)] fn check_ability_specialization( arena: &Bump, subs: &mut Subs, From 5e1ab8225ef2bc46129a14f8b753edb256323417 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 13 Apr 2022 10:11:24 -0400 Subject: [PATCH 20/21] Report when ability member binds >1 variable to parent --- compiler/can/src/def.rs | 20 +++++++++++++++ compiler/problem/src/can.rs | 7 +++++ reporting/src/error/canonicalize.rs | 29 +++++++++++++++++++++ reporting/tests/test_reporting.rs | 40 +++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 514d43d8e6..83fff31fbc 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -491,6 +491,26 @@ pub fn canonicalize_defs<'a>( bad_has_clauses = true; } + if variables_bound_to_ability.len() > 1 { + // There is more than one variable bound to the member signature, so something like + // Eq has eq : a, b -> Bool | a has Eq, b has Eq + // We have no way of telling what type implements a particular instance of Eq in + // this case (a or b?), so disallow it. + let span_has_clauses = + Region::across_all(variables_bound_to_ability.iter().map(|v| &v.first_seen)); + let bound_var_names = variables_bound_to_ability + .iter() + .map(|v| v.name.clone()) + .collect(); + env.problem(Problem::AbilityMemberMultipleBoundVars { + member: member_sym, + ability: loc_ability_name.value, + span_has_clauses, + bound_var_names, + }); + bad_has_clauses = true; + } + if !variables_bound_to_other_abilities.is_empty() { // Disallow variables bound to other abilities, for now. for bad_variable in variables_bound_to_other_abilities.iter() { diff --git a/compiler/problem/src/can.rs b/compiler/problem/src/can.rs index d35d93b307..f8d1684418 100644 --- a/compiler/problem/src/can.rs +++ b/compiler/problem/src/can.rs @@ -119,6 +119,13 @@ pub enum Problem { ability: Symbol, region: Region, }, + AbilityMemberMultipleBoundVars { + member: Symbol, + ability: Symbol, + span_has_clauses: Region, + bound_var_names: Vec, + }, + // TODO(abilities): remove me when ability hierarchies are supported AbilityMemberBindsExternalAbility { member: Symbol, ability: Symbol, diff --git a/reporting/src/error/canonicalize.rs b/reporting/src/error/canonicalize.rs index c804905ae2..d366baf6f2 100644 --- a/reporting/src/error/canonicalize.rs +++ b/reporting/src/error/canonicalize.rs @@ -44,6 +44,7 @@ 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"; +const ABILITY_MEMBER_BINDS_MULTIPLE_VARIABLES: &str = "ABILITY MEMBER BINDS MULTIPLE VARIABLES"; pub fn can_problem<'b>( alloc: &'b RocDocAllocator<'b>, @@ -684,6 +685,34 @@ pub fn can_problem<'b>( severity = Severity::RuntimeError; } + Problem::AbilityMemberMultipleBoundVars { + member, + ability, + span_has_clauses, + mut bound_var_names, + } => { + doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The definition of the ability member "), + alloc.symbol_unqualified(member), + alloc.reflow(" includes multiple variables bound to the "), + alloc.symbol_unqualified(ability), + alloc.keyword(" ability:"), + ]), + alloc.region(lines.convert_region(span_has_clauses)), + alloc.reflow("Ability members can only bind one type variable to their parent ability. Otherwise, I wouldn't know what type implements an ability by looking at specializations!"), + alloc.concat(vec![ + alloc.hint("Did you mean to only bind "), + alloc.type_variable(bound_var_names.swap_remove(0)), + alloc.reflow(" to "), + alloc.symbol_unqualified(ability), + alloc.reflow("?"), + ]) + ]); + title = ABILITY_MEMBER_BINDS_MULTIPLE_VARIABLES.to_string(); + severity = Severity::RuntimeError; + } + Problem::AbilityMemberBindsExternalAbility { member, ability, diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 140926a53a..6585008ab9 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9397,6 +9397,46 @@ I need all branches in an `if` to have the same type! ) } + #[test] + fn ability_member_binds_parent_twice() { + new_report_problem_as( + indoc!( + r#" + app "test" provides [ ] to "./platform" + + Eq has eq : a, b -> Bool | a has Eq, b has Eq + "# + ), + indoc!( + r#" + ── ABILITY MEMBER BINDS MULTIPLE VARIABLES ───────────────────────────────────── + + The definition of the ability member `eq` includes multiple variables + bound to the `Eq`` ability:` + + 3│ Eq has eq : a, b -> Bool | a has Eq, b has Eq + ^^^^^^^^^^^^^^^^^^ + + Ability members can only bind one type variable to their parent + ability. Otherwise, I wouldn't know what type implements an ability by + looking at specializations! + + Hint: Did you mean to only bind `a` to `Eq`? + + ── UNUSED DEFINITION ─────────────────────────────────────────────────────────── + + `eq` is not used anywhere in your code. + + 3│ Eq has eq : a, b -> Bool | a has Eq, b 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. + "# + ), + ) + } + #[test] fn has_clause_outside_of_ability() { new_report_problem_as( From 389c46edcf1540234e66d8dc56944cf0b6c7387d Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 13 Apr 2022 10:12:45 -0400 Subject: [PATCH 21/21] Remove unneeded var --- compiler/can/src/abilities.rs | 12 +++--------- compiler/can/src/def.rs | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index 558f73441d..4132456a21 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -1,14 +1,13 @@ use roc_collections::all::MutMap; use roc_module::symbol::Symbol; use roc_region::all::Region; -use roc_types::{subs::Variable, types::Type}; +use roc_types::types::Type; /// 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, Clone, PartialEq, Eq)] pub struct AbilityMemberData { pub parent_ability: Symbol, - pub signature_var: Variable, pub signature: Type, pub region: Region, } @@ -49,19 +48,14 @@ pub struct AbilitiesStore { impl AbilitiesStore { /// Records the definition of an ability, including its members. - pub fn register_ability( - &mut self, - ability: Symbol, - members: Vec<(Symbol, Region, Variable, Type)>, - ) { + pub fn register_ability(&mut self, ability: Symbol, members: Vec<(Symbol, Region, Type)>) { let mut members_vec = Vec::with_capacity(members.len()); - for (member, region, signature_var, signature) in members.into_iter() { + for (member, region, signature) in members.into_iter() { members_vec.push(member); let old_member = self.ability_members.insert( member, AbilityMemberData { parent_ability: ability, - signature_var, signature, region, }, diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 83fff31fbc..2a8a6d1880 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -533,7 +533,7 @@ pub fn canonicalize_defs<'a>( .introduced_variables .union(&member_annot.introduced_variables); - can_members.push((member_sym, name_region, var_store.fresh(), member_annot.typ)); + can_members.push((member_sym, name_region, member_annot.typ)); } // Store what symbols a type must define implementations for to have this ability.