roc-lang / roc

A fast, friendly, functional language.
https://roc-lang.org
Universal Permissive License v1.0
4.17k stars 294 forks source link

alias analysis panic involving optional record field annotations #3651

Open rtfeldman opened 2 years ago

rtfeldman commented 2 years ago

To reproduce, replace examples/hello-world/main.roc with this and run it. It panics with:

'Error in alias analysis: error in module ModName("UserApp"), function definition FuncName("\x01\x00\x00\x00\x0e\x00\x00\x00\x04\xabC{&\xb1\xde\xe0"): expected type '(heap_cell,)', found type '((heap_cell,), (heap_cell,))'', crates/compiler/gen_llvm/src/llvm/build.rs:4345:19

Note that the type annotation is necessary to reproduce it!

app "helloWorld"
    packages { pf: "platform/main.roc" }
    imports []
    provides [main] to pf

foo : { b : Str, c ? Str, a ? Str } -> [Arg { b : Str, c ? Str, a ? Str }]*
foo = \config -> Arg config

main =
    x = foo { a: "blah", b: "foo" }

    "Hello, World!\n"

Possibly related to https://github.com/rtfeldman/roc/issues/3650

ayazhafiz commented 2 years ago

Yeah, this is unfortunate. Basically what's happening is we're prepopulating the type of foo with the type signature, where the record on the left and the record on the right are disjoint. When we unify that signature with the inferred type of \config -> Arg config, the type variables for { b : Str, c ? Str, a ? Str } on the left and { b : Str, c ? Str, a ? Str } on the right are unified but not merged.

it might seem like the way to resolve that is to infer the type of foo first, and then check the inferred type against the annotation. However that fails for the same reason, the type of config will be unified on one side first.

I think the way to resolve this is to explicitly merge record field variables. Unfortunately that might eat a lot of time though.

rtfeldman commented 2 years ago

Hm, would it be noticeable in the case where these are intended to be used? (That is, you pass a record literal into function arguments and that's it.)

I suppose people could use it for more than that though, which could slow down compile times... 🤔

ayazhafiz commented 2 years ago

Thinking about this more, I'm not sure we have another option, we may just have to eat the cost here. Will run some benches