Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[spec] Reverse subtyping #110
[spec] Reverse subtyping #110
Changes from 1 commit
5308d5e
6122335
fb06196
638e80f
5e84198
ee84d78
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this rule is actually just an instance of the
rule, isn’t it?
Ah, but they have different elaboration of course… this makes your rules overlapping (I think you avoided that before…) A problem? Since this is just defining a binary relation on types at this point (and not an elaboration) it feels strange to use more rules than necessary.
Maybe it’s worth defining
<:
in a non-overlapping way, with simply an unrestrictedAnd then doing the different cases in the elaborated version.
Or simply only define the elaborated version…
But I thought we’d agree we need different relations for “sensible evolution” and “decoding”?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would be better if this wasn't there. Same problem as above. For now, I added similar handwaving to pepper over the non-determinism.
I realised that doesn't actually work. You need to allow all of decoding in the evolution check, because cases like that might not be under your own control: some type you use may originate from another canister, that has performed multiple evolutionary steps that are transitively non-coherent, but you have missed the intermediate step that would avoid the "ugly" transitive case when you upgrade yourself.
Hence the informal language in L785+ instead, about discouraging users to create that case and implementations to warn about it.
IOW, we need the completeness property after all, even if some cases hopefully never occur in practice if everybody follows good style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow the example. Even if you miss a step, and even if there there is incoherence, the “interface evolution” relation would still be transitive, woudn’t it? But maybe there is some complicate higher-order case where some canister is in the unfortunate situation where it is force to upgrade it’s interface in an undesirable way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our previous conclusion was (unless I misremember something) that the transitive rule (opt t <: opt t' with incompatible t, t') should not be allowed in the upgrade check, because that check always deals with a single evolutionary step where you simply shouldn't be doing that. And that we hence should define two relations, one with and the other without this rule (or something like that).
But the assumption that the check always deals with a single evolutionary step is bogus if your interface has to adapt to (the latest version of) somebody else's interface types, whose evolution might have a different pace, and who makes bad transitive evolutionary choices outside your control. In that case you may need the "bad" rule on options even during upgrade checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes… but
wasn’t the reason, I think. I thought the reason was that we only had problems when you need the bad rules due to transitivity because you composed two serivces, and each was doing one (or more) sensible steps. But the transtive relation was only relevant for decoding, not an actual service evolution.
That’s why I hope we can have a “can sensibly evolve” relation (probably with polarities), that’s transitive, and a separate relation “can be decoded at” relation that has the unwanted relations.
But maybe I am too optimistic, and there will always be corner cases where a service needs to change its interface is a “bad” way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think we can allow such subtyping premises on the elaborated rules, as these define deserialization.
Shouldn’t this rule (without the premise, and with the
if ∃
elaboration) subsume the normal rule?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I tried to keep the structure of the rules without elaboration. But I think the problem is that this rules try to express a phase separation that doesn't actually exist. All the rules should be considered "runtime". I removed the other rule and reformulated this one non-deterministically and added a comment admitting that the formulation is somewhat hand-wavy. Doing it properly would require a 4-place relation like
v:t <: v':t'
, I think, but I'd prefer leaving that for another time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can clean up separtely.
I don’t think it’d be a four-place relation: Didn’t we determine that the input data should be considered untyped (and the type description just be an encoding help)? So I’d expect
v <: v' : t
or maybe more suggestivelyv ~> v':t
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we could phrase it like that, but that would be a bit of a disconnect with the actual serialisation format, which does include a type -- it just may not be "principal".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that disconnect would be intentional: Didn't we just last week realize that it was misguided to take that type as more than just a way to help understand the binary structure of the value?
I think the conceptual phase distinction between “decoding binary to abstract value” and “interpreting a value at the expected type” is very helpful (and how many implementatoins will naturally approach this, too).