From b2c60b4f53fa7a3a296ce91efeb6cbe4d3d84b70 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 8 Feb 2023 14:17:26 -0800 Subject: [PATCH] Change the builtin enum approach to enforce addition. (#2584) It feels like every time I add a builtin, I forget to add it to the builtin IR. This approach enforces the addition of builtin nodes, and also takes advantage of the normalized structure to set types appropriately (I think the new approach is simpler on this aspect). --- .../semantics/semantics_builtin_kind.cpp | 3 +- .../semantics/semantics_builtin_kind.def | 42 +++++++++++++++---- toolchain/semantics/semantics_builtin_kind.h | 7 ++-- toolchain/semantics/semantics_ir.cpp | 32 ++++---------- toolchain/semantics/semantics_node.h | 4 +- 5 files changed, 51 insertions(+), 37 deletions(-) diff --git a/toolchain/semantics/semantics_builtin_kind.cpp b/toolchain/semantics/semantics_builtin_kind.cpp index 4828912684fd..100015b7359e 100644 --- a/toolchain/semantics/semantics_builtin_kind.cpp +++ b/toolchain/semantics/semantics_builtin_kind.cpp @@ -7,7 +7,8 @@ namespace Carbon { CARBON_DEFINE_ENUM_CLASS_NAMES(SemanticsBuiltinKind) = { -#define CARBON_SEMANTICS_BUILTIN_KIND(Name) CARBON_ENUM_CLASS_NAME_STRING(Name) +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \ + CARBON_ENUM_CLASS_NAME_STRING(Name) #include "toolchain/semantics/semantics_builtin_kind.def" }; diff --git a/toolchain/semantics/semantics_builtin_kind.def b/toolchain/semantics/semantics_builtin_kind.def index e45848ddd885..ebe0c9c96e49 100644 --- a/toolchain/semantics/semantics_builtin_kind.def +++ b/toolchain/semantics/semantics_builtin_kind.def @@ -6,30 +6,56 @@ // // It does not use `#include` guards, and instead is designed to be `#include`ed // after the x-macro is defined in order for its inclusion to expand to the -// desired output. The x-macro for this header is `CARBON_PARSE_NODE_KIND`. The -// definition provided will be removed at the end of this file to clean up. +// desired output. +// +// x-macros come in two forms: +// CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) +// Used as a fallback if other macros are missing. +// CARBON_SEMANTICS_BUILTIN_KIND(Name, Type) +// Defines a builtin kind with the associated type, which must also be +// builtin. +// +// Note that Invalid is provided with CARBON_SEMANTICS_BUILTIN_KIND_NAME but not +// CARBON_SEMANTICS_BUILTIN_KIND. +#if !(defined(CARBON_SEMANTICS_BUILTIN_KIND_NAME) || \ + defined(CARBON_SEMANTICS_BUILTIN_KIND)) +#error \ + "Must define CARBON_SEMANTICS_BUILTIN_KIND family x-macros to use this file." +#endif + +// If CARBON_SEMANTICS_BUILTIN_KIND_NAME is undefined, ignore calls. +#ifndef CARBON_SEMANTICS_BUILTIN_KIND_NAME +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) +#endif + +// If CARBON_SEMANTICS_BUILTIN_KIND is undefined, delegate calls to +// CARBON_SEMANTICS_BUILTIN_KIND_NAME. #ifndef CARBON_SEMANTICS_BUILTIN_KIND -#error "Must define the x-macro to use this file." +#define CARBON_SEMANTICS_BUILTIN_KIND(Name, ...) \ + CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) #endif // Tracks expressions which are valid as types. -CARBON_SEMANTICS_BUILTIN_KIND(TypeType) +// This has a deliberately self-referential type. +CARBON_SEMANTICS_BUILTIN_KIND(TypeType, TypeType) // Used when a SemanticNode has an invalid type, which should then be ignored // for future type checking. -CARBON_SEMANTICS_BUILTIN_KIND(InvalidType) +// This has a deliberately self-referential type. +CARBON_SEMANTICS_BUILTIN_KIND(InvalidType, InvalidType) // The type of integers and integer literals, currently always i32. Long-term // we may not want it this way, but for now this is the approach. -CARBON_SEMANTICS_BUILTIN_KIND(IntegerType) +CARBON_SEMANTICS_BUILTIN_KIND(IntegerType, TypeType) // The type of reals and real literals, currently always f64. Long-term // we may not want it this way, but for now this is the approach. -CARBON_SEMANTICS_BUILTIN_KIND(RealType) +CARBON_SEMANTICS_BUILTIN_KIND(RealType, TypeType) // Keep invalid last, so that we can use values as array indices without needing // an invalid entry. -CARBON_SEMANTICS_BUILTIN_KIND(Invalid) +CARBON_SEMANTICS_BUILTIN_KIND_NAME(Invalid) +#undef CARBON_SEMANTICS_BUILTIN_KIND_NAME #undef CARBON_SEMANTICS_BUILTIN_KIND diff --git a/toolchain/semantics/semantics_builtin_kind.h b/toolchain/semantics/semantics_builtin_kind.h index c48287736a1f..53a9d919c64b 100644 --- a/toolchain/semantics/semantics_builtin_kind.h +++ b/toolchain/semantics/semantics_builtin_kind.h @@ -12,13 +12,14 @@ namespace Carbon { CARBON_DEFINE_RAW_ENUM_CLASS(SemanticsBuiltinKind, uint8_t) { -#define CARBON_SEMANTICS_BUILTIN_KIND(Name) CARBON_RAW_ENUM_ENUMERATOR(Name) +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \ + CARBON_RAW_ENUM_ENUMERATOR(Name) #include "toolchain/semantics/semantics_builtin_kind.def" }; class SemanticsBuiltinKind : public CARBON_ENUM_BASE(SemanticsBuiltinKind) { public: -#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \ +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \ CARBON_ENUM_CONSTANT_DECLARATION(Name) #include "toolchain/semantics/semantics_builtin_kind.def" @@ -34,7 +35,7 @@ class SemanticsBuiltinKind : public CARBON_ENUM_BASE(SemanticsBuiltinKind) { using EnumBase::FromInt; }; -#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \ +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \ CARBON_ENUM_CONSTANT_DEFINITION(SemanticsBuiltinKind, Name) #include "toolchain/semantics/semantics_builtin_kind.def" diff --git a/toolchain/semantics/semantics_ir.cpp b/toolchain/semantics/semantics_ir.cpp index c99d95880a1a..3485de59e5d7 100644 --- a/toolchain/semantics/semantics_ir.cpp +++ b/toolchain/semantics/semantics_ir.cpp @@ -14,33 +14,19 @@ namespace Carbon { auto SemanticsIR::MakeBuiltinIR() -> SemanticsIR { SemanticsIR semantics(/*builtin_ir=*/nullptr); - auto block_id = semantics.AddNodeBlock(); semantics.nodes_.reserve(SemanticsBuiltinKind::ValidCount); - constexpr int32_t TypeOfTypeType = 0; - auto type_type = semantics.AddNode( - block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::TypeType, - SemanticsNodeId(TypeOfTypeType))); - CARBON_CHECK(type_type.index == TypeOfTypeType) - << "TypeType's type must be self-referential."; +#define CARBON_SEMANTICS_BUILTIN_KIND(Name, Type) \ + semantics.nodes_.push_back(SemanticsNode::MakeBuiltin( \ + SemanticsBuiltinKind::Name, SemanticsNodeId::Builtin##Type)); +#include "toolchain/semantics/semantics_builtin_kind.def" - constexpr int32_t TypeOfInvalidType = 1; - auto invalid_type = semantics.AddNode( - block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::InvalidType, - SemanticsNodeId(TypeOfInvalidType))); - CARBON_CHECK(invalid_type.index == TypeOfInvalidType) - << "InvalidType's type must be self-referential."; - - semantics.AddNode( - block_id, - SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::IntegerType, type_type)); - - semantics.AddNode(block_id, SemanticsNode::MakeBuiltin( - SemanticsBuiltinKind::RealType, type_type)); - - CARBON_CHECK(semantics.node_blocks_.size() == 2) - << "BuildBuiltins should produce 2 blocks, actual: " + CARBON_CHECK(semantics.node_blocks_.size() == 1) + << "BuildBuiltins should only have the empty block, actual: " << semantics.node_blocks_.size(); + CARBON_CHECK(semantics.nodes_.size() == SemanticsBuiltinKind::ValidCount) + << "BuildBuiltins should produce " << SemanticsBuiltinKind::ValidCount + << " nodes, actual: " << semantics.nodes_.size(); return semantics; } diff --git a/toolchain/semantics/semantics_node.h b/toolchain/semantics/semantics_node.h index a366985e5b99..1e8fdf0721cf 100644 --- a/toolchain/semantics/semantics_node.h +++ b/toolchain/semantics/semantics_node.h @@ -22,7 +22,7 @@ struct SemanticsNodeId : public IndexBase { static const SemanticsNodeId Invalid; // Builtin node IDs. -#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \ +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \ static const SemanticsNodeId Builtin##Name; #include "toolchain/semantics/semantics_builtin_kind.def" @@ -37,7 +37,7 @@ 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) \ +#define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \ constexpr SemanticsNodeId SemanticsNodeId::Builtin##Name = \ SemanticsNodeId(SemanticsBuiltinKind::Name.AsInt()); #include "toolchain/semantics/semantics_builtin_kind.def"