Repository navigation
Reboot TypeGeneralizing to use SubtypingDiscoverer #9263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
950784a
eb92c6f
479cf92
327c476
78630e6
01ec083
7d4362d
30d4880
62d6f45
6df116a
d5649c8
3b83bd7
e2745e8
cc70c10
3aad80b
7f496de
13d2b15
4dd3c5f
c3067a1
ac8eaa5
1bedbe6
3c629de
7dd90b7
44bff47
84e4977
95d7501
03f21b1
72bb947
5973fe5
5decf16
11b5a9b
fcaf2fc
8ad69f0
b9bffdc
d9800a2
740348a
9f5cb46
b8108cf
4751238
ee64c80
b1e6c14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,7 +252,10 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| self()->getModule()->getTable(curr->table)->type); | ||
| } | ||
| void visitTableSize(TableSize* curr) {} | ||
| void visitTableGrow(TableGrow* curr) {} | ||
| void visitTableGrow(TableGrow* curr) { | ||
| self()->noteSubtype(curr->value, | ||
| self()->getModule()->getTable(curr->table)->type); | ||
| } | ||
| void visitTableFill(TableFill* curr) { | ||
| self()->noteSubtype(curr->value, | ||
| self()->getModule()->getTable(curr->table)->type); | ||
|
|
@@ -288,8 +291,18 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| } | ||
| } | ||
| void visitRethrow(Rethrow* curr) {} | ||
| void visitThrowRef(ThrowRef* curr) {} | ||
| void visitTupleMake(TupleMake* curr) {} | ||
| void visitThrowRef(ThrowRef* curr) { | ||
| self()->noteNonFlowSubtype(curr->exnref, Type(HeapType::exn, Nullable)); | ||
| } | ||
| void visitTupleMake(TupleMake* curr) { | ||
| if (curr->type != Type::unreachable) { | ||
| assert(curr->operands.size() == curr->type.size()); | ||
| // Ensure the inputs make sense for the tuple. | ||
| for (size_t i = 0, size = curr->operands.size(); i < size; ++i) { | ||
| self()->noteSubtype(curr->operands[i], curr->type[i]); | ||
| } | ||
| } | ||
| } | ||
| void visitTupleExtract(TupleExtract* curr) {} | ||
| void visitRefI31(RefI31* curr) {} | ||
| void visitI31Get(I31Get* curr) { | ||
|
|
@@ -330,31 +343,86 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| void visitRefTest(RefTest* curr) { | ||
| self()->noteCast(curr->ref, curr->castType); | ||
| } | ||
| void visitRefCast(RefCast* curr) { self()->noteCast(curr->ref, curr); } | ||
| void visitRefGetDesc(RefGetDesc* curr) {} | ||
| void visitRefCast(RefCast* curr) { | ||
| self()->noteCast(curr->ref, curr); | ||
|
|
||
| if (curr->desc) { | ||
| // As with CallRef, we must keep a descriptor valid: it must be a declared | ||
| // descriptor type. Until we have addStructSubtype, make it keep its type. | ||
| self()->noteSubtype(curr->desc, curr->desc->type); | ||
| } | ||
| } | ||
| void visitRefGetDesc(RefGetDesc* curr) { | ||
| // Similar to RefCastDesc, we must keep the input a valid struct type, one | ||
| // with a descriptor. Until we have addStructSubtype or such, make it keep | ||
| // its type. | ||
| self()->noteSubtype(curr->ref, curr->ref->type); | ||
| } | ||
| void visitBrOn(BrOn* curr) { | ||
| auto* breakTarget = self()->findBreakTarget(curr->name); | ||
| switch (curr->op) { | ||
| case BrOnNull: | ||
| // If we do not branch, we flow out the existing value as non-null. | ||
| if (curr->type.isRef()) { | ||
| self()->noteSubtype(curr->ref, curr->type.with(Nullable)); | ||
| } | ||
| break; | ||
| case BrOnNonNull: | ||
| // The value sent is made non-nullable and then sent. That means it | ||
| // must have the right heap type. | ||
| if (breakTarget->type.isRef()) { | ||
| // Enforce the heap type requirement by making sure we are a subtype | ||
| // of the target, made nullable (i.e., without any non-nullability | ||
| // requirement, as we are making it non-nullable anyhow). | ||
| self()->noteSubtype(curr->ref, breakTarget->type.with(Nullable)); | ||
| } | ||
| break; | ||
| case BrOnCast: | ||
| case BrOnCastFail: | ||
| case BrOnCastDescEq: | ||
| case BrOnCastDescEqFail: | ||
| self()->noteCast(curr->ref, curr->castType); | ||
| if (curr->desc) { | ||
| // As with RefCast, we must keep a descriptor valid. | ||
| self()->noteSubtype(curr->desc, curr->desc->type); | ||
| } | ||
| if (curr->op == BrOnCast || curr->op == BrOnCastDescEq) { | ||
| // If we do not branch, we flow out the original input when the cast | ||
| // fails. If the cast type is nullable then nulls take the branch, so | ||
| // the fallthrough value is made non-nullable. | ||
| if (curr->type.isRef()) { | ||
| auto type = curr->type; | ||
| if (curr->castType.isNullableRef()) { | ||
| type = type.with(Nullable); | ||
| } | ||
| self()->noteSubtype(curr->ref, type); | ||
| } | ||
| } else { | ||
| // We send the original input along when the cast fails, so the input | ||
| // must be sendable to the target. | ||
| if (breakTarget->type.isRef()) { | ||
| self()->noteSubtype(curr->ref, breakTarget->type); | ||
| } | ||
| } | ||
| break; | ||
| } | ||
| self()->noteSubtype(curr->getSentType(), | ||
| self()->findBreakTarget(curr->name)); | ||
| self()->noteSubtype(curr->getSentType(), breakTarget); | ||
| } | ||
| void visitStructNew(StructNew* curr) { | ||
| if (!curr->type.isStruct() || curr->isWithDefault()) { | ||
| if (!curr->type.isStruct()) { | ||
| return; | ||
| } | ||
| const auto& fields = curr->type.getHeapType().getStruct().fields; | ||
| assert(fields.size() == curr->operands.size()); | ||
| for (size_t i = 0, size = fields.size(); i < size; ++i) { | ||
| self()->noteSubtype(curr->operands[i], fields[i].type); | ||
| if (!curr->isWithDefault()) { | ||
| const auto& fields = curr->type.getHeapType().getStruct().fields; | ||
| assert(fields.size() == curr->operands.size()); | ||
| for (size_t i = 0, size = fields.size(); i < size; ++i) { | ||
| self()->noteSubtype(curr->operands[i], fields[i].type); | ||
| } | ||
| } | ||
| if (curr->desc) { | ||
| auto descType = curr->type.getHeapType().getDescriptorType(); | ||
| assert(descType); | ||
| self()->noteSubtype(curr->desc, Type(*descType, Nullable, Exact)); | ||
| } | ||
|
Comment on lines
+422
to
+426
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If visitation order matters, this should come after visiting the fields. (And even if visitation order doesn't matter now, it would probably be better to be consistent about it.)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The order doesn't matter, but I agree it is nice to follow. Fixed.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
How about leaving those for later + tests? This PR is mainly focused on the reboot, but not adding new functionality. |
||
| } | ||
| void visitStructGet(StructGet* curr) {} | ||
|
|
@@ -404,7 +472,7 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| self()->noteSubtype(curr->waitqueue, | ||
| Type(HeapTypes::sharedWaitqueue, Nullable)); | ||
| } | ||
| void visitPublish(Publish* curr) {} | ||
| void visitPublish(Publish* curr) { self()->noteSubtype(curr->ref, curr); } | ||
| void visitArrayNew(ArrayNew* curr) { | ||
| if (!curr->type.isArray() || curr->isWithDefault()) { | ||
| return; | ||
|
|
@@ -441,7 +509,14 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| } | ||
| void visitArrayLoad(ArrayLoad* curr) {} | ||
| void visitArrayStore(ArrayStore* curr) {} | ||
| void visitArrayLen(ArrayLen* curr) {} | ||
| void visitArrayLen(ArrayLen* curr) { | ||
| auto share = Unshared; | ||
| if (curr->ref->type.isRef() && curr->ref->type.getHeapType().isShared()) { | ||
| share = Shared; | ||
| } | ||
| self()->noteNonFlowSubtype( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, why non-flow? Previously the only use of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is correct (for the reasons I mentioned above) but yes, there might be more places it makes sense to use. I didn't try to find them all in this PR - would make more sense in a place where we can cover them more easily with tests, though I don't know that we have a better pass, actually...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can remove the ones not directly needed for tests in this PR if you prefer, though e.g. the String ones seem obvious?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Aside from them, the ThrowRef one can also be removed from this PR, it looks like.) |
||
| curr->ref, Type(HeapTypes::array.getBasic(share), Nullable)); | ||
| } | ||
| void visitArrayCopy(ArrayCopy* curr) { | ||
| if (!curr->srcRef->type.isArray() || !curr->destRef->type.isArray()) { | ||
| return; | ||
|
|
@@ -513,24 +588,64 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| return; | ||
| } | ||
| } | ||
| void visitStringNew(StringNew* curr) {} | ||
| void visitStringNew(StringNew* curr) { | ||
| switch (curr->op) { | ||
| case StringNewLossyUTF8Array: | ||
| self()->noteNonFlowSubtype(curr->ref, | ||
| Type(HeapTypes::getMutI8Array(), Nullable)); | ||
| break; | ||
| case StringNewWTF16Array: | ||
| self()->noteNonFlowSubtype(curr->ref, | ||
| Type(HeapTypes::getMutI16Array(), Nullable)); | ||
| break; | ||
| case StringNewFromCodePoint: | ||
| break; | ||
| } | ||
| } | ||
| void visitStringConst(StringConst* curr) {} | ||
| void visitStringMeasure(StringMeasure* curr) {} | ||
| void visitStringEncode(StringEncode* curr) {} | ||
| void visitStringConcat(StringConcat* curr) {} | ||
| void visitStringEq(StringEq* curr) {} | ||
| void visitStringTest(StringTest* curr) {} | ||
| void visitStringWTF16Get(StringWTF16Get* curr) {} | ||
| void visitStringSliceWTF(StringSliceWTF* curr) {} | ||
| void visitStringMeasure(StringMeasure* curr) { | ||
| self()->noteNonFlowSubtype(curr->ref, Type(HeapType::string, Nullable)); | ||
| } | ||
| void visitStringEncode(StringEncode* curr) { | ||
| self()->noteNonFlowSubtype(curr->str, Type(HeapType::string, Nullable)); | ||
| switch (curr->op) { | ||
| case StringEncodeLossyUTF8Array: | ||
| self()->noteNonFlowSubtype(curr->array, | ||
| Type(HeapTypes::getMutI8Array(), Nullable)); | ||
| break; | ||
| case StringEncodeWTF16Array: | ||
| self()->noteNonFlowSubtype(curr->array, | ||
| Type(HeapTypes::getMutI16Array(), Nullable)); | ||
| break; | ||
| } | ||
| } | ||
| void visitStringConcat(StringConcat* curr) { | ||
| self()->noteNonFlowSubtype(curr->left, Type(HeapType::string, Nullable)); | ||
| self()->noteNonFlowSubtype(curr->right, Type(HeapType::string, Nullable)); | ||
| } | ||
| void visitStringEq(StringEq* curr) { | ||
| self()->noteNonFlowSubtype(curr->left, Type(HeapType::string, Nullable)); | ||
| self()->noteNonFlowSubtype(curr->right, Type(HeapType::string, Nullable)); | ||
| } | ||
| void visitStringTest(StringTest* curr) { | ||
| self()->noteNonFlowSubtype(curr->ref, Type(HeapType::ext, Nullable)); | ||
| } | ||
| void visitStringWTF16Get(StringWTF16Get* curr) { | ||
| self()->noteNonFlowSubtype(curr->ref, Type(HeapType::string, Nullable)); | ||
| } | ||
| void visitStringSliceWTF(StringSliceWTF* curr) { | ||
| self()->noteNonFlowSubtype(curr->ref, Type(HeapType::string, Nullable)); | ||
| } | ||
|
|
||
| void visitContNew(ContNew* curr) { | ||
| if (!curr->type.isContinuation()) { | ||
| return; | ||
| } | ||
| // The type of the function reference must remain a subtype of the function | ||
| // type expected by the continuation. | ||
| self()->noteSubtype(curr->func->type.getHeapType(), | ||
| curr->type.getHeapType().getContinuation().type); | ||
| self()->noteSubtype( | ||
| curr->func, | ||
| Type(curr->type.getHeapType().getContinuation().type, Nullable)); | ||
| } | ||
| void visitContBind(ContBind* curr) { | ||
| if (!curr->cont->type.isContinuation() || !curr->type.isContinuation()) { | ||
|
|
@@ -602,6 +717,11 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| if (!curr->cont->type.isContinuation()) { | ||
| return; | ||
| } | ||
|
|
||
| // Like CallRef, we cannot generalize the ref into a basic type, it must | ||
| // here remain a Continuation. | ||
| self()->noteSubtype(curr->cont, curr->cont->type); | ||
|
|
||
| processResumeHandlers( | ||
| curr->cont->type, curr->handlerTags, curr->handlerBlocks); | ||
| // The types we send to the resumed continuation must remain subtypes of the | ||
|
|
@@ -631,6 +751,10 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> { | |
| for (Index i = 0; i < curr->operands.size(); ++i) { | ||
| self()->noteSubtype(curr->operands[i], params[i]); | ||
| } | ||
| } else { | ||
| assert(curr->operands.size() == 1); | ||
| self()->noteNonFlowSubtype(curr->operands[0], | ||
| Type(HeapType::exn, Nullable)); | ||
| } | ||
| } | ||
| void visitStackSwitch(StackSwitch* curr) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "non-flow" here? The ref does flow into the throw and out of a later catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but it doesn't flow out of this expression. Reading the docs on top, I guess this can be ambiguous. I can reword that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need. It's probably worth going through later and using
noteNonFlowSubtypein most other places.