dfinity / motoko

Simple high-level language for writing Internet Computer canisters
Apache License 2.0
515 stars 97 forks source link

Crash with variants and `or`-patterns #4236

Closed ggreif closed 1 year ago

ggreif commented 1 year ago

This snippet crashes the compiler:

type Account = {
        holder : Principal;
        balance : Nat;
};
type Amount = {
        amount : Nat
};
type Movement = {
        #deposit : Account and Amount;
        #withdraw : Account and Amount;
};

func getAccount(m : Movement) : Account =
      switch m {
        case (#deposit accnt or #withdraw accnt) accnt
      }
}

Of course the workaround is to write

    case (#deposit accnt) accnt;
    case (#withdraw accnt) accnt

instead. Here is the backtrace:

Motoko 0.9.7 (source 1i44jx8l-n223pr9s-fcds19w3-cca3spvb)

Fatal error: exception File "ir_def/rename.ml", line 102, characters 21-27: Assertion failed
Raised at Ir_def__Rename.pat' in file "ir_def/rename.ml", line 102, characters 21-59
Called from Ir_def__Rename.pat in file "ir_def/rename.ml", line 84, characters 17-30
Called from Ir_def__Rename.case' in file "ir_def/rename.ml", line 119, characters 19-28
Called from Ir_def__Rename.case in file "ir_def/rename.ml", line 117, characters 15-29
Called from Stdlib__list.map in file "list.ml", line 92, characters 20-23
Called from Ir_def__Rename.exp' in file "ir_def/rename.ml", line 47, characters 47-59
Called from Ir_def__Rename.exp in file "ir_def/rename.ml", line 25, characters 31-44
Called from Ir_def__Rename.exp' in file "ir_def/rename.ml", line 57, characters 14-24
Called from Ir_def__Rename.exp in file "ir_def/rename.ml", line 25, characters 31-44
Called from Ir_def__Rename.dec'.(fun) in file "ir_def/rename.ml", line 132, characters 27-37
Called from Ir_def__Rename.decs.(fun) in file "ir_def/rename.ml", line 153, characters 51-63
Called from Stdlib__list.map in file "list.ml", line 92, characters 20-23
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Stdlib__list.map in file "list.ml", line 92, characters 32-39
Called from Ir_def__Rename.decs in file "ir_def/rename.ml", line 153, characters 12-73
Called from Ir_def__Rename.comp_unit in file "ir_def/rename.ml", line 170, characters 21-33
Called from Lowering__Desugar.inject_decs in file "lowering/desugar.ml", line 1008, characters 12-52
Called from Lowering__Desugar.transform_unit in file "lowering/desugar.ml", line 1073, characters 2-28
Called from Pipeline.desugar_unit in file "pipeline/pipeline.ml", line 602, characters 4-22
Called from Pipeline.compile_unit in file "pipeline/pipeline.ml", line 691, characters 16-43
Called from Pipeline.compile_unit_to_wasm in file "pipeline/pipeline.ml", line 698, characters 17-49
Called from Pipeline.compile_libs.go in file "pipeline/pipeline.ml", line 683, characters 19-54
Called from Pipeline.compile_progs in file "pipeline/pipeline.ml", line 703, characters 16-38
Called from Pipeline.compile_files in file "pipeline/pipeline.ml", line 713, characters 19-56
Called from Diag.bind in file "lang_utils/diag.ml", line 32, characters 27-30
Called from Diag.bind in file "lang_utils/diag.ml", line 32, characters 27-30
Called from Dune__exe__Moc.process_files in file "exes/moc.ml", line 225, characters 49-94
Called from Dune__exe__Moc in file "exes/moc.ml", line 320, characters 4-23
crusso commented 1 year ago

My hunch is that rename wasn't updated to accommodate or patterns with bindings...

crusso commented 1 year ago

ir_def/rename.ml

   | AltP (p1, p2) -> assert(Freevars.(M.is_empty (pat p1)));
                     assert(Freevars.(M.is_empty (pat p2)));
                     let (p1', _) = pat rho p1 in
                     let (p2' ,_) = pat rho p2 in
                     (AltP (p1', p2'), rho)

unfortunately, not entirely trivial to fix because the recursive calls will need to agree on the renaming - this is the only construct that introduces two binding occurrences for the same identifier. Annoying.

UPDATE: the solution may be to rename in p1, but just substitute in p2 (without extending rho1 or returning rho2). Except that this won't work if its a tree, not a list, of patterns. Hmmm.

crusso commented 1 year ago

I'll try to fix it.

crusso commented 1 year ago

Fixed in https://github.com/dfinity/motoko/pull/4244

ggreif commented 1 year ago

Confirmed. Using moc from dfx 0.15.2(-beta.2) and it works just fine.