Fix sharedness bug in inhabitable type fuzzer #6807
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kripken
approved these changes
Aug 5, 2024
Since there is only one unshared string heap type, the `isString()` method did not do anything that could not be done as well with other, more fundamental methods. Although using `isString` saved characters compared with its replacement, that's not enough of a benefit to offset the advantages of reducing redundant API surface and being more explicit at callsites. This would be NFC, but I updated many call sites of `isString` to handle shared string types as well, or at least error more eagerly on them, which may have changed observable behavior.
The HeapType API has functions like `isBasic()`, `isStruct()`, `isSignature()`, etc. to test the classification of a heap type. Many users have to call these functions in sequence and handle all or most of the possible classifications. When we add a new kind of heap type, finding and updating all these sites is a manual and error-prone process. To make adding new heap type kinds easier, introduce a new API that returns an enum classifying the heap type. The enum can be used in switch statements and the compiler's exhaustiveness checker will flag use sites that need to be updated when we add a new kind of heap type. This commit uses the new enum internally in the type system, but follow-on commits will add new uses and convert uses of the existing APIs to use `getKind` instead.
The code for collecting inhabitable types incorrectly considered shared, non-nullable externrefs to be inhabitable, which disagreed with the code for rewriting types to be inhabitable, which was correct, causing the type fuzzer to report an error.
tlively
force-pushed
the
heaptype-getkind
branch
from
0a14db4
to
19384e6
Compare
tlively
force-pushed
the
fix-type-fuzzer-shared
branch
from
a4472a7
to
977c7c4
Compare
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The code for collecting inhabitable types incorrectly considered shared,
non-nullable externrefs to be inhabitable, which disagreed with the code
for rewriting types to be inhabitable, which was correct, causing the
type fuzzer to report an error.