Fix Unsubtyping and GTO for configureAll #8267
Conversation
Take into account the implicit casts and conversions that happen on the boundary with JS as well as the fact that JS can essentially read the first field on descriptors with configured prototypes.
tlively
force-pushed
the
fix-gto-unsubypting-configure-all
branch
from
3d31e7e to
224c6c9
Compare
tlively
changed the title
src/wasm-type.h
Outdated
|
|
||
| // Whether this is a struct type whose first field is immutable and a subtype | ||
| // of externref. | ||
| bool hasPossibleJSPrototypeField() const; |
There was a problem hiding this comment.
This feels like a somewhat specific property beyond the core type system - perhaps it can go in struct-utils.h?
| if (!curr->value->type.isRef()) { | ||
| return; | ||
| } | ||
| auto exact = curr->value->type.getExactness(); |
| return; | ||
| } | ||
| for (auto func : Intrinsics(wasm).getConfigureAllFunctions()) { | ||
| for (auto type : wasm.getFunction(func)->getResults()) { |
There was a problem hiding this comment.
Perhaps add a comment here as to why only the results matter?
| // This descriptor will expose a prototype to JS, so we must keep | ||
| // it. | ||
| noteDescriptor(heapType, *desc); | ||
| } |
There was a problem hiding this comment.
Why is the cast symmetric in params and results, but the prototype only appears for the results?
There was a problem hiding this comment.
The parameters are values flowing from JS to Wasm and the results are values flowing from Wasm to JS. Only JS->Wasm involves an implicit cast, only Wasm->JS involves an implict extern.convert_any, and only Wasm->JS makes a configured prototype visible to JS.
| collectedInfo.casts.insert({src, dst}); | ||
| } | ||
| dsts.clear(); | ||
| } |
There was a problem hiding this comment.
No. We now have casts that were noted before the parallel function analysis. This is merging them into the parallel function analysis sets so we don't end up with unnecessary duplicated entries.
|
|
||
| (module | ||
| (rec | ||
| ;; We can optimize out the externref field on the field because it is never |
There was a problem hiding this comment.
| ;; We can optimize out the externref field on the field because it is never | |
| ;; We can optimize out the externref field on the struct because it is never |
| ;; Externalizing a reference may in general make it available to JS and | ||
| ;; and force us to keep fields holding exposed prototypes. |
There was a problem hiding this comment.
| ;; Externalizing a reference may in general make it available to JS and | |
| ;; and force us to keep fields holding exposed prototypes. |
unneeded duplicate from above
| (type $struct-not-sent (descriptor $desc-not-sent) (struct)) | ||
| (type $desc-not-sent (describes $struct-not-sent) (struct (field externref))) | ||
|
|
||
| ;; Receiving a type from JS as means it is implicitly cast from any. |
There was a problem hiding this comment.
What implications does that have? Unlike the above comments, it's not clear what to expect in the output here.
There was a problem hiding this comment.
From the test function where this type is used:
;; There is an implicit cast from any to $super-param on the boundary. This
;; combined with $sub-param flowing into any here means that the subtyping
;; must be maintained.
Do you have a preference for what information we duplicate (if any) between the comments on the types and the comments in the functions?
There was a problem hiding this comment.
It might be nice to keep comments near the test outputs. So in this case, up here. I don't feel strongly though if it's a lot of work.
Address feedback missed in #8267.
2 participants
Take into account the implicit casts and conversions that happen on the
boundary with JS as well as the fact that JS can essentially read the
first field on descriptors with configured prototypes.