Make final types the default (#5918)

Match the spec and parse the shorthand binary and text formats as final and emit final types without supertypes using the shorthands as well. This is a potentially-breaking change, since the text and binary shorthands can no longer be used to define types that have subtypes. Also make TypeBuilder entries final by default to better match the spec and update the internal APIs to use the "open" terminology rather than "final" terminology. Future changes will update the text format to use the standard "sub open" rather than the current "sub final" keywords. The exception is the new wat parser, which supporst "sub open" as of this change, since it didn't support final types at all previously.
WebAssembly · Sep 9, 2023 · 4e58466 · 4e58466
1 parent 9057105
commit 4e58466
Show file tree

Hide file tree

Showing 79 changed files with 729 additions and 730 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,8 @@ Current Trunk
  and as a convenience.
 - The nonstandard, experimental gc-nn-locals feature has been removed now that
  standard non-nullable locals are supported.
+- Heap types are now final by default and openness must be opted into both in
+ the text and binary formats as well as in the TypeBuilder API.
 
 v114
 ----

diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
@@ -6413,6 +6413,9 @@ void TypeBuilderSetSubType(TypeBuilderRef builder,
  BinaryenHeapType superType) {
  ((TypeBuilder*)builder)->setSubType(index, HeapType(superType));
 }
+void TypeBuilderSetOpen(TypeBuilderRef builder, BinaryenIndex index) {
+ ((TypeBuilder*)builder)->setOpen(index);
+}
 void TypeBuilderCreateRecGroup(TypeBuilderRef builder,
  BinaryenIndex index,
  BinaryenIndex length) {

diff --git a/src/binaryen-c.h b/src/binaryen-c.h
@@ -3572,6 +3572,9 @@ BINARYEN_API BinaryenType TypeBuilderGetTempRefType(TypeBuilderRef builder,
 BINARYEN_API void TypeBuilderSetSubType(TypeBuilderRef builder,
  BinaryenIndex index,
  BinaryenHeapType superType);
+// Sets the type at `index` to be open (i.e. non-final).
+BINARYEN_API void TypeBuilderSetOpen(TypeBuilderRef builder,
+ BinaryenIndex index);
 // Creates a new recursion group in the range `index` inclusive to `index +
 // length` exclusive. Recursion groups must not overlap.
 BINARYEN_API void TypeBuilderCreateRecGroup(TypeBuilderRef builder,

diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp
@@ -72,7 +72,7 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes() {
  // Create the temporary heap types.
  i = 0;
  for (auto [type, _] : typeIndices) {
- typeBuilder[i].setFinal(type.isFinal());
+ typeBuilder[i].setOpen(type.isOpen());
  if (type.isSignature()) {
  auto sig = type.getSignature();
  TypeList newParams, newResults;

diff --git a/src/passes/TypeMerging.cpp b/src/passes/TypeMerging.cpp
@@ -538,7 +538,7 @@ bool shapeEq(HeapType a, HeapType b) {
  // Check whether `a` and `b` have the same top-level structure, including the
  // position and identity of any children that are not included as transitions
  // in the DFA, i.e. any children that are not nontrivial references.
- if (a.isFinal() != b.isFinal()) {
+ if (a.isOpen() != b.isOpen()) {
  return false;
  }
  if (a.isStruct() && b.isStruct()) {
@@ -554,7 +554,7 @@ bool shapeEq(HeapType a, HeapType b) {
 }
 
 size_t shapeHash(HeapType a) {
- size_t digest = hash(a.isFinal());
+ size_t digest = hash(a.isOpen());
  if (a.isStruct()) {
  rehash(digest, 0);
  hash_combine(digest, shapeHash(a.getStruct()));

diff --git a/src/passes/TypeSSA.cpp b/src/passes/TypeSSA.cpp
@@ -110,7 +110,7 @@ std::vector<HeapType> ensureTypesAreInNewRecGroup(RecGroup recGroup,
  if (auto super = type.getSuperType()) {
  builder[i].subTypeOf(*super);
  }
- builder[i].setFinal(type.isFinal());
+ builder[i].setOpen(type.isOpen());
  }
 
  // Implement the hash as a struct with "random" fields, and add it.
@@ -255,6 +255,7 @@ struct TypeSSA : public Pass {
  builder[i] = oldType.getArray();
  }
  builder[i].subTypeOf(oldType);
+ builder[i].setOpen();
  }
  builder.createRecGroup(0, num);
  auto result = builder.build();
@@ -311,7 +312,7 @@ struct TypeSSA : public Pass {
  return false;
  }
 
- if (curr->type.getHeapType().isFinal()) {
+ if (!curr->type.getHeapType().isOpen()) {
  // We can't create new subtypes of a final type anyway.
  return false;
  }

diff --git a/src/tools/fuzzing/heap-types.cpp b/src/tools/fuzzing/heap-types.cpp
@@ -87,9 +87,7 @@ struct HeapTypeGeneratorImpl {
 
  // Types without nontrivial subtypes may be marked final.
  for (Index i = 0; i < builder.size(); ++i) {
- if (subtypeIndices[i].size() == 1 && rand.oneIn(2)) {
- builder[i].setFinal();
- }
+ builder[i].setOpen(subtypeIndices[i].size() > 1 || rand.oneIn(2));
  }
 
  // Initialize the recursion groups.
@@ -894,7 +892,7 @@ std::vector<HeapType> Inhabitator::build() {
  builder[i].subTypeOf(*super);
  }
  }
- builder[i].setFinal(types[i].isFinal());
+ builder[i].setOpen(types[i].isOpen());
  }
 
  auto built = builder.build();

diff --git a/src/tools/wasm-fuzz-types.cpp b/src/tools/wasm-fuzz-types.cpp
@@ -264,7 +264,7 @@ void Fuzzer::checkCanonicalization() {
 
  // Set finality
  for (size_t i = 0; i < types.size(); ++i) {
- builder[i].setFinal(types[i].isFinal());
+ builder[i].setOpen(types[i].isOpen());
  }
 
  // Set up recursion groups and record group ends to ensure we only select

diff --git a/src/wasm-type.h b/src/wasm-type.h
@@ -361,7 +361,7 @@ class HeapType {
  bool isArray() const;
  bool isString() const;
  bool isBottom() const;
- bool isFinal() const;
+ bool isOpen() const;
 
  Signature getSignature() const;
  const Struct& getStruct() const;
@@ -587,7 +587,7 @@ struct TypeBuilder {
  // not overlap or go out of bounds.
  void createRecGroup(size_t i, size_t length);
 
- void setFinal(size_t i, bool final = true);
+ void setOpen(size_t i, bool open = true);
 
  enum class ErrorReason {
  // There is a cycle in the supertype relation.
@@ -650,8 +650,8 @@ struct TypeBuilder {
  builder.setSubType(index, other);
  return *this;
  }
- Entry& setFinal(bool final = true) {
- builder.setFinal(index, final);
+ Entry& setOpen(bool open = true) {
+ builder.setOpen(index, open);
  return *this;
  }
  };

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
@@ -265,13 +265,11 @@ void WasmBinaryWriter::writeTypes() {
  // Emit the type definition.
  BYN_TRACE("write " << type << std::endl);
  auto super = type.getSuperType();
- // TODO: Use the binary shorthand for final types once we parse MVP
- // signatures as final.
- if (type.isFinal() || super || hasSubtypes[i]) {
- if (type.isFinal()) {
- o << S32LEB(BinaryConsts::EncodedType::SubFinal);
- } else {
+ if (super || type.isOpen()) {
+ if (type.isOpen()) {
  o << S32LEB(BinaryConsts::EncodedType::Sub);
+ } else {
+ o << S32LEB(BinaryConsts::EncodedType::SubFinal);
  }
  if (super) {
  o << U32LEB(1);
@@ -2268,9 +2266,8 @@ void WasmBinaryReader::readTypes() {
  std::optional<uint32_t> superIndex;
  if (form == BinaryConsts::EncodedType::Sub ||
  form == BinaryConsts::EncodedType::SubFinal) {
- if (form == BinaryConsts::EncodedType::SubFinal) {
- // TODO: Interpret type definitions without any `sub` as final as well.
- builder[i].setFinal();
+ if (form == BinaryConsts::EncodedType::Sub) {
+ builder[i].setOpen();
  }
  uint32_t supers = getU32LEB();
  if (supers > 0) {

diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
@@ -929,8 +929,9 @@ void SExpressionWasmBuilder::preParseHeapTypes(Element& module) {
  if (kind == SUB) {
  Index i = 1;
  if (*def[i] == FINAL) {
- builder[index].setFinal();
  ++i;
+ } else {
+ builder[index].setOpen();
  }
  if (def[i]->dollared()) {
  super = def[i];
@@ -959,6 +960,7 @@ void SExpressionWasmBuilder::preParseHeapTypes(Element& module) {
  if (kind == FUNC) {
  builder[index] = parseSignatureDef(def, 0);
  } else if (kind == FUNC_SUBTYPE) {
+ builder[index].setOpen();
  builder[index] = parseSignatureDef(def, 1);
  super = def[def.size() - 1];
  if (!super->dollared() && super->str() == FUNC) {
@@ -968,6 +970,7 @@ void SExpressionWasmBuilder::preParseHeapTypes(Element& module) {
  } else if (kind == STRUCT) {
  builder[index] = parseStructDef(def, index, 0);
  } else if (kind == STRUCT_SUBTYPE) {
+ builder[index].setOpen();
  builder[index] = parseStructDef(def, index, 1);
  super = def[def.size() - 1];
  if (!super->dollared() && super->str() == DATA) {
@@ -977,6 +980,7 @@ void SExpressionWasmBuilder::preParseHeapTypes(Element& module) {
  } else if (kind == ARRAY) {
  builder[index] = parseArrayDef(def);
  } else if (kind == ARRAY_SUBTYPE) {
+ builder[index].setOpen();
  builder[index] = parseArrayDef(def);
  super = def[def.size() - 1];
  if (!super->dollared() && super->str() == DATA) {
@@ -993,6 +997,7 @@ void SExpressionWasmBuilder::preParseHeapTypes(Element& module) {
  }
  } else if (elementStartsWith(elem[elem.size() - 1], EXTENDS)) {
  // '(' 'extends' $supertype ')'
+ builder[index].setOpen();
  Element& extends = *elem[elem.size() - 1];
  super = extends[1];
  }

diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
@@ -85,7 +85,7 @@ struct HeapTypeInfo {
  // Used in assertions to ensure that temporary types don't leak into the
  // global store.
  bool isTemp = false;
- bool isFinal = false;
+ bool isOpen = false;
  // The supertype of this HeapType, if it exists.
  HeapTypeInfo* supertype = nullptr;
  // The recursion group of this type or null if the recursion group is trivial
@@ -1180,11 +1180,11 @@ bool HeapType::isBottom() const {
  return false;
 }
 
-bool HeapType::isFinal() const {
+bool HeapType::isOpen() const {
  if (isBasic()) {
  return false;
  } else {
- return getHeapTypeInfo(*this)->isFinal;
+ return getHeapTypeInfo(*this)->isOpen;
  }
 }
 
@@ -1771,13 +1771,12 @@ std::ostream& TypePrinter::print(HeapType type) {
  os << "(; temp ;) ";
  }
 
- // TODO: Use shorthand for final types once we parse MVP signatures as final.
  bool useSub = false;
  auto super = type.getSuperType();
- if (super || type.isFinal()) {
+ if (super || type.isOpen()) {
  useSub = true;
  os << "(sub ";
- if (type.isFinal()) {
+ if (!type.isOpen()) {
  os << "final ";
  }
  if (super) {
@@ -1946,7 +1945,7 @@ size_t RecGroupHasher::hash(const HeapTypeInfo& info) const {
  if (info.supertype) {
  hash_combine(digest, hash(HeapType(uintptr_t(info.supertype))));
  }
- wasm::rehash(digest, info.isFinal);
+ wasm::rehash(digest, info.isOpen);
  wasm::rehash(digest, info.kind);
  switch (info.kind) {
  case HeapTypeInfo::SignatureKind:
@@ -2069,7 +2068,7 @@ bool RecGroupEquator::eq(const HeapTypeInfo& a, const HeapTypeInfo& b) const {
  return false;
  }
  }
- if (a.isFinal != b.isFinal) {
+ if (a.isOpen != b.isOpen) {
  return false;
  }
  if (a.kind != b.kind) {
@@ -2325,15 +2324,15 @@ void TypeBuilder::createRecGroup(size_t index, size_t length) {
  {RecGroup(uintptr_t(groupInfo.get())), std::move(groupInfo)});
 }
 
-void TypeBuilder::setFinal(size_t i, bool final) {
+void TypeBuilder::setOpen(size_t i, bool open) {
  assert(i < size() && "index out of bounds");
- impl->entries[i].info->isFinal = final;
+ impl->entries[i].info->isOpen = open;
 }
 
 namespace {
 
 bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) {
- if (super.isFinal) {
+ if (!super.isOpen) {
  return false;
  }
  if (sub.kind != super.kind) {

diff --git a/src/wasm/wat-parser.cpp b/src/wasm/wat-parser.cpp
@@ -849,6 +849,7 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx {
  void addFuncType(SignatureT) {}
  void addStructType(StructT) {}
  void addArrayType(ArrayT) {}
+ void setOpen() {}
  Result<> addSubtype(Index) { return Ok{}; }
  void finishSubtype(Name name, Index pos) {
  subtypeDefs.push_back({name, pos, Index(subtypeDefs.size())});
@@ -1078,6 +1079,8 @@ struct ParseTypeDefsCtx : TypeParserCtx<ParseTypeDefsCtx> {
 
  void addArrayType(ArrayT& type) { builder[index] = type; }
 
+ void setOpen() { builder[index].setOpen(); }
+
  Result<> addSubtype(Index super) {
  if (super >= builder.size()) {
  return in.err("supertype index out of bounds");
@@ -3463,6 +3466,9 @@ template<typename Ctx> MaybeResult<> subtype(Ctx& ctx) {
  }
 
  if (ctx.in.takeSExprStart("sub"sv)) {
+ if (ctx.in.takeKeyword("open"sv)) {
+ ctx.setOpen();
+ }
  if (auto super = maybeTypeidx(ctx)) {
  CHECK_ERR(super);
  CHECK_ERR(ctx.addSubtype(*super));

diff --git a/test/ctor-eval/gc-2.wast.out b/test/ctor-eval/gc-2.wast.out
@@ -1,5 +1,5 @@
 (module
- (type $struct (struct (field i32)))
+ (type $struct (sub (struct (field i32))))
  (type $1 (func (result i32)))
  (global $ctor-eval$global_4 (ref $struct) (struct.new $struct
  (i32.const 9999)

diff --git a/test/ctor-eval/gc.wast.out b/test/ctor-eval/gc.wast.out
@@ -1,5 +1,5 @@
 (module
- (type $struct (struct (field i32)))
+ (type $struct (sub (struct (field i32))))
  (type $1 (func (param anyref)))
  (type $2 (func (result i32)))
  (type $3 (func))

diff --git a/test/example/c-api-kitchen-sink.c b/test/example/c-api-kitchen-sink.c
@@ -2160,6 +2160,7 @@ void test_typebuilder() {
  tempArrayType,
  BinaryenPackedTypeNotPacked(),
  true);
+ TypeBuilderSetOpen(builder, tempArrayIndex);
 
  // Create a recursive struct with a field of its own type
  const BinaryenIndex tempStructIndex = 1;
@@ -2173,6 +2174,7 @@ void test_typebuilder() {
  bool fieldMutables[] = {true};
  TypeBuilderSetStructType(
  builder, tempStructIndex, fieldTypes, fieldPackedTypes, fieldMutables, 1);
+ TypeBuilderSetOpen(builder, tempStructIndex);
  }
 
  // Create a recursive signature with parameter and result including its own
@@ -2189,6 +2191,7 @@ void test_typebuilder() {
  tempSignatureIndex,
  TypeBuilderGetTempTupleType(builder, (BinaryenType*)&paramTypes, 2),
  tempSignatureType);
+ TypeBuilderSetOpen(builder, tempSignatureIndex);
  }
 
  // Create a subtype (with an additional immutable packed field)
@@ -2209,11 +2212,10 @@ void test_typebuilder() {
  fieldPackedTypes,
  fieldMutables,
  2);
+ TypeBuilderSetOpen(builder, tempSubStructIndex);
  }
  TypeBuilderSetSubType(builder, tempSubStructIndex, tempStructHeapType);
 
- // TODO: Rtts (post-MVP?)
-
  // Build the type hierarchy and dispose the builder
  BinaryenHeapType heapTypes[4];
  BinaryenIndex errorIndex;

diff --git a/test/example/c-api-kitchen-sink.txt b/test/example/c-api-kitchen-sink.txt
@@ -3166,9 +3166,9 @@ TypeBuilderErrorReasonForwardSupertypeReference: 2
 TypeBuilderErrorReasonForwardChildReference: 3
 module with recursive GC types:
 (module
- (type $SomeArray (array (mut (ref null $SomeArray))))
- (type $SomeSignature (func (param (ref null $SomeSignature) (ref null $SomeArray)) (result (ref null $SomeSignature))))
- (type $SomeStruct (struct (field $SomeField (mut (ref null $SomeStruct)))))
+ (type $SomeArray (sub (array (mut (ref null $SomeArray)))))
+ (type $SomeSignature (sub (func (param (ref null $SomeSignature) (ref null $SomeArray)) (result (ref null $SomeSignature)))))
+ (type $SomeStruct (sub (struct (field $SomeField (mut (ref null $SomeStruct))))))
  (type $3 (func))
  (type $SomeSubStruct (sub $SomeStruct (struct (field $SomeField (mut (ref null $SomeStruct))) (field $SomePackedField i8))))
  (func $test (type $3)

diff --git a/test/gtest/possible-contents.cpp b/test/gtest/possible-contents.cpp
@@ -539,14 +539,11 @@ TEST_F(PossibleContentsTest, TestStructCones) {
  */
  TypeBuilder builder(5);
  builder.createRecGroup(0, 5);
- builder.setHeapType(0, Struct(FieldList{}));
- builder.setHeapType(1, Struct(FieldList{}));
- builder.setHeapType(2, Struct(FieldList{}));
- builder.setHeapType(3, Struct(FieldList{}));
- builder.setHeapType(4, Struct(FieldList{}));
- builder.setSubType(1, builder.getTempHeapType(0));
- builder.setSubType(2, builder.getTempHeapType(0));
- builder.setSubType(3, builder.getTempHeapType(2));
+ builder[0].setOpen() = Struct(FieldList{});
+ builder[1].setOpen().subTypeOf(builder[0]) = Struct(FieldList{});
+ builder[2].setOpen().subTypeOf(builder[0]) = Struct(FieldList{});
+ builder[3].setOpen().subTypeOf(builder[2]) = Struct(FieldList{});
+ builder[4].setOpen() = Struct(FieldList{});
  auto result = builder.build();
  ASSERT_TRUE(result);
  auto types = *result;