Skip to content

More coercions: reserved ~> null : opt _#134

Merged
nomeata merged 2 commits into
masterfrom
joachim/reserved-opt
Nov 13, 2020
Merged

More coercions: reserved ~> null : opt _#134
nomeata merged 2 commits into
masterfrom
joachim/reserved-opt

Conversation

@nomeata
Copy link
Copy Markdown
Contributor

@nomeata nomeata commented Nov 4, 2020

I won’t believe in this system until we have a formal proof… anyways,
still finding holes.

Imagine the sender evolves as follows:

record {} <: record { foo : reserved }

and the receiver evolves (using fancy new opt field rules) as follows:

record {} :> record { foo : opt bool }

Both are allowed, so coercion must not fail. One way to fix that is to
add

(null : reserved) ~> null : opt <t>

to the rules.

I won’t believe in this system until we have a formal proof… anyways,
still finding holes.

Imagine the sender evolves as follows:
```
record {} <: record { foo : reserved }
```
and the receiver evolves (using fancy new opt field rules) as follows:
```
record {} :> record { foo : opt bool }
```

Both are allowed, so decoding must not fail. One way to fix that is to
add
```
(null : reserved) ~> null : opt <t>
```
to the rules.
@nomeata nomeata requested a review from rossberg November 4, 2020 09:38
@nomeata nomeata changed the title More decoding: reserved ~> null : opt _ More coercions: reserved ~> null : opt _ Nov 4, 2020
@nomeata
Copy link
Copy Markdown
Contributor Author

nomeata commented Nov 13, 2020

@rossberg , did you have a chance to double check this?

@rossberg
Copy link
Copy Markdown
Contributor

I think you're right.

@nomeata nomeata merged commit d768917 into master Nov 13, 2020
@nomeata nomeata deleted the joachim/reserved-opt branch November 13, 2020 21:35
nomeata added a commit that referenced this pull request Nov 13, 2020
nomeata added a commit that referenced this pull request Nov 21, 2020
this adds some tests to account for the new behaviour of
#110 and #128 and #134 and #137.

Co-authored-by: chenyan-dfinity <yan.chen@dfinity.org>
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.

3 participants