From e672ce9ebdc7875e247cbc506abeb8daec990613 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 18 Jul 2022 12:39:31 -0400 Subject: [PATCH] First pass at canonicalizing and reporting syntactic abilities --- crates/compiler/can/src/def.rs | 242 +++++++++++++-------- crates/compiler/can/src/scope.rs | 1 + crates/compiler/parse/src/ast.rs | 2 +- crates/compiler/problem/src/can.rs | 18 +- crates/compiler/types/src/types.rs | 15 -- crates/reporting/src/error/canonicalize.rs | 63 +++++- crates/reporting/tests/test_reporting.rs | 81 +++++++ 7 files changed, 307 insertions(+), 115 deletions(-) diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs index 7bf9ac7a5d..136a7eb30e 100644 --- a/crates/compiler/can/src/def.rs +++ b/crates/compiler/can/src/def.rs @@ -42,7 +42,6 @@ use roc_types::types::AliasCommon; use roc_types::types::AliasKind; use roc_types::types::AliasVar; use roc_types::types::LambdaSet; -use roc_types::types::Opaque; use roc_types::types::OpaqueSupports; use roc_types::types::OptAbleType; use roc_types::types::{Alias, Type}; @@ -416,6 +415,116 @@ fn canonicalize_alias<'a>( )) } +/// Canonicalizes a claimed ability implementation like `{ eq }` or `{ eq: myEq }`. +/// Returns a mapping of the ability member to the implementation symbol. +/// If there was an error, a problem will be recorded and nothing is returned. +fn canonicalize_claimed_ability_impl<'a>( + env: &mut Env<'a>, + scope: &mut Scope, + ability: Symbol, + loc_impl: &Loc>>, +) -> Result<(Symbol, Symbol), ()> { + let ability_home = ability.module_id(); + + match loc_impl.extract_spaces().item { + AssignedField::LabelOnly(label) => { + let label_str = label.value; + let region = label.region; + + let _member_symbol = + match env.qualified_lookup_with_module_id(scope, ability_home, label_str, region) { + Ok(symbol) => symbol, + Err(_) => { + env.problem(Problem::NotAnAbilityMember { + ability, + name: label_str.to_owned(), + region, + }); + return Err(()); + } + }; + + todo!(); + // match scope.lookup(&label_str.into(), label.region) { + // Ok(symbol) => { + // return Ok((member_symbol, symbol)); + // } + // Err(_) => { + // // [Eq { eq }] + + // env.problem(Problem::ImplementationNotFound { + // ability, + // member: scope.lookup(), + // region: label.region, + // }); + // } + // } + } + AssignedField::RequiredValue(label, _spaces, value) => { + let impl_ident = match value.value { + ast::Expr::Var { module_name, ident } => { + if module_name.is_empty() { + ident + } else { + env.problem(Problem::QualifiedAbilityImpl { + region: value.region, + }); + return Err(()); + } + } + _ => { + env.problem(Problem::AbilityImplNotIdent { + region: value.region, + }); + return Err(()); + } + }; + let impl_region = value.region; + + let member_symbol = match env.qualified_lookup_with_module_id( + scope, + ability_home, + label.value, + label.region, + ) { + Ok(symbol) => symbol, + Err(_) => { + env.problem(Problem::NotAnAbilityMember { + ability, + name: label.value.to_owned(), + region: label.region, + }); + return Err(()); + } + }; + + let impl_symbol = match scope.lookup(&impl_ident.into(), impl_region) { + Ok(symbol) => symbol, + Err(err) => { + env.problem(Problem::RuntimeError(err)); + return Err(()); + } + }; + + Ok((member_symbol, impl_symbol)) + } + AssignedField::OptionalValue(_, _, _) => { + env.problem(Problem::OptionalAbilityImpl { + ability, + region: loc_impl.region, + }); + Err(()) + } + AssignedField::Malformed(_) => { + // An error will already have been reported + Err(()) + } + AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => { + internal_error!("unreachable") + } + } +} + #[inline(always)] #[allow(clippy::too_many_arguments)] fn canonicalize_opaque<'a>( @@ -429,7 +538,7 @@ fn canonicalize_opaque<'a>( ann: &'a Loc>, vars: &[Loc], has_abilities: Option<&'a Loc>>, -) -> Result { +) -> Result { let alias = canonicalize_alias( env, output, @@ -454,107 +563,58 @@ fn canonicalize_opaque<'a>( ast::HasAbility::HasAbility { ability, impls } => (ability, impls), _ => internal_error!("spaces not extracted"), }; - match ability.value { + + let ability = match ability.value { ast::TypeAnnotation::Apply(module_name, ident, []) => { match make_apply_symbol(env, region, scope, module_name, ident) { - Ok(ability) => { - if let Some(impls) = opt_impls { - // #[derive(Debug, Copy, Clone, PartialEq)] - // pub enum HasImpls<'a> { - // // `{ eq: myEq }` - // HasImpls(Collection<'a, Loc>>>), - - // // We preserve this for the formatter; canonicalization ignores it. - // SpaceBefore(&'a HasImpls<'a>, &'a [CommentOrNewline<'a>]), - // SpaceAfter(&'a HasImpls<'a>, &'a [CommentOrNewline<'a>]), - // } - let mut impl_map: VecMap = VecMap::default(); - - let ability_home = ability.module_id(); - - for loc_field in impls.extract_spaces().item.items { - match loc_field.value { - AssignedField::LabelOnly(label) => { - let label_ident = label.into(); - let region = label.region; - - let member_symbol = match env.qualified_lookup_with_module_id(scope, ability_home, label_ident, region) { - Ok(symbol) => { - symbol - } - Err(_) => { - env.problem(Problem::NotAnAbilityMember { - ability, - region, - }); - } - } - - match scope.lookup(label.value.into(), label.region) { - Ok(symbol) => { - impl_map.insert(symbol, symbol); - } - Err(_) => { - // [Eq { eq }] - - env.problem(Problem::ImplementationNotFound{ - - ability, - member: scope.lookup() - region: label.region, - }); - } - } - } - AssignedField::RequiredValue( - label, - _spaces, - Loc { - value: Expr::Var(var_name), - .. - }, - ) => { - let key_sym: Symbol = todo!(); // label - let value_sym: Symbol = todo!(); // var_name - - impl_map.insert(key_sym, value_sym); - } - } - } - - supported_abilities.push(OpaqueSupports::Implemented { - ability_name: ability, - impls: impl_map, - }); - } else { - if ability.is_builtin_ability() { - derived_abilities.push(Loc::at(region, ability)); - supported_abilities.push(OpaqueSupports::Derived(ability)); - } else { - // There was no record specified of functions to use for - // members, but also this isn't a builtin ability, so we don't - // know how to auto-derive it. - // - // Register the problem but keep going, we may still be able to compile the - // program even if a derive is missing. - env.problem(Problem::IllegalDerive(region)); - } - } - } + Ok(ability) => ability, Err(_) => { // This is bad apply; an error will have been reported for it // already. + continue; } } } _ => { + // Register the problem but keep going. + env.problem(Problem::IllegalClaimedAbility(region)); + continue; + } + }; + + if let Some(impls) = opt_impls { + let mut impl_map: VecMap = VecMap::default(); + + for loc_impl in impls.extract_spaces().item.items { + match canonicalize_claimed_ability_impl(env, scope, ability, loc_impl) { + Ok((member, opaque_impl)) => { + impl_map.insert(member, opaque_impl); + } + Err(()) => continue, + } + } + + supported_abilities.push(OpaqueSupports::Implemented { + ability_name: ability, + impls: impl_map, + }); + } else { + if ability.is_builtin_ability() { + derived_abilities.push(Loc::at(region, ability)); + supported_abilities.push(OpaqueSupports::Derived(ability)); + } else { + // There was no record specified of functions to use for + // members, but also this isn't a builtin ability, so we don't + // know how to auto-derive it. + // // Register the problem but keep going, we may still be able to compile the // program even if a derive is missing. - env.problem(Problem::IllegalDerive(region)); + env.problem(Problem::IllegalClaimedAbility(region)); } } } + // TODO: properly validate all supported_abilities if !derived_abilities.is_empty() { // Fresh instance of this opaque to be checked for derivability during solving. let fresh_inst = Type::DelayedAlias(AliasCommon { @@ -610,8 +670,6 @@ pub(crate) fn canonicalize_defs<'a>( let mut pending_type_defs = Vec::with_capacity(loc_defs.type_defs.len()); let mut pending_value_defs = Vec::with_capacity(loc_defs.value_defs.len()); - let mut output = Output::default(); - for (index, either_index) in loc_defs.tags.iter().enumerate() { match either_index.split() { Ok(type_index) => { @@ -656,7 +714,7 @@ pub(crate) fn canonicalize_defs<'a>( output, var_store, scope, - &pending_value_defs, + pending_value_defs, pattern_type, aliases, symbols_introduced, @@ -669,7 +727,7 @@ fn canonicalize_value_defs<'a>( mut output: Output, var_store: &mut VarStore, scope: &mut Scope, - value_defs: &[Loc>], + value_defs: Vec>>, pattern_type: PatternType, mut aliases: VecMap, mut symbols_introduced: MutMap, diff --git a/crates/compiler/can/src/scope.rs b/crates/compiler/can/src/scope.rs index 62ee0cf3da..b03364b290 100644 --- a/crates/compiler/can/src/scope.rs +++ b/crates/compiler/can/src/scope.rs @@ -298,6 +298,7 @@ impl Scope { let shadow_symbol = self.scopeless_symbol(ident, region); if self.abilities_store.is_ability_member_name(original_symbol) { + // TODO: remove register_specializing_symbol self.abilities_store .register_specializing_symbol(shadow_symbol, original_symbol); diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index 222ff2597e..18912e963b 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/crates/compiler/parse/src/ast.rs @@ -1173,7 +1173,7 @@ impl<'a> ExtractSpaces<'a> for HasImpls<'a> { }, HasImpls::SpaceBefore(item, before) => match item { HasImpls::HasImpls(inner) => Spaces { - before: before, + before, item: *inner, after: &[], }, diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index eadf10d0ef..4df0296f35 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -130,14 +130,20 @@ pub enum Problem { }, AbilityUsedAsType(Lowercase, Symbol, Region), NestedSpecialization(Symbol, Region), - IllegalDerive(Region), - ImplementationNotFound { - ability: Symbol, - member: Symbol, - region: Region, - }, + IllegalClaimedAbility(Region), NotAnAbilityMember { ability: Symbol, + name: String, + region: Region, + }, + OptionalAbilityImpl { + ability: Symbol, + region: Region, + }, + QualifiedAbilityImpl { + region: Region, + }, + AbilityImplNotIdent { region: Region, }, } diff --git a/crates/compiler/types/src/types.rs b/crates/compiler/types/src/types.rs index 40fd9e4bda..dc7b2bec99 100644 --- a/crates/compiler/types/src/types.rs +++ b/crates/compiler/types/src/types.rs @@ -2056,21 +2056,6 @@ impl From<&AliasVar> for OptAbleVar { } } -#[derive(Clone, Debug)] -pub struct Opaque { - pub region: Region, - pub type_variables: Vec>, - - /// lambda set variables, e.g. the one annotating the arrow in - /// a |c|-> b - pub lambda_set_variables: Vec, - pub recursion_variables: MutSet, - pub typ: Type, - pub kind: AliasKind, - - pub supports: Vec, -} - #[derive(Clone, Debug)] pub enum OpaqueSupports { Derived(Symbol), diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index da17170cad..b78276baf7 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -48,6 +48,10 @@ const ABILITY_NOT_ON_TOPLEVEL: &str = "ABILITY NOT ON TOP-LEVEL"; const SPECIALIZATION_NOT_ON_TOPLEVEL: &str = "SPECIALIZATION NOT ON TOP-LEVEL"; const ABILITY_USED_AS_TYPE: &str = "ABILITY USED AS TYPE"; const ILLEGAL_DERIVE: &str = "ILLEGAL DERIVE"; +const NOT_AN_ABILITY_MEMBER: &str = "NOT AN ABILITY MEMBER"; +const OPTIONAL_ABILITY_IMPLEMENTATION: &str = "OPTIONAL ABILITY IMPLEMENTATION"; +const QUALIFIED_ABILITY_IMPLEMENTATION: &str = "QUALIFIED ABILITY IMPLEMENTATION"; +const ABILITY_IMPLEMENTATION_NOT_IDENTIFIER: &str = "ABILITY IMPLEMENTATION NOT IDENTIFIER"; pub fn can_problem<'b>( alloc: &'b RocDocAllocator<'b>, @@ -738,7 +742,7 @@ pub fn can_problem<'b>( title = SPECIALIZATION_NOT_ON_TOPLEVEL.to_string(); severity = Severity::Warning; } - Problem::IllegalDerive(region) => { + Problem::IllegalClaimedAbility(region) => { doc = alloc.stack([ alloc.reflow("This ability cannot be derived:"), alloc.region(lines.convert_region(region)), @@ -750,6 +754,63 @@ pub fn can_problem<'b>( title = ILLEGAL_DERIVE.to_string(); severity = Severity::Warning; } + Problem::NotAnAbilityMember { + ability, + name, + region, + } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("The "), alloc.symbol_unqualified(ability), alloc.reflow(" ability does not have a member "), alloc.string(name), + ]), + alloc.region(lines.convert_region(region)), + alloc.reflow("Only implementations for members an ability has can be specified in this location.") + ]); + title = NOT_AN_ABILITY_MEMBER.to_string(); + severity = Severity::RuntimeError; + } + Problem::OptionalAbilityImpl { ability, region } => { + let hint = if ability.is_builtin() { + alloc.hint("").append( + alloc.reflow("if you want this implementation to be derived, don't include a record of implementations. For example,") + .append(alloc.type_block(alloc.concat([alloc.type_str("has ["), alloc.symbol_unqualified(ability), alloc.type_str("]")]))) + .append(alloc.reflow(" will attempt to derive ").append(alloc.symbol_unqualified(ability)))) + } else { + alloc.nil() + }; + + doc = alloc.stack([ + alloc.reflow("Ability implementations cannot be optional:"), + alloc.region(lines.convert_region(region)), + alloc.reflow("Custom implementations must be supplied fully."), + hint, + ]); + title = OPTIONAL_ABILITY_IMPLEMENTATION.to_string(); + severity = Severity::RuntimeError; + } + Problem::QualifiedAbilityImpl { region } => { + doc = alloc.stack([ + alloc.reflow("This ability implementation is qualified:"), + alloc.region(lines.convert_region(region)), + alloc.reflow( + "Custom implementations must be defined in the local scope, and unqualified.", + ), + ]); + title = QUALIFIED_ABILITY_IMPLEMENTATION.to_string(); + severity = Severity::RuntimeError; + } + Problem::AbilityImplNotIdent { region } => { + doc = alloc.stack([ + alloc.reflow("This ability implementation is not an identifier:"), + alloc.region(lines.convert_region(region)), + alloc.reflow( + "Custom ability implementations defined in this position can only be unqualified identifiers, not arbitrary expressions.", + ), + alloc.tip().append(alloc.reflow("consider defining this expression as a variable.")) + ]); + title = ABILITY_IMPLEMENTATION_NOT_IDENTIFIER.to_string(); + severity = Severity::RuntimeError; + } }; Report { diff --git a/crates/reporting/tests/test_reporting.rs b/crates/reporting/tests/test_reporting.rs index 1a1aefb334..ebb76971fe 100644 --- a/crates/reporting/tests/test_reporting.rs +++ b/crates/reporting/tests/test_reporting.rs @@ -9162,6 +9162,87 @@ All branches in an `if` must have the same type! "# ); + test_report!( + opaque_ability_impl_not_found, + indoc!( + r#" + app "test" provides [A, myEq] to "./platform" + + Eq has eq : a, a -> Bool | a has Eq + + A := U8 has [ Eq {eq: aEq} ] + + myEq = \m, n -> m == n + "# + ), + @r###" + "### + ); + + test_report!( + opaque_ability_impl_optional, + indoc!( + r#" + app "test" provides [myEq] to "./platform" + + Eq has eq : a, a -> Bool | a has Eq + + A := U8 has [ Eq {eq ? aEq} ] + + myEq = \m, n -> m == n + "# + ), + @r###" + "### + ); + + test_report!( + opaque_builtin_ability_impl_optional, + indoc!( + r#" + app "test" + imports [Encode.{ Encoding }] + provides [A, myEncoder] to "./platform" + + A := U8 has [ Encoding {toEncoder ? myEncoder} ] + + myEncoder = 1 + "# + ), + @r###" + "### + ); + + test_report!( + opaque_ability_impl_qualified, + indoc!( + r#" + app "test" provides [A] to "./platform" + + Eq has eq : a, a -> Bool | a has Eq + + A := U8 has [ Eq {eq : Bool.eq} ] + "# + ), + @r###" + "### + ); + + test_report!( + opaque_ability_impl_not_identifier, + indoc!( + r#" + app "test" provides [A] to "./platform" + + Eq has eq : a, a -> Bool | a has Eq + + A := U8 has [ Eq {eq : \m, n -> m == n} ] + "# + ), + @r###" + "### + ); + test_report!( derive_non_builtin_ability, indoc!(