Track params in the parser

toolchain/check/handle_alias.cpp

@@ -16,11 +16,6 @@ auto HandleParseNode(Context& context, Parse::AliasIntroducerId /*node_id*/)
     -> bool {
   context.decl_introducer_state_stack().Push<Lex::TokenKind::Alias>();
   context.decl_name_stack().PushScopeAndStartName();
-
-  // Push a pattern block to handle parameters of the alias declaration.
-  // TODO: Disallow these in parse, instead of check, so we don't have to do
-  // this.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/handle_class.cpp

@@ -42,10 +42,6 @@ auto HandleParseNode(Context& context, Parse::ClassIntroducerId node_id)
   context.decl_name_stack().PushScopeAndStartName();
   // This class is potentially generic.
   StartGenericDecl(context);
-  // Push a pattern block for the signature (if any) of the first NameComponent.
-  // TODO: Instead use a separate parse node kind for an identifier that's
-  // followed by a pattern, and push a pattern block when handling it.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/handle_export.cpp

@@ -18,9 +18,6 @@ auto HandleParseNode(Context& context, Parse::ExportIntroducerId /*node_id*/)
   context.decl_introducer_state_stack().Push<Lex::TokenKind::Export>();
   // TODO: Probably need to update DeclNameStack to restrict to only namespaces.
   context.decl_name_stack().PushScopeAndStartName();
-  // The parser supports patterns after `export`, so we need a pattern block
-  // to handle them.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/handle_function.cpp

@@ -36,8 +36,6 @@ auto HandleParseNode(Context& context, Parse::FunctionIntroducerId node_id)
   context.decl_name_stack().PushScopeAndStartName();
   // The function is potentially generic.
   StartGenericDecl(context);
-  // Start a new pattern block for the signature.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/handle_interface.cpp

@@ -25,10 +25,6 @@ auto HandleParseNode(Context& context, Parse::InterfaceIntroducerId node_id)
   context.decl_name_stack().PushScopeAndStartName();
   // This interface is potentially generic.
   StartGenericDecl(context);
-  // Push a pattern block for the signature (if any) of the first NameComponent.
-  // TODO: Instead use a separate parse node kind for an identifier that's
-  // followed by a pattern, and push a pattern block when handling it.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/handle_name.cpp

@@ -123,7 +123,8 @@ static auto HandleNameAsExpr(Context& context, Parse::NodeId node_id,
       {.type_id = type_id, .name_id = name_id, .value_id = result.inst_id});
 }
 
-auto HandleParseNode(Context& context, Parse::IdentifierNameId node_id)
+static auto HandleIdentifierName(Context& context,
+                                 Parse::AnyNonExprIdentifierNameId node_id)
     -> bool {
   // The parent is responsible for binding the name.
   auto name_id = GetIdentifierAsName(context, node_id);
@@ -133,6 +134,18 @@ auto HandleParseNode(Context& context, Parse::IdentifierNameId node_id)
   return true;
 }
 
+auto HandleParseNode(Context& context,
+                     Parse::IdentifierNameNotBeforeParamsId node_id) -> bool {
+  return HandleIdentifierName(context, node_id);
+}
+
+auto HandleParseNode(Context& context,
+                     Parse::IdentifierNameBeforeParamsId node_id) -> bool {
+  // Push a pattern block stack entry to handle the parameter pattern.
+  context.pattern_block_stack().Push();
+  return HandleIdentifierName(context, node_id);
+}
+
 auto HandleParseNode(Context& context, Parse::IdentifierNameExprId node_id)
     -> bool {
   auto name_id = GetIdentifierAsName(context, node_id);
@@ -172,13 +185,15 @@ auto HandleParseNode(Context& context, Parse::SelfValueNameExprId node_id)
   return true;
 }
 
-auto HandleParseNode(Context& context, Parse::NameQualifierId /*node_id*/)
-    -> bool {
+auto HandleParseNode(Context& context,
+                     Parse::NameQualifierWithParamsId /*node_id*/) -> bool {
+  context.decl_name_stack().ApplyNameQualifier(PopNameComponent(context));
+  return true;
+}
+
+auto HandleParseNode(Context& context,
+                     Parse::NameQualifierWithoutParamsId /*node_id*/) -> bool {
   context.decl_name_stack().ApplyNameQualifier(PopNameComponent(context));
-  // Push a pattern block for the signature (if any) of the next NameComponent.
-  // TODO: Instead use a separate parse node kind for an identifier that's
-  // followed by a pattern, and push a pattern block when handling it.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/handle_namespace.cpp

@@ -18,11 +18,6 @@ auto HandleParseNode(Context& context, Parse::NamespaceStartId /*node_id*/)
   // Optional modifiers and the name follow.
   context.decl_introducer_state_stack().Push<Lex::TokenKind::Namespace>();
   context.decl_name_stack().PushScopeAndStartName();
-
-  // Push a pattern block to handle parameters of the namespace declaration.
-  // TODO: Disallow these in parse, instead of check, so we don't have to do
-  // this.
-  context.pattern_block_stack().Push();
   return true;
 }
 

toolchain/check/inst_block_stack.h

@@ -54,7 +54,7 @@ class InstBlockStack {
 
   // Adds the given instruction ID to the block at the top of the stack.
   auto AddInstId(SemIR::InstId inst_id) -> void {
-    CARBON_CHECK(!empty(), "no current block");
+    CARBON_CHECK(!empty(), "{0} has no current block", name_);
     insts_stack_.AppendToTop(inst_id);
   }
 
@@ -74,7 +74,7 @@ class InstBlockStack {
 
   // Runs verification that the processing cleanly finished.
   auto VerifyOnFinish() const -> void {
-    CARBON_CHECK(empty(), "{0}", id_stack_.size());
+    CARBON_CHECK(empty(), "{0} still has {1} entries", name_, id_stack_.size());
   }
 
   auto empty() const -> bool { return id_stack_.empty(); }

toolchain/check/name_component.cpp

@@ -11,56 +11,65 @@ namespace Carbon::Check {
 
 auto PopNameComponent(Context& context, SemIR::InstId return_slot_pattern_id)
     -> NameComponent {
-  Parse::NodeId first_param_node_id = Parse::InvalidNodeId();
-  Parse::NodeId last_param_node_id = Parse::InvalidNodeId();
+  NameComponent name_component = {
+      .name_loc_id = Parse::NodeId::Invalid,
+      .name_id = SemIR::NameId::Invalid,
+      .first_param_node_id = Parse::NodeId::Invalid,
+      .last_param_node_id = Parse::NodeId::Invalid,
+      .implicit_params_loc_id = Parse::NodeId::Invalid,
+      .implicit_param_patterns_id = SemIR::InstBlockId::Invalid,
+      .params_loc_id = Parse::NodeId::Invalid,
+      .param_patterns_id = SemIR::InstBlockId::Invalid,
+      .call_params_id = SemIR::InstBlockId::Invalid,
+      .return_slot_pattern_id = return_slot_pattern_id,
+      .pattern_block_id = SemIR::InstBlockId::Invalid,
+  };
 
   // Explicit params.
-  auto [params_loc_id, param_patterns_id] =
-      context.node_stack().PopWithNodeIdIf<Parse::NodeKind::TuplePattern>();
-  if (param_patterns_id) {
-    first_param_node_id =
+  if (auto [params_loc_id, param_patterns_id] =
+          context.node_stack().PopWithNodeIdIf<Parse::NodeKind::TuplePattern>();
+      param_patterns_id) {
+    name_component.first_param_node_id =
         context.node_stack()
             .PopForSoloNodeId<Parse::NodeKind::TuplePatternStart>();
-    last_param_node_id = params_loc_id;
-  } else {
-    param_patterns_id = SemIR::InstBlockId::Invalid;
+    name_component.last_param_node_id = params_loc_id;
+    name_component.params_loc_id = params_loc_id;
+    name_component.param_patterns_id = *param_patterns_id;
   }
 
   // Implicit params.
-  auto [implicit_params_loc_id, implicit_param_patterns_id] =
-      context.node_stack()
-          .PopWithNodeIdIf<Parse::NodeKind::ImplicitParamList>();
-  if (implicit_param_patterns_id) {
+  if (auto [implicit_params_loc_id, implicit_param_patterns_id] =
+          context.node_stack()
+              .PopWithNodeIdIf<Parse::NodeKind::ImplicitParamList>();
+      implicit_param_patterns_id) {
     // Implicit params always come before explicit params.
-    first_param_node_id =
+    name_component.first_param_node_id =
         context.node_stack()
             .PopForSoloNodeId<Parse::NodeKind::ImplicitParamListStart>();
     // Only use the end of implicit params if there weren't explicit params.
-    if (last_param_node_id.is_valid()) {
-      last_param_node_id = params_loc_id;
+    if (!name_component.last_param_node_id.is_valid()) {
+      name_component.last_param_node_id = implicit_params_loc_id;
     }
-  } else {
-    implicit_param_patterns_id = SemIR::InstBlockId::Invalid;
+    name_component.implicit_params_loc_id = implicit_params_loc_id;
+    name_component.implicit_param_patterns_id = *implicit_param_patterns_id;
   }
 
-  auto call_params_id =
-      CalleePatternMatch(context, *implicit_param_patterns_id,
-                         *param_patterns_id, return_slot_pattern_id);
+  if (name_component.param_patterns_id.is_valid() ||
+      name_component.implicit_param_patterns_id.is_valid()) {
+    std::tie(name_component.name_loc_id, name_component.name_id) =
+        context.node_stack()
+            .PopWithNodeId<Parse::NodeKind::IdentifierNameBeforeParams>();
+    name_component.call_params_id = CalleePatternMatch(
+        context, name_component.implicit_param_patterns_id,
+        name_component.param_patterns_id, return_slot_pattern_id);
+    name_component.pattern_block_id = context.pattern_block_stack().Pop();
+  } else {
+    std::tie(name_component.name_loc_id, name_component.name_id) =
+        context.node_stack()
+            .PopWithNodeId<Parse::NodeKind::IdentifierNameNotBeforeParams>();
+  }
 
-  auto [name_loc_id, name_id] = context.node_stack().PopNameWithNodeId();
-  return {
-      .name_loc_id = name_loc_id,
-      .name_id = name_id,
-      .first_param_node_id = first_param_node_id,
-      .last_param_node_id = last_param_node_id,
-      .implicit_params_loc_id = implicit_params_loc_id,
-      .implicit_param_patterns_id = *implicit_param_patterns_id,
-      .params_loc_id = params_loc_id,
-      .param_patterns_id = *param_patterns_id,
-      .call_params_id = call_params_id,
-      .return_slot_pattern_id = return_slot_pattern_id,
-      .pattern_block_id = context.pattern_block_stack().Pop(),
-  };
+  return name_component;
 }
 
 // Pop the name of a declaration from the node stack, and diagnose if it has

toolchain/check/name_component.h

@@ -49,13 +49,14 @@ struct NameComponent {
   SemIR::InstBlockId pattern_block_id;
 };
 
-// Pop a name component from the node stack and pattern block stack.
+// Pops a name component from the node stack (and pattern block stack, if it has
+// parameters).
 auto PopNameComponent(Context& context, SemIR::InstId return_slot_pattern_id =
                                             SemIR::InstId::Invalid)
     -> NameComponent;
 
-// Pop the name of a declaration from the node stack and pattern block stack,
-// and diagnose if it has parameters.
+// Equivalent to PopNameComponent, but also diagnoses if the name component has
+// parameters.
 auto PopNameComponentWithoutParams(Context& context, Lex::TokenKind introducer)
     -> NameComponent;
 

toolchain/check/node_stack.h

@@ -521,7 +521,8 @@ class NodeStack {
         case Parse::NodeKind::ForStatement:
         case Parse::NodeKind::FunctionDecl:
         case Parse::NodeKind::FunctionDefinition:
-        case Parse::NodeKind::IdentifierName:
+        case Parse::NodeKind::IdentifierNameNotBeforeParams:
+        case Parse::NodeKind::IdentifierNameBeforeParams:
         case Parse::NodeKind::IdentifierNameExpr:
         case Parse::NodeKind::IfConditionStart:
         case Parse::NodeKind::IfExprElse:
@@ -594,7 +595,8 @@ class NodeStack {
         case Parse::NodeKind::NamedConstraintDefinition:
         case Parse::NodeKind::NamedConstraintDefinitionStart:
         case Parse::NodeKind::NamedConstraintIntroducer:
-        case Parse::NodeKind::NameQualifier:
+        case Parse::NodeKind::NameQualifierWithParams:
+        case Parse::NodeKind::NameQualifierWithoutParams:
         case Parse::NodeKind::Namespace:
         case Parse::NodeKind::NamespaceStart:
         case Parse::NodeKind::PackageDecl:

toolchain/language_server/handle_document_symbol.cpp

@@ -13,19 +13,25 @@
 
 namespace Carbon::LanguageServer {
 
-// Returns the text of first child of kind Parse::NodeKind::IdentifierName.
+// Returns the text of first child of kind IdentifierNameBeforeParams or
+// IdentifierNameNotBeforeParams.
 static auto GetIdentifierName(const SharedValueStores& value_stores,
                               const Lex::TokenizedBuffer& tokens,
                               const Parse::TreeAndSubtrees& tree_and_subtrees,
                               Parse::NodeId node)
     -> std::optional<llvm::StringRef> {
   for (auto child : tree_and_subtrees.children(node)) {
-    if (tree_and_subtrees.tree().node_kind(child) ==
-        Parse::NodeKind::IdentifierName) {
-      auto token = tree_and_subtrees.tree().node_token(child);
-      if (tokens.GetKind(token) == Lex::TokenKind::Identifier) {
-        return value_stores.identifiers().Get(tokens.GetIdentifier(token));
+    switch (tree_and_subtrees.tree().node_kind(child)) {
+      case Parse::NodeKind::IdentifierNameBeforeParams:
+      case Parse::NodeKind::IdentifierNameNotBeforeParams: {
+        auto token = tree_and_subtrees.tree().node_token(child);
+        if (tokens.GetKind(token) == Lex::TokenKind::Identifier) {
+          return value_stores.identifiers().Get(tokens.GetIdentifier(token));
+        }
+        break;
       }
+      default:
+        break;
     }
   }
   return std::nullopt;

toolchain/parse/handle_binding_pattern.cpp

@@ -40,7 +40,7 @@ auto HandleBindingPattern(Context& context) -> void {
   // The first item should be an identifier or `self`.
   bool has_name = false;
   if (auto identifier = context.ConsumeIf(Lex::TokenKind::Identifier)) {
-    context.AddLeafNode(NodeKind::IdentifierName, *identifier);
+    context.AddLeafNode(NodeKind::IdentifierNameNotBeforeParams, *identifier);
     has_name = true;
   } else if (auto self =
                  context.ConsumeIf(Lex::TokenKind::SelfValueIdentifier)) {
@@ -51,7 +51,8 @@ auto HandleBindingPattern(Context& context) -> void {
   }
   if (!has_name) {
     // Add a placeholder for the name.
-    context.AddLeafNode(NodeKind::IdentifierName, *context.position(),
+    context.AddLeafNode(NodeKind::IdentifierNameNotBeforeParams,
+                        *context.position(),
                         /*has_error=*/true);
     on_error(/*expected_name=*/true);
   }

toolchain/parse/handle_choice.cpp

@@ -50,8 +50,8 @@ auto HandleChoiceAlternative(Context& context) -> void {
 
   context.PushState(State::ChoiceAlternativeFinish);
 
-  if (!context.ConsumeAndAddLeafNodeIf(Lex::TokenKind::Identifier,
-                                       NodeKind::IdentifierName)) {
+  auto token = context.ConsumeIf(Lex::TokenKind::Identifier);
+  if (!token) {
     if (!state.has_error) {
       CARBON_DIAGNOSTIC(ExpectedChoiceAlternativeName, Error,
                         "expected choice alternative name");
@@ -67,7 +67,10 @@ auto HandleChoiceAlternative(Context& context) -> void {
   }
 
   if (context.PositionIs(Lex::TokenKind::OpenParen)) {
+    context.AddLeafNode(NodeKind::IdentifierNameBeforeParams, *token);
     context.PushState(State::PatternListAsTuple);
+  } else {
+    context.AddLeafNode(NodeKind::IdentifierNameNotBeforeParams, *token);
   }
 }
 

toolchain/parse/handle_decl_name_and_params.cpp

@@ -34,29 +34,31 @@ auto HandleDeclNameAndParams(Context& context) -> void {
     return;
   }
 
-  context.AddLeafNode(NodeKind::IdentifierName, *identifier);
-
   switch (context.PositionKind()) {
     case Lex::TokenKind::Period:
-      context.AddNode(NodeKind::NameQualifier,
+      context.AddLeafNode(NodeKind::IdentifierNameNotBeforeParams, *identifier);
+      context.AddNode(NodeKind::NameQualifierWithoutParams,
                       context.ConsumeChecked(Lex::TokenKind::Period),
                       state.has_error);
       context.PushState(State::DeclNameAndParams);
       break;
 
     case Lex::TokenKind::OpenSquareBracket:
+      context.AddLeafNode(NodeKind::IdentifierNameBeforeParams, *identifier);
       state.state = State::DeclNameAndParamsAfterImplicit;
       context.PushState(state);
       context.PushState(State::PatternListAsImplicit);
       break;
 
     case Lex::TokenKind::OpenParen:
+      context.AddLeafNode(NodeKind::IdentifierNameBeforeParams, *identifier);
       state.state = State::DeclNameAndParamsAfterParams;
       context.PushState(state);
       context.PushState(State::PatternListAsTuple);
       break;
 
     default:
+      context.AddLeafNode(NodeKind::IdentifierNameNotBeforeParams, *identifier);
       break;
   }
 }
@@ -82,7 +84,8 @@ auto HandleDeclNameAndParamsAfterParams(Context& context) -> void {
   auto state = context.PopState();
 
   if (auto period = context.ConsumeIf(Lex::TokenKind::Period)) {
-    context.AddNode(NodeKind::NameQualifier, *period, state.has_error);
+    context.AddNode(NodeKind::NameQualifierWithParams, *period,
+                    state.has_error);
     context.PushState(State::DeclNameAndParams);
   }
 }