Skip to content

Desguar all objects to NewObjE, and compile it properly#109

Merged
nomeata merged 3 commits intomasterfrom
joachim/desguar-obj
Dec 14, 2018
Merged

Desguar all objects to NewObjE, and compile it properly#109
nomeata merged 3 commits intomasterfrom
joachim/desguar-obj

Conversation

@nomeata
Copy link
Contributor

@nomeata nomeata commented Dec 7, 2018

if we have to do it for some, lets do it for all! So in desguar, we
now replace ObjE by BlockE { LetD … ; Var D ; LetD o NewObjE },
properly implementing the idea of an object as a bunch of closures.

Object class functions are also desugared here. The class identity is
lost. In light of getting rid of instance-of (#81) that seems fine for
now.

Actors cannot be compiled that way, so they survive untouched (as
ActorE and ActorClassD to be explicit).

The compiler now implements NewObjE so that {x = y} makes x an
alias for y, so that mutable fields stay mutable.

This fixes the semantics of test/run-dfinity/async-obj-mut.as. Yay!

For now, all actor fields have this level of indirection. A further
refinement may restrict that treatment only to mutable fields.

Small wart: If the compiler finds that a mutable variable is not
captured by a function, it will allocate it on the stack. If such a
variable is put in NewObjE, the content will be put in a new box. This
is fine for the output of the desguaring: If it was not captured, the
local x is not referenced after the object has been created.
But this is shaky ground, and may need to be revisited (probably by
beefing up the AllocHow algorithm that determines whether to
heap-allocate a variable).

@nomeata nomeata changed the title Joachim/desguar obj @nomeata Desguar all objects to NewObjE, and compile it properly Dec 7, 2018
@nomeata nomeata changed the title @nomeata Desguar all objects to NewObjE, and compile it properly Desguar all objects to NewObjE, and compile it properly Dec 7, 2018
if we have to do it for some, lets do it for all! So in `desguar`, we
now replace `ObjE` by `BlockE { LetD … ; Var D ; LetD o NewObjE }`,
properly implementing the idea of an object as a bunch of closures.

Object class functions are also desugared here. The class identity is
lost. In light of getting rid of `instance-of` (#81) that seems fine for
now.

Actors cannot be compiled that way, so they survive untouched (as
`ActorE` and `ActorClassD` to be explicit).

The compiler now implements `NewObjE` so that `{x = y}` makes `x` an
alias for `y`, so that mutable fields stay mutable.

This fixes the semantics of `test/run-dfinity/async-obj-mut.as`. Yay!

For now, *all* actor fields have this level of indirection.  A further
refinement may restrict that treatment only to mutable fields.

Small wart: If the compiler finds that a mutable variable is not
captured by a function, it will allocate it on the stack. If such a
variable is put in `NewObjE`, the content will be put in a new box. This
is fine for the output of the desguaring: If it was not captured, the
local `x` is not referenced after the object has been created.
But this is shaky ground, and may need to be revisited (probably by
beefing up the `AllocHow` algorithm that determines whether to
heap-allocate a variable).
@nomeata nomeata force-pushed the joachim/desguar-obj branch from 7fbfcf0 to ff0d5b4 Compare December 10, 2018 10:53
@crusso crusso self-requested a review December 13, 2018 11:55
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was at first worried that the desugaring of objects expressions to blocks ending in let this = NewObj ... didn't follow the same declaration order as in the interpreter (which promises 'this' first) but it turns out it doesn't matter as you can't shadow fields names or this by virtue of type-checking. I take the same liberty in the async translation, so you're in bad company;-> If we ever change our mind to allow shadowing of 'this' (or perhaps curtailing type annotations on this) then we would be in trouble so it might be safer if we both here and in async(opt).ml elaborated to { this = {fields;NewObj(...);} }.

Why can't you apply the same elaboration to actor expressions? Or is this just a todo?

| Some -> 7l
| Text -> 8l
| Indirection -> 9l
| ObjInd -> 2l
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just the value represtion of type Ref ? Why not call it that and add a comment to disambiguate from Dfn Reference

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, this is something else - MutBox represents a Ref, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MutBox is for mutable local variables. It is like ObjInd (or Some) in terms of heap layout. But MutBox should never have to be serialized when sending messages, hence I keep them separate here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is it for promises stored in fields? I think I need to study the compiler a bit more, but go ahead and merge. I'm having trouble finding a button to actually approve the PR...

@nomeata
Copy link
Contributor Author

nomeata commented Dec 13, 2018

Actually, in light of let x = e and let x = e; x not being the same thing, as you discovered recently, I will have to fix that here.

{ this = {fields;NewObj(...);}

I am not sure I follow. Can you give an example of what could go wrong?

Why can't you apply the same elaboration to actor expressions? Or is this just a todo?

Not a TODO, a “don’t know how to do”. Moving the bindings out of actor { … } would move their execution from inside the new actor to outside it. Or, put differently, actor { … } can’t capture non-sharable things, so we can’t move non-sharable things out.

This is very dissatisfying, but I don't have a good idea here.

@crusso
Copy link
Contributor

crusso commented Dec 13, 2018

I was thinking if we allowed:
new this { this = 1;}
Then the translation:
{ let this = 1; let this = newobj [this,this]; this}
wouldn't work; but
{this = { this = 1; newobj [this,this] }
might.

That's is indeed a bummer about actors. I'm pretty sure I break the semantics if an actor expression does an await in its body, but don't if it doesn't. Perhaps that's a clue to how we prevent the breakage.

@nomeata
Copy link
Contributor Author

nomeata commented Dec 13, 2018

Thanks for elaborating! Yeah, that might work… but as long as we don’t allow this, we don't have to bother, I think. (And it seems to be good to not allow this).

That's is indeed a bummer about actors. I'm pretty sure I break the semantics if an actor expression does an await in its body, but don't if it doesn't.

await in actor bodies should be ruled out since 3ef5ae1. You should not have to even look in to them when doing the translation.

@crusso
Copy link
Contributor

crusso commented Dec 13, 2018

That's good, but probably not quite true since the actor methods themselves may need rewriting internally. But nice!

@crusso
Copy link
Contributor

crusso commented Dec 13, 2018

You have my approval to merge ..

@nomeata nomeata merged commit 79fe50c into master Dec 14, 2018
@nomeata nomeata deleted the joachim/desguar-obj branch December 14, 2018 10:24
nomeata added a commit that referenced this pull request Dec 14, 2018
I noticed tha #109 had a bug: It would compile an actor class like a
local class, creating a local object instead of an actor. This is wrong.

So now, actor classes are desguared to functions returning actors. This
is better, but not perfect yet: In particular:
 * this has no chance of implementing instance-of (but we might get rid
   of that, see #81)
 * it means that the pattern matching of the class function is done by
   the creating actor, not within the created actor. This is fine as long
   as
   - pattern matching is pure, and
   - the pattern-bound types are sharable.
   But since we don’t implement non-closed actors anyways, most of this
   code does not work at all yet.
ggreif added a commit that referenced this pull request Jan 23, 2020
...no specific reason, but it's fun! Ge get

```
$ git log --oneline --first-parent 8c07b4a592e7c54ff43adf0420575d4069bfe8a9..233f63f23f0a207f291693fc1983a92a53e28b59
```
233f63f (HEAD -> master, origin/master, origin/HEAD) Merge pull request #111 from dfinity-lab/nm-update-naersk
acb95af Merge pull request #110 from dfinity-lab/nm-force-docheck
6b8ecf7 Merge pull request #108 from dfinity-lab/basvandijk/fetch-sources.nix
1b0691a Merge pull request #109 from dfinity-lab/basvandijk/filter-dot-git-from-nix-fmt
f0db4f4 Merge pull request #107 from dfinity-lab/nm-cmake-bash
dfinity-bot added a commit that referenced this pull request Jul 9, 2020
mergify bot pushed a commit that referenced this pull request Jul 9, 2020
dfinity-bot added a commit that referenced this pull request Oct 28, 2020
## Changelog for candid:
Branch: 
Commits: [dfinity/candid@a1dcbad4...3e3ad95a](dfinity/candid@a1dcbad...3e3ad95)

* [`119703ba`](dfinity/candid@119703b) [Spec] Relax LEB128 decoding ([dfinity/candid⁠#79](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/79))
* [`10f08432`](dfinity/candid@10f0843) Update prim.test.did
* [`b2524816`](dfinity/candid@b252481) parser for test suite ([dfinity/candid⁠#78](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/78))
* [`71bf6e76`](dfinity/candid@71bf6e7) release 0.5.2 ([dfinity/candid⁠#80](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/80))
* [`b9f387e3`](dfinity/candid@b9f387e) test suite for JS ([dfinity/candid⁠#81](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/81))
* [`9e5dc775`](dfinity/candid@9e5dc77) Release ([dfinity/candid⁠#82](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/82))
* [`1df9d2d7`](dfinity/candid@1df9d2d) more candid test data ([dfinity/candid⁠#83](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/83))
* [`9e4156d9`](dfinity/candid@9e4156d) fix newtype ([dfinity/candid⁠#85](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/85))
* [`6880a430`](dfinity/candid@6880a43) display for types ([dfinity/candid⁠#86](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/86))
* [`04b1b068`](dfinity/candid@04b1b06) release ([dfinity/candid⁠#87](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/87))
* [`117c6436`](dfinity/candid@117c643) Refactor Lexer ([dfinity/candid⁠#89](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/89))
* [`0a5789f9`](dfinity/candid@0a5789f) fix value pretty printer ([dfinity/candid⁠#92](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/92))
* [`9f35a5aa`](dfinity/candid@9f35a5a) refactor error ([dfinity/candid⁠#94](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/94))
* [`2e742927`](dfinity/candid@2e74292) Parse annvals in textual format ([dfinity/candid⁠#93](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/93))
* [`0a144c79`](dfinity/candid@0a144c7) use principal from ic-types ([dfinity/candid⁠#84](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/84))
* [`01412b14`](dfinity/candid@01412b1) release ([dfinity/candid⁠#95](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/95))
* [`f540df54`](dfinity/candid@f540df5) release ([dfinity/candid⁠#98](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/98))
* [`798675d8`](dfinity/candid@798675d) Add generic functions to encode/decode around a tuple ([dfinity/candid⁠#99](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/99))
* [`0d26e568`](dfinity/candid@0d26e56) release ([dfinity/candid⁠#100](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/100))
* [`191b6f1f`](dfinity/candid@191b6f1) Reset record_nesting_depth after each value ([dfinity/candid⁠#101](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/101))
* [`8e7be65d`](dfinity/candid@8e7be65) fix record ([dfinity/candid⁠#103](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/103))
* [`159533b2`](dfinity/candid@159533b) Update construct.test.did
* [`a6ea0991`](dfinity/candid@a6ea099) add service initialization parameters ([dfinity/candid⁠#88](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/88))
* [`3a1f56fa`](dfinity/candid@3a1f56f) refactor: sort dependencies and add traits for error types ([dfinity/candid⁠#105](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/105))
* [`89df78ee`](dfinity/candid@89df78e) support service constructor ([dfinity/candid⁠#106](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/106))
* [`738d5ed4`](dfinity/candid@738d5ed) fix for actor class codegen ([dfinity/candid⁠#107](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/107))
* [`97ba7a0f`](dfinity/candid@97ba7a0) export init args in js ([dfinity/candid⁠#108](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/108))
* [`d4e00adc`](dfinity/candid@d4e00ad) fix js init export ([dfinity/candid⁠#109](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/109))
* [`c1662abe`](dfinity/candid@c1662ab) [spec] Reverse subtyping ([dfinity/candid⁠#110](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/110))
* [`713595be`](dfinity/candid@713595b) The “reverse variant extension rule” is redundand ([dfinity/candid⁠#113](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/113))
* [`79d49a01`](dfinity/candid@79d49a0) Spec: Opt decoding also from non-opt values ([dfinity/candid⁠#114](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/114))
* [`2cfc0ecf`](dfinity/candid@2cfc0ec) improve pretty printing for values ([dfinity/candid⁠#116](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/116))
* [`8fafe345`](dfinity/candid@8fafe34) Un-rename Soundness document ([dfinity/candid⁠#115](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/115))
* [`8e6fc502`](dfinity/candid@8e6fc50) Bump spec version ([dfinity/candid⁠#112](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/112))
* [`7cedebcb`](dfinity/candid@7cedebc) fix clippy ([dfinity/candid⁠#117](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/117))
* [`a732a639`](dfinity/candid@a732a63) Candid UI Canister ([dfinity/candid⁠#111](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/111))
* [`d97b271c`](dfinity/candid@d97b271) disable pretty printing for large vectors ([dfinity/candid⁠#118](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/118))
* [`aceb7f92`](dfinity/candid@aceb7f9) derive candid type for functions ([dfinity/candid⁠#119](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/119))
* [`c3dc0ad7`](dfinity/candid@c3dc0ad) rename derived code for CDK ([dfinity/candid⁠#120](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/120))
* [`d1f8de7d`](dfinity/candid@d1f8de7) release ([dfinity/candid⁠#121](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/121))
* [`3e3ad95a`](dfinity/candid@3e3ad95) remove multi-line string in test suites ([dfinity/candid⁠#125](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/125))
dfinity-bot added a commit that referenced this pull request Dec 2, 2022
## Changelog for ic-hs:
Branch: master
Commits: [dfinity/ic-hs@0f87c270...20e23e1a](dfinity/ic-hs@0f87c27...20e23e1)

* [`8c2207ee`](dfinity/ic-hs@8c2207e) query balance and balance128 within the same query ([dfinity/ic-hs⁠#106](https://github.com/dfinity/ic-hs/issues/106))
* [`5a076ad1`](dfinity/ic-hs@5a076ad) rename canister state counter to canister version ([dfinity/ic-hs⁠#109](https://github.com/dfinity/ic-hs/issues/109))
* [`d095478b`](dfinity/ic-hs@d095478) Include ic-ref-test and universal canister in release build artifacts ([dfinity/ic-hs⁠#111](https://github.com/dfinity/ic-hs/issues/111))
* [`3a58e164`](dfinity/ic-hs@3a58e16) Add canister global timer ([dfinity/ic-hs⁠#107](https://github.com/dfinity/ic-hs/issues/107))
* [`c863b6ef`](dfinity/ic-hs@c863b6e) fix provisional top up test ([dfinity/ic-hs⁠#112](https://github.com/dfinity/ic-hs/issues/112))
* [`2cd76efb`](dfinity/ic-hs@2cd76ef) run system tasks periodically ([dfinity/ic-hs⁠#113](https://github.com/dfinity/ic-hs/issues/113))
* [`3c2eb69a`](dfinity/ic-hs@3c2eb69) narrow down gap for canister http_requests between ic-ref(-test) and Interface Spec ([dfinity/ic-hs⁠#100](https://github.com/dfinity/ic-hs/issues/100))
* [`fb76b646`](dfinity/ic-hs@fb76b64) ic-ref-run: Execute heartbeats before any submitted message ([dfinity/ic-hs⁠#114](https://github.com/dfinity/ic-hs/issues/114))
* [`20e23e1a`](dfinity/ic-hs@20e23e1) do not return an HTTP error for calls to stopping/stopped canisters ([dfinity/ic-hs⁠#115](https://github.com/dfinity/ic-hs/issues/115))
mergify bot pushed a commit that referenced this pull request Dec 6, 2022
## Changelog for ic-hs:
Branch: master
Commits: [dfinity/ic-hs@0f87c270...20e23e1a](dfinity/ic-hs@0f87c27...20e23e1)

* [`8c2207ee`](dfinity/ic-hs@8c2207e) query balance and balance128 within the same query ([dfinity/ic-hs⁠#106](https://github.com/dfinity/ic-hs/issues/106))
* [`5a076ad1`](dfinity/ic-hs@5a076ad) rename canister state counter to canister version ([dfinity/ic-hs⁠#109](https://github.com/dfinity/ic-hs/issues/109))
* [`d095478b`](dfinity/ic-hs@d095478) Include ic-ref-test and universal canister in release build artifacts ([dfinity/ic-hs⁠#111](https://github.com/dfinity/ic-hs/issues/111))
* [`3a58e164`](dfinity/ic-hs@3a58e16) Add canister global timer ([dfinity/ic-hs⁠#107](https://github.com/dfinity/ic-hs/issues/107))
* [`c863b6ef`](dfinity/ic-hs@c863b6e) fix provisional top up test ([dfinity/ic-hs⁠#112](https://github.com/dfinity/ic-hs/issues/112))
* [`2cd76efb`](dfinity/ic-hs@2cd76ef) run system tasks periodically ([dfinity/ic-hs⁠#113](https://github.com/dfinity/ic-hs/issues/113))
* [`3c2eb69a`](dfinity/ic-hs@3c2eb69) narrow down gap for canister http_requests between ic-ref(-test) and Interface Spec ([dfinity/ic-hs⁠#100](https://github.com/dfinity/ic-hs/issues/100))
* [`fb76b646`](dfinity/ic-hs@fb76b64) ic-ref-run: Execute heartbeats before any submitted message ([dfinity/ic-hs⁠#114](https://github.com/dfinity/ic-hs/issues/114))
* [`20e23e1a`](dfinity/ic-hs@20e23e1) do not return an HTTP error for calls to stopping/stopped canisters ([dfinity/ic-hs⁠#115](https://github.com/dfinity/ic-hs/issues/115))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants