From c6bc2cbb3df863ca8348526405d7350612184e89 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 30 Nov 2023 10:50:59 -0800 Subject: [PATCH] Rename IndexBase -> IdBase, ComparableIndexBase -> IndexBase. (#3436) This reflects how we're naming classes that derive from these classes, and matches usage for each existing `Id` and `Index` type, except: - `Parse::NodeId` previously inherited from `ComparableIndexBase`, and is no longer comparable. - `SemIR::MemberIndex` previously inherited from `IndexBase`, and is now comparable. Making `Parse::NodeId` non-comparable reflects that it's intended to be an opaque identifier for a node and that the ordering is an implementation detail rather than part of the intended public interface. `PostorderIterator` and `SiblingIterator` still rely on the numerical meaning of `NodeId`s, but that's OK since they're part of the node implementation. --- toolchain/base/index_base.h | 65 ++++++++++++++++++------------- toolchain/base/value_store.h | 30 +++++++------- toolchain/check/decl_name_stack.h | 4 +- toolchain/lex/tokenized_buffer.h | 8 ++-- toolchain/parse/tree.h | 14 ++----- toolchain/sem_ir/ids.h | 58 +++++++++++++-------------- toolchain/sem_ir/inst.h | 2 +- 7 files changed, 93 insertions(+), 88 deletions(-) diff --git a/toolchain/base/index_base.h b/toolchain/base/index_base.h index 7b680dcff4ee..6c24c9b0f30c 100644 --- a/toolchain/base/index_base.h +++ b/toolchain/base/index_base.h @@ -15,15 +15,20 @@ namespace Carbon { template class DataIterator; -// A lightweight handle to an item in a vector. +// A lightweight handle to an item identified by an opaque ID. // -// DataIndex is designed to be passed by value, not reference or pointer. They -// are also designed to be small and efficient to store in data structures. -struct IndexBase : public Printable { +// This class is intended to be derived from by classes representing a specific +// kind of ID, whose meaning as an integer is an implementation detail of the +// type that vends the IDs. Typically this will be a vector index. +// +// Classes derived from IdBase are designed to be passed by value, not +// reference or pointer. They are also designed to be small and efficient to +// store in data structures. +struct IdBase : public Printable { static constexpr int32_t InvalidIndex = -1; - IndexBase() = delete; - constexpr explicit IndexBase(int index) : index(index) {} + IdBase() = delete; + constexpr explicit IdBase(int index) : index(index) {} auto Print(llvm::raw_ostream& output) const -> void { if (is_valid()) { @@ -38,48 +43,56 @@ struct IndexBase : public Printable { int32_t index; }; -// Like IndexBase, but also provides < and > comparison operators. -struct ComparableIndexBase : public IndexBase { - using IndexBase::IndexBase; +// A lightweight handle to an item that behaves like an index. +// +// Unlike IdBase, classes derived from IndexBase are not completely opaque, and +// provide at least an ordering between indexes that has meaning to an API +// user. Additional semantics may be specified by the derived class. +struct IndexBase : public IdBase { + using IdBase::IdBase; }; -// Equality comparison for both IndexBase and ComparableIndexBase. -template >* = - nullptr> +// Equality comparison for both IdBase and IndexBase. +template < + typename IndexType, + typename std::enable_if_t>* = nullptr> auto operator==(IndexType lhs, IndexType rhs) -> bool { return lhs.index == rhs.index; } -template >* = - nullptr> +template < + typename IndexType, + typename std::enable_if_t>* = nullptr> auto operator!=(IndexType lhs, IndexType rhs) -> bool { return lhs.index != rhs.index; } -// The < and > comparisons for only ComparableIndexBase. -template >* = nullptr> +// The < and > comparisons for only IndexBase. +template >* = + nullptr> auto operator<(IndexType lhs, IndexType rhs) -> bool { return lhs.index < rhs.index; } -template >* = nullptr> +template >* = + nullptr> auto operator<=(IndexType lhs, IndexType rhs) -> bool { return lhs.index <= rhs.index; } -template >* = nullptr> +template >* = + nullptr> auto operator>(IndexType lhs, IndexType rhs) -> bool { return lhs.index > rhs.index; } -template >* = nullptr> +template >* = + nullptr> auto operator>=(IndexType lhs, IndexType rhs) -> bool { return lhs.index >= rhs.index; } -// Provides base support for use of IndexBase types as DenseMap/DenseSet keys. +// Provides base support for use of IdBase types as DenseMap/DenseSet keys. // // Usage (in global namespace): // template <> diff --git a/toolchain/base/value_store.h b/toolchain/base/value_store.h index 0ec09b87a75d..78d2a62d0504 100644 --- a/toolchain/base/value_store.h +++ b/toolchain/base/value_store.h @@ -48,37 +48,37 @@ class Real : public Printable { }; // Corresponds to an integer value represented by an APInt. -struct IntId : public IndexBase, public Printable { +struct IntId : public IdBase, public Printable { using IndexedType = const llvm::APInt; static const IntId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "int"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr IntId IntId::Invalid(IntId::InvalidIndex); // Corresponds to a Real value. -struct RealId : public IndexBase, public Printable { +struct RealId : public IdBase, public Printable { using IndexedType = const Real; static const RealId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "real"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr RealId RealId::Invalid(RealId::InvalidIndex); // Corresponds to a StringRef. -struct StringId : public IndexBase, public Printable { +struct StringId : public IdBase, public Printable { using IndexedType = const std::string; static const StringId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "str"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr StringId StringId::Invalid(StringId::InvalidIndex); @@ -87,23 +87,23 @@ constexpr StringId StringId::Invalid(StringId::InvalidIndex); // // `NameId` relies on the values of this type other than `Invalid` all being // non-negative. -struct IdentifierId : public IndexBase, public Printable { +struct IdentifierId : public IdBase, public Printable { static const IdentifierId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "strId"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr IdentifierId IdentifierId::Invalid(IdentifierId::InvalidIndex); // Adapts StringId for string literals. -struct StringLiteralId : public IndexBase, public Printable { +struct StringLiteralId : public IdBase, public Printable { static const StringLiteralId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "strLit"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr StringLiteralId StringLiteralId::Invalid( diff --git a/toolchain/check/decl_name_stack.h b/toolchain/check/decl_name_stack.h index f739363849c4..d98b200e3776 100644 --- a/toolchain/check/decl_name_stack.h +++ b/toolchain/check/decl_name_stack.h @@ -21,8 +21,8 @@ namespace Carbon::Check { // // TODO: Move this struct and the name lookup code in context.h to a separate // file. -struct ScopeIndex : public ComparableIndexBase, public Printable { - using ComparableIndexBase::ComparableIndexBase; +struct ScopeIndex : public IndexBase, public Printable { + using IndexBase::IndexBase; }; class Context; diff --git a/toolchain/lex/tokenized_buffer.h b/toolchain/lex/tokenized_buffer.h index 4de2d7b50e24..6c7f0ed932c7 100644 --- a/toolchain/lex/tokenized_buffer.h +++ b/toolchain/lex/tokenized_buffer.h @@ -39,11 +39,11 @@ class TokenizedBuffer; // meaningfully compared. // // All other APIs to query a `TokenIndex` are on the `TokenizedBuffer`. -struct TokenIndex : public ComparableIndexBase { +struct TokenIndex : public IndexBase { static const TokenIndex Invalid; // Comments aren't tokenized, so this is the first token after FileStart. static const TokenIndex FirstNonCommentToken; - using ComparableIndexBase::ComparableIndexBase; + using IndexBase::IndexBase; }; constexpr TokenIndex TokenIndex::Invalid(TokenIndex::InvalidIndex); @@ -60,9 +60,9 @@ constexpr TokenIndex TokenIndex::FirstNonCommentToken(1); // same line or the relative position of different lines within the source. // // All other APIs to query a `LineIndex` are on the `TokenizedBuffer`. -struct LineIndex : public ComparableIndexBase { +struct LineIndex : public IndexBase { static const LineIndex Invalid; - using ComparableIndexBase::ComparableIndexBase; + using IndexBase::IndexBase; }; constexpr LineIndex LineIndex::Invalid(LineIndex::InvalidIndex); diff --git a/toolchain/parse/tree.h b/toolchain/parse/tree.h index 74e3a94b9638..5f5cad37dee2 100644 --- a/toolchain/parse/tree.h +++ b/toolchain/parse/tree.h @@ -23,14 +23,11 @@ namespace Carbon::Parse { // Objects of this type are small and cheap to copy and store. They don't // contain any of the information about the node, and serve as a handle that // can be used with the underlying tree to query for detailed information. -// -// That said, nodes can be compared and are part of a depth-first pre-order -// sequence across all nodes in the parse tree. -struct NodeId : public ComparableIndexBase { +struct NodeId : public IdBase { // An explicitly invalid instance. static const NodeId Invalid; - using ComparableIndexBase::ComparableIndexBase; + using IdBase::IdBase; }; constexpr NodeId NodeId::Invalid = NodeId(NodeId::InvalidIndex); @@ -282,7 +279,7 @@ class Tree::PostorderIterator return node_ == rhs.node_; } auto operator<(const PostorderIterator& rhs) const -> bool { - return node_ < rhs.node_; + return node_.index < rhs.node_.index; } auto operator*() const -> NodeId { return node_; } @@ -333,11 +330,6 @@ class Tree::SiblingIterator auto operator==(const SiblingIterator& rhs) const -> bool { return node_ == rhs.node_; } - auto operator<(const SiblingIterator& rhs) const -> bool { - // Note that child iterators walk in reverse compared to the postorder - // index. - return node_ > rhs.node_; - } auto operator*() const -> NodeId { return node_; } diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 52cd8a2caf7e..353a98686d30 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -14,7 +14,7 @@ namespace Carbon::SemIR { // The ID of an instruction. -struct InstId : public IndexBase, public Printable { +struct InstId : public IdBase, public Printable { // An explicitly invalid instruction ID. static const InstId Invalid; @@ -28,11 +28,11 @@ struct InstId : public IndexBase, public Printable { return InstId(kind.AsInt()); } - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "inst"; if (!is_valid()) { - IndexBase::Print(out); + IdBase::Print(out); } else if (index < BuiltinKind::ValidCount) { out << BuiltinKind::FromInt(index); } else { @@ -51,51 +51,51 @@ constexpr InstId InstId::Invalid = InstId(InstId::InvalidIndex); #include "toolchain/sem_ir/builtin_kind.def" // The ID of a function. -struct FunctionId : public IndexBase, public Printable { +struct FunctionId : public IdBase, public Printable { // An explicitly invalid function ID. static const FunctionId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "function"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr FunctionId FunctionId::Invalid = FunctionId(FunctionId::InvalidIndex); // The ID of a class. -struct ClassId : public IndexBase, public Printable { +struct ClassId : public IdBase, public Printable { // An explicitly invalid class ID. static const ClassId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "class"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr ClassId ClassId::Invalid = ClassId(ClassId::InvalidIndex); // The ID of a cross-referenced IR. -struct CrossRefIRId : public IndexBase, public Printable { +struct CrossRefIRId : public IdBase, public Printable { static const CrossRefIRId Builtins; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "ir"; - IndexBase::Print(out); + IdBase::Print(out); } }; constexpr CrossRefIRId CrossRefIRId::Builtins = CrossRefIRId(0); // A boolean value. -struct BoolValue : public IndexBase, public Printable { +struct BoolValue : public IdBase, public Printable { static const BoolValue False; static const BoolValue True; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { switch (index) { case 0: @@ -115,7 +115,7 @@ constexpr BoolValue BoolValue::True = BoolValue(1); // The ID of a name. A name is either a string or a special name such as // `self`, or eventually `Self` or `base`. -struct NameId : public IndexBase, public Printable { +struct NameId : public IdBase, public Printable { // An explicitly invalid ID. static const NameId Invalid; // The name of `self`. @@ -134,7 +134,7 @@ struct NameId : public IndexBase, public Printable { return NameId(id.index); } - using IndexBase::IndexBase; + using IdBase::IdBase; // Returns the IdentifierId corresponding to this NameId, or an invalid // IdentifierId if this is a special name. @@ -152,7 +152,7 @@ struct NameId : public IndexBase, public Printable { out << "ReturnSlot"; } else { CARBON_CHECK(index >= 0) << "Unknown index"; - IndexBase::Print(out); + IdBase::Print(out); } } }; @@ -163,14 +163,14 @@ constexpr NameId NameId::SelfType = NameId(NameId::InvalidIndex - 2); constexpr NameId NameId::ReturnSlot = NameId(NameId::InvalidIndex - 3); // The ID of a name scope. -struct NameScopeId : public IndexBase, public Printable { +struct NameScopeId : public IdBase, public Printable { // An explicitly invalid ID. static const NameScopeId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "name_scope"; - IndexBase::Print(out); + IdBase::Print(out); } }; @@ -178,7 +178,7 @@ constexpr NameScopeId NameScopeId::Invalid = NameScopeId(NameScopeId::InvalidIndex); // The ID of an instruction block. -struct InstBlockId : public IndexBase, public Printable { +struct InstBlockId : public IdBase, public Printable { // All File instances must provide the 0th instruction block as empty. static const InstBlockId Empty; @@ -188,13 +188,13 @@ struct InstBlockId : public IndexBase, public Printable { // An ID for unreachable code. static const InstBlockId Unreachable; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { if (index == Unreachable.index) { out << "unreachable"; } else { out << "block"; - IndexBase::Print(out); + IdBase::Print(out); } } }; @@ -206,7 +206,7 @@ constexpr InstBlockId InstBlockId::Unreachable = InstBlockId(InstBlockId::InvalidIndex - 1); // The ID of a type. -struct TypeId : public IndexBase, public Printable { +struct TypeId : public IdBase, public Printable { // The builtin TypeType. static const TypeId TypeType; @@ -216,7 +216,7 @@ struct TypeId : public IndexBase, public Printable { // An explicitly invalid ID. static const TypeId Invalid; - using IndexBase::IndexBase; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "type"; if (index == TypeType.index) { @@ -224,7 +224,7 @@ struct TypeId : public IndexBase, public Printable { } else if (index == Error.index) { out << "Error"; } else { - IndexBase::Print(out); + IdBase::Print(out); } } }; @@ -234,11 +234,11 @@ constexpr TypeId TypeId::Error = TypeId(TypeId::InvalidIndex - 1); constexpr TypeId TypeId::Invalid = TypeId(TypeId::InvalidIndex); // The ID of a type block. -struct TypeBlockId : public IndexBase, public Printable { - using IndexBase::IndexBase; +struct TypeBlockId : public IdBase, public Printable { + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "typeBlock"; - IndexBase::Print(out); + IdBase::Print(out); } }; diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index a655b0c13f86..6f20f75a866c 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -159,7 +159,7 @@ class Inst : public Printable { arg1_(arg1) {} // Convert a field to its raw representation, used as `arg0_` / `arg1_`. - static constexpr auto ToRaw(IndexBase base) -> int32_t { return base.index; } + static constexpr auto ToRaw(IdBase base) -> int32_t { return base.index; } static constexpr auto ToRaw(BuiltinKind kind) -> int32_t { return kind.AsInt(); }