Add getters and setters (#235) - #701
Conversation
|
Should this be emoji-gated? With resource instance getters/setters spec'ed & implemented, how big do you expect the jump will be to supporting static resource getters/setters and interface-level getters/setters as well?. E.g. interface a {
foo: get() -> u64;
foo: set(value: u64);
resource r {
bar: get() -> u64;
bar: set(value: u64);
baz: static get() -> u64;
baz: static set(value: u64);
}
}If not too much, it might be nice to include these at the same time. Especially interface-level getters/setters already have a couple of use cases in WASI:
|
That seems overkill to me, since getters and setters are basically sugar with a couple extra validation rules.
I was initially surprised by this question, but on further review, it seems like this happens in WebIDL often enough that we should probably go ahead and spec it right away. For example, The same goes for interface getters with properties like My main concern is codegen; I would expect that many languages don't have any kind of native syntax for interface-level getters and setters in particular. A cursory LLM exploration indicates that newer versions of Python, for example, forbid you to combine But, if some languages have to fall back to plain old functions with
To actually answer the question...probably not that hard. As far as validation, it's just dropping |
|
Another fun thing to consider: WebIDL's interface mixin ElementCSSInlineStyle {
[SameObject, PutForwards=cssText] readonly attribute CSSStyleProperties style;
};In other words, the setter for |
|
Per our new WASI 0.3.* CM feature release process, even small additions (e.g., recently, For "static getters/setters": could we instead consider cases like
I think this makes sense. |
Ryan dug into this a bit and these properties don't even require a As for the getter/setter agreement rule, I'll just go ahead and drop that. Bindings generators can just check the type agreement themselves when deciding what code to generate, and the majority of the time they should agree. |
Ah, ok. So I guess this is unlike Having to fall back to explicit |
From what I could see in #235 (comment) (see bottom of comment), the answer seems to be: |
It seems like Python might be an exception - per this page, |
|
PR is updated with emoji-gating, new canonicalization rules to match #702, a rule requiring getters to precede setters for the same property, and without the rule requiring getter and setter types to agree. I would appreciate guidance on how to specify names for static getters. How do you combine |
(As a bit of background for folks who saw the last iteration on this design question with |
|
I was hoping you'd say |
|
Updated to support static and interface-level getters and setters. I also opted to go with a slightly different name-mangling scheme that feels to me more consistent and makes it easier to express the matrix of validation constraints: I also still have a rule requiring getters to precede setters, because it just feels better to me that way. I could be talked out of this for WIT but I see no reason to tolerate the order mismatch in components themselves. I have some additional thoughts about this in #235. |
|
Ok @lukewagner, I think this PR is now up to date with all the concerns raised both here and the original issue. So it should actually be ready for a real review now, whenever that works for you. I'm probably going to start implementing the current shape of things in SM but will not be upset if anything needs to change. |
| | 'from' | ||
| | 'func' | ||
| | 'future' | ||
| | 'get' 📡 |
There was a problem hiding this comment.
It's not clear to me that these actually need to be added to a global keyword list, given that it will likely cause breakage and they can only appear in a specific place.
There was a problem hiding this comment.
will likely cause breakage
Can confirm it will definitely cause breakage:
- https://github.com/WebAssembly/WASI/blob/3071db04c857b3a2c047d3d1ac694bc41f021796/proposals/http/wit/types.wit#L12
- https://github.com/WebAssembly/WASI/blob/3071db04c857b3a2c047d3d1ac694bc41f021796/proposals/http/wit/types.wit#L220
- https://github.com/WebAssembly/WASI/blob/3071db04c857b3a2c047d3d1ac694bc41f021796/proposals/http/wit/types.wit#L233
but only syntactically in the WITs. The binary compatibility is not affected, of course
| to be `(param "self" (borrow $R))`, where `$R` is the named resource type. | ||
| * 📡 Validation of `[get]` names requires that the function have no parameters, | ||
| unless the name is also annotated with `[method]`, in which case `self` must | ||
| be the only parameter. |
There was a problem hiding this comment.
| be the only parameter. | |
| be the only parameter (subject to the `[method]` validation rules above). |
| * 📡 Validation of `[get]` names requires that the function have a result type. | ||
| * 📡 Validation of `[set]` names requires that the function have exactly one | ||
| parameter, unless the name is also annotated with `[method]`, in which case | ||
| there must be two parameters, the first of which is `self`. |
There was a problem hiding this comment.
| there must be two parameters, the first of which is `self`. | |
| there must be two parameters, the first of which is `self` (subject to the | |
| `[method]` validation rules above). |
| | '[get]' <label> 📡 | ||
| | '[set]' <label> 📡 |
There was a problem hiding this comment.
Are these 2 cases still intended and, if so, do they have a 3rd meaning distinct from [static] and [method]?
There was a problem hiding this comment.
Yes, these are the interface-level getters and setters we discussed for things like CSS.highlights. They are not associated with any resource type, so they have only a single label in their name. As I say that, though, I'm not entirely sure what our web bindings are expected to do in that case, so I should run this by Ryan before merging.
| * 📡 If a name with `[set]` is defined as an import or export within a | ||
| particular scope, the equivalent name with `[get]` must have already been | ||
| defined as an import or export respectively in that same scope—that is, all | ||
| labels must be equal (before canonicalization), and all annotations must be | ||
| the same except that `[set]` is replaced with `[get]`, and the `[get]` | ||
| import/export must precede the `[set]` import/export. For example, | ||
| `[set]prop` requires `[get]prop`, and `[method][set]foo.bar` requires | ||
| `[method][get]foo.bar`. |
There was a problem hiding this comment.
Thinking about this some more:
The rule on its own makes sense, though the tooling side of things prbably makes it impractical:
- We can't assume that toolchains (like GCC or LLVM) emit their external symbols in any well-defined order.
- When guest code imports a property but only invokes its setter, the getter import may end up being optimized away as partof dead-code elimination.
This makes me lean towards:
- at the binary level; treat getters & setters as independent items, validated separately, just like their method equivalents are today
- move the validation into WIT, or even further out into an external wit "linter" that steers authors away from BadIdeas™ such as: setters without getters, identifiers called
class, etc. There have been ideas about such a linter tool before, but as of yet no such thing exists.
| | 'from' | ||
| | 'func' | ||
| | 'future' | ||
| | 'get' 📡 |
There was a problem hiding this comment.
will likely cause breakage
Can confirm it will definitely cause breakage:
- https://github.com/WebAssembly/WASI/blob/3071db04c857b3a2c047d3d1ac694bc41f021796/proposals/http/wit/types.wit#L12
- https://github.com/WebAssembly/WASI/blob/3071db04c857b3a2c047d3d1ac694bc41f021796/proposals/http/wit/types.wit#L220
- https://github.com/WebAssembly/WASI/blob/3071db04c857b3a2c047d3d1ac694bc41f021796/proposals/http/wit/types.wit#L233
but only syntactically in the WITs. The binary compatibility is not affected, of course
| 2. If the name is `[...]*l.l` for any annotations `[...]*` and some `label` | ||
| `l`, replace the name with `l` (e.g. `[method]foo.foo` becomes `foo`). |
There was a problem hiding this comment.
| 2. If the name is `[...]*l.l` for any annotations `[...]*` and some `label` | |
| `l`, replace the name with `l` (e.g. `[method]foo.foo` becomes `foo`). | |
| 2. If the name is `[...]*l.l` for any annotations `[...]*` and some `label` | |
| `l`, replace the name with `l` (e.g. `[method]foo.bar` becomes `bar`). |
Not really related to this PR, but the pre-existing example was ambiguous as to which label it will pick.
Adds
getandsetto WIT and adds[get]and[set]annotations to function names, along with validation conditions.Right now my strongly-unique rules forbid
[get]foo.propand[static]foo.propfrom existing on the same resource. This seems like a reasonable and conservative choice to me. It does not forbid[get]foo.propfrom existing alongside[method]foo.get-propor[static]foo.get-prop, since we need to keep that WASI migration path open. This could be pretty easily added in the future but will make me sad because it will make the strongly-unique rules even more bonkers to implement than they already are >:(Resolves #235.