From bc25978b30a6f9d99854196589691398478d598e Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 25 Jun 2022 11:07:56 -0400 Subject: [PATCH 1/4] Put specialization lambda sets type behind alias --- compiler/can/src/abilities.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index 464527f9e4..e164a4b10b 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -71,6 +71,17 @@ pub type SpecializationsMap = VecMap<(Symbol, Symbol), MemberSpecializati pub type PendingSpecializations = SpecializationsMap; pub type ResolvedSpecializations = SpecializationsMap; +/// Solved lambda sets for an ability member specialization. For example, if we have +/// +/// Default has default : {} -[[] + a:default:1]-> a | a has Default +/// +/// A := {} +/// default = \{} -[[closA]]-> @A {} +/// +/// and this [MemberSpecialization] is for `A`, then there is a mapping of +/// `1` to the variable representing `[[closA]]`. +pub type SpecializationLambdaSets = VecMap; + /// A particular specialization of an ability member. #[derive(Debug, Clone)] pub struct MemberSpecialization { @@ -78,20 +89,11 @@ pub struct MemberSpecialization { pub symbol: Symbol, - /// Solved lambda sets for an ability member specialization. For example, if we have - /// - /// Default has default : {} -[[] + a:default:1]-> a | a has Default - /// - /// A := {} - /// default = \{} -[[closA]]-> @A {} - /// - /// and this [MemberSpecialization] is for `A`, then there is a mapping of - /// `1` to the variable representing `[[closA]]`. - pub specialization_lambda_sets: VecMap, + pub specialization_lambda_sets: SpecializationLambdaSets, } impl MemberSpecialization { - pub fn new(symbol: Symbol, specialization_lambda_sets: VecMap) -> Self { + pub fn new(symbol: Symbol, specialization_lambda_sets: SpecializationLambdaSets) -> Self { Self { _phase: Default::default(), symbol, From 9aeda3efd71b51394c266a20ad9584ff6101dbfb Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 25 Jun 2022 11:10:35 -0400 Subject: [PATCH 2/4] Make Option have same size as SpecializationId Derivers will use ability members but not use a specialization ID, so we'll need to allow `None`. --- compiler/can/src/abilities.rs | 40 +++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/compiler/can/src/abilities.rs b/compiler/can/src/abilities.rs index e164a4b10b..18a644a77c 100644 --- a/compiler/can/src/abilities.rs +++ b/compiler/can/src/abilities.rs @@ -1,3 +1,5 @@ +use std::num::NonZeroU32; + use roc_collections::{all::MutMap, VecMap, VecSet}; use roc_error_macros::internal_error; use roc_module::symbol::{ModuleId, Symbol}; @@ -103,14 +105,9 @@ impl MemberSpecialization { } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct SpecializationId(u32); +pub struct SpecializationId(NonZeroU32); -#[allow(clippy::derivable_impls)] // let's be explicit about this -impl Default for SpecializationId { - fn default() -> Self { - Self(0) - } -} +static_assertions::assert_eq_size!(SpecializationId, Option); pub enum SpecializationLambdaSetError {} @@ -119,7 +116,7 @@ pub enum SpecializationLambdaSetError {} // TODO(abilities): this should probably go on the Scope, I don't put it there for now because we // are only dealing with intra-module abilities for now. // TODO(abilities): many of these should be `VecMap`s. Do some benchmarking. -#[derive(Default, Debug, Clone)] +#[derive(Debug, Clone)] pub struct IAbilitiesStore { /// Maps an ability to the members defining it. members_of_ability: MutMap>, @@ -141,13 +138,28 @@ pub struct IAbilitiesStore { /// member `member`, to the exact symbol that implements the ability. declared_specializations: SpecializationsMap, - next_specialization_id: u32, + next_specialization_id: NonZeroU32, /// Resolved specializations for a symbol. These might be ephemeral (known due to type solving), /// or resolved on-the-fly during mono. resolved_specializations: MutMap, } +impl Default for IAbilitiesStore { + fn default() -> Self { + Self { + members_of_ability: Default::default(), + ability_members: Default::default(), + specialization_to_root: Default::default(), + declared_specializations: Default::default(), + next_specialization_id: + // Safety: 1 != 0 + unsafe { NonZeroU32::new_unchecked(1) }, + resolved_specializations: Default::default(), + } + } +} + pub type AbilitiesStore = IAbilitiesStore; pub type PendingAbilitiesStore = IAbilitiesStore; @@ -216,10 +228,12 @@ impl IAbilitiesStore { } pub fn fresh_specialization_id(&mut self) -> SpecializationId { - debug_assert!(self.next_specialization_id != std::u32::MAX); + debug_assert!(self.next_specialization_id.get() != std::u32::MAX); let id = SpecializationId(self.next_specialization_id); - self.next_specialization_id += 1; + // Safety: we already checked this won't overflow, and we started > 0. + self.next_specialization_id = + unsafe { NonZeroU32::new_unchecked(self.next_specialization_id.get() + 1) }; id } @@ -433,8 +447,8 @@ impl IAbilitiesStore { ); } - debug_assert!(next_specialization_id == 0); - debug_assert!(self.next_specialization_id == 0); + debug_assert_eq!(next_specialization_id.get(), 1); + debug_assert_eq!(self.next_specialization_id.get(), 1); debug_assert!(resolved_specializations.is_empty()); debug_assert!(self.resolved_specializations.is_empty()); } From d22c1be05b26a2b6d616025c23b19487541077a7 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 25 Jun 2022 11:15:10 -0400 Subject: [PATCH 3/4] Make specialization IDs optional in the can AST Derived impls won't use specialization IDs --- compiler/can/src/expr.rs | 10 ++++++---- compiler/can/src/traverse.rs | 4 +++- compiler/constrain/src/expr.rs | 20 +++++++++++--------- compiler/mono/src/ir.rs | 24 ++++++++++++------------ 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index 886e9a4875..134764b955 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -99,8 +99,10 @@ pub enum Expr { AbilityMember( /// Actual member name Symbol, - /// Specialization to use, and its variable - SpecializationId, + /// Specialization to use, and its variable. + /// The specialization id may be [`None`] if construction of an ability member usage can + /// prove the usage is polymorphic. + Option, Variable, ), @@ -1351,7 +1353,7 @@ fn canonicalize_var_lookup( if scope.abilities_store.is_ability_member_name(symbol) { AbilityMember( symbol, - scope.abilities_store.fresh_specialization_id(), + Some(scope.abilities_store.fresh_specialization_id()), var_store.fresh(), ) } else { @@ -1374,7 +1376,7 @@ fn canonicalize_var_lookup( if scope.abilities_store.is_ability_member_name(symbol) { AbilityMember( symbol, - scope.abilities_store.fresh_specialization_id(), + Some(scope.abilities_store.fresh_specialization_id()), var_store.fresh(), ) } else { diff --git a/compiler/can/src/traverse.rs b/compiler/can/src/traverse.rs index 525b7c1af8..4c95c5d9d0 100644 --- a/compiler/can/src/traverse.rs +++ b/compiler/can/src/traverse.rs @@ -462,7 +462,9 @@ pub fn find_ability_member_and_owning_type_at( if region == self.region { if let &Expr::AbilityMember(member_symbol, specialization_id, _var) = expr { debug_assert!(self.found.is_none()); - self.found = match self.abilities_store.get_resolved(specialization_id) { + self.found = match specialization_id + .and_then(|id| self.abilities_store.get_resolved(id)) + { Some(spec_symbol) => { let spec_type = find_specialization_type_of_symbol( spec_symbol, diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index ac02f1fbb3..1707481319 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -351,15 +351,17 @@ pub fn constrain_expr( region, ); - // Make sure we attempt to resolve the specialization. - env.resolutions_to_make.push(OpportunisticResolve { - specialization_variable: specialization_var, - specialization_expectation: constraints.push_expected_type( - Expected::NoExpectation(Type::Variable(specialization_var)), - ), - member: symbol, - specialization_id, - }); + // Make sure we attempt to resolve the specialization, if we need to. + if let Some(specialization_id) = specialization_id { + env.resolutions_to_make.push(OpportunisticResolve { + specialization_variable: specialization_var, + specialization_expectation: constraints.push_expected_type( + Expected::NoExpectation(Type::Variable(specialization_var)), + ), + member: symbol, + specialization_id, + }); + } constraints.and_constraint([store_expected, lookup_constr]) } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 535184de30..1db11ff8fb 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -3772,14 +3772,13 @@ pub fn with_hole<'a>( specialize_naked_symbol(env, variable, procs, layout_cache, assigned, hole, symbol) } - AbilityMember(_member, specialization_id, _) => { - let specialization_symbol = - env.abilities - .with_module_abilities_store(env.home, |store| { - store - .get_resolved(specialization_id) - .expect("Specialization was never made!") - }); + AbilityMember(member, specialization_id, specialization_var) => { + let specialization_symbol = late_resolve_ability_specialization( + env, + member, + specialization_id, + specialization_var, + ); specialize_naked_symbol( env, @@ -5161,12 +5160,13 @@ pub fn with_hole<'a>( fn late_resolve_ability_specialization<'a>( env: &mut Env<'a, '_>, member: Symbol, - specialization_id: SpecializationId, + specialization_id: Option, specialization_var: Variable, ) -> Symbol { - let opt_resolved = env - .abilities - .with_module_abilities_store(env.home, |store| store.get_resolved(specialization_id)); + let opt_resolved = specialization_id.and_then(|id| { + env.abilities + .with_module_abilities_store(env.home, |store| store.get_resolved(id)) + }); if let Some(spec_symbol) = opt_resolved { // Fast path: specialization is monomorphic, was found during solving. From fe5063de2d11b11b2b0ce6b240268f6bea78781d Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 25 Jun 2022 11:17:04 -0400 Subject: [PATCH 4/4] Rename solve's DeriveKey to RequestedDeriveKey To avoid conflicts with the roc_derive_key crate. --- compiler/solve/src/ability.rs | 22 +++++++++++----------- compiler/solve/src/solve.rs | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/solve/src/ability.rs b/compiler/solve/src/ability.rs index 467529a990..aa9f37f820 100644 --- a/compiler/solve/src/ability.rs +++ b/compiler/solve/src/ability.rs @@ -57,7 +57,7 @@ pub enum Unfulfilled { /// Indexes a deriving of an ability for an opaque type. #[derive(Debug, PartialEq, Clone, Copy)] -pub struct DeriveKey { +pub struct RequestedDeriveKey { pub opaque: Symbol, pub ability: Symbol, } @@ -72,7 +72,7 @@ struct ImplKey { #[derive(Debug)] pub struct PendingDerivesTable( /// derive key -> (opaque type var to use for checking, derive region) - VecMap, + VecMap, ); impl PendingDerivesTable { @@ -89,7 +89,7 @@ impl PendingDerivesTable { ability.is_builtin_ability(), "Not a builtin - should have been caught during can" ); - let derive_key = DeriveKey { opaque, ability }; + let derive_key = RequestedDeriveKey { opaque, ability }; // Neither rank nor pools should matter here. let opaque_var = @@ -117,7 +117,7 @@ pub struct DeferredObligations { /// Derives that are claimed, but have also been determined to have /// specializations. Maps to the first member specialization of the same /// ability. - dominated_derives: VecMap, + dominated_derives: VecMap, } impl DeferredObligations { @@ -133,7 +133,7 @@ impl DeferredObligations { self.obligations.push((must_implement, on_error)); } - pub fn dominate(&mut self, key: DeriveKey, impl_region: Region) { + pub fn dominate(&mut self, key: RequestedDeriveKey, impl_region: Region) { // Only builtin abilities can be derived, and hence dominated. if self.pending_derives.0.contains_key(&key) && !self.dominated_derives.contains_key(&key) { self.dominated_derives.insert(key, impl_region); @@ -153,7 +153,7 @@ impl DeferredObligations { self, subs: &mut Subs, abilities_store: &AbilitiesStore, - ) -> (Vec, Vec) { + ) -> (Vec, Vec) { let mut problems = vec![]; let Self { @@ -282,11 +282,11 @@ type ObligationResult = Result<(), Unfulfilled>; struct ObligationCache<'a> { abilities_store: &'a AbilitiesStore, - dominated_derives: &'a VecMap, + dominated_derives: &'a VecMap, pending_derives: &'a PendingDerivesTable, impl_cache: VecMap, - derive_cache: VecMap, + derive_cache: VecMap, } enum ReadCache { @@ -342,7 +342,7 @@ impl ObligationCache<'_> { fn check_opaque(&mut self, subs: &mut Subs, opaque: Symbol, ability: Symbol) -> ReadCache { let impl_key = ImplKey { opaque, ability }; - let derive_key = DeriveKey { opaque, ability }; + let derive_key = RequestedDeriveKey { opaque, ability }; match self.pending_derives.0.get(&derive_key) { Some(&(opaque_real_var, derive_region)) => { @@ -377,7 +377,7 @@ impl ObligationCache<'_> { ReadCache::Impl => self.impl_cache.get(&ImplKey { opaque, ability }).unwrap(), ReadCache::Derive => self .derive_cache - .get(&DeriveKey { opaque, ability }) + .get(&RequestedDeriveKey { opaque, ability }) .unwrap(), } } @@ -418,7 +418,7 @@ impl ObligationCache<'_> { fn check_derive( &mut self, subs: &mut Subs, - derive_key: DeriveKey, + derive_key: RequestedDeriveKey, opaque_real_var: Variable, derive_region: Region, ) { diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 233ee42aba..dde52063c1 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1,6 +1,6 @@ use crate::ability::{ resolve_ability_specialization, type_implementing_specialization, AbilityImplError, - DeferredObligations, DeriveKey, PendingDerivesTable, Resolved, Unfulfilled, + DeferredObligations, PendingDerivesTable, RequestedDeriveKey, Resolved, Unfulfilled, }; use bumpalo::Bump; use roc_can::abilities::{AbilitiesStore, MemberSpecialization}; @@ -1660,7 +1660,7 @@ fn check_ability_specialization( .add(must_implement_ability, AbilityImplError::IncompleteAbility); // This specialization dominates any derives that might be present. deferred_obligations.dominate( - DeriveKey { + RequestedDeriveKey { opaque, ability: parent_ability, },