-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add support for type dynamic #292
base: master
Are you sure you want to change the base?
Conversation
That looks plausible but only deals with a single value, right? For the use case we were discussing, the arguments and return of a generic call forwarding mechanism, wouldn't the dyn value need to pack a sequence of values? The forwarding mechanism needs to work for any arg/return arity. |
@nomeata, interested in your view on this (but can't add you as reviewer). |
Yes, the second class nature of the argument tuple type may cause issues here. |
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.
Not sure where to discuss this, but I find that the content in the issue has been helpful to remind myself of the past discussion, so I added a comment there:
#245 (comment)
As mentioned elsewhere, I think I'm fine if this works just for a fixed arity for now. The wallet example could add methods for different return arities if that's needed. Happy to defer thinking about tuple conversions later. I think the bigger problem with the wallet example actually is a different one: the callee that somebody wants to forward to presumably is not expecting a |
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.
[Deleted, wrong PR. :)]
From an implementation point of view, I think it wouldn't be too much of an issue - just recurse into the nested encoding. Although, what do we do on failure - trap? Let the application handle that? Is I'd be inclined to let these dyn values be complete encoded candid sequences, with type table, for the same reasons as in the closure discussion. |
That would be nice. Is that possible? |
Not sure. If we have |
Let's discuss that, perhaps? To me, it seems reasonable to treat these Later, when it becomes clear how to make everything that developers and we understand explicitly into something more implicit, I would embrace that too.
I don't see why this is necessary -- the goal is generic data, and that could reasonably mean explicit generic data, so long as there are (explicit) conversions in Motoko and the |
To be clear, I'm not saying we should not have an explicit representation of type
This is supposed to be generic, i.e., work with any function, including all those whose result has not been typed as
I'd hope so, but don't know how well-prepared the decoder is for reentrancy.
If we allow both a Motoko-side
Isn't that what they are already? (Note how the encoding uses the |
Yes, my bad, I had pages out the concrete proposed change. |
While gradual-typing-like features seem inevitable in the limit, I wonder if something much simpler and more explicit could still work in the interim? My thinking was that the MVP would merely introduce Hence, a closure of type Rather, that coercion (which could fail) happens when some response-handling Motoko/Rust/Wasm code eventually tries to unpack and use the I realize that my more limited conception of this feature is not as powerful as the one that @rossberg envisions. But, I'm not in favor of having silent, implicit traps when these coercions fail, at least not in the MVP. Wallets and use cases like them are not that common, and we do not need to optimize the code's concision for them yet. IMO this implicit feature does not offer enough "bang" to justify all of the many, many footguns that could be fashioned from it from its misuse.
Ah, I see --- that's a more liberal design that I first imagined. I was only thinking about the former choice, and not considering that we support the latter initially. Is there a way to avoid offering the second choice initially? You seem to indicate no, but I still wonder:
I think there's room for a "less generic" interpretation, where all of the types match on the nose and there is no implicit conversion -- in that realm, |
Hm, an argument sequence makes no sense for most use cases. Typically, you need individual values, e.g. the generic value of a key/value store.
But how would that address the wallet use case? The wallet doesn't get to choose whether some other pre-existing interface uses return type dynamic. And if it actually did, it would ultimately mean that everybody would have to use dynamic for every method, which obviously is highly undesirable. |
Some explicit primitives, e.g., in a Motoko library, would introduce and elim the argument sequences (much like we envision for converting to/from
Well, no, only the methods that are to be used in a higher-order and generic way need to use this type. Presumably, not every API call needs to be used that way, and the ones that do can be re-exposed with the requisite Do I think this is the "right design"? No, of course not. I'm merely pushing back on the idea that we must have implicit type coercions to have an MVP that has some value. Arguably, this idea lacks sufficient value to execute (for the reasons that you are giving, which I find compelling), but to me, that's a separate question. ;) |
spec/Candid.md
Outdated
@@ -1265,12 +1310,14 @@ Deserialisation at an expected type sequence `(<t'>,*)` proceeds by | |||
|
|||
Deserialisation uses the following mechanism for robustness towards future extensions: | |||
|
|||
* A serialised type may be headed by an opcode other than the ones defined above (i.e., less than -24). Any such opcode is followed by an LEB128-encoded count, and then a number of bytes corresponding to this count. A type represented that way is called a *future type*. | |||
* A serialised type may be headed by an other than -1 to -24 . Any such opcode is followed by an LEB128-encoded count, and then a number of bytes corresponding to this count. A type represented that way is called a *future type*. |
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.
the word "opcode" got dropped during the edits?
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.
Oops, fixed.
Hm, I believe it's a crucial point for an abstraction like the wallet's call forwarding that it's applicable to every method. Without this ability, it's not a solution to that problem, and in addition, would create bad incentives (like using |
Okay, I extended the proposal with proper support for gradual typing. |
I just noticed that the sketched http_request interface also works around the lack of a generic type, as it has this:
It then uses |
spec/Candid.md
Outdated
``` | ||
C[dynamic <: <t>](dynamic <v'> : <t'>) = C[<t'> <: <t>](<v'>) if <t> =/= dynamic | ||
``` | ||
Note: Type `<t>` is not known statically, so it cannot be decided statically whether `C[<t'> <: <t>]` is defined. Hence this amouts to a runtime type check. |
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.
That's a bit too sneaky; if we go this route we should explicitly define this function to be partial (it was total before), with suitable notation. Also the interaction with opt
stuff needs to be clear, especially if we go back to making that more dynamic again.
How does this affect our formal guarantees? They can only hold when no dynamic
is in use, right?
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.
Fair enough. It's probably necessary to distinguish gradual conversion from subtyping anyway, because transitively, subtyping collapses with the above rules, i.e., everything becomes a subtype of everything.
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.
@nomeata, you are right, this was taking at least one shortcut too many. I pushed a change that should properly separate graduality from subtyping and coercions. Subtyping now only allows t <: dynamic
, but not the other way round. Graduality itself is handled in de/serialisation itself. Let me know what you think.
* update: changes to agent and authentication packages * update: locking repo to node 12 * fix: typescript type safety * greening tests * modifying node version * updating linting * Dont use typescript 'as string' override in idp-protocol/request (dfinity#291) * use lockfileVersion=2 (npm) (dfinity#292) * implementing feedback from @Bengo Co-authored-by: Benjamin Goering <benjamin.goering@dfinity.org>
To represent generic data, as discussed on #245 ("Idea 1") on various occasions.
A value of type
dynamic
is serialised as a nested self-contained Candid blob.