Parse milestone 4 nominal types #4222
Conversation
Implement parsing the new {func,struct,array}_subtype format for nominal types.
For now, the new format is parsed the same way the old-style (extends X) format
is parsed, i.e. in --nominal mode types are parsed as nominal but otherwise they
are parsed as equirecursive. Intentionally do not parse the new types
unconditionally as nominal for now to allow frontends to update their nominal
text format while continuing to use the workflow of running wasm-opt without
--nominal to lower nominal types to structural types. A future PR will introduce
a new pass to perform this lowering explicitly, allowing the parsers to create
true nominal types without breaking that workflow.
Also switch the printer to use the new format, since the old format will
eventually be removed.
Do we need that? I'd check with j2cl as I think they can feed nominal types to v8 now, if you haven't already. |
src/passes/Print.cpp
Outdated
| @@ -2504,9 +2504,15 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> { | |||
| visitExpression(curr); | |||
| } | |||
| // Module-level visitors | |||
| void handleSignature(HeapType curr, Name name = Name()) { | |||
| void handleSignature(HeapType curr, Module* module, Name name = Name()) { | |||
There was a problem hiding this comment.
There is already currModule which you can use.
src/passes/Print.cpp
Outdated
| Signature sig = curr.getSignature(); | ||
| o << "(func"; | ||
| HeapType super; | ||
| bool nominal = curr.getSuperType(super); |
There was a problem hiding this comment.
This will be true for the old nominal types as well as M4 ones, correct? So we are changing all of our printing of nominal types?
test/lit/lub-bug-3843.wast
Outdated
| @@ -12,11 +12,11 @@ | |||
| (type $A (struct (field (ref null $C)))) | |||
|
|
|||
| ;; CHECK: (type $B (struct (field (ref null $D)))) | |||
| ;; NOMNL: (type $B (struct (field (ref null $D))) (extends $A)) | |||
| ;; NOMNL: (type $B (struct_subtype (field (ref null $D)) $A)) | |||
There was a problem hiding this comment.
Continuing the previous question about changing all of our nominal printing, it looks like that is indeed the case - all nominal types are now printed as M4? In particular that means reading text and printing emits something different if the input was the old form and not M4, as shown here. That may break roundtripping via text, which is unfortunately still disabled, in the fuzzer,
Lines 1012 to 1013 in c2fa907
If there are no updates on #3989 yet then before landing this PR we should at least verify manually that we have sufficient text roundtripping tests in the test suite - I can only remember a very small number.
(My concern is that, other than fuzzing + roundtripping tests in the test suite, the new form that we now print is only checked as an input in the new tiny file test/lit/parse-nominal-types.wast. So we'd be writing something with minimal testing for reading.)
tlively
changed the title
|
I reverted the changes to the printer for now and beefed up the parsing test, so this should be good for another review. |
Implement parsing the new {func,struct,array}_subtype format for nominal types.
For now, the new format is parsed the same way the old-style (extends X) format
is parsed, i.e. in --nominal mode types are parsed as nominal but otherwise they
are parsed as equirecursive. Intentionally do not parse the new types
unconditionally as nominal for now to allow frontends to update their nominal
text format while continuing to use the workflow of running wasm-opt without
--nominal to lower nominal types to structural types. A future PR will introduce
a new pass to perform this lowering explicitly, allowing the parsers to create
true nominal types without breaking that workflow.