Skip to content

fix(aggregation): on includes, don't include the module environment

Alistair O'Brien requested to merge ajob410@fix-include into dev

Motivation and Context

Closes: #2163 (closed)

Example program:

module Test1 = Test.Next

module C = struct
  [@entry] let f () () : operation list * unit = [], ()
end

let test =
  let orig = Test1.originate (contract_of C) () 0tez in
  let _c = Test1.to_contract orig.taddr in
  ()

Instrumenting the interpreter a bit I got the following trace:

src/main/interpreter/common/interpreter.ml:1735:13:
loc: File "test.mligo", line 9, characters 29-39
expr: orig#2286.taddr
record': {addr = KT1MoPRoithHNa7i6LYHqeQfZB4oyWThinnS ; code = { parameter unit ;
  storage unit ;
  code { DROP ; UNIT ; NIL operation ; PAIR } } ; size = 30}
src/main/interpreter/common/interpreter.ml:1741:15:
Uncaught exception:
  
  (Not_found_s
   ("Map.find_exn: not found"
    (Label taddr (File (test.mligo:9:34 test.mligo:9:39)))))

The interpreter was trying to access the label .taddr in the record:
{addr = KT1MoPRoithHNa7i6LYHqeQfZB4oyWThinnS ; code = { parameter unit ;
  storage unit ;
  code { DROP ; UNIT ; NIL operation ; PAIR } } ; size = 30}

Unsurprisingly, this is a fatal error since taddr doesn't exist in this record. Looking at the result of aggregation, it appears that Test1.originate was resolving to the old Test.originate instead of Test.Next.originate -- which returns a record without taddr .

So, the bug lies with aggregation. Looking at the aggregation pass, a subtle bug exists in Data.include_:

let include_ : t -> t -> t =
   fun data to_include ->
    (* TODO: would be smart to use map .. ughh .. *)
    let exp =
      (* Filter variables shadowed by the include *)
      List.filter data.env.exp ~f:(fun x ->
          not
          @@ List.exists to_include.env.exp ~f:(fun new_ ->
                 Value_var.equal x.old new_.old))
    in
    let mod_ =
      (* Filter module variables shadowed by the include *)
      List.filter data.env.mod_ ~f:(fun x ->
          not
          @@ List.exists to_include.env.mod_ ~f:(fun new_ ->
                 Module_var.equal x.name new_.name))
    in
    { env = { exp = to_include.env.exp @ exp; mod_ = to_include.env.mod_ @ mod_ }
    ; content = data.content @ [ new_global_decl (Incl to_include.content) ]
    }

Namely, while include should update the content of the Data IR, it shouldn't update the environment env with the exception of the declarations defined in the content. This is because we could have something like this:

let x = 1
module A = struct
  let y = 2 + x
end

let x = "Hello world"
include A

The environment for A would be { exp = [ "x"; "y" ]; ... }, including would thus overwrite let x = "Hello world", even though we only want to include y

The offending line in my std_lib.mligo that causes this error is:

    let originate = Originate.contract
    let failwith = Assert.failwith

    include Typed_address

Including Typed_address overwrites originate with Test.originate which is in the environment of Typed_address since Typed_address is defined after it in the Test module.

You can easily verify this behaviour by swapping the order of the include and the let originate:

    include Typed_address
    let originate = Originate.contract
    let failwith = Assert.failwith

Description

The fix is surprisingly simple, we only include the module's content and not it's environment in Data.include_.

Component

  • compiler
  • website
  • webide
  • vscode-plugin
  • debugger

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Performance improvement (non-breaking change that improves performance)
  • None (change with no changelog)

Changelog

fix(aggregation): on includes, don't include the module environment

Checklist:

  • If a new syntax has been introduced, put a message on slack ligo-lsp
  • Changes follow the existing coding style (use dune @fmt to check).
  • Tests for the changes have been added (for bug fixes / feature).
  • Documentation has been updated.
  • Changelog description has been added (if appropriate).
  • Start titles under ## Changelog section with #### (if appropriate).
  • There is no image or uploaded file in changelog
  • Examples in changed behaviour have been added to the changelog (for breaking change / feature).

Merge request reports

Loading