Skip to content

Commit 838ec9a

Browse files
committed
Refactor InstNamer::GetScopeIdOffset() to make it easier to comprehend and maintain
Instead of having a switch case with `fallthrough` where each case adds the number of entities of the next id type case, have an array that maps id type to a function that gets the number of the matching entities. This array is less confusing and easier to maintain to avoid bugs like the one fixed in carbon-language#6151. The logic that sums only the entities above the given id is now a common logic so no need to change it when changing the set of ids.
1 parent 16999a7 commit 838ec9a

File tree

1 file changed

+45
-35
lines changed

1 file changed

+45
-35
lines changed

toolchain/sem_ir/inst_namer.cpp

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -121,45 +121,55 @@ InstNamer::InstNamer(const File* sem_ir, int total_ir_count)
121121
}
122122

123123
auto InstNamer::GetScopeIdOffset(ScopeIdTypeEnum id_enum) const -> int {
124-
int offset = 0;
124+
// Map between Id type and a function to get the number of its entities.
125+
struct IdEnumGetSizeEntry {
126+
ScopeIdTypeEnum id_enum = ScopeIdTypeEnum::None;
127+
using GetSizeFunc = size_t (*)(const File*);
128+
GetSizeFunc get_size;
129+
};
130+
constexpr IdEnumGetSizeEntry IdEnumGetSizeArray[] = {
131+
{ScopeIdTypeEnum::For<AssociatedConstantId>,
132+
[](const File* sem_ir) {
133+
return sem_ir->associated_constants().size();
134+
}},
135+
{ScopeIdTypeEnum::For<ClassId>,
136+
[](const File* sem_ir) { return sem_ir->classes().size(); }},
137+
{ScopeIdTypeEnum::For<CppOverloadSetId>,
138+
[](const File* sem_ir) { return sem_ir->cpp_overload_sets().size(); }},
139+
{ScopeIdTypeEnum::For<FunctionId>,
140+
[](const File* sem_ir) { return sem_ir->functions().size(); }},
141+
{ScopeIdTypeEnum::For<ImplId>,
142+
[](const File* sem_ir) { return sem_ir->impls().size(); }},
143+
{ScopeIdTypeEnum::For<InterfaceId>,
144+
[](const File* sem_ir) { return sem_ir->interfaces().size(); }},
145+
{ScopeIdTypeEnum::For<SpecificInterfaceId>,
146+
[](const File* sem_ir) { return sem_ir->specific_interfaces().size(); }},
147+
{ScopeIdTypeEnum::For<VtableId>,
148+
[](const File* sem_ir) { return sem_ir->vtables().size(); }},
149+
};
125150

126-
// For each Id type, add the number of entities *above* its case; for example,
127-
// the offset for functions excludes the functions themselves. The fallthrough
151+
// For each Id type, add the number of entities *above* it; for example, the
152+
// offset for functions excludes the functions themselves. The fallthrough
128153
// handles summing to get uniqueness; order isn't special.
129-
switch (id_enum) {
130-
case ScopeIdTypeEnum::None:
131-
// `None` will be getting a full count of scopes.
132-
offset += sem_ir_->associated_constants().size();
133-
[[fallthrough]];
134-
case ScopeIdTypeEnum::For<AssociatedConstantId>:
135-
offset += sem_ir_->classes().size();
136-
[[fallthrough]];
137-
case ScopeIdTypeEnum::For<ClassId>:
138-
offset += sem_ir_->cpp_overload_sets().size();
139-
[[fallthrough]];
140-
case ScopeIdTypeEnum::For<CppOverloadSetId>:
141-
offset += sem_ir_->functions().size();
142-
[[fallthrough]];
143-
case ScopeIdTypeEnum::For<FunctionId>:
144-
offset += sem_ir_->impls().size();
145-
[[fallthrough]];
146-
case ScopeIdTypeEnum::For<ImplId>:
147-
offset += sem_ir_->interfaces().size();
148-
[[fallthrough]];
149-
case ScopeIdTypeEnum::For<InterfaceId>:
150-
offset += sem_ir_->specific_interfaces().size();
151-
[[fallthrough]];
152-
case ScopeIdTypeEnum::For<SpecificInterfaceId>:
153-
offset += sem_ir_->vtables().size();
154-
[[fallthrough]];
155-
case ScopeIdTypeEnum::For<VtableId>:
156-
// All type-specific scopes are offset by `FirstEntityScope`.
157-
offset += static_cast<int>(ScopeId::FirstEntityScope);
158-
return offset;
159154

160-
default:
161-
CARBON_FATAL("Unexpected ScopeIdTypeEnum: {0}", id_enum);
155+
// `FirstEntityScope` is always above.
156+
int offset = static_cast<int>(ScopeId::FirstEntityScope);
157+
158+
// All entities are above *None*.
159+
bool found_enum = id_enum == ScopeIdTypeEnum::None;
160+
161+
for (const auto& id_enum_get_size : IdEnumGetSizeArray) {
162+
// We sum the offsets before comparing id_enum to exclude the given id_enum
163+
// from the offset.
164+
if (found_enum) {
165+
offset += id_enum_get_size.get_size(sem_ir_);
166+
} else {
167+
found_enum = id_enum_get_size.id_enum == id_enum;
168+
}
162169
}
170+
CARBON_CHECK(found_enum, "Unexpected ScopeIdTypeEnum: {0}", id_enum);
171+
172+
return offset;
163173
}
164174

165175
auto InstNamer::GetScopeName(ScopeId scope) const -> std::string {

0 commit comments

Comments
 (0)