Post-Christmas kitchen sink PR by nomeata · Pull Request #118 · caffeinelabs/motoko

nomeata · 2018-12-15T14:28:22Z

And no re-boxing around let (x,y) = foo() and similar constructors. Parked has a PR until #115 has been merged.

because of multi-value support.

this avoids much useless tuple boxing and reboxing.

with a bidirectional data flow, somehow, and a dedicate data type.

since `wasm` doesn’t support multi-value stack types yet.

because we need to include multi-value support. I tried to get rid of the wasm reference interpreter dependency alltogether, given that we really only needs the AST (we already have a copy of the serialization module), and it would be nice to have a single `Source` module, for example, but we are using a lot of stuff from `Wasm.Lib`, e.g. for value types. Once multi-value support is officially supported by the `wasm` library I am happy to undo these changes and start using the official version again.

by cherry-picking the changes to `ast.ml` and `encode.ml` from https://github.com/WebAssembly/multi-value into our code copy.

which leads to nicer code. This needs multi-value support in the Wasm AST.

this means that the compiler produces much less boxing of integers (naturals, booleans) immediately followed by such boxing.

to make the Wasm validator happy. E.g. even if two branches of an `IfE` end with a `return`, the wasm validator needs to be told that the code after the `IfE` is unreachable.

it can change the behaviour. Instead, just unbox in `Bittagged.if_unboxed` if not asked for, and leave it to the user. Actually makes the code a tad easier to understand.

and always create new named locals when needed

@paulyoung

by adding an appropriate combinator to the `InstrList` module, and setting it at crucial positions in `Compile`. Makes the code in `compile.ml` actually nicer (no `G.i_` / `G.i` any more), and should produce a much more detailed and comprehensive source map than before. In that sense, a little Christmas present for @paulyoung.

and do not pass boolean values through `box`/`unbox`. This is in preparation of using proper WebAssembly types for the various number types.

Not that this is very useful, but it was a good oportunity to add more needed structure to the compiler, e.g. operator overloading (for example, booleans are still 32 bit). This now traps upon `0-1:Nat`.

otherwise `(0:Int) - (1:Nat) == (-1:Int)` traps erronously.

to avoid a bit of boxing/unboxing. This also resolves the overloading of `.` (objects vs. actors) in the desguarer. This code is currently incomplete for type constructors, as we don't have a consenv around there.

to properly recognize objects that are instances of classes

…rray

the type infernce code knows the precise type that it has determined is compatible with a given operation (`t` in `typing`). But previously, this was not stored anywhere, and interpreter and compiler had to make educated guesses based on the type annotation of the arguments and/or the return values. This is brittle, due to subtyping, and led to bugs. Therefore, we not simply store the type within the constructor. This fixes #122.

thanks to https://github.com/dfinity-lab/dev/pull/408 But in order to keep the test suite useful, `run.sh` now automatically appends print("Top-level code done.\n") to all test cases (in `--dfinity` mode). This way, a trap in the top-level code will be observable in the `.dvm-run.ok` file It does not help with, say ignore(async {assert False}); so tests that want to check the absence of traps need to somehow make that visible.

also catch out-of-bounds error in the interpreter and treat them as traps.

this undoes 8783539.

This reverts commit a6a8008.

…him/n-ary-return

nomeata · 2019-01-08T14:39:02Z

A bunch of stuff accumulated here on this branch over christmas and early January. Most of it is purely internal to the compiler, but not all, so I would appreciate a second pair of eyes that glances the changes over?

crusso · 2019-01-08T17:47:56Z

Sorry, I just saw this. I'll take a look tomorrow - I'm actually forked off this branch for the IR type checker (claudio/check-ir) so am keen to get it in ;->

crusso · 2019-01-09T10:38:31Z

src/desugar.ml

+    | S.OptE e -> I.OptE (exp ce e)
+    | S.ObjE (s, i, es) -> obj ce at s None i es
+    | S.DotE (e, n) ->
+      begin match Type.as_immut (Type.promote ce (e.Source.note.S.note_typ)) with


I don't think the environment ce used here will be correct since it isn't constructed from the initial environment and all intervening type declarations. I guess you can either try to reconstruct the environment accurately, or generalize S.DotE to contain the information you need as a result of typechecking. The latter would be easiest...

It is the ce that comes out of typechecking the whole module that we are compiling right now, see compile_with in pipeline.ml … so it should contain at least the top-level constructors.

I don’t know if it would work with locally defined type constructors: Do they escape to the top-level

No, they shouldn't. I beleive its a constructor environment (as it should be) not a global store.
I'll do the fix as part of the check-ir branch.

crusso · 2019-01-09T11:03:58Z

src/pipeline.ml

    let prog = tailcall_optimization true prog name in
-    let prog = Desugar.prog prog in
-    let prelude = Desugar.prog prelude in
+    let scope' = Typing.adjoin_scope initial_stat_env scope in


The use of scope' to desugar prog seems dubious since scope is the environment resulting from prog, note the environment in which prog is checked. Or am I missing something? Regardless, I think just annotating S.DotE in the checked would avoid the need to pass an environment to Desugar.prog.

Ah, see above. But yeah, an annotation would be great. Can you do that? Should I wait with master for this, or is this corner-case enough to still land this in master (I guess it is not a regression…)

crusso

LGTM except for the issue with the constructor environment used for desugaring (see comments).

crusso · 2019-01-09T12:11:54Z

Adding comments is great!

## Changelog for ic-ref: Branch: release-0.8 Commits: [dfinity-lab/ic-ref@0990d033...c47126c8](https://github.com/dfinity-lab/ic-ref/compare/0990d033452af92cbea2e60c44c301aa551d7b0e...c47126c8cff6aa039f70148c92f31eaad574c5f4) * [`d16c8d15`](https://github.com/dfinity-lab/ic-ref/commit/d16c8d1567e3a2961e01f34e29e1c0ca97c1727d) Pass sectanchors to asciidoc * [`c47126c8`](https://github.com/dfinity-lab/ic-ref/commit/c47126c8cff6aa039f70148c92f31eaad574c5f4) Do not copy libs to the $out of the ic-ref derivation ([dfinity-lab/ic-ref⁠#118](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/ic-ref/issues/118))

nomeata added 30 commits December 15, 2018 13:09

Use wasm-interp in the test suite

a6a8008

because of multi-value support.

Add test case for nary calls and returns

218fc2f

Bump dev to allow n-ary function calls

acef67f

Merge remote-tracking branch 'origin/master' into HEAD

61cbbe6

Use n-ary return values

e55b821

N-ary expression refactor part 1

5a91b6f

this avoids much useless tuple boxing and reboxing.

Compiler: Avoid tuple boxing even when returning from Blocks

c4361c1

Optimize let (x,y) = foo() to avoid tuple

02dc038

Nix tweaks

329ea1a

Further refactoring of stack representations in the compiler

4e9b8ba

with a bidirectional data flow, somehow, and a dedicate data type.

Use Vanilla StackRep around Label and Break

aa7efd4

since `wasm` doesn’t support multi-value stack types yet.

Generate proper multi-value Wasm

680ade7

by cherry-picking the changes to `ast.ml` and `encode.ml` from https://github.com/WebAssembly/multi-value into our code copy.

Join the stack representations of the branches of an if

6be25b1

which leads to nicer code. This needs multi-value support in the Wasm AST.

New StackRep: UnboxedInt

9ca47b9

this means that the compiler produces much less boxing of integers (naturals, booleans) immediately followed by such boxing.

Less boxing around IsE

1e74dc9

Less boxing/unboxing around binary operations

0f5b08e

Less unboxing around binary relations

08458d2

Less unboxing around unary operations

4f89398

Correctly drop scrutinee if compile_n_ary_pat encounters WildP

f13e13e

Throw in some extra Unreachable

751f92e

to make the Wasm validator happy. E.g. even if two branches of an `IfE` end with a `return`, the wasm validator needs to be told that the code after the `IfE` is unreachable.

Delete more dead Wasm code in instrList.optimize.

05e76b0

Remove Binary-Drop-eliminiation in instrList.optimize

16aee16

it can change the behaviour. Instead, just unbox in `Bittagged.if_unboxed` if not asked for, and leave it to the user. Actually makes the code a tad easier to understand.

Get rid of tmp local in all functions

81cc2c9

and always create new named locals when needed

Merge branch 'master' into joachim/n-ary-return

38d85fa

Resolve operator overloading in desugarer

f232d02

and do not pass boolean values through `box`/`unbox`. This is in preparation of using proper WebAssembly types for the various number types.

Correctly implement == on Text

8a0960b

Numbers are now 64 bits!

a431331

Not that this is very useful, but it was a good oportunity to add more needed structure to the compiler, e.g. operator overloading (for example, booleans are still 32 bit). This now traps upon `0-1:Nat`.

Overloading of BinE: Use result type, not argument type

0e465e6

otherwise `(0:Int) - (1:Nat) == (-1:Int)` traps erronously.

nomeata added 12 commits January 2, 2019 12:33

Introduce a stack representation for (unboxed) references

a5b2d13

to avoid a bit of boxing/unboxing. This also resolves the overloading of `.` (objects vs. actors) in the desguarer. This code is currently incomplete for type constructors, as we don't have a consenv around there.

Pass con_env to desguar

c4d27a5

to properly recognize objects that are instances of classes

Only do dynamic dispatch on DotE if the field could be that of an a…

0bdd494

…rray

Update dev (no particular reason)

025d3bc

Add test case for #122

d3af081

A simple test case for trap-on-array-out-of-bounds

45544b7

also catch out-of-bounds error in the interpreter and treat them as traps.

Switch to multi-value fork of wasm-spec

429c7ad

Switch back to using the AST from the wasm library

878f2be

this undoes 8783539.

Revert "Use wasm-interp in the test suite"

34bcf44

This reverts commit a6a8008.

Merge branch 'master' of github.com:dfinity-lab/actorscript into joac…

45556f5

…him/n-ary-return

nomeata changed the title ~~n-ary returns~~ Post-Christmas kitchen sink PR Jan 8, 2019

nomeata requested a review from crusso January 8, 2019 14:39

crusso reviewed Jan 9, 2019

View reviewed changes

nomeata added 2 commits January 9, 2019 12:31

Lots of new comments in compile.ml (and some mild code cleanup)

ae7542f

Avoid building asc when invoking nix-shell

c95a5e6

nomeata merged commit c95a5e6 into master Jan 9, 2019

nomeata deleted the joachim/n-ary-return branch January 9, 2019 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Post-Christmas kitchen sink PR#118

Post-Christmas kitchen sink PR#118
nomeata merged 44 commits intomasterfrom
joachim/n-ary-return

nomeata commented Dec 15, 2018

Uh oh!

nomeata commented Jan 8, 2019

Uh oh!

crusso commented Jan 8, 2019

Uh oh!

crusso Jan 9, 2019

Uh oh!

nomeata Jan 9, 2019

Uh oh!

crusso Jan 9, 2019

Uh oh!

crusso Jan 9, 2019 •

edited

Loading

Uh oh!

nomeata Jan 9, 2019

Uh oh!

crusso left a comment

Uh oh!

crusso commented Jan 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nomeata commented Dec 15, 2018

Uh oh!

nomeata commented Jan 8, 2019

Uh oh!

crusso commented Jan 8, 2019

Uh oh!

crusso Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

nomeata Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

crusso Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

crusso Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nomeata Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

crusso left a comment

Choose a reason for hiding this comment

Uh oh!

crusso commented Jan 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

crusso Jan 9, 2019 •

edited

Loading