Fix glue generation of main : Effect Str #5688
Conversation
| // TODO investigate: is LambdaSetId(1) always correct here? | ||
| // It's supposed to be the LambdaSetId for this thunk, | ||
| // but what LambdaSetId should that be, if not 1? | ||
| extern_name: fn_caller_name(&name, LambdaSetId(1)), |
There was a problem hiding this comment.
this should indeed be correct because thunks only dispatch to one destination, and more over only have place in their type that they can dispatch
| if is_function(env.subs, var) { | ||
| types.entry_points.push((name, id)); | ||
| entry_points.remove(&k); | ||
| } else { | ||
| // This top-level value isn't a function, so we will end up code-genning a thunk | ||
| // for it. We need to represent that thunk explicitly in glue! | ||
| env.add_thunk_entry_point(&mut types, name, id); | ||
| } |
There was a problem hiding this comment.
I'm not sure this logic is correct. Wouldn't the following program run into the same issue?
mainImpl1 = \{} -> ""
mainImpl2 = \{} -> ""
mainForHost : {} -> Str
mainForHost = if Bool.true then mainImpl1 else mainImpl2
This would hit the first branch on 120, but mainForHost should also be a thunk here.
This needs further investigation, clearly
|
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
We hit an edge case with this, which is that glue would unwrap the opaque
Effecttype and see that it's a function (becauseEffectis an opaque wrapper around a function), and therefore not generate a thunk formain.