Previously we would pass the annotation down as the
type-to-be-monomorphized for, but that would just mean the annotation
would unify with itself. We instead want to use the variable the thunk
is being used as to be the one unified with the thunk's annotation.
Closes#2445Closes#2446
This is a bit.. ugly, or at least seems suboptimal, but I can't think of
a better way to do it currently aside from demanding a uniform
representation, which we probably don't want to do.
Another option is something like the defunctionalization we perform
today, except also capturing potential uses of nested functions in the
closure tag of an encompassing lambda. So for example,
```
f = \x -> \y -> 1
```
would now record a lambdaset with the data `[Test.f
[TypeOfInnerClos1]]`, where `TypeOfInnerClos1` is e.g.
`[Test.f.innerClos1 I8, Test.f.innerClos1 I16]`, symbolizing that the
inner closure may be specialized to take an I8 or I16. Then at the time
that we create the capture set for `f`, we create a tag noting what
specialization should be used for the inner closure, and apply the
current defunctionalization algorithm. So effectively, the type of the
inner closure becomes a capture.
I'm not sure if this is any better, or if it has more problems.
@folkertdev any thoughts?
Closes#2322
This code has a shadowing error:
```
b = False
f = \b -> b
f b
```
but prior to this commit, the compiler would hit an internal error
during monomorphization and not even get to report the error. The reason
was that when we entered the closure `\b -> b`, we would try to
introduce the identifier `b` to the scope, see that it shadows an
existing identifier, and not insert the identifier. But this meant that
when checking the body of `\b -> b`, we would think that we captured the
value `b` in the outer scope, but that's incorrect!
The present patch fixes the issue by generating new symbols for
shadowing identifiers, so deeper scopes pick up the correct reference.
This also means in the future we may be able to compile and execute code
with shadows, even though it will still be an error.
Closes#2343
This commit fixes a long-standing bug wherein bindings to polymorphic,
non-function expressions would be lowered at binding site, rather than
being specialized at the call site.
Concretely, consider the program
```
main =
n = 1
idU8 : U8 -> U8
idU8 = \m -> m
idU8 n
```
Prior to this commit, we would lower `n = 1` as part of the IR, and the
`n` at the call site `idU8 n` would reference the lowered definition.
However, at the definition site, `1` has the polymorphic type `Num *` -
it is not until the the call site that we are able to refine the type
bound by `n`, but at that point it's too late. Since the default layout
for `Num *` is a signed 64-bit int, we would generate IR like
```
procedure main():
let App.n : Builtin(Int(I64)) = 1i64;
...
let App.5 : Builtin(Int(U8)) = CallByName Add.idU8 App.n;
ret App.5;
```
But we know `idU8` expects a `u8`; giving it an `i64` is nonsense.
Indeed this would trigger LLVM miscompilations later on.
To remedy this, we now keep a sidecar table that maps symbols to the
polymorphic expression they reference, when they do so. We then
specialize references to symbols on the fly at usage sites, similar to
how we specialize function usages.
Looking at our example, the definition `n = 1` is now never lowered to
the IR directly. We only generate code for `1` at each place `n` is
referenced. As a larger example, you can imagine that
```
main =
n = 1
asU8 : U8 -> U8
asU32 : U32 -> U8
asU8 n + asU32 n
```
is lowered to the moral equivalent of
```
main =
asU8 : U8 -> U8
asU32 : U32 -> U8
asU8 1 + asU32 1
```
Moreover, transient usages of polymorphic expressions are lowered
successfully with this approach. See for example the
`monomorphized_tag_with_polymorphic_arg_and_monomorphic_arg` test in
this commit, which checks that
```
main =
mono : U8
mono = 15
poly = A
wrap = Wrapped poly mono
useWrap1 : [Wrapped [A] U8, Other] -> U8
useWrap1 =
\w -> when w is
Wrapped A n -> n
Other -> 0
useWrap2 : [Wrapped [A, B] U8] -> U8
useWrap2 =
\w -> when w is
Wrapped A n -> n
Wrapped B _ -> 0
useWrap1 wrap * useWrap2 wrap
```
has proper code generated for it, in the presence of the polymorphic
`wrap` which references the polymorphic `poly`.
https://github.com/rtfeldman/roc/pull/2347 had a different approach to
this - polymorphic expressions would be converted to (possibly capturing) thunks.
This has the benefit of reducing code size if there are many polymorphic
usages, but may make the generated code slower and makes integration
with the existing IR implementation harder. In practice I think the
average number of polymorphic usages of an expression will be very
small.
Closes https://github.com/rtfeldman/roc/issues/2336
Closes https://github.com/rtfeldman/roc/issues/2254
Closes https://github.com/rtfeldman/roc/issues/2344
Previously we would expand optional record fields to assignments when
converting record patterns to "when" expressions. This resulted in
incorrect code being generated.
Figuring out what this module was doing, and why, took me a bit less
than half an hour. We should document what's happening for others in the
future so they don't need to follow up on Zulip necessarily.
