WebAssembly / component-model

Repository for design and specification of the Component Model
Other
897 stars 75 forks source link

Add @feature and @since gates to WIT #332

Closed lukewagner closed 1 month ago

lukewagner commented 3 months ago

Based on a number of other folks' ideas and feedback (esp. @yoshuawuyts, @ricochet, @alexcrichton), this proposal adds purely-WIT-level "gates" in order to reduce the churn necessary to make minor version releases (in preparation for WASI 0.2.1).

The motivation for adding these gates is that, without them, when adding a new function or type to an interface as part of a minor release, the transition is fairly toilsome and may require multiple copies of WIT documents be maintained (e.g., a "stable" snapshot, a "candidate" snapshot, and an "actively-developed" draft). By definition, minor updates must be purely additive, so the idea is to "compress" these snapshots into a single document, by syntactically annotating the new feature with what minor version they were introduced in. Importantly, these gates do not show up in a component or runtime; they are erased as part of the process of building a component (selecting the target version and experimental features to enable).

(At some point, we may add function-level subtyping, at which point minor updates won't just add new functions, but also modify existing function types and so we'll need to nuance gating. But I think that should be possible and until then we don't have to worry about it.)

The PR explains the new WIT syntax and give some examples.

yordis commented 3 months ago

Should the spec include any tagging with @[tag name] and reserved tags such as @feature and @since? We have something similar in the Elixir land.

lukewagner commented 3 months ago

I might be misunderstanding what you're suggesting, but my hope here wasn't to add a generic annotation syntax since I think that opens a can of worms that require a lot more thought and discussion. Technically, all @[tag name]s (other than @feature and @since) are still reserved after this PR since they are all rejected by the WIT parser.

Mossaka commented 3 months ago

I like these additions to WIT!

Why are @feature and @since gates only apply withint the interface scope? I think the same motivation applies to worlds too, no?

lukewagner commented 3 months ago

@Mossaka Great point! I should probably add the gates to world-items as well.

yordis commented 3 months ago

but my hope here wasn't to add a generic annotation syntax

@lukewagner, that is what I meant 😅 ignore me for now; the conversation could happen another day in a different space!

theduke commented 3 months ago

For reference: discussion about generic annotations: #58

lukewagner commented 2 months ago

As an update, based on @lann's suggestion, I renamed @feature to @unstable. Doing this made me realize that, instead of allowing these annotations to both be applied, it's much nicer to read if we allow the feature field (of @unstable) to be optionally present in @since.

alexcrichton commented 2 months ago

Ok I have an initial implementation of this for wasm-tools over at https://github.com/bytecodealliance/wasm-tools/pull/1508 which has a lot more nitty-gritty as well. If folks want to kick the tires on that and make sure it covers all the use cases as well that'd be appreciated.

lukewagner commented 2 months ago

Great points @alexcrichton! Replying to them individually:

alexcrichton commented 2 months ago

Oh to be clear though for referencing ungated items the snippet I pasted is intended to be an error when the feature is disabled. The example added here shows an un-tagged item referencing a tagged item, which would be an error if the tagged item were omitted. In that sense should the example be updated to have a tag for t2 as well? The prose mentions that t2 is transitively disabled but I wasn't planning on doing that, instead making it a WIT-parse-error requiring the WIT document to get updated to pass parsing.

Personally when it comes to feature gating I'm a fan of explicitness, but I'll note that I'm biased here. In Rust we require all stable items to have #[stable] (individually) and it's only #[unstable] which is inherited internally. I'd personally be in favor of tagging everything as @since and @unstable in WIT.

lukewagner commented 2 months ago

In that sense should the example be updated to have a tag for t2 as well?

Oh yeah, good point, that would be simpler and less magical. Updated here to just mention it as a validation rule.

I'd personally be in favor of tagging everything as @since and @unstable in WIT.

Yeah, I suppose it makes sense, and I'm also seeing the symmetry with the validation rule above. Added. Syntax errors in examples fixed.

brooksmtownsend commented 2 months ago
  1. Good job GH, tagging users with the @ image
  2. Does this have support for "overwriting" or "shadowing" existing definitions with newer versions? For example, say we updated function a to return a string in version 0.2.1. Is this valid?:
interface foo {
  a: func();
  @since(version = 0.2.1)
  a: func() -> string;
}

No hard requirement here, I think it makes more sense for the above example to be @since(version = 0.3.0) to simulate a breaking change, but I figured it's worth considering either way

lukewagner commented 2 months ago

@brooksmtownsend Unfortunately not since, particularly in the absence of function-level subtyping, that would be a breaking change and we're only talking about @since for minor version changes. For major changes, the workflow is still "create a new directory with a copied set of WITs and then just edit them".

brooksmtownsend commented 2 months ago

I think that makes sense. That will probably generally encourage forwards compatible changes as well, rather than breaking existing methods when moving from version to version. Thanks for the quick response!

alexcrichton commented 1 month ago

Er I apologize for the perhaps early merge for this, I'm apparently still waking up and I thought that this PR was https://github.com/bytecodealliance/wasm-tools/pull/1508. I was planning on merging the implementation there to help get this merged. I think there's general agreement on this, but if anyone would like I'm happy to revert this on the component-model repository and make a second PR. Again sorry for jumping the gun!

lukewagner commented 1 month ago

Hah, no worries; I was planning to merge as soon as we had a working implementation merged.