fix(aggregation): on includes, don't include the module environment
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).