From a53da2bc2412bbab506578a518f66096436430be Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Thu, 30 Nov 2023 22:16:01 -0600 Subject: [PATCH] Make sure late specializations of opaques inherit Inspect as needed A "late specialization" of a type is an ability specialization that is not visible or needed until after type-specialization; i.e. during monomorphization. The `Inspect.toInspector` ability is special-cased for opaques that do not claim or explicitly implement `Inspect`. In such cases, they are treated as structural types, and given the immediate specialization of `Inpect.inspectOpaque`. However, prior to this commit, that special-casing would only be applied during early specialiation (i.e. specializations visible during generalized type inference). This commit applies the special case to late specialization as well - the specialization decision for an opaque type is always the specialization of the opaque type in the late case, but now, when we go to look up the ambient lambda set of the specialization, if it does not exist and corresponds to `Inspect.toInspector`, we fall back to the immediate. One concern I have here is that in a case like ``` Op := {} x = dbg (@Op {}) ``` the specialization of `Inspect.toInspector` for `Op` should be known early. Indeed, the program ``` Op := {} x = Inspect.toInspector (@Op {}) |> Inspect.apply (Inspect.init {}) |> Inspect.toDbgStr ``` Compiles fine without this change. This makes me suspect there is an issue with the implementation of `dbg`'s desugaring. If possible, this should be addressed sooner rather than later. Closes #6127 --- crates/compiler/load_internal/src/file.rs | 4 +- crates/compiler/solve/src/specialize.rs | 220 +++++++++++------- crates/compiler/test_gen/src/gen_abilities.rs | 38 +++ .../specialize/inspect/opaque_automatic.txt | 54 +++++ .../inspect/opaque_automatic_late.txt | 61 +++++ 5 files changed, 286 insertions(+), 91 deletions(-) create mode 100644 crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic.txt create mode 100644 crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic_late.txt diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs index ea2af62253e..d24f8b96e22 100644 --- a/crates/compiler/load_internal/src/file.rs +++ b/crates/compiler/load_internal/src/file.rs @@ -4662,7 +4662,9 @@ pub fn add_imports( let mut cached_symbol_vars = VecMap::default(); - for &symbol in &exposed_for_module.imported_values { + let imported_values = exposed_for_module.imported_values.iter().copied(); + + for symbol in imported_values { import_variable_for_symbol( subs, constraints, diff --git a/crates/compiler/solve/src/specialize.rs b/crates/compiler/solve/src/specialize.rs index 54b6fd358a7..cfde2e45e35 100644 --- a/crates/compiler/solve/src/specialize.rs +++ b/crates/compiler/solve/src/specialize.rs @@ -723,115 +723,155 @@ fn get_specialization_lambda_set_ambient_function( phase: &P, ability_member: Symbol, lset_region: u8, - specialization_key: SpecializationTypeKey, + mut specialization_key: SpecializationTypeKey, target_rank: Rank, ) -> Result { - match specialization_key { - SpecializationTypeKey::Opaque(opaque) => { - let opaque_home = opaque.module_id(); - let external_specialized_lset = - phase.with_module_abilities_store(opaque_home, |abilities_store| { - let impl_key = roc_can::abilities::ImplKey { + loop { + match specialization_key { + SpecializationTypeKey::Opaque(opaque) => { + let opaque_home = opaque.module_id(); + let found = phase.with_module_abilities_store(opaque_home, |abilities_store| { + find_opaque_specialization_ambient_function( + abilities_store, opaque, ability_member, - }; - - let opt_specialization = - abilities_store.get_implementation(impl_key); - match opt_specialization { - None => { - if P::IS_LATE { - internal_error!( - "expected to know a specialization for {:?}#{:?}, but it wasn't found", - opaque, - ability_member - ); - } else { - // doesn't specialize, we'll have reported an error for this - Err(()) - } + lset_region, + ) + }); + + let external_specialized_lset = match found { + FoundOpaqueSpecialization::UpdatedSpecializationKey(key) => { + specialization_key = key; + continue; + } + FoundOpaqueSpecialization::AmbientFunction(lset) => lset, + FoundOpaqueSpecialization::NotFound => { + if P::IS_LATE { + internal_error!( + "expected to know a specialization for {:?}#{:?}, but it wasn't found", + opaque, + ability_member + ); + } else { + // We'll have reported an error for this. + return Err(()); } - Some(member_impl) => match member_impl { - MemberImpl::Impl(spec_symbol) => { - let specialization = - abilities_store.specialization_info(*spec_symbol).expect("expected custom implementations to always have complete specialization info by this point"); - - let specialized_lambda_set = *specialization - .specialization_lambda_sets - .get(&lset_region) - .unwrap_or_else(|| panic!("lambda set region not resolved: {:?}", (spec_symbol, specialization))); - Ok(specialized_lambda_set) - } - MemberImpl::Error => todo_abilities!(), - }, } - })?; + }; - let specialized_ambient = phase.copy_lambda_set_ambient_function_to_home_subs( - external_specialized_lset, - opaque_home, - subs, - ); + let specialized_ambient = phase.copy_lambda_set_ambient_function_to_home_subs( + external_specialized_lset, + opaque_home, + subs, + ); - Ok(specialized_ambient) - } + return Ok(specialized_ambient); + } - SpecializationTypeKey::Derived(derive_key) => { - let mut derived_module = derived_env.derived_module.lock().unwrap(); + SpecializationTypeKey::Derived(derive_key) => { + let mut derived_module = derived_env.derived_module.lock().unwrap(); - let (_, _, specialization_lambda_sets) = - derived_module.get_or_insert(derived_env.exposed_types, derive_key); + let (_, _, specialization_lambda_sets) = + derived_module.get_or_insert(derived_env.exposed_types, derive_key); - let specialized_lambda_set = *specialization_lambda_sets - .get(&lset_region) - .expect("lambda set region not resolved"); + let specialized_lambda_set = *specialization_lambda_sets + .get(&lset_region) + .expect("lambda set region not resolved"); - let specialized_ambient = derived_module.copy_lambda_set_ambient_function_to_subs( - specialized_lambda_set, - subs, - target_rank, - ); + let specialized_ambient = derived_module.copy_lambda_set_ambient_function_to_subs( + specialized_lambda_set, + subs, + target_rank, + ); - Ok(specialized_ambient) - } + return Ok(specialized_ambient); + } - SpecializationTypeKey::Immediate(imm) => { - // Immediates are like opaques in that we can simply look up their type definition in - // the ability store, there is nothing new to synthesize. - // - // THEORY: if something can become an immediate, it will always be available in the - // local ability store, because the transformation is local (?) - // - // TODO: I actually think we can get what we need here by examining `derived_env.exposed_types`, - // since immediates can only refer to builtins - and in userspace, all builtin types - // are available in `exposed_types`. - let immediate_lambda_set_at_region = - phase.get_and_copy_ability_member_ambient_function(imm, lset_region, subs); - - Ok(immediate_lambda_set_at_region) - } + SpecializationTypeKey::Immediate(imm) => { + // Immediates are like opaques in that we can simply look up their type definition in + // the ability store, there is nothing new to synthesize. + // + // THEORY: if something can become an immediate, it will always be available in the + // local ability store, because the transformation is local (?) + // + // TODO: I actually think we can get what we need here by examining `derived_env.exposed_types`, + // since immediates can only refer to builtins - and in userspace, all builtin types + // are available in `exposed_types`. + let immediate_lambda_set_at_region = + phase.get_and_copy_ability_member_ambient_function(imm, lset_region, subs); + + return Ok(immediate_lambda_set_at_region); + } - SpecializationTypeKey::SingleLambdaSetImmediate(imm) => { - let module_id = imm.module_id(); - debug_assert!(module_id.is_builtin()); + SpecializationTypeKey::SingleLambdaSetImmediate(imm) => { + let module_id = imm.module_id(); + debug_assert!(module_id.is_builtin()); - let module_types = &derived_env - .exposed_types - .get(&module_id) - .unwrap() - .exposed_types_storage_subs; + let module_types = &derived_env + .exposed_types + .get(&module_id) + .unwrap() + .exposed_types_storage_subs; - // Since this immediate has only one lambda set, the region must be pointing to 1, and - // moreover the imported function type is the ambient function of the single lset. - debug_assert_eq!(lset_region, 1); - let storage_var = module_types.stored_vars_by_symbol.get(&imm).unwrap(); - let imported = module_types - .storage_subs - .export_variable_to(subs, *storage_var); + // Since this immediate has only one lambda set, the region must be pointing to 1, and + // moreover the imported function type is the ambient function of the single lset. + debug_assert_eq!(lset_region, 1); + let storage_var = module_types.stored_vars_by_symbol.get(&imm).unwrap(); + let imported = module_types + .storage_subs + .export_variable_to(subs, *storage_var); - roc_types::subs::instantiate_rigids(subs, imported.variable); + roc_types::subs::instantiate_rigids(subs, imported.variable); - Ok(imported.variable) + return Ok(imported.variable); + } } } } + +enum FoundOpaqueSpecialization { + UpdatedSpecializationKey(SpecializationTypeKey), + AmbientFunction(Variable), + NotFound, +} + +fn find_opaque_specialization_ambient_function( + abilities_store: &AbilitiesStore, + opaque: Symbol, + ability_member: Symbol, + lset_region: u8, +) -> FoundOpaqueSpecialization { + let impl_key = roc_can::abilities::ImplKey { + opaque, + ability_member, + }; + + let opt_specialization = abilities_store.get_implementation(impl_key); + match opt_specialization { + None => match ability_member { + Symbol::INSPECT_TO_INSPECTOR => FoundOpaqueSpecialization::UpdatedSpecializationKey( + SpecializationTypeKey::Immediate(Symbol::INSPECT_OPAQUE), + ), + _ => FoundOpaqueSpecialization::NotFound, + }, + Some(member_impl) => match member_impl { + MemberImpl::Impl(spec_symbol) => { + let specialization = + abilities_store.specialization_info(*spec_symbol).expect("expected custom implementations to always have complete specialization info by this point"); + + let specialized_lambda_set = *specialization + .specialization_lambda_sets + .get(&lset_region) + .unwrap_or_else(|| { + panic!( + "lambda set region not resolved: {:?}", + (spec_symbol, specialization) + ) + }); + + FoundOpaqueSpecialization::AmbientFunction(specialized_lambda_set) + } + MemberImpl::Error => todo_abilities!(), + }, + } +} diff --git a/crates/compiler/test_gen/src/gen_abilities.rs b/crates/compiler/test_gen/src/gen_abilities.rs index 2bc4791b758..7c96408390e 100644 --- a/crates/compiler/test_gen/src/gen_abilities.rs +++ b/crates/compiler/test_gen/src/gen_abilities.rs @@ -2288,4 +2288,42 @@ mod inspect { RocStr ); } + + #[test] + #[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))] + fn opaque_automatic() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [main] to "./platform" + + Op := {} + + main = Inspect.toDbgStr (Inspect.inspect (@Op {})) + "# + ), + RocStr::from(r#""#), + RocStr + ); + } + + #[test] + #[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))] + fn opaque_automatic_with_polymorphic_call() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [main] to "./platform" + + Op := {} + + late = \a -> Inspect.toDbgStr (Inspect.inspect a) + + main = late (@Op {}) + "# + ), + RocStr::from(r#""#), + RocStr + ); + } } diff --git a/crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic.txt b/crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic.txt new file mode 100644 index 00000000000..ffe19f6da3d --- /dev/null +++ b/crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic.txt @@ -0,0 +1,54 @@ +# +emit:mono +app "test" provides [main] to "./platform" + +Op := {} + +main = + dbg (@Op {}) + 1 + +# -emit:mono +procedure Inspect.251 (Inspect.252): + let Inspect.317 : Str = ""; + let Inspect.316 : Str = CallByName Inspect.61 Inspect.252 Inspect.317; + ret Inspect.316; + +procedure Inspect.30 (Inspect.147): + ret Inspect.147; + +procedure Inspect.35 (Inspect.300): + ret Inspect.300; + +procedure Inspect.36 (Inspect.304): + let Inspect.311 : Str = ""; + ret Inspect.311; + +procedure Inspect.45 (Inspect.302): + let Inspect.314 : {} = Struct {}; + let Inspect.313 : {} = CallByName Inspect.30 Inspect.314; + ret Inspect.313; + +procedure Inspect.5 (Inspect.150): + let Inspect.312 : {} = CallByName Inspect.45 Inspect.150; + let Inspect.309 : {} = Struct {}; + let Inspect.308 : Str = CallByName Inspect.36 Inspect.309; + let Inspect.307 : Str = CallByName Inspect.251 Inspect.308; + ret Inspect.307; + +procedure Inspect.61 (Inspect.303, Inspect.298): + let Inspect.319 : Str = CallByName Str.3 Inspect.303 Inspect.298; + dec Inspect.298; + ret Inspect.319; + +procedure Str.3 (#Attr.2, #Attr.3): + let Str.292 : Str = lowlevel StrConcat #Attr.2 #Attr.3; + ret Str.292; + +procedure Test.0 (): + let Test.5 : {} = Struct {}; + let Test.4 : Str = CallByName Inspect.5 Test.5; + let Test.2 : Str = CallByName Inspect.35 Test.4; + dbg Test.2; + dec Test.2; + let Test.3 : I64 = 1i64; + ret Test.3; diff --git a/crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic_late.txt b/crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic_late.txt new file mode 100644 index 00000000000..198b0d15dc5 --- /dev/null +++ b/crates/compiler/uitest/tests/ability/specialize/inspect/opaque_automatic_late.txt @@ -0,0 +1,61 @@ +# +emit:mono +app "test" provides [main] to "./platform" + +Op := {} + +late = \a -> + dbg a + 1 + +main = + late (@Op {}) + +# -emit:mono +procedure Inspect.251 (Inspect.252): + let Inspect.317 : Str = ""; + let Inspect.316 : Str = CallByName Inspect.61 Inspect.252 Inspect.317; + ret Inspect.316; + +procedure Inspect.30 (Inspect.147): + ret Inspect.147; + +procedure Inspect.35 (Inspect.300): + ret Inspect.300; + +procedure Inspect.36 (Inspect.304): + let Inspect.311 : Str = ""; + ret Inspect.311; + +procedure Inspect.45 (Inspect.302): + let Inspect.314 : {} = Struct {}; + let Inspect.313 : {} = CallByName Inspect.30 Inspect.314; + ret Inspect.313; + +procedure Inspect.5 (Inspect.150): + let Inspect.312 : {} = CallByName Inspect.45 Inspect.150; + let Inspect.309 : {} = Struct {}; + let Inspect.308 : Str = CallByName Inspect.36 Inspect.309; + let Inspect.307 : Str = CallByName Inspect.251 Inspect.308; + ret Inspect.307; + +procedure Inspect.61 (Inspect.303, Inspect.298): + let Inspect.319 : Str = CallByName Str.3 Inspect.303 Inspect.298; + dec Inspect.298; + ret Inspect.319; + +procedure Str.3 (#Attr.2, #Attr.3): + let Str.292 : Str = lowlevel StrConcat #Attr.2 #Attr.3; + ret Str.292; + +procedure Test.2 (Test.3): + let Test.8 : Str = CallByName Inspect.5 Test.3; + let Test.4 : Str = CallByName Inspect.35 Test.8; + dbg Test.4; + dec Test.4; + let Test.7 : I64 = 1i64; + ret Test.7; + +procedure Test.0 (): + let Test.6 : {} = Struct {}; + let Test.5 : I64 = CallByName Test.2 Test.6; + ret Test.5;