Skip to content

Commit 5436efc

Browse files
authored
Remove unused isNominal field on HeapTypeInfo (#4465)
This field was originally added with the goal of allowing types from multiple type systems to coexist by determining the type system on a per-type level rather than globally. This goal was never fully achieved and the `isNominal` field is not used outside of tests. Now that we are working on implementing the hybrid isorecursive system, it does not look like having types from multiple systems coexist will be useful in the near term, so clean up this tech debt.
1 parent 4205846 commit 5436efc

File tree

6 files changed

+13
-620
lines changed

6 files changed

+13
-620
lines changed

src/wasm-type.h

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -385,22 +385,13 @@ class HeapType {
385385
Array getArray() const;
386386

387387
// If there is a nontrivial (i.e. non-basic) nominal supertype, return it,
388-
// else an empty optional. Nominal types (in the sense of isNominal,
389-
// i.e. Milestone 4 nominal types) may always have supertypes and other types
390-
// may have supertypes in `TypeSystem::Nominal` mode but not in
391-
// `TypeSystem::Equirecursive` mode.
388+
// else an empty optional.
392389
std::optional<HeapType> getSuperType() const;
393390

394391
// Return the depth of this heap type in the nominal type hierarchy, i.e. the
395392
// number of supertypes in its supertype chain.
396393
size_t getDepth() const;
397394

398-
// Whether this is a nominal type in the sense of being a GC Milestone 4
399-
// nominal type. Although all non-basic HeapTypes are nominal in
400-
// `TypeSystem::Nominal` mode, this will still return false unless the type is
401-
// specifically constructed as a Milestone 4 nominal type.
402-
bool isNominal() const;
403-
404395
constexpr TypeID getID() const { return id; }
405396
constexpr BasicHeapType getBasic() const {
406397
assert(isBasic() && "Basic heap type expected");
@@ -605,10 +596,6 @@ struct TypeBuilder {
605596
// `j`. Does nothing for equirecursive types.
606597
void setSubType(size_t i, size_t j);
607598

608-
// Make this type nominal in the sense of the Milestone 4 GC spec, independent
609-
// of the current TypeSystem configuration.
610-
void setNominal(size_t i);
611-
612599
// Returns all of the newly constructed heap types. May only be called once
613600
// all of the heap types have been initialized with `setHeapType`. In nominal
614601
// mode, all of the constructed HeapTypes will be fresh and distinct. In
@@ -647,10 +634,6 @@ struct TypeBuilder {
647634
builder.setSubType(index, other.index);
648635
return *this;
649636
}
650-
Entry& setNominal() {
651-
builder.setNominal(index);
652-
return *this;
653-
}
654637
};
655638

656639
Entry operator[](size_t i) { return Entry{*this, i}; }

src/wasm/wasm-binary.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ void WasmBinaryWriter::writeTypes() {
224224
o << U32LEB(indexedTypes.types.size());
225225
for (Index i = 0; i < indexedTypes.types.size(); ++i) {
226226
auto type = indexedTypes.types[i];
227-
bool nominal = type.isNominal() || getTypeSystem() == TypeSystem::Nominal;
227+
bool nominal = getTypeSystem() == TypeSystem::Nominal;
228228
BYN_TRACE("write " << type << std::endl);
229229
if (type.isSignature()) {
230230
o << S32LEB(nominal ? BinaryConsts::EncodedType::FuncExtending
@@ -1962,8 +1962,6 @@ void WasmBinaryBuilder::readTypes() {
19621962
if (form == BinaryConsts::EncodedType::FuncExtending ||
19631963
form == BinaryConsts::EncodedType::StructExtending ||
19641964
form == BinaryConsts::EncodedType::ArrayExtending) {
1965-
// TODO: Let the new nominal types coexist with equirecursive types
1966-
// builder[i].setNominal();
19671965
auto superIndex = getS64LEB(); // TODO: Actually s33
19681966
if (superIndex >= 0) {
19691967
if (size_t(superIndex) >= numTypes) {

src/wasm/wasm-s-parser.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -876,8 +876,6 @@ void SExpressionWasmBuilder::preParseHeapTypes(Element& module) {
876876
}
877877
Element* super = nullptr;
878878
if (nominal) {
879-
// TODO: Let the new nominal types coexist with equirecursive types
880-
// builder[index].setNominal();
881879
super = def[def.size() - 1];
882880
if (super->dollared()) {
883881
// OK

src/wasm/wasm-type.cpp

Lines changed: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ struct HeapTypeInfo {
105105
// Otherwise, the type definition tree is still being constructed via the
106106
// TypeBuilder interface, so hashing and equality use pointer identity.
107107
bool isFinalized = true;
108-
bool isNominal = false;
109-
// In nominal mode, the supertype of this HeapType, if it exists.
108+
// In nominal or isorecursive mode, the supertype of this HeapType, if it
109+
// exists.
110110
HeapTypeInfo* supertype = nullptr;
111111
enum Kind {
112112
BasicKind,
@@ -581,7 +581,6 @@ bool TypeInfo::operator==(const TypeInfo& other) const {
581581
HeapTypeInfo::HeapTypeInfo(const HeapTypeInfo& other) {
582582
kind = other.kind;
583583
supertype = other.supertype;
584-
isNominal = other.isNominal;
585584
switch (kind) {
586585
case BasicKind:
587586
new (&basic) auto(other.basic);
@@ -690,7 +689,7 @@ template<typename Info> struct Store {
690689
std::lock_guard<std::recursive_mutex> lock(mutex);
691690
// Nominal HeapTypes are always unique, so don't bother deduplicating them.
692691
if constexpr (std::is_same_v<Info, HeapTypeInfo>) {
693-
if (info.isNominal || typeSystem == TypeSystem::Nominal) {
692+
if (typeSystem == TypeSystem::Nominal) {
694693
return insertNew();
695694
}
696695
}
@@ -1215,14 +1214,6 @@ size_t HeapType::getDepth() const {
12151214
return depth;
12161215
}
12171216

1218-
bool HeapType::isNominal() const {
1219-
if (isBasic()) {
1220-
return false;
1221-
} else {
1222-
return getHeapTypeInfo(*this)->isNominal;
1223-
}
1224-
}
1225-
12261217
bool HeapType::isSubType(HeapType left, HeapType right) {
12271218
// As an optimization, in the common case do not even construct a SubTyper.
12281219
if (left == right) {
@@ -1342,11 +1333,7 @@ bool SubTyper::isSubType(HeapType a, HeapType b) {
13421333
// Basic HeapTypes are never subtypes of compound HeapTypes.
13431334
return false;
13441335
}
1345-
// Nominal and structural types are never subtypes of each other.
1346-
if (a.isNominal() != b.isNominal()) {
1347-
return false;
1348-
}
1349-
if (a.isNominal() || typeSystem == TypeSystem::Nominal) {
1336+
if (typeSystem == TypeSystem::Nominal) {
13501337
// Subtyping must be declared in a nominal system, not derived from
13511338
// structure, so we will not recurse. TODO: optimize this search with some
13521339
// form of caching.
@@ -1525,9 +1512,6 @@ HeapType TypeBounder::lub(HeapType a, HeapType b) {
15251512
if (a.isBasic() || b.isBasic()) {
15261513
return getBasicLUB();
15271514
}
1528-
if (a.isNominal() != b.isNominal()) {
1529-
return getBasicLUB();
1530-
}
15311515

15321516
HeapTypeInfo* infoA = getHeapTypeInfo(a);
15331517
HeapTypeInfo* infoB = getHeapTypeInfo(b);
@@ -1536,7 +1520,7 @@ HeapType TypeBounder::lub(HeapType a, HeapType b) {
15361520
return getBasicLUB();
15371521
}
15381522

1539-
if (a.isNominal() || typeSystem == TypeSystem::Nominal) {
1523+
if (typeSystem == TypeSystem::Nominal) {
15401524
// Walk up the subtype tree to find the LUB. Ascend the tree from both `a`
15411525
// and `b` in lockstep. The first type we see for a second time must be the
15421526
// LUB because there are no cycles and the only way to encounter a type
@@ -1971,15 +1955,13 @@ size_t FiniteShapeHasher::hash(const TypeInfo& info) {
19711955
}
19721956

19731957
size_t FiniteShapeHasher::hash(const HeapTypeInfo& info) {
1974-
size_t digest = wasm::hash(info.isNominal);
1975-
if (info.isNominal || getTypeSystem() == TypeSystem::Nominal) {
1976-
rehash(digest, uintptr_t(&info));
1977-
return digest;
1958+
if (getTypeSystem() == TypeSystem::Nominal) {
1959+
return wasm::hash(uintptr_t(&info));
19781960
}
19791961
// If the HeapTypeInfo is not finalized, then it is mutable and its shape
19801962
// might change in the future. In that case, fall back to pointer identity to
19811963
// keep the hash consistent until all the TypeBuilder's types are finalized.
1982-
digest = wasm::hash(info.isFinalized);
1964+
size_t digest = wasm::hash(info.isFinalized);
19831965
if (!info.isFinalized) {
19841966
rehash(digest, uintptr_t(&info));
19851967
return digest;
@@ -2094,9 +2076,7 @@ bool FiniteShapeEquator::eq(const TypeInfo& a, const TypeInfo& b) {
20942076
}
20952077

20962078
bool FiniteShapeEquator::eq(const HeapTypeInfo& a, const HeapTypeInfo& b) {
2097-
if (a.isNominal != b.isNominal) {
2098-
return false;
2099-
} else if (a.isNominal || getTypeSystem() == TypeSystem::Nominal) {
2079+
if (getTypeSystem() == TypeSystem::Nominal) {
21002080
return &a == &b;
21012081
}
21022082
if (a.isFinalized != b.isFinalized) {
@@ -2264,7 +2244,6 @@ struct TypeBuilder::Impl {
22642244
}
22652245
void set(HeapTypeInfo&& hti) {
22662246
hti.supertype = info->supertype;
2267-
hti.isNominal = info->isNominal;
22682247
*info = std::move(hti);
22692248
info->isTemp = true;
22702249
info->isFinalized = false;
@@ -2359,11 +2338,6 @@ void TypeBuilder::setSubType(size_t i, size_t j) {
23592338
sub->supertype = super;
23602339
}
23612340

2362-
void TypeBuilder::setNominal(size_t i) {
2363-
assert(i < size() && "index out of bounds");
2364-
impl->entries[i].info->isNominal = true;
2365-
}
2366-
23672341
namespace {
23682342

23692343
// Helper for TypeBuilder::build() that keeps track of temporary types and
@@ -3036,9 +3010,7 @@ void globallyCanonicalize(CanonicalizationState& state) {
30363010
void canonicalizeEquirecursive(CanonicalizationState& state) {
30373011
// Equirecursive types always have null supertypes.
30383012
for (auto& info : state.newInfos) {
3039-
if (!info->isNominal) {
3040-
info->supertype = nullptr;
3041-
}
3013+
info->supertype = nullptr;
30423014
}
30433015

30443016
#if TIME_CANONICALIZATION
@@ -3219,8 +3191,7 @@ std::vector<HeapType> TypeBuilder::build() {
32193191

32203192
// Note built signature types. See comment in `HeapType::HeapType(Signature)`.
32213193
for (auto type : state.results) {
3222-
if (type.isSignature() &&
3223-
(type.isNominal() || getTypeSystem() == TypeSystem::Nominal)) {
3194+
if (type.isSignature() && (getTypeSystem() == TypeSystem::Nominal)) {
32243195
nominalSignatureCache.insertType(type);
32253196
}
32263197
}

0 commit comments

Comments
 (0)