Skip to content

Conversation

@lukewagner
Copy link
Member

As discussed in #475, as a temporary expediency, the CABI currently traps when trying to lift and lower in the same component instance to avoid the otherwise subtle ordering issues that arise with memory aliasing. However, if the element type is empty (as enabled by #440), there is no such memory aliasing issue. Moreover, such futures/streams can be used as a simple way to support #459 use cases without having to add anything fancier like the wait-async built-in suggested in #459. Thus, this tiny PR loosens the same-instance trapping condition to only trap if there is a non-empty element type.

@lukewagner
Copy link
Member Author

Trying to actually test this, I just noticed that #440 forgot to update Binary.md to provide an encoding for the "none" case in all the stream/future built-ins (they currently unconditionally take a typeidx).

In the type section, which was updated, the future/stream type constructors currently take a <valtype>?. Updating the stream built-ins to do likewise would thus be a binary breaking change. I was wondering why the stream built-ins took a typeidx and not a valtype (given that the element type is often a primvaltype), and I think what happened is that the typeidx initially referred to the whole stream<T>/future<T> (which can never be a primvaltype) and then we changed it without changing the encoding. Thinking about how stream/future built-ins would work with stream<T,U,E>/future<T,E>, though, I think that several of the stream built-ins need to see the whole compound stream/future type, not just the T (since e.g. a future read would be able to produce a T or an E).

Based on this, what I think the fix is here is to keep the binary format taking a typeidx, but require it to refer to a stream/future. wasip3-prototyping could keep allowing typeidx to refer to non-stream/future types to avoid breaking producers during the transition, although if any pre-transition producer used a stream<stream<T>>, it would still break.

WDYT?

@alexcrichton
Copy link
Collaborator

I think the prototyping repo already implements the behavior you describe where the typeidx is required to be a (future ...) or (stream ...) depending on the intrinsinc. I suspect that "violated" the spec at the time, but may prove useful in aligning to this direction nonetheless?

@lukewagner
Copy link
Member Author

Sweet; that looks great. I expect it matched spec before the spec changed. I'll file a new PR to switch back.

@lukewagner lukewagner merged commit 9a0e59b into main Apr 30, 2025
2 checks passed
@lukewagner lukewagner deleted the same-instance branch April 30, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants