Skip to content

Commit fbf08ef

Browse files
authored
[NFC] Introduce HeapTypeDef and use it for printing (#7444)
Now that we have exact heap types, non-abstract heap types no longer correspond 1:1 with heap type definitions. We previously used a single type, `HeapType`, to represent both heap types and heap type definitions. Now that `HeapType` can represent multiple heap types corresponding to the same definition, there is a new class of potential bugs in which code that expects `HeapType` values to map 1:1 with heap type definitions observes both an exact and inexact heap type for the same definition. To eliminate this class of bugs, introduce a new type, `HeapTypeDef`, whose values do correspond 1:1 with heap type definitions. `HeapTypeDef` is a subclass of `HeapType`, so it supports all the same functionality, but it cannot represent exact heap types because its constructor clears the exact bit. As an initial proof-of-concept, use HeapTypeDef in the type printing machinery, which only cares about heap type names corresponding to heap type definitions. Future PRs will use HeapTypeDef in more places.
1 parent 3ce02b6 commit fbf08ef

File tree

6 files changed

+51
-19
lines changed

6 files changed

+51
-19
lines changed

src/passes/Print.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
198198
}
199199
}
200200

201-
TypeNames getNames(HeapType type) {
201+
TypeNames getNames(HeapTypeDef type) {
202202
if (parent.currModule) {
203203
if (auto it = parent.currModule->typeNames.find(type);
204204
it != parent.currModule->typeNames.end()) {

src/tools/wasm-fuzz-types.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ void Fuzzer::printTypes(const std::vector<HeapType>& types) {
9797
std::cout << "Built " << types.size() << " types:\n";
9898
struct FatalTypeNameGenerator
9999
: TypeNameGeneratorBase<FatalTypeNameGenerator> {
100-
TypeNames getNames(HeapType type) {
100+
TypeNames getNames(HeapTypeDef type) {
101101
Fatal() << "trying to print unknown heap type";
102102
}
103103
} fatalGenerator;

src/wasm-type-printing.h

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,32 @@ namespace wasm {
3232
// ability to use the generator as a function to print Types and HeapTypes to
3333
// streams.
3434
template<typename Subclass> struct TypeNameGeneratorBase {
35-
TypeNames getNames(HeapType type) {
36-
static_assert(&TypeNameGeneratorBase<Subclass>::getNames !=
37-
&Subclass::getNames,
38-
"Derived class must implement getNames");
35+
TypeNameGeneratorBase() { assertValidUsage(); }
36+
37+
TypeNames getNames(HeapTypeDef type) {
3938
WASM_UNREACHABLE("Derived class must implement getNames");
4039
}
41-
HeapType::Printed operator()(HeapType type) {
42-
return type.print(
43-
[&](HeapType ht) { return static_cast<Subclass*>(this)->getNames(ht); });
40+
HeapType::Printed operator()(HeapTypeDef type) {
41+
return type.print([&](HeapTypeDef ht) {
42+
return static_cast<Subclass*>(this)->getNames(ht);
43+
});
4444
}
4545
Type::Printed operator()(Type type) {
46-
return type.print(
47-
[&](HeapType ht) { return static_cast<Subclass*>(this)->getNames(ht); });
46+
return type.print([&](HeapTypeDef ht) {
47+
return static_cast<Subclass*>(this)->getNames(ht);
48+
});
49+
}
50+
51+
private:
52+
constexpr void assertValidUsage() {
53+
#if !defined(__GNUC__) || __GNUC__ >= 14
54+
// Check that the subclass provides `getNames` with the correct type.
55+
using Self = TypeNameGeneratorBase<Subclass>;
56+
static_assert(
57+
static_cast<TypeNames (Self::*)(HeapTypeDef)>(&Self::getNames) !=
58+
static_cast<TypeNames (Self::*)(HeapTypeDef)>(&Subclass::getNames),
59+
"Derived class must implement getNames");
60+
#endif
4861
}
4962
};
5063

@@ -60,7 +73,7 @@ struct DefaultTypeNameGenerator
6073
// Cached names for types that have already been seen.
6174
std::unordered_map<HeapType, TypeNames> nameCache;
6275

63-
TypeNames getNames(HeapType type);
76+
TypeNames getNames(HeapTypeDef type);
6477
};
6578

6679
// Generates names based on the indices of types in some collection, falling
@@ -71,7 +84,7 @@ struct IndexedTypeNameGenerator
7184
: TypeNameGeneratorBase<IndexedTypeNameGenerator<FallbackGenerator>> {
7285
DefaultTypeNameGenerator defaultGenerator;
7386
FallbackGenerator& fallback;
74-
std::unordered_map<HeapType, TypeNames> names;
87+
std::unordered_map<HeapTypeDef, TypeNames> names;
7588

7689
template<typename T>
7790
IndexedTypeNameGenerator(T& types,
@@ -86,7 +99,7 @@ struct IndexedTypeNameGenerator
8699
IndexedTypeNameGenerator(T& types, const std::string& prefix = "")
87100
: IndexedTypeNameGenerator(types, defaultGenerator, prefix) {}
88101

89-
TypeNames getNames(HeapType type) {
102+
TypeNames getNames(HeapTypeDef type) {
90103
if (auto it = names.find(type); it != names.end()) {
91104
return it->second;
92105
} else {
@@ -117,7 +130,7 @@ struct ModuleTypeNameGenerator
117130
std::enable_if_t<std::is_same_v<T, DefaultTypeNameGenerator>>* = nullptr)
118131
: ModuleTypeNameGenerator(wasm, defaultGenerator) {}
119132

120-
TypeNames getNames(HeapType type) {
133+
TypeNames getNames(HeapTypeDef type) {
121134
if (auto it = wasm.typeNames.find(type); it != wasm.typeNames.end()) {
122135
return it->second;
123136
}

src/wasm-type.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ void destroyAllTypesForTestingPurposesOnly();
5050
// data.
5151
class Type;
5252
class HeapType;
53+
class HeapTypeDef;
5354
class RecGroup;
5455
struct Signature;
5556
struct Continuation;
@@ -73,7 +74,7 @@ struct TypeNames {
7374
};
7475

7576
// Used to generate HeapType names.
76-
using HeapTypeNameGenerator = std::function<TypeNames(HeapType)>;
77+
using HeapTypeNameGenerator = std::function<TypeNames(HeapTypeDef)>;
7778

7879
// The type used for interning IDs in the public interfaces of Type and
7980
// HeapType.
@@ -294,6 +295,16 @@ class HeapType {
294295
std::string toString() const;
295296
};
296297

298+
// Like `HeapType`, but used to represent heap type definitions and abstract
299+
// heap types rather than arbitrary heap types. Use this whenever it would be a
300+
// category error to use an exact heap type.
301+
class HeapTypeDef : public HeapType {
302+
public:
303+
// Allow implicit conversions from HeapType.
304+
constexpr HeapTypeDef(HeapType type) : HeapType(type.with(Inexact)) {}
305+
constexpr HeapTypeDef() = default;
306+
};
307+
297308
class Type {
298309
// The `id` uniquely represents each type, so type equality is just a
299310
// comparison of the ids. The basic types are packed at the bottom of the
@@ -1007,6 +1018,10 @@ template<> class hash<wasm::HeapType> {
10071018
public:
10081019
size_t operator()(const wasm::HeapType&) const;
10091020
};
1021+
template<> class hash<wasm::HeapTypeDef> {
1022+
public:
1023+
size_t operator()(const wasm::HeapTypeDef&) const;
1024+
};
10101025
template<> class hash<wasm::RecGroup> {
10111026
public:
10121027
size_t operator()(const wasm::RecGroup&) const;

src/wasm.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,8 +2415,8 @@ class Module {
24152415
// Module name, if specified. Serves a documentary role only.
24162416
Name name;
24172417

2418-
std::unordered_map<HeapType, TypeNames> typeNames;
2419-
std::unordered_map<HeapType, Index> typeIndices;
2418+
std::unordered_map<HeapTypeDef, TypeNames> typeNames;
2419+
std::unordered_map<HeapTypeDef, Index> typeIndices;
24202420

24212421
MixedArena allocator;
24222422

src/wasm/wasm-type.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,7 @@ size_t RecGroup::size() const {
13691369
}
13701370
}
13711371

1372-
TypeNames DefaultTypeNameGenerator::getNames(HeapType type) {
1372+
TypeNames DefaultTypeNameGenerator::getNames(HeapTypeDef type) {
13731373
auto [it, inserted] = nameCache.insert({type, {}});
13741374
if (inserted) {
13751375
// Generate a new name for this type we have not previously seen.
@@ -2768,6 +2768,10 @@ size_t hash<wasm::HeapType>::operator()(const wasm::HeapType& heapType) const {
27682768
return wasm::hash(heapType.getID());
27692769
}
27702770

2771+
size_t hash<wasm::HeapTypeDef>::operator()(const wasm::HeapTypeDef& def) const {
2772+
return wasm::hash(def.getID());
2773+
}
2774+
27712775
size_t hash<wasm::RecGroup>::operator()(const wasm::RecGroup& group) const {
27722776
return wasm::hash(group.getID());
27732777
}

0 commit comments

Comments
 (0)