Drop stability for external package includes

[jsturtevant]

fixes #2113

[alexcrichton]

I'm trying to think through the implications of this in terms of bindgen, and I'm wondering if we perhaps want a different merging algorithm related to unstable. My worry here is that we could have a chain of events like:

• wasi:tls/imports has @unstable import wasi:tls/types;
• wasi:cli/imports has @unstable include wasi:tls/imports;
• wasi:cli/run has @stable include wasi:cli/imports

I think that the instability will get propagated to wasi:cli/imports, but I think that the instability won't get propagated to wasi:cli/run? Wouldn't the @stable tag there override the @unstable of the world item for wasi:tls/types?

Basically I'm worried about accidentally flagging something originally marked @unstable as @stable as it transitions between worlds and include. One possible solution might be to reorder the Stable and Unstable variants so unstable is "greater", but that may cause havoc elsewhere

[jsturtevant]

I think the individual items would get upgraded. Do I understand correctly that include and use also do different things in the types system. (ie. use is a reference and include would redefine it (unless it then calls a use))

I found there are all sorts of edge cases here based on what use/included first. And gets tricky when there are nested includes too. This took the simple approach of overriding everything of with the value on the include.

I was re-reading the spec and the implications of using/including to different worlds means that we break one of the "rules" of features gates: Either @since or @unstable should be used, but not both (exclusive or). When we add setting feature-gates on the include or use statement I think we further add ambiguity and complexity to solving for the correct version: i.e. Does that apply to all sub items?

Maybe we need to detail this out a bit more in the design/spec? I can think of a couple possibilities:

• either includes can't have since/unstable and the underlying stability info comes from the items
• since/unstable can only determine if the include/use are in scope of current package but the stability of the underlying items that are being included are the ones that get applied.
• must match the stabilities of individual items across all imports/includes (this wouldn't work in practice i think)

[alexcrichton]

You're right yeah that include and use are different, and you're also right that the spec is pretty amibugous about what to do here. One of the main differences is that include attempts to inline one world into another, moving items around, where use just refers to something somewhere else, and nothing is moved. In that sense stability makes a lot more sense with use and less so with include.

Even if we disallow stability on include though I think we have the same issues. For example include wasi:cli/imports will transitively refer to import wasi:tls/types which is unstable, and somehow that should be resolved. If it's unstable it's ok to copy over the instability, but if it's stable it's not valid to copy over the stable information since the version of the current package could be completely different (and IIRC was one of the impetuses for this change)

How about a solution along the lines of:

• intra-package include (e.g. one world in a package include-ing a world in the same package) requires no stability on the include and the annotation of each item is just copied over from the source, failing on conflict if something was already in the world. This is IIRC mostly the original behavior of include
• inter-package include would also require no stability on include and would discard @stable annotations while preserving @unstable annotations, and would probably return an error on clash with an explicit import.

That way the stability of include is never considered and unstable things at least get propagated as unstable while stable things just sort of lose their annotation which shouldn't be the end of the world

[jsturtevant]

Honestly, I think I missed the difference between same package and external package includes. It makes sense that inside the same packages the Stability to should align.

Even if we disallow stability on include though I think we have the same issues.

I think you are right, but I think you would still want to take the stability on the include into account as to whether the include is processed.

I'm drafting up a few concrete examples and will post them back here.

[alexcrichton]

Oh wow that was a pretty small implementation! Was there other tests you wanted to add or other examples? Otherwise this looks good to go to me

[jsturtevant]

I saw you added some support to wit-smith for includes. Does it make sense to add a simple round trip to the tests in wit-parser as an initial test of the samples or do you think the fuzzing tests running later is enough to catch issues?

[alexcrichton]

I did add support yeah but wit-smith still has no support for stability annotations, so while include is now there it still doesn't use @stable or @unstable at all, so fuzzing won't catch this.

Fuzzing does though do round-tripping so once it generates all the things here it should catch this, in theory.

[alexcrichton]

I'm going to go ahead and land this, and if necessary can follow-up with extra tests/fuzzing.