Merge pull request #5733 from roc-lang/glue-earlier

fix off-by-one error in glue generation
roc-lang · Aug 9, 2023 · e6e1a13 · e6e1a13
2 parents 713afc7 + bec0e52
commit e6e1a13
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 93 deletions.
diff --git a/crates/cli_testing_examples/benchmarks/platform/main.roc b/crates/cli_testing_examples/benchmarks/platform/main.roc
@@ -5,5 +5,5 @@ platform "benchmarks"
     imports [Task.{ Task }]
     provides [mainForHost]
 
-mainForHost : Task {} [] as Fx
+mainForHost : Task {} []
 mainForHost = main
diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs
@@ -41,7 +41,6 @@ use roc_mono::ir::{
     CapturedSymbols, ExternalSpecializations, GlueLayouts, PartialProc, Proc, ProcLayout, Procs,
     ProcsBase, UpdateModeIds, UsageTrackingMap,
 };
-use roc_mono::layout::LayoutInterner;
 use roc_mono::layout::{
     GlobalLayoutInterner, LambdaName, Layout, LayoutCache, LayoutProblem, Niche, STLayoutInterner,
 };
@@ -2982,8 +2981,8 @@ fn finish_specialization<'a>(
     arena: &'a Bump,
     state: State<'a>,
     subs: Subs,
-    mut layout_interner: STLayoutInterner<'a>,
-    mut exposed_to_host: ExposedToHost,
+    layout_interner: STLayoutInterner<'a>,
+    exposed_to_host: ExposedToHost,
     module_expectations: VecMap<ModuleId, Expectations>,
 ) -> Result<MonomorphizedModule<'a>, LoadingProblem<'a>> {
     if false {
@@ -3101,53 +3100,6 @@ fn finish_specialization<'a>(
         .collect();
 
     let module_id = state.root_id;
-    let mut glue_getters = Vec::new();
-
-    // the REPL does not have any platform data
-    if let (
-        EntryPoint::Executable {
-            exposed_to_host: exposed_top_levels,
-            ..
-        },
-        Some(platform_data),
-    ) = (&entry_point, platform_data.as_ref())
-    {
-        // Expose glue for the platform, not for the app module!
-        let module_id = platform_data.module_id;
-
-        for (_name, proc_layout) in exposed_top_levels.iter() {
-            let ret = &proc_layout.result;
-            for in_layout in proc_layout.arguments.iter().chain([ret]) {
-                let layout = layout_interner.get(*in_layout);
-                let ident_ids = interns.all_ident_ids.get_mut(&module_id).unwrap();
-                let all_glue_procs = roc_mono::ir::generate_glue_procs(
-                    module_id,
-                    ident_ids,
-                    arena,
-                    &mut layout_interner,
-                    arena.alloc(layout),
-                );
-
-                let lambda_set_names = all_glue_procs
-                    .legacy_layout_based_extern_names
-                    .iter()
-                    .map(|(lambda_set_id, _)| (*_name, *lambda_set_id));
-                exposed_to_host.lambda_sets.extend(lambda_set_names);
-
-                let getter_names = all_glue_procs
-                    .getters
-                    .iter()
-                    .flat_map(|(_, glue_procs)| glue_procs.iter().map(|glue_proc| glue_proc.name));
-                exposed_to_host.getters.extend(getter_names);
-
-                glue_getters.extend(all_glue_procs.getters.iter().flat_map(|(_, glue_procs)| {
-                    glue_procs
-                        .iter()
-                        .map(|glue_proc| (glue_proc.name, glue_proc.proc_layout))
-                }));
-            }
-        }
-    }
 
     let output_path = match output_path {
         Some(path_str) => Path::new(path_str).into(),
@@ -3176,9 +3128,7 @@ fn finish_specialization<'a>(
         sources,
         timings: state.timings,
         toplevel_expects,
-        glue_layouts: GlueLayouts {
-            getters: glue_getters,
-        },
+        glue_layouts: GlueLayouts { getters: vec![] },
         uses_prebuilt_platform,
     })
 }

diff --git a/crates/compiler/mono/src/ir.rs b/crates/compiler/mono/src/ir.rs
@@ -3069,9 +3069,7 @@ fn specialize_host_specializations<'a>(
         let Some(from_platform) = opt_from_platform else { continue };
 
         // now run the lambda set numbering scheme
-        let mut layout_env =
-            layout::Env::from_components(layout_cache, env.subs, env.arena, env.target_info);
-        let hels = find_lambda_sets(&mut layout_env, from_platform);
+        let hels = find_lambda_sets(env.arena, env.subs, from_platform);
 
         // now unify
         let mut unify_env = roc_unify::Env::new(
@@ -3099,10 +3097,18 @@ fn specialize_host_specializations<'a>(
 
         let mut aliases = BumpMap::default();
 
-        for (id, _, raw_function_layout) in hels {
+        for (var, id) in hels {
             let symbol = env.unique_symbol();
             let lambda_name = LambdaName::no_niche(symbol);
 
+            let mut layout_env =
+                layout::Env::from_components(layout_cache, env.subs, env.arena, env.target_info);
+            let lambda_set = env.subs.get_lambda_set(var);
+            let raw_function_layout =
+                RawFunctionLayout::from_var(&mut layout_env, lambda_set.ambient_function)
+                    .value()
+                    .unwrap();
+
             let (key, (top_level, proc)) = generate_host_exposed_function(
                 env,
                 procs,
@@ -10001,16 +10007,17 @@ impl LambdaSetId {
     }
 }
 
-fn find_lambda_sets<'a>(
-    env: &mut crate::layout::Env<'a, '_>,
+pub fn find_lambda_sets(
+    arena: &Bump,
+    subs: &Subs,
     initial: Variable,
-) -> Vec<'a, (LambdaSetId, Variable, RawFunctionLayout<'a>)> {
-    let mut stack = bumpalo::collections::Vec::new_in(env.arena);
+) -> MutMap<Variable, LambdaSetId> {
+    let mut stack = bumpalo::collections::Vec::new_in(arena);
 
     // ignore the lambda set of top-level functions
-    match env.subs.get_without_compacting(initial).content {
+    match subs.get_without_compacting(initial).content {
         Content::Structure(FlatType::Func(arguments, _, result)) => {
-            let arguments = &env.subs.variables[arguments.indices()];
+            let arguments = &subs.variables[arguments.indices()];
 
             stack.extend(arguments.iter().copied());
             stack.push(result);
@@ -10020,24 +10027,10 @@ fn find_lambda_sets<'a>(
         }
     }
 
-    let lambda_set_variables = find_lambda_sets_help(env.subs, stack);
-    let mut answer =
-        bumpalo::collections::Vec::with_capacity_in(lambda_set_variables.len(), env.arena);
-
-    for (variable, lambda_set_id) in lambda_set_variables {
-        let lambda_set = env.subs.get_lambda_set(variable);
-        let raw_function_layout = RawFunctionLayout::from_var(env, lambda_set.ambient_function)
-            .value()
-            .unwrap();
-
-        let key = (lambda_set_id, variable, raw_function_layout);
-        answer.push(key);
-    }
-
-    answer
+    find_lambda_sets_help(subs, stack)
 }
 
-pub fn find_lambda_sets_help(
+fn find_lambda_sets_help(
     subs: &Subs,
     mut stack: Vec<'_, Variable>,
 ) -> MutMap<Variable, LambdaSetId> {

diff --git a/crates/compiler/test_mono/generated/capture_void_layout_task.txt b/crates/compiler/test_mono/generated/capture_void_layout_task.txt
@@ -14,7 +14,7 @@ procedure List.66 (#Attr.2, #Attr.3):
     let List.537 : [] = lowlevel ListGetUnsafe #Attr.2 #Attr.3;
     ret List.537;
 
-procedure List.80 (#Derived_gen.11, #Derived_gen.12, #Derived_gen.13, #Derived_gen.14, #Derived_gen.15):
+procedure List.80 (#Derived_gen.13, #Derived_gen.14, #Derived_gen.15, #Derived_gen.16, #Derived_gen.17):
     joinpoint List.527 List.439 List.440 List.441 List.442 List.443:
         let List.529 : Int1 = CallByName Num.22 List.442 List.443;
         if List.529 then
@@ -27,7 +27,7 @@ procedure List.80 (#Derived_gen.11, #Derived_gen.12, #Derived_gen.13, #Derived_g
             dec List.439;
             ret List.440;
     in
-    jump List.527 #Derived_gen.11 #Derived_gen.12 #Derived_gen.13 #Derived_gen.14 #Derived_gen.15;
+    jump List.527 #Derived_gen.13 #Derived_gen.14 #Derived_gen.15 #Derived_gen.16 #Derived_gen.17;
 
 procedure List.93 (List.436, List.437, List.438):
     let List.525 : U64 = 0i64;
@@ -156,8 +156,16 @@ procedure Test.80 (Test.81):
     ret Test.83;
 
 procedure Test.84 (Test.86, #Attr.12):
-    let Test.87 : Str = "a Lambda Set is empty. Most likely there is a type error in your program.";
-    Crash Test.87
+    let Test.87 : U8 = GetTagId #Attr.12;
+    switch Test.87:
+        case 0:
+            let Test.85 : {} = CallByName Test.10 Test.86 #Attr.12;
+            ret Test.85;
+
+        default:
+            let Test.85 : {} = CallByName Test.14 Test.86 #Attr.12;
+            ret Test.85;
+
 
 procedure Test.0 ():
     let Test.35 : List [] = Array [];

diff --git a/crates/glue/src/types.rs b/crates/glue/src/types.rs
@@ -111,8 +111,7 @@ impl Types {
             target,
         );
 
-        let variables: Vec<_> = entry_points.values().copied().collect();
-        for var in variables {
+        for (_symbol, var) in entry_points.clone() {
             env.lambda_set_ids = env.find_lambda_sets(var);
             let id = env.add_toplevel_type(var, &mut types);
 
@@ -1223,8 +1222,7 @@ impl<'a> Env<'a> {
     }
 
     fn find_lambda_sets(&self, root: Variable) -> MutMap<Variable, LambdaSetId> {
-        let stack = bumpalo::vec![in self.arena; root];
-        roc_mono::ir::find_lambda_sets_help(self.subs, stack)
+        roc_mono::ir::find_lambda_sets(self.arena, self.subs, root)
     }
 
     fn add_toplevel_type(&mut self, var: Variable, types: &mut Types) -> TypeId {
@@ -1268,8 +1266,13 @@ fn add_function_type<'a>(
 
     let name = format!("RocFunction_{closure_var:?}");
 
-    let id = env.lambda_set_ids.get(&closure_var).unwrap();
-    let extern_name = format!("roc__mainForHost_{}_caller", id.0);
+    let extern_name = match env.lambda_set_ids.get(&closure_var) {
+        Some(id) => format!("roc__mainForHost_{}_caller", id.0),
+        None => {
+            debug_assert!(is_toplevel);
+            String::from("this_extern_should_not_be_used_this_is_a_bug")
+        }
+    };
 
     for arg_var in args {
         let arg_layout = env

diff --git a/examples/cli/cli-platform/main.roc b/examples/cli/cli-platform/main.roc
@@ -5,5 +5,5 @@ platform "cli"
     imports [Task.{ Task }]
     provides [mainForHost]
 
-mainForHost : Task {} [] as Fx
+mainForHost : Task {} []
 mainForHost = main
diff --git a/examples/cli/effects-platform/main.roc b/examples/cli/effects-platform/main.roc
@@ -5,5 +5,5 @@ platform "effects"
     imports [pf.Effect]
     provides [mainForHost]
 
-mainForHost : Effect.Effect {} as Fx
+mainForHost : Effect.Effect {}
 mainForHost = main
diff --git a/examples/cli/false-interpreter/platform/main.roc b/examples/cli/false-interpreter/platform/main.roc
@@ -5,5 +5,5 @@ platform "false-interpreter"
     imports [Task.{ Task }]
     provides [mainForHost]
 
-mainForHost : Str -> Task {} [] as Fx
+mainForHost : Str -> Task {} []
 mainForHost = \file -> main file