Reduce macro use for node kinds to get flexibility (#2354)

As I'm looking at adding builtins, it feels a bit like the types of args will end up scaling close to the number of semantic kinds. Rather than having this lead to a bunch of macros, this approach drops the macros for plain code.

Note, this adds Get functions too -- I'd planned those regardless for type safety, this is just getting ahead of the issue so that the Print function is pretty clean.

toolchain/semantics/semantics_node.cpp

@@ -7,41 +7,24 @@
 namespace Carbon {
 
 static auto PrintArgs(llvm::raw_ostream& /*out*/,
-                      const SemanticsNodeArgs::None /*no_args*/) {}
+                      const SemanticsNode::NoArgs /*no_args*/) {}
 
-static auto PrintArgs(llvm::raw_ostream& out, SemanticsNodeId one_node) {
-  out << one_node;
+template <typename T>
+static auto PrintArgs(llvm::raw_ostream& out, T arg) {
+  out << arg;
 }
 
-static auto PrintArgs(llvm::raw_ostream& out, SemanticsTwoNodeIds two_nodes) {
-  out << two_nodes.nodes[0] << ", " << two_nodes.nodes[1];
-}
-
-static auto PrintArgs(llvm::raw_ostream& out,
-                      SemanticsIdentifierId identifier) {
-  out << identifier;
-}
-
-static auto PrintArgs(llvm::raw_ostream& out,
-                      SemanticsIntegerLiteralId integer_literal) {
-  out << integer_literal;
-}
-
-static auto PrintArgs(llvm::raw_ostream& out, SemanticsNodeBlockId node_block) {
-  out << node_block;
-}
-
-static auto PrintArgs(llvm::raw_ostream& out,
-                      SemanticsNodeIdAndNodeBlockId node_and_node_block) {
-  out << node_and_node_block.node << ", " << node_and_node_block.node_block;
+template <typename T0, typename T1>
+static auto PrintArgs(llvm::raw_ostream& out, std::pair<T0, T1> args) {
+  out << args.first << ", " << args.second;
 }
 
 void SemanticsNode::Print(llvm::raw_ostream& out) const {
   out << kind_ << "(";
   switch (kind_) {
-#define CARBON_SEMANTICS_NODE_KIND(Name, Args) \
-  case SemanticsNodeKind::Name():              \
-    PrintArgs(out, one_of_args_.Args);         \
+#define CARBON_SEMANTICS_NODE_KIND(Name) \
+  case SemanticsNodeKind::Name():        \
+    PrintArgs(out, GetAs##Name());       \
     break;
 #include "toolchain/semantics/semantics_node_kind.def"
   }

toolchain/semantics/semantics_node.h

@@ -7,6 +7,7 @@
 
 #include <cstdint>
 
+#include "common/check.h"
 #include "common/ostream.h"
 #include "toolchain/semantics/semantics_node_kind.h"
 
@@ -54,123 +55,108 @@ struct SemanticsNodeBlockId {
   int32_t id;
 };
 
-struct SemanticsTwoNodeIds {
-  SemanticsNodeId nodes[2];
-};
-struct SemanticsNodeIdAndNodeBlockId {
-  SemanticsNodeId node;
-  SemanticsNodeBlockId node_block;
-};
-
-union SemanticsNodeArgs {
-  struct None {};
-
-  SemanticsNodeArgs() : no_args() {}
-  explicit SemanticsNodeArgs(SemanticsNodeId one_node) : one_node(one_node) {}
-  explicit SemanticsNodeArgs(SemanticsTwoNodeIds two_nodes)
-      : two_nodes(two_nodes) {}
-
-  explicit SemanticsNodeArgs(SemanticsIdentifierId identifier)
-      : identifier(identifier) {}
-  explicit SemanticsNodeArgs(SemanticsIntegerLiteralId integer_literal)
-      : integer_literal(integer_literal) {}
-  explicit SemanticsNodeArgs(SemanticsNodeBlockId node_block)
-      : node_block(node_block) {}
-  explicit SemanticsNodeArgs(SemanticsNodeIdAndNodeBlockId node_and_node_block)
-      : node_and_node_block(node_and_node_block) {}
-
-  None no_args;
-  SemanticsNodeId one_node;
-  SemanticsTwoNodeIds two_nodes;
-
-  SemanticsIdentifierId identifier;
-  SemanticsIntegerLiteralId integer_literal;
-  SemanticsNodeBlockId node_block;
-  SemanticsNodeIdAndNodeBlockId node_and_node_block;
-};
-// TODO: This is currently 8 bytes only because of two_nodes; others are only 4
-// bytes. The NodeKind is 1 byte; if we reduced this structure to 7 bytes (3.5
-// bytes per node), we could potentially change SemanticsNode from 12 bytes to 8
-// bytes. This may be worth investigating further.
-static_assert(sizeof(SemanticsNodeArgs) == 8, "Unexpected OneOfArgs size");
-
 // The standard structure for nodes.
 class SemanticsNode {
  public:
-  // Define factory functions for each node kind. These should improve type
-  // safety by enforcing argument counts.
-  // `clang-format` has a bug with spacing around `->` returns here. See
-  // https://bugs.llvm.org/show_bug.cgi?id=48320 for details.
-#define CARBON_SEMANTICS_MAKE_no_args(Name)                               \
-  static auto Make##Name()->SemanticsNode {                               \
-    return SemanticsNode(SemanticsNodeKind::Name(), SemanticsNodeArgs()); \
-  }
-#define CARBON_SEMANTICS_MAKE_one_node(Name)                        \
-  static auto Make##Name(SemanticsNodeId one_node)->SemanticsNode { \
-    return SemanticsNode(SemanticsNodeKind::Name(),                 \
-                         SemanticsNodeArgs(one_node));              \
-  }
-#define CARBON_SEMANTICS_MAKE_two_nodes(Name)                          \
-  static auto Make##Name(SemanticsNodeId node1, SemanticsNodeId node2) \
-      ->SemanticsNode {                                                \
-    return SemanticsNode(                                              \
-        SemanticsNodeKind::Name(),                                     \
-        SemanticsNodeArgs(SemanticsTwoNodeIds{node1, node2}));         \
-  }
-
-#define CARBON_SEMANTICS_MAKE_identifier(Name)                              \
-  static auto Make##Name(SemanticsIdentifierId identifier)->SemanticsNode { \
-    return SemanticsNode(SemanticsNodeKind::Name(),                         \
-                         SemanticsNodeArgs(identifier));                    \
-  }
-#define CARBON_SEMANTICS_MAKE_integer_literal(Name)                 \
-  static auto Make##Name(SemanticsIntegerLiteralId integer_literal) \
-      ->SemanticsNode {                                             \
-    return SemanticsNode(SemanticsNodeKind::Name(),                 \
-                         SemanticsNodeArgs(integer_literal));       \
-  }
-#define CARBON_SEMANTICS_MAKE_node_block(Name)                             \
-  static auto Make##Name(SemanticsNodeBlockId node_block)->SemanticsNode { \
-    return SemanticsNode(SemanticsNodeKind::Name(),                        \
-                         SemanticsNodeArgs(node_block));                   \
-  }
-#define CARBON_SEMANTICS_MAKE_node_and_node_block(Name)                      \
-  static auto Make##Name(SemanticsNodeId node,                               \
-                         SemanticsNodeBlockId node_block)                    \
-      ->SemanticsNode {                                                      \
-    return SemanticsNode(                                                    \
-        SemanticsNodeKind::Name(),                                           \
-        SemanticsNodeArgs(SemanticsNodeIdAndNodeBlockId{node, node_block})); \
-  }
-
-#define CARBON_SEMANTICS_NODE_KIND(Name, ArgsType) \
-  CARBON_SEMANTICS_MAKE_##ArgsType(Name)
-#include "toolchain/semantics/semantics_node_kind.def"
-
-#undef CARBON_SEMANTICS_MAKE_no_args
-#undef CARBON_SEMANTICS_MAKE_one_node
-#undef CARBON_SEMANTICS_MAKE_two_nodes
-
-#undef CARBON_SEMANTICS_MAKE_identifier
-#undef CARBON_SEMANTICS_MAKE_integer_literal
-#undef CARBON_SEMANTICS_MAKE_node_block
-#undef CARBON_SEMANTICS_MAKE_node_and_node_block
-
-  SemanticsNode() : kind_(SemanticsNodeKind::Invalid()) {}
+  struct NoArgs {};
+
+  auto GetAsInvalid() const -> NoArgs { CARBON_FATAL() << "Invalid access"; }
+
+  static auto MakeBinaryOperatorAdd(SemanticsNodeId lhs, SemanticsNodeId rhs)
+      -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::BinaryOperatorAdd(), lhs.id,
+                         rhs.id);
+  }
+  auto GetAsBinaryOperatorAdd() const
+      -> std::pair<SemanticsNodeId, SemanticsNodeId> {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::BinaryOperatorAdd());
+    return {SemanticsNodeId(arg0_), SemanticsNodeId(arg1_)};
+  }
+
+  static auto MakeCodeBlock(SemanticsNodeBlockId node_block) -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::CodeBlock(), node_block.id);
+  }
+  auto GetAsCodeBlock() const -> SemanticsNodeBlockId {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::CodeBlock());
+    return SemanticsNodeBlockId(arg0_);
+  }
+
+  static auto MakeFunctionDeclaration(SemanticsNodeId name) -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::FunctionDeclaration(), name.id);
+  }
+  auto GetAsFunctionDeclaration() const -> SemanticsNodeId {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::FunctionDeclaration());
+    return SemanticsNodeId(arg0_);
+  }
+
+  static auto MakeFunctionDefinition(SemanticsNodeId decl,
+                                     SemanticsNodeBlockId node_block)
+      -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::FunctionDefinition(), decl.id,
+                         node_block.id);
+  }
+  auto GetAsFunctionDefinition() const
+      -> std::pair<SemanticsNodeId, SemanticsNodeBlockId> {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::FunctionDefinition());
+    return {SemanticsNodeId(arg0_), SemanticsNodeBlockId(arg1_)};
+  }
+
+  static auto MakeIdentifier(SemanticsIdentifierId identifier)
+      -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::Identifier(), identifier.id);
+  }
+  auto GetAsIdentifier() const -> SemanticsIdentifierId {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::Identifier());
+    return SemanticsIdentifierId(arg0_);
+  }
+
+  static auto MakeIntegerLiteral(SemanticsIntegerLiteralId integer)
+      -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::IntegerLiteral(), integer.id);
+  }
+  auto GetAsIntegerLiteral() const -> SemanticsIntegerLiteralId {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::IntegerLiteral());
+    return SemanticsIntegerLiteralId(arg0_);
+  }
+
+  static auto MakeReturn() -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::Return());
+  }
+  auto GetAsReturn() const -> NoArgs {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::Return());
+    return {};
+  }
+
+  static auto MakeReturnExpression(SemanticsNodeId expr) -> SemanticsNode {
+    return SemanticsNode(SemanticsNodeKind::ReturnExpression(), expr.id);
+  }
+  auto GetAsReturnExpression() const -> SemanticsNodeId {
+    CARBON_CHECK(kind_ == SemanticsNodeKind::ReturnExpression());
+    return SemanticsNodeId(arg0_);
+  }
+
+  SemanticsNode() : SemanticsNode(SemanticsNodeKind::Invalid()) {}
 
   auto kind() -> SemanticsNodeKind { return kind_; }
 
   void Print(llvm::raw_ostream& out) const;
 
  private:
-  SemanticsNode(SemanticsNodeKind kind, SemanticsNodeArgs one_of_args)
-      : kind_(kind), one_of_args_(one_of_args) {}
+  explicit SemanticsNode(SemanticsNodeKind kind, int32_t arg0 = -1,
+                         int32_t arg1 = -1)
+      : kind_(kind), arg0_(arg0), arg1_(arg1) {}
 
   SemanticsNodeKind kind_;
-
-  SemanticsNodeArgs one_of_args_;
+  int32_t arg0_;
+  int32_t arg1_;
 };
 
+// TODO: This is currently 12 bytes because we sometimes have 2 arguments for a
+// pair of SemanticsNodes. If SemanticsNode was tracked in 3.5 bytes, we could
+// potentially change SemanticsNode to 8 bytes. This may be worth investigating
+// further.
+static_assert(sizeof(SemanticsNode) == 12, "Unexpected SemanticsNode size");
+
 }  // namespace Carbon
 
 #endif  // CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_NODE_H_

toolchain/semantics/semantics_node_kind.def

@@ -13,32 +13,15 @@
 #error "Must define the x-macro to use this file."
 #endif
 
-// No args.
-CARBON_SEMANTICS_NODE_KIND(Invalid, no_args)
-
-// Two nodes: lhs and rhs.
-CARBON_SEMANTICS_NODE_KIND(BinaryOperatorAdd, two_nodes)
-
-// The code block.
-CARBON_SEMANTICS_NODE_KIND(CodeBlock, node_block)
-
-// One node: the name.
-// TODO: Add a declaration scope as a second arg.
-CARBON_SEMANTICS_NODE_KIND(FunctionDeclaration, one_node)
-
-// Two nodes: a FunctionDeclaration and its body.
-CARBON_SEMANTICS_NODE_KIND(FunctionDefinition, node_and_node_block)
-
-// The declared IdentifierId.
-CARBON_SEMANTICS_NODE_KIND(Identifier, identifier)
-
-// The declared IntegerLiteralId.
-CARBON_SEMANTICS_NODE_KIND(IntegerLiteral, integer_literal)
-
-// No args.
-CARBON_SEMANTICS_NODE_KIND(Return, no_args)
-
-// One node: the return expression.
-CARBON_SEMANTICS_NODE_KIND(ReturnExpression, one_node)
+CARBON_SEMANTICS_NODE_KIND(Invalid)
+
+CARBON_SEMANTICS_NODE_KIND(BinaryOperatorAdd)
+CARBON_SEMANTICS_NODE_KIND(CodeBlock)
+CARBON_SEMANTICS_NODE_KIND(FunctionDeclaration)
+CARBON_SEMANTICS_NODE_KIND(FunctionDefinition)
+CARBON_SEMANTICS_NODE_KIND(Identifier)
+CARBON_SEMANTICS_NODE_KIND(IntegerLiteral)
+CARBON_SEMANTICS_NODE_KIND(Return)
+CARBON_SEMANTICS_NODE_KIND(ReturnExpression)
 
 #undef CARBON_SEMANTICS_NODE_KIND