From 5a0613283bf7f121dd9c0cf58dfd01f0204a5d9d Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Mon, 6 Feb 2023 15:57:33 -0800 Subject: [PATCH] Switch constexpr factory functions to constexpr values. (#2581) Using the same const/constexpr done in EnumBase, adds Invalid and Builtin* values to replace Make functions that produced the same. This should make it clearer at call sites what the cost actually is, and reduces the syntactic overhead for MakeBuiltinReference in particular. Really, this is that MakeBuiltinReference has been feeling pretty verbose, so I did that, and then one MakeInvalid is right next to it, and then obviously I should replace the other MakeInvalid for consistency... --- toolchain/parser/parse_tree.h | 7 ++-- toolchain/semantics/semantics_node.h | 35 +++++++++++-------- toolchain/semantics/semantics_node_stack.h | 5 ++- .../semantics_parse_tree_handler.cpp | 15 ++++---- 4 files changed, 33 insertions(+), 29 deletions(-) diff --git a/toolchain/parser/parse_tree.h b/toolchain/parser/parse_tree.h index bb0159d6bf6f..de4e3a1360b4 100644 --- a/toolchain/parser/parse_tree.h +++ b/toolchain/parser/parse_tree.h @@ -242,12 +242,15 @@ class ParseTree { // That said, nodes can be compared and are part of a depth-first pre-order // sequence across all nodes in the parse tree. struct ParseTree::Node : public ComparableIndexBase { - // Constructs an explicitly invalid instance. - static auto MakeInvalid() -> Node { return Node(); } + // An explicitly invalid instance. + // NOLINTNEXTLINE(readability-identifier-naming) + static const Node Invalid; using ComparableIndexBase::ComparableIndexBase; }; +constexpr ParseTree::Node ParseTree::Node::Invalid; + // A random-access iterator to the depth-first postorder sequence of parse nodes // in the parse tree. It produces `ParseTree::Node` objects which are opaque // handles and must be used in conjunction with the `ParseTree` itself. diff --git a/toolchain/semantics/semantics_node.h b/toolchain/semantics/semantics_node.h index 4e65decbb87d..a366985e5b99 100644 --- a/toolchain/semantics/semantics_node.h +++ b/toolchain/semantics/semantics_node.h @@ -17,15 +17,14 @@ namespace Carbon { // Type-safe storage of Node IDs. struct SemanticsNodeId : public IndexBase { - // Uses the cross-reference node ID for a builtin. This relies on SemanticsIR - // guarantees for builtin cross-reference placement. - static auto MakeBuiltinReference(SemanticsBuiltinKind kind) - -> SemanticsNodeId { - return SemanticsNodeId(kind.AsInt()); - } + // An explicitly invalid node ID. + // NOLINTNEXTLINE(readability-identifier-naming) + static const SemanticsNodeId Invalid; - // Constructs an explicitly invalid instance. - static auto MakeInvalid() -> SemanticsNodeId { return SemanticsNodeId(); } +// Builtin node IDs. +#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \ + static const SemanticsNodeId Builtin##Name; +#include "toolchain/semantics/semantics_builtin_kind.def" using IndexBase::IndexBase; auto Print(llvm::raw_ostream& out) const -> void { @@ -34,6 +33,15 @@ struct SemanticsNodeId : public IndexBase { } }; +constexpr SemanticsNodeId SemanticsNodeId::Invalid = SemanticsNodeId(); + +// Uses the cross-reference node ID for a builtin. This relies on SemanticsIR +// guarantees for builtin cross-reference placement. +#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \ + constexpr SemanticsNodeId SemanticsNodeId::Builtin##Name = \ + SemanticsNodeId(SemanticsBuiltinKind::Name.AsInt()); +#include "toolchain/semantics/semantics_builtin_kind.def" + // The ID of a callable, such as a function. struct SemanticsCallableId : public IndexBase { using IndexBase::IndexBase; @@ -157,7 +165,7 @@ class SemanticsNode { static auto MakeCrossReference(SemanticsNodeId type, SemanticsCrossReferenceIRId ir, SemanticsNodeId node) -> SemanticsNode { - return SemanticsNode(ParseTree::Node::MakeInvalid(), + return SemanticsNode(ParseTree::Node::Invalid, SemanticsNodeKind::CrossReference, type, ir.index, node.index); } @@ -196,9 +204,7 @@ class SemanticsNode { SemanticsIntegerLiteralId integer) -> SemanticsNode { return SemanticsNode(parse_node, SemanticsNodeKind::IntegerLiteral, - SemanticsNodeId::MakeBuiltinReference( - SemanticsBuiltinKind::IntegerType), - integer.index); + SemanticsNodeId::BuiltinIntegerType, integer.index); } auto GetAsIntegerLiteral() const -> SemanticsIntegerLiteralId { CARBON_CHECK(kind_ == SemanticsNodeKind::IntegerLiteral); @@ -206,9 +212,8 @@ class SemanticsNode { } static auto MakeRealLiteral(ParseTree::Node parse_node) -> SemanticsNode { - return SemanticsNode( - parse_node, SemanticsNodeKind::RealLiteral, - SemanticsNodeId::MakeBuiltinReference(SemanticsBuiltinKind::RealType)); + return SemanticsNode(parse_node, SemanticsNodeKind::RealLiteral, + SemanticsNodeId::BuiltinRealType); } auto GetAsRealLiteral() const -> NoArgs { CARBON_CHECK(kind_ == SemanticsNodeKind::RealLiteral); diff --git a/toolchain/semantics/semantics_node_stack.h b/toolchain/semantics/semantics_node_stack.h index 0fe9faa71e60..5f9ff0a7f93c 100644 --- a/toolchain/semantics/semantics_node_stack.h +++ b/toolchain/semantics/semantics_node_stack.h @@ -36,9 +36,8 @@ class SemanticsNodeStack { // Pushes a solo parse tree node onto the stack. Used when there is no // IR generated by the node. auto Push(ParseTree::Node parse_node) -> void { - PushEntry( - {.parse_node = parse_node, .node_id = SemanticsNodeId::MakeInvalid()}, - DebugLog::None); + PushEntry({.parse_node = parse_node, .node_id = SemanticsNodeId::Invalid}, + DebugLog::None); } // Pushes a parse tree node onto the stack. diff --git a/toolchain/semantics/semantics_parse_tree_handler.cpp b/toolchain/semantics/semantics_parse_tree_handler.cpp index b0d8522baf5c..d2796794c981 100644 --- a/toolchain/semantics/semantics_parse_tree_handler.cpp +++ b/toolchain/semantics/semantics_parse_tree_handler.cpp @@ -140,16 +140,15 @@ auto SemanticsParseTreeHandler::TryTypeConversion(ParseTree::Node parse_node, // TODO: This should attempt a type conversion, but there's not enough // implemented to do that right now. if (lhs_type != rhs_type) { - auto invalid_type = SemanticsNodeId::MakeBuiltinReference( - SemanticsBuiltinKind::InvalidType); - if (lhs_type != invalid_type && rhs_type != invalid_type) { + if (lhs_type != SemanticsNodeId::BuiltinInvalidType && + rhs_type != SemanticsNodeId::BuiltinInvalidType) { // TODO: This is a poor diagnostic, and should be expanded. CARBON_DIAGNOSTIC(TypeMismatch, Error, "Type mismatch: lhs is {0}, rhs is {1}", SemanticsNodeId, SemanticsNodeId); emitter_->Emit(parse_node, TypeMismatch, lhs_type, rhs_type); } - return invalid_type; + return SemanticsNodeId::BuiltinInvalidType; } return lhs_type; } @@ -332,7 +331,7 @@ auto SemanticsParseTreeHandler::HandleFunctionDefinitionStart( auto decl_id = AddNode(SemanticsNode::MakeFunctionDeclaration(fn_node, callable_id)); // TODO: Propagate the type of the function. - BindName(name_node, SemanticsNodeId::MakeInvalid(), decl_id); + BindName(name_node, SemanticsNodeId::Invalid, decl_id); node_block_stack_.Push(); PushScope(); @@ -436,8 +435,7 @@ auto SemanticsParseTreeHandler::HandleLiteral(ParseTree::Node parse_node) "Currently only i32 is allowed"); return false; } - node_stack_.Push(parse_node, SemanticsNodeId::MakeBuiltinReference( - SemanticsBuiltinKind::IntegerType)); + node_stack_.Push(parse_node, SemanticsNodeId::BuiltinIntegerType); break; } default: @@ -456,8 +454,7 @@ auto SemanticsParseTreeHandler::HandleNameReference(ParseTree::Node parse_node) CARBON_DIAGNOSTIC(NameNotFound, Error, "Name {0} not found", llvm::StringRef); emitter_->Emit(parse_node, NameNotFound, name_str); - node_stack_.Push(parse_node, SemanticsNodeId::MakeBuiltinReference( - SemanticsBuiltinKind::InvalidType)); + node_stack_.Push(parse_node, SemanticsNodeId::BuiltinInvalidType); }; auto name_id = semantics_->GetString(name_str);