Fix `NodeCategory` printing. (#4983)

Rearrange `NodeCategory` printing so we get a compile-time error for
missing switch cases if it's missing any categories. Add several missing
categories.

In passing, fix some minor things in the `NodeCategory` class
definition, and fix an overly-permissive typed node.

toolchain/parse/node_kind.cpp

@@ -10,25 +10,34 @@
 namespace Carbon::Parse {
 
 auto NodeCategory::Print(llvm::raw_ostream& out) const -> void {
-  if (!value_) {
-    out << "<none>";
-  } else {
-    llvm::ListSeparator sep("|");
-
-#define CARBON_NODE_CATEGORY(Name)   \
-  if (value_ & NodeCategory::Name) { \
-    out << sep << #Name;             \
+  llvm::ListSeparator sep("|");
+  auto value = value_;
+  do {
+    // The lowest set bit in the value, or 0 (`None`) if no bits are set.
+    auto lowest_bit = static_cast<RawEnumType>(value & -value);
+    switch (lowest_bit) {
+#define CARBON_NODE_CATEGORY(Name) \
+  case NodeCategory::Name: {       \
+    out << sep << #Name;           \
+    break;                         \
   }
-    CARBON_NODE_CATEGORY(Decl);
-    CARBON_NODE_CATEGORY(Expr);
-    CARBON_NODE_CATEGORY(ImplAs);
-    CARBON_NODE_CATEGORY(MemberExpr);
-    CARBON_NODE_CATEGORY(MemberName);
-    CARBON_NODE_CATEGORY(Modifier);
-    CARBON_NODE_CATEGORY(Pattern);
-    CARBON_NODE_CATEGORY(Statement);
+      CARBON_NODE_CATEGORY(Decl);
+      CARBON_NODE_CATEGORY(Expr);
+      CARBON_NODE_CATEGORY(ImplAs);
+      CARBON_NODE_CATEGORY(MemberExpr);
+      CARBON_NODE_CATEGORY(MemberName);
+      CARBON_NODE_CATEGORY(Modifier);
+      CARBON_NODE_CATEGORY(Pattern);
+      CARBON_NODE_CATEGORY(Statement);
+      CARBON_NODE_CATEGORY(IntConst);
+      CARBON_NODE_CATEGORY(Requirement);
+      CARBON_NODE_CATEGORY(NonExprIdentifierName);
+      CARBON_NODE_CATEGORY(PackageName);
+      CARBON_NODE_CATEGORY(None);
 #undef CARBON_NODE_CATEGORY
-  }
+    }
+    value &= ~lowest_bit;
+  } while (value);
 }
 
 CARBON_DEFINE_ENUM_CLASS_NAMES(NodeKind) = {

toolchain/parse/node_kind.h

@@ -37,6 +37,7 @@ class NodeCategory : public Printable<NodeCategory> {
     Requirement = 1 << 9,
     NonExprIdentifierName = 1 << 10,
     PackageName = 1 << 11,
+    // If you add a new category here, also add it to the Print function.
     None = 0,
 
     LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/PackageName)
@@ -48,15 +49,14 @@ class NodeCategory : public Printable<NodeCategory> {
   constexpr NodeCategory(RawEnumType value) : value_(value) {}
 
   // Returns true if there's a non-empty set intersection.
-  constexpr auto HasAnyOf(NodeCategory other) -> bool {
+  constexpr auto HasAnyOf(NodeCategory other) const -> bool {
     return value_ & other.value_;
   }
 
   // Returns the set inverse.
-  constexpr auto operator~() -> NodeCategory { return ~value_; }
+  constexpr auto operator~() const -> NodeCategory { return ~value_; }
 
-  friend auto operator==(const NodeCategory& lhs, const NodeCategory& rhs)
-      -> bool {
+  friend auto operator==(NodeCategory lhs, NodeCategory rhs) -> bool {
     return lhs.value_ == rhs.value_;
   }
 

toolchain/parse/typed_nodes.h

@@ -236,7 +236,7 @@ struct PackageDecl {
 
   PackageIntroducerId introducer;
   llvm::SmallVector<AnyModifierId> modifiers;
-  std::optional<AnyPackageNameId> name;
+  AnyPackageNameId name;
   std::optional<LibrarySpecifierId> library;
   Lex::SemiTokenIndex token;
 };

toolchain/parse/typed_nodes_test.cpp

@@ -138,6 +138,34 @@ TEST_F(TypedNodeTest, For) {
   ASSERT_TRUE(for_var_name.has_value());
 }
 
+TEST_F(TypedNodeTest, VerifyExtractTracePackage) {
+  auto& tree = compile_helper_.GetTreeAndSubtrees(R"carbon(
+    impl package Banana;
+  )carbon");
+  auto file = tree.ExtractFile();
+
+  ASSERT_EQ(file.decls.size(), 1);
+  ErrorBuilder trace;
+  auto library =
+      Peer::VerifyExtractAs<PackageDecl>(tree, file.decls[0], &trace);
+  EXPECT_TRUE(library.has_value());
+  Error err = trace;
+  // Use Regex matching to avoid hard-coding the result of `typeinfo(T).name()`.
+  EXPECT_THAT(err.message(), testing::MatchesRegex(
+                                 R"Trace(Aggregate [^:]*: begin
+Optional [^:]*: begin
+NodeIdForKind error: wrong kind IdentifierPackageName, expected LibrarySpecifier
+Optional [^:]*: missing
+NodeIdInCategory PackageName: kind IdentifierPackageName consumed
+Vector: begin
+NodeIdInCategory Modifier: kind ImplModifier consumed
+NodeIdInCategory Modifier error: kind PackageIntroducer doesn't match
+Vector: end
+NodeIdForKind: PackageIntroducer consumed
+Aggregate [^:]*: success
+)Trace"));
+}
+
 TEST_F(TypedNodeTest, VerifyExtractTraceLibrary) {
   auto& tree = compile_helper_.GetTreeAndSubtrees(R"carbon(
     impl library default;
@@ -234,7 +262,7 @@ Aggregate [^:]*: success
   // Use Regex matching to avoid hard-coding the result of `typeinfo(T).name()`.
   EXPECT_THAT(err2.message(), testing::MatchesRegex(
                                   R"Trace(Aggregate [^:]*: begin
-NodeIdInCategory MemberExpr\|MemberName: kind IdentifierNameNotBeforeParams consumed
+NodeIdInCategory MemberExpr\|MemberName\|IntConst: kind IdentifierNameNotBeforeParams consumed
 NodeIdInCategory Expr: kind PointerMemberAccessExpr consumed
 Aggregate [^:]*: success
 )Trace"));
@@ -262,7 +290,7 @@ Optional [^:]*: found
 Optional [^:]*: begin
 NodeIdForKind error: wrong kind IdentifierNameBeforeParams, expected ImplicitParamList
 Optional [^:]*: missing
-NodeIdInCategory : kind IdentifierNameBeforeParams consumed
+NodeIdInCategory NonExprIdentifierName: kind IdentifierNameBeforeParams consumed
 Vector: begin
 NodeIdOneOf NameQualifierWithParams or NameQualifierWithoutParams: NameQualifierWithoutParams consumed
 NodeIdOneOf error: wrong kind AbstractModifier, expected NameQualifierWithParams or NameQualifierWithoutParams