Merge pull request #2750 from rtfeldman/delay-instantiating-aliases-fix-limitations

Delay instantiating aliases fix limitations

compiler/can/src/annotation.rs

@@ -367,12 +367,8 @@ fn can_annotation_help(
                         return error;
                     }
 
-                    // For now, aliases of function types cannot be delayed.
-                    // This is a limitation of the current implementation,
-                    // and this totally should be possible in the future.
-                    let is_import = !symbol.is_builtin() && (env.home != symbol.module_id());
                     let is_structural = alias.kind == AliasKind::Structural;
-                    if !is_import && is_structural && alias.lambda_set_variables.is_empty() {
+                    if is_structural {
                         let mut type_var_to_arg = Vec::new();
 
                         for (loc_var, arg_ann) in alias.type_variables.iter().zip(args) {
@@ -381,10 +377,21 @@ fn can_annotation_help(
                             type_var_to_arg.push((name, arg_ann));
                         }
 
+                        let mut lambda_set_variables =
+                            Vec::with_capacity(alias.lambda_set_variables.len());
+
+                        for _ in 0..alias.lambda_set_variables.len() {
+                            let lvar = var_store.fresh();
+
+                            introduced_variables.insert_lambda_set(lvar);
+
+                            lambda_set_variables.push(LambdaSet(Type::Variable(lvar)));
+                        }
+
                         Type::DelayedAlias(AliasCommon {
                             symbol,
                             type_arguments: type_var_to_arg,
-                            lambda_set_variables: alias.lambda_set_variables.clone(),
+                            lambda_set_variables,
                         })
                     } else {
                         let (type_arguments, lambda_set_variables, actual) =

compiler/can/src/def.rs

@@ -22,6 +22,7 @@ use roc_problem::can::{CycleEntry, Problem, RuntimeError};
 use roc_region::all::{Loc, Region};
 use roc_types::subs::{VarStore, Variable};
 use roc_types::types::AliasKind;
+use roc_types::types::LambdaSet;
 use roc_types::types::{Alias, Type};
 use std::collections::HashMap;
 use std::fmt::Debug;
@@ -1722,12 +1723,21 @@ fn correct_mutual_recursive_type_alias<'a>(
             let alias = pending_aliases.get_mut(&rec).unwrap();
             // Don't try to instantiate the alias itself in its definition.
             let original_alias_def = to_instantiate.remove(&rec).unwrap();
+
+            let mut new_lambda_sets = ImSet::default();
             alias.typ.instantiate_aliases(
                 alias.region,
                 &to_instantiate,
                 var_store,
-                &mut ImSet::default(),
+                &mut new_lambda_sets,
             );
+
+            for lambda_set_var in new_lambda_sets {
+                alias
+                    .lambda_set_variables
+                    .push(LambdaSet(Type::Variable(lambda_set_var)));
+            }
+
             to_instantiate.insert(rec, original_alias_def);
 
             // Now mark the alias recursive, if it needs to be.

compiler/can/src/module.rs

@@ -25,7 +25,8 @@ pub struct Module {
     pub exposed_symbols: MutSet<Symbol>,
     pub referenced_values: MutSet<Symbol>,
     pub referenced_types: MutSet<Symbol>,
-    pub aliases: MutMap<Symbol, Alias>,
+    /// all aliases. `bool` indicates whether it is exposed
+    pub aliases: MutMap<Symbol, (bool, Alias)>,
     pub rigid_variables: RigidVariables,
 }
 
@@ -500,7 +501,7 @@ pub fn canonicalize_module_defs<'a>(
                 }
             }
 
-            Ok(ModuleOutput {
+            let output = ModuleOutput {
                 scope,
                 aliases,
                 rigid_variables,
@@ -511,7 +512,9 @@ pub fn canonicalize_module_defs<'a>(
                 problems: env.problems,
                 lookups,
                 ident_ids: env.ident_ids,
-            })
+            };
+
+            Ok(output)
         }
         (Err(runtime_error), _) => Err(runtime_error),
     }

compiler/can/src/scope.rs

@@ -17,7 +17,7 @@ pub struct Scope {
     symbols: SendMap<Symbol, Region>,
 
     /// The type aliases currently in scope
-    aliases: SendMap<Symbol, Alias>,
+    pub aliases: SendMap<Symbol, Alias>,
 
     /// The current module being processed. This will be used to turn
     /// unqualified idents into Symbols.

compiler/load/src/file.rs

@@ -83,7 +83,7 @@ struct ModuleCache<'a> {
     /// Phases
     headers: MutMap<ModuleId, ModuleHeader<'a>>,
     parsed: MutMap<ModuleId, ParsedModule<'a>>,
-    aliases: MutMap<ModuleId, MutMap<Symbol, Alias>>,
+    aliases: MutMap<ModuleId, MutMap<Symbol, (bool, Alias)>>,
     constrained: MutMap<ModuleId, ConstrainedModule>,
     typechecked: MutMap<ModuleId, TypeCheckedModule<'a>>,
     found_specializations: MutMap<ModuleId, FoundSpecializationsModule<'a>>,
@@ -208,8 +208,14 @@ fn start_phase<'a>(
                             imported, parsed.module_id,
                         ),
                         Some(new) => {
-                            // TODO filter to only add imported aliases
+                            aliases.extend(new.iter().filter_map(|(s, (exposed, a))| {
-                            aliases.extend(new.iter().map(|(s, a)| (*s, a.clone())));
+                                // only pass this on if it's exposed, or the alias is a transitive import
+                                if *exposed || s.module_id() != *imported {
+                                    Some((*s, a.clone()))
+                                } else {
+                                    None
+                                }
+                            }));
                         }
                     }
                 }
@@ -505,7 +511,7 @@ enum Msg<'a> {
     FinishedAllTypeChecking {
         solved_subs: Solved<Subs>,
         exposed_vars_by_symbol: Vec<(Symbol, Variable)>,
-        exposed_aliases_by_symbol: MutMap<Symbol, Alias>,
+        exposed_aliases_by_symbol: MutMap<Symbol, (bool, Alias)>,
         dep_idents: MutMap<ModuleId, IdentIds>,
         documentation: MutMap<ModuleId, ModuleDocumentation>,
     },
@@ -1151,6 +1157,11 @@ fn state_thread_step<'a>(
                     // We're done! There should be no more messages pending.
                     debug_assert!(msg_rx.is_empty());
 
+                    let exposed_aliases_by_symbol = exposed_aliases_by_symbol
+                        .into_iter()
+                        .map(|(k, (_, v))| (k, v))
+                        .collect();
+
                     let typechecked = finish(
                         state,
                         solved_subs,
@@ -1863,6 +1874,7 @@ fn update<'a>(
                         .insert(module_id, typechecked);
                 } else {
                     state.constrained_ident_ids.insert(module_id, ident_ids);
+                    state.timings.insert(module_id, module_timing);
                 }
 
                 start_tasks(arena, &mut state, work, injector, worker_listeners)?;
@@ -3169,7 +3181,7 @@ fn run_solve<'a>(
 
     let mut solve_aliases = default_aliases();
 
-    for (name, alias) in aliases.iter() {
+    for (name, (_, alias)) in aliases.iter() {
         solve_aliases.insert(*name, alias.clone());
     }
 
@@ -3287,6 +3299,8 @@ fn canonicalize_and_constrain<'a>(
         ..
     } = parsed;
 
+    let before = roc_types::types::get_type_clone_count();
+
     let mut var_store = VarStore::default();
     let canonicalized = canonicalize_module_defs(
         arena,
@@ -3302,6 +3316,16 @@ fn canonicalize_and_constrain<'a>(
         &mut var_store,
     );
 
+    let after = roc_types::types::get_type_clone_count();
+
+    log!(
+        "canonicalize of {:?} cloned Type {} times ({} -> {})",
+        module_id,
+        after - before,
+        before,
+        after
+    );
+
     let canonicalize_end = SystemTime::now();
 
     module_timing.canonicalize = canonicalize_end.duration_since(canonicalize_start).unwrap();
@@ -3315,7 +3339,7 @@ fn canonicalize_and_constrain<'a>(
                 ModuleNameEnum::App(_) => None,
                 ModuleNameEnum::Interface(name) | ModuleNameEnum::Hosted(name) => {
                     let docs = crate::docs::generate_module_docs(
-                        module_output.scope,
+                        module_output.scope.clone(),
                         name.as_str().into(),
                         &module_output.ident_ids,
                         parsed_defs,
@@ -3325,17 +3349,50 @@ fn canonicalize_and_constrain<'a>(
                 }
             };
 
+            let before = roc_types::types::get_type_clone_count();
+
             let mut constraints = Constraints::new();
+
             let constraint =
                 constrain_module(&mut constraints, &module_output.declarations, module_id);
 
+            let after = roc_types::types::get_type_clone_count();
+
+            log!(
+                "constraint gen of {:?} cloned Type {} times ({} -> {})",
+                module_id,
+                after - before,
+                before,
+                after
+            );
+
+            // scope has imported aliases, but misses aliases from inner scopes
+            // module_output.aliases does have those aliases, so we combine them
+            let mut aliases: MutMap<Symbol, (bool, Alias)> = module_output
+                .aliases
+                .into_iter()
+                .map(|(k, v)| (k, (true, v)))
+                .collect();
+            for (name, alias) in module_output.scope.aliases {
+                match aliases.entry(name) {
+                    Occupied(_) => {
+                        // do nothing
+                    }
+                    Vacant(vacant) => {
+                        if !name.is_builtin() {
+                            vacant.insert((false, alias));
+                        }
+                    }
+                }
+            }
+
             let module = Module {
                 module_id,
                 exposed_imports: module_output.exposed_imports,
                 exposed_symbols,
                 referenced_values: module_output.referenced_values,
                 referenced_types: module_output.referenced_types,
-                aliases: module_output.aliases,
+                aliases,
                 rigid_variables: module_output.rigid_variables,
             };
 

compiler/solve/src/module.rs

@@ -13,7 +13,7 @@ pub struct SolvedModule {
 
     /// all aliases and their definitions. this has to include non-exposed aliases
     /// because exposed aliases can depend on non-exposed ones)
-    pub aliases: MutMap<Symbol, Alias>,
+    pub aliases: MutMap<Symbol, (bool, Alias)>,
 
     /// Used when the goal phase is TypeChecking, and
     /// to create the types for HostExposed. This

compiler/test_gen/src/gen_primitives.rs

@@ -1908,6 +1908,35 @@ fn wildcard_rigid() {
     );
 }
 
+#[test]
+#[cfg(any(feature = "gen-llvm"))]
+fn alias_of_alias_with_type_arguments() {
+    assert_non_opt_evals_to!(
+        indoc!(
+            r#"
+            app "test" provides [ main ] to "./platform"
+
+            Effect a : [ @Effect a ]
+
+            Task a err : Effect (Result a err)
+
+            always : a -> Task a *
+            always = \x ->
+                inner = (Ok x)
+
+                @Effect inner
+
+
+            main : Task {} (Float *)
+            main = always {}
+            "#
+        ),
+        0,
+        i64,
+        |_| 0
+    );
+}
+
 #[test]
 #[cfg(any(feature = "gen-llvm"))]
 #[ignore]

compiler/types/src/types.rs

@@ -229,12 +229,20 @@ static mut TYPE_CLONE_COUNT: std::sync::atomic::AtomicUsize =
     std::sync::atomic::AtomicUsize::new(0);
 
 pub fn get_type_clone_count() -> usize {
-    unsafe { TYPE_CLONE_COUNT.load(std::sync::atomic::Ordering::SeqCst) }
+    if cfg!(debug_assertions) {
+        unsafe { TYPE_CLONE_COUNT.load(std::sync::atomic::Ordering::SeqCst) }
+    } else {
+        0
+    }
 }
 
 impl Clone for Type {
     fn clone(&self) -> Self {
-        unsafe { TYPE_CLONE_COUNT.fetch_add(1, std::sync::atomic::Ordering::SeqCst) };
+        #[cfg(debug_assertions)]
+        unsafe {
+            TYPE_CLONE_COUNT.fetch_add(1, std::sync::atomic::Ordering::SeqCst)
+        };
+
         match self {
             Self::EmptyRec => Self::EmptyRec,
             Self::EmptyTagUnion => Self::EmptyTagUnion,
@@ -886,7 +894,11 @@ impl Type {
                     TypeExtension::Closed => Ok(()),
                 }
             }
-            DelayedAlias(AliasCommon { type_arguments, .. }) => {
+            DelayedAlias(AliasCommon {
+                type_arguments,
+                lambda_set_variables: _no_aliases_in_lambda_sets,
+                ..
+            }) => {
                 for (_, ta) in type_arguments {
                     ta.substitute_alias(rep_symbol, rep_args, actual)?;
                 }
@@ -966,12 +978,16 @@ impl Type {
             DelayedAlias(AliasCommon {
                 symbol,
                 type_arguments,
+                lambda_set_variables,
                 ..
             }) => {
                 symbol == &rep_symbol
                     || type_arguments
                         .iter()
                         .any(|v| v.1.contains_symbol(rep_symbol))
+                    || lambda_set_variables
+                        .iter()
+                        .any(|v| v.0.contains_symbol(rep_symbol))
             }
             Alias {
                 symbol: alias_symbol,
@@ -1055,41 +1071,41 @@ impl Type {
         region: Region,
         aliases: &ImMap<Symbol, Alias>,
         var_store: &mut VarStore,
-        introduced: &mut ImSet<Variable>,
+        new_lambda_set_variables: &mut ImSet<Variable>,
     ) {
         use Type::*;
 
         match self {
             Function(args, closure, ret) => {
                 for arg in args {
-                    arg.instantiate_aliases(region, aliases, var_store, introduced);
+                    arg.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
-                closure.instantiate_aliases(region, aliases, var_store, introduced);
+                closure.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
-                ret.instantiate_aliases(region, aliases, var_store, introduced);
+                ret.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
             }
             FunctionOrTagUnion(_, _, ext) => {
                 if let TypeExtension::Open(ext) = ext {
-                    ext.instantiate_aliases(region, aliases, var_store, introduced);
+                    ext.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
             }
             RecursiveTagUnion(_, tags, ext) | TagUnion(tags, ext) => {
                 for (_, args) in tags {
                     for x in args {
-                        x.instantiate_aliases(region, aliases, var_store, introduced);
+                        x.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                     }
                 }
 
                 if let TypeExtension::Open(ext) = ext {
-                    ext.instantiate_aliases(region, aliases, var_store, introduced);
+                    ext.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
             }
             Record(fields, ext) => {
                 for (_, x) in fields.iter_mut() {
-                    x.instantiate_aliases(region, aliases, var_store, introduced);
+                    x.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
 
                 if let TypeExtension::Open(ext) = ext {
-                    ext.instantiate_aliases(region, aliases, var_store, introduced);
+                    ext.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
             }
             DelayedAlias(AliasCommon { .. }) => {
@@ -1109,98 +1125,145 @@ impl Type {
             } => {
                 for arg in type_args {
                     arg.1
-                        .instantiate_aliases(region, aliases, var_store, introduced);
+                        .instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
 
                 for arg in lambda_set_variables {
-                    arg.instantiate_aliases(region, aliases, var_store, introduced);
+                    arg.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                 }
 
-                actual_type.instantiate_aliases(region, aliases, var_store, introduced);
+                actual_type.instantiate_aliases(
+                    region,
+                    aliases,
+                    var_store,
+                    new_lambda_set_variables,
+                );
             }
             Apply(symbol, args, _) => {
                 if let Some(alias) = aliases.get(symbol) {
-                    if args.len() != alias.type_variables.len() {
+                    // TODO switch to this, but we still need to check for recursion with the
-                        *self = Type::Erroneous(Problem::BadTypeArguments {
+                    // `else` branch
+                    if false {
+                        let mut type_var_to_arg = Vec::new();
+
+                        for (loc_var, arg_ann) in alias.type_variables.iter().zip(args) {
+                            let name = loc_var.value.0.clone();
+
+                            type_var_to_arg.push((name, arg_ann.clone()));
+                        }
+
+                        let mut lambda_set_variables =
+                            Vec::with_capacity(alias.lambda_set_variables.len());
+
+                        for _ in 0..alias.lambda_set_variables.len() {
+                            let lvar = var_store.fresh();
+
+                            new_lambda_set_variables.insert(lvar);
+
+                            lambda_set_variables.push(LambdaSet(Type::Variable(lvar)));
+                        }
+
+                        let alias = Type::DelayedAlias(AliasCommon {
                             symbol: *symbol,
-                            region,
+                            type_arguments: type_var_to_arg,
-                            type_got: args.len() as u8,
+                            lambda_set_variables,
-                            alias_needs: alias.type_variables.len() as u8,
                         });
-                        return;
-                    }
 
-                    let mut actual = alias.typ.clone();
+                        *self = alias;
-
+                    } else {
-                    let mut named_args = Vec::with_capacity(args.len());
+                        if args.len() != alias.type_variables.len() {
-                    let mut substitution = ImMap::default();
+                            *self = Type::Erroneous(Problem::BadTypeArguments {
-
+                                symbol: *symbol,
-                    // TODO substitute further in args
+                                region,
-                    for (
+                                type_got: args.len() as u8,
-                        Loc {
+                                alias_needs: alias.type_variables.len() as u8,
-                            value: (lowercase, placeholder),
+                            });
-                            ..
+                            return;
-                        },
-                        filler,
-                    ) in alias.type_variables.iter().zip(args.iter())
-                    {
-                        let mut filler = filler.clone();
-                        filler.instantiate_aliases(region, aliases, var_store, introduced);
-                        named_args.push((lowercase.clone(), filler.clone()));
-                        substitution.insert(*placeholder, filler);
-                    }
-
-                    // make sure hidden variables are freshly instantiated
-                    let mut lambda_set_variables =
-                        Vec::with_capacity(alias.lambda_set_variables.len());
-                    for typ in alias.lambda_set_variables.iter() {
-                        if let Type::Variable(var) = typ.0 {
-                            let fresh = var_store.fresh();
-                            introduced.insert(fresh);
-                            substitution.insert(var, Type::Variable(fresh));
-                            lambda_set_variables.push(LambdaSet(Type::Variable(fresh)));
-                        } else {
-                            unreachable!("at this point there should be only vars in there");
-                        }
-                    }
-
-                    actual.instantiate_aliases(region, aliases, var_store, introduced);
-
-                    actual.substitute(&substitution);
-
-                    // instantiate recursion variable!
-                    if let Type::RecursiveTagUnion(rec_var, mut tags, mut ext) = actual {
-                        let new_rec_var = var_store.fresh();
-                        substitution.clear();
-                        substitution.insert(rec_var, Type::Variable(new_rec_var));
-
-                        for typ in tags.iter_mut().map(|v| v.1.iter_mut()).flatten() {
-                            typ.substitute(&substitution);
                         }
 
-                        if let TypeExtension::Open(ext) = &mut ext {
+                        let mut actual = alias.typ.clone();
-                            ext.substitute(&substitution);
+
+                        let mut named_args = Vec::with_capacity(args.len());
+                        let mut substitution = ImMap::default();
+
+                        // TODO substitute further in args
+                        for (
+                            Loc {
+                                value: (lowercase, placeholder),
+                                ..
+                            },
+                            filler,
+                        ) in alias.type_variables.iter().zip(args.iter())
+                        {
+                            let mut filler = filler.clone();
+                            filler.instantiate_aliases(
+                                region,
+                                aliases,
+                                var_store,
+                                new_lambda_set_variables,
+                            );
+                            named_args.push((lowercase.clone(), filler.clone()));
+                            substitution.insert(*placeholder, filler);
                         }
 
-                        actual = Type::RecursiveTagUnion(new_rec_var, tags, ext);
+                        // make sure hidden variables are freshly instantiated
-                    }
+                        let mut lambda_set_variables =
+                            Vec::with_capacity(alias.lambda_set_variables.len());
+                        for typ in alias.lambda_set_variables.iter() {
+                            if let Type::Variable(var) = typ.0 {
+                                let fresh = var_store.fresh();
+                                new_lambda_set_variables.insert(fresh);
+                                substitution.insert(var, Type::Variable(fresh));
+                                lambda_set_variables.push(LambdaSet(Type::Variable(fresh)));
+                            } else {
+                                unreachable!("at this point there should be only vars in there");
+                            }
+                        }
 
-                    *self = Type::Alias {
+                        actual.instantiate_aliases(
-                        symbol: *symbol,
+                            region,
-                        type_arguments: named_args,
+                            aliases,
-                        lambda_set_variables,
+                            var_store,
-                        actual: Box::new(actual),
+                            new_lambda_set_variables,
-                        kind: alias.kind,
+                        );
-                    };
+
+                        actual.substitute(&substitution);
+
+                        // instantiate recursion variable!
+                        if let Type::RecursiveTagUnion(rec_var, mut tags, mut ext) = actual {
+                            let new_rec_var = var_store.fresh();
+                            substitution.clear();
+                            substitution.insert(rec_var, Type::Variable(new_rec_var));
+
+                            for typ in tags.iter_mut().map(|v| v.1.iter_mut()).flatten() {
+                                typ.substitute(&substitution);
+                            }
+
+                            if let TypeExtension::Open(ext) = &mut ext {
+                                ext.substitute(&substitution);
+                            }
+
+                            actual = Type::RecursiveTagUnion(new_rec_var, tags, ext);
+                        }
+                        let alias = Type::Alias {
+                            symbol: *symbol,
+                            type_arguments: named_args,
+                            lambda_set_variables,
+                            actual: Box::new(actual),
+                            kind: alias.kind,
+                        };
+
+                        *self = alias;
+                    }
                 } else {
                     // one of the special-cased Apply types.
                     for x in args {
-                        x.instantiate_aliases(region, aliases, var_store, introduced);
+                        x.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
                     }
                 }
             }
             RangedNumber(typ, _) => {
-                typ.instantiate_aliases(region, aliases, var_store, introduced);
+                typ.instantiate_aliases(region, aliases, var_store, new_lambda_set_variables);
             }
             EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous(_) | Variable(_) => {}
         }
@@ -1406,10 +1469,18 @@ fn variables_help(tipe: &Type, accum: &mut ImSet<Variable>) {
             // this rec var doesn't need to be in flex_vars or rigid_vars
             accum.remove(rec);
         }
-        DelayedAlias(AliasCommon { type_arguments, .. }) => {
+        DelayedAlias(AliasCommon {
+            type_arguments,
+            lambda_set_variables,
+            ..
+        }) => {
             for (_, arg) in type_arguments {
                 variables_help(arg, accum);
             }
+
+            for lambda_set in lambda_set_variables {
+                variables_help(&lambda_set.0, accum);
+            }
         }
         Alias {
             type_arguments,
@@ -1530,10 +1601,22 @@ fn variables_help_detailed(tipe: &Type, accum: &mut VariableDetail) {
 
             accum.recursion_variables.insert(*rec);
         }
-        DelayedAlias(AliasCommon { type_arguments, .. }) => {
+        DelayedAlias(AliasCommon {
+            type_arguments,
+            lambda_set_variables,
+            ..
+        }) => {
             for (_, arg) in type_arguments {
                 variables_help_detailed(arg, accum);
             }
+
+            for lambda_set in lambda_set_variables {
+                if let Type::Variable(v) = lambda_set.0 {
+                    accum.lambda_set_variables.push(v);
+                } else {
+                    variables_help_detailed(&lambda_set.0, accum);
+                }
+            }
         }
         Alias {
             type_arguments,