Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDL: Implement width subtyping for sequences directly #755

Closed
wants to merge 4 commits into from

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Oct 14, 2019

without appealing to records, and clarifying that it can be done lazily.

This allows functions to add new result values and new (optional)
argument values just fine. It seems.

without appealing to records, and clarifying that it can be done lazily.

This allows functions to add new result values and new (optional)
argument values just fine. It seems.
Co-Authored-By: Claudio Russo <claudio@dfinity.org>
Copy link
Contributor

@rossberg rossberg left a comment

Choose a reason for hiding this comment

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

Hm, I am not sure this is correct. It seems to me that it runs afoul the same contra-variance violation that we have run into before, i.e., the subtyping against null/any is the wrong way around in the higher-order case.

@nomeata
Copy link
Collaborator Author

nomeata commented Oct 14, 2019

It is still different from earlier attempts (e.g. indexing the judgment) because it is not “deep”. We do not mess with the subtyping/contravariance machinery in any deep or tricky way; we just have two different type constructors on the left and right of the arrow.

Because of this, I am fairly confident that it is sound, and reasonably confident that it does not forbid any useful upgrade behavior.

Co-Authored-By: Claudio Russo <claudio@dfinity.org>
@crusso
Copy link
Contributor

crusso commented Oct 14, 2019

Intriguing, but my head hurts.
I'm trying to figure out what this might mean without any coercions. An argument sequence would always have a prefix of the sequence in the type, and projecting outside the argument sequence would return null. A function would always return all of the results in the type and thus be compatible with any prefix of that type.

@crusso crusso closed this Oct 14, 2019
@crusso crusso reopened this Oct 14, 2019
nomeata added a commit that referenced this pull request Oct 15, 2019
the decoder did not ignore extra arguments, as envisioned by the current
verison of the IDL.

(This would change again a bit if we’d go for #755..)
@nomeata
Copy link
Collaborator Author

nomeata commented Oct 17, 2019

An argument sequence would always have a prefix of the sequence in the type, and projecting outside the argument sequence would return null.
A function would always return all of the results in the type and thus be compatible with any prefix of that type.

Correct

What you lose is:

  • Sending extra arguments to a function that you have no reason to believe that function is accepting.
  • Returning less results than you used to.

@nomeata nomeata mentioned this pull request Oct 23, 2019
mergify bot pushed a commit that referenced this pull request Oct 24, 2019
#757)

* IDL Decoder: Ignore extra args (vanilla argument list width subtyping)

the decoder did not ignore extra arguments, as envisioned by the current
verison of the IDL.

(This would change again a bit if we’d go for #755..)

* Fix bug in reference serialization

I had an off-by-factor-of-4 bug, which lead to extra references being
transmitted.

This also uncovered a mild bug with `dvm`, so this depends on
https://github.com/dfinity-lab/dev/pull/1117

* Fix dev import

* Back to `dev` master

* More normalization

* Update test output
@nomeata
Copy link
Collaborator Author

nomeata commented Nov 7, 2019

Probably obsoleted by #832 (As the opt machinery there will also be available in argument/result sequences.)

@dfinity-ci
Copy link

In terms of size, no changes are observed in 2 tests.
In terms of gas, no changes are observed in 2 tests.

@nomeata nomeata closed this Oct 22, 2020
@nomeata nomeata deleted the joachim/IDL/arg-res-subtype branch October 22, 2020 14:55
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.

4 participants