Refactor state construction and change how the decl loop makes state. (#3467)

Building on #3463. The PushState+PopState to construct a state feels
worth cleanup. The rest is just kind of making it easier to do without
adding another PushState overload.

toolchain/parse/context.h

@@ -52,15 +52,6 @@ class Context {
 
   // Used to track state on state_stack_.
   struct StateStackEntry : public Printable<StateStackEntry> {
-    explicit StateStackEntry(State state, PrecedenceGroup ambient_precedence,
-                             PrecedenceGroup lhs_precedence,
-                             Lex::TokenIndex token, int32_t subtree_start)
-        : state(state),
-          ambient_precedence(ambient_precedence),
-          lhs_precedence(lhs_precedence),
-          token(token),
-          subtree_start(subtree_start) {}
-
     // Prints state information for verbose output.
     auto Print(llvm::raw_ostream& output) const -> void {
       output << state << " @" << token << " subtree_start=" << subtree_start
@@ -77,8 +68,8 @@ class Context {
     // operator precedence. The ambient_precedence deals with how the expression
     // should interact with outside context, while the lhs_precedence is
     // specific to the lhs of an operator expression.
-    PrecedenceGroup ambient_precedence;
-    PrecedenceGroup lhs_precedence;
+    PrecedenceGroup ambient_precedence = PrecedenceGroup::ForTopLevelExpr();
+    PrecedenceGroup lhs_precedence = PrecedenceGroup::ForTopLevelExpr();
 
     // A token providing context based on the subtree. This will typically be
     // the first token in the subtree, but may sometimes be a token within. It
@@ -248,32 +239,30 @@ class Context {
   }
 
   // Pushes a new state with the current position for context.
-  auto PushState(State state) -> void {
-    PushState(StateStackEntry(state, PrecedenceGroup::ForTopLevelExpr(),
-                              PrecedenceGroup::ForTopLevelExpr(), *position_,
-                              tree_->size()));
-  }
+  auto PushState(State state) -> void { PushState(state, *position_); }
 
   // Pushes a new state with a specific token for context. Used when forming a
-  // new subtree with a token that isn't the start of the subtree.
+  // new subtree when the current position isn't the start of the subtree.
   auto PushState(State state, Lex::TokenIndex token) -> void {
-    PushState(StateStackEntry(state, PrecedenceGroup::ForTopLevelExpr(),
-                              PrecedenceGroup::ForTopLevelExpr(), token,
-                              tree_->size()));
+    PushState({.state = state, .token = token, .subtree_start = tree_->size()});
   }
 
   // Pushes a new expression state with specific precedence.
   auto PushStateForExpr(PrecedenceGroup ambient_precedence) -> void {
-    PushState(StateStackEntry(State::Expr, ambient_precedence,
-                              PrecedenceGroup::ForTopLevelExpr(), *position_,
-                              tree_->size()));
+    PushState({.state = State::Expr,
+               .ambient_precedence = ambient_precedence,
+               .token = *position_,
+               .subtree_start = tree_->size()});
   }
 
   // Pushes a new state with detailed precedence for expression resume states.
   auto PushStateForExprLoop(State state, PrecedenceGroup ambient_precedence,
                             PrecedenceGroup lhs_precedence) -> void {
-    PushState(StateStackEntry(state, ambient_precedence, lhs_precedence,
-                              *position_, tree_->size()));
+    PushState({.state = state,
+               .ambient_precedence = ambient_precedence,
+               .lhs_precedence = lhs_precedence,
+               .token = *position_,
+               .subtree_start = tree_->size()});
   }
 
   // Pushes a constructed state onto the stack.

toolchain/parse/handle_binding_pattern.cpp

@@ -12,15 +12,15 @@ auto HandleBindingPattern(Context& context) -> void {
   // Parameters may have keywords prefixing the pattern. They become the parent
   // for the full BindingPattern.
   if (auto token = context.ConsumeIf(Lex::TokenKind::Template)) {
-    context.PushState(Context::StateStackEntry(
-        State::BindingPatternTemplate, PrecedenceGroup::ForTopLevelExpr(),
-        PrecedenceGroup::ForTopLevelExpr(), *token, state.subtree_start));
+    context.PushState({.state = State::BindingPatternTemplate,
+                       .token = *token,
+                       .subtree_start = state.subtree_start});
   }
 
   if (auto token = context.ConsumeIf(Lex::TokenKind::Addr)) {
-    context.PushState(Context::StateStackEntry(
-        State::BindingPatternAddress, PrecedenceGroup::ForTopLevelExpr(),
-        PrecedenceGroup::ForTopLevelExpr(), *token, state.subtree_start));
+    context.PushState({.state = State::BindingPatternAddress,
+                       .token = *token,
+                       .subtree_start = state.subtree_start});
   }
 
   // Handle an invalid pattern introducer for parameters and variables.

toolchain/parse/handle_decl_scope_loop.cpp

@@ -77,6 +77,18 @@ static auto HandleUnrecognizedDecl(Context& context, int32_t subtree_start)
   FinishAndSkipInvalidDecl(context, subtree_start);
 }
 
+// Replaces the introducer placeholder node, and pushes the introducer state for
+// processing.
+static auto ApplyIntroducer(Context& context, Context::StateStackEntry state,
+                            NodeKind introducer_kind, State next_state)
+    -> void {
+  context.ReplacePlaceholderNode(state.subtree_start, introducer_kind,
+                                 context.Consume());
+  // Reuse state here to retain its `subtree_start`.
+  state.state = next_state;
+  context.PushState(state);
+}
+
 // Handles `base` as a declaration.
 static auto HandleBaseAsDecl(Context& context, Context::StateStackEntry state)
     -> void {
@@ -84,11 +96,7 @@ static auto HandleBaseAsDecl(Context& context, Context::StateStackEntry state)
   // it's followed by a colon, it's an introducer (`extend base: BaseType;`).
   // Otherwise it's an error.
   if (context.PositionIs(Lex::TokenKind::Colon, Lookahead::NextToken)) {
-    context.ReplacePlaceholderNode(state.subtree_start,
-                                   NodeKind::BaseIntroducer, context.Consume());
-    // Reuse state here to retain its `subtree_start`
-    state.state = State::BaseDecl;
-    context.PushState(state);
+    ApplyIntroducer(context, state, NodeKind::BaseIntroducer, State::BaseDecl);
     context.PushState(State::Expr);
     context.AddLeafNode(NodeKind::BaseColon, context.Consume());
   } else {
@@ -109,26 +117,19 @@ static auto HandleBaseAsDecl(Context& context, Context::StateStackEntry state)
 // to a state to parse the rest of the declaration.
 static auto TryHandleAsDecl(Context& context, Context::StateStackEntry state,
                             bool saw_modifier) -> bool {
-  auto introducer = [&](NodeKind node_kind, State next_state) {
-    context.ReplacePlaceholderNode(state.subtree_start, node_kind,
-                                   context.Consume());
-    // Reuse state here to retain its `subtree_start`.
-    state.state = next_state;
-    context.PushState(state);
-  };
-
   switch (context.PositionKind()) {
     case Lex::TokenKind::Base: {
       HandleBaseAsDecl(context, state);
       return true;
     }
     case Lex::TokenKind::Class: {
-      introducer(NodeKind::ClassIntroducer, State::TypeAfterIntroducerAsClass);
+      ApplyIntroducer(context, state, NodeKind::ClassIntroducer,
+                      State::TypeAfterIntroducerAsClass);
       return true;
     }
     case Lex::TokenKind::Constraint: {
-      introducer(NodeKind::NamedConstraintIntroducer,
-                 State::TypeAfterIntroducerAsNamedConstraint);
+      ApplyIntroducer(context, state, NodeKind::NamedConstraintIntroducer,
+                      State::TypeAfterIntroducerAsNamedConstraint);
       return true;
     }
     case Lex::TokenKind::Extend: {
@@ -137,7 +138,8 @@ static auto TryHandleAsDecl(Context& context, Context::StateStackEntry state,
       return true;
     }
     case Lex::TokenKind::Fn: {
-      introducer(NodeKind::FunctionIntroducer, State::FunctionIntroducer);
+      ApplyIntroducer(context, state, NodeKind::FunctionIntroducer,
+                      State::FunctionIntroducer);
       return true;
     }
     case Lex::TokenKind::Impl: {
@@ -146,20 +148,22 @@ static auto TryHandleAsDecl(Context& context, Context::StateStackEntry state,
       return true;
     }
     case Lex::TokenKind::Interface: {
-      introducer(NodeKind::InterfaceIntroducer,
-                 State::TypeAfterIntroducerAsInterface);
+      ApplyIntroducer(context, state, NodeKind::InterfaceIntroducer,
+                      State::TypeAfterIntroducerAsInterface);
       return true;
     }
     case Lex::TokenKind::Namespace: {
-      introducer(NodeKind::NamespaceStart, State::Namespace);
+      ApplyIntroducer(context, state, NodeKind::NamespaceStart,
+                      State::Namespace);
       return true;
     }
     case Lex::TokenKind::Let: {
-      introducer(NodeKind::LetIntroducer, State::Let);
+      ApplyIntroducer(context, state, NodeKind::LetIntroducer, State::Let);
       return true;
     }
     case Lex::TokenKind::Var: {
-      introducer(NodeKind::VariableIntroducer, State::VarAsDecl);
+      ApplyIntroducer(context, state, NodeKind::VariableIntroducer,
+                      State::VarAsDecl);
       return true;
     }
 
@@ -243,10 +247,11 @@ auto HandleDeclScopeLoop(Context& context) -> void {
     return;
   }
 
-  // Create a state with the correct starting position, with a dummy kind until
-  // we see the declaration's introducer.
-  context.PushState(State::DeclScopeLoop);
-  auto state = context.PopState();
+  // Create a state with the correct starting position, with a dummy kind
+  // until we see the declaration's introducer.
+  Context::StateStackEntry state{.state = State::Invalid,
+                                 .token = *context.position(),
+                                 .subtree_start = context.tree().size()};
 
   // Add a placeholder node, to be replaced by the declaration introducer once
   // it is found.

toolchain/parse/state.def

@@ -53,6 +53,10 @@
   CARBON_PARSE_STATE_VARIANT(State, Variant1)                             \
   CARBON_PARSE_STATE_VARIANTS3(State, Variant2, Variant3, Variant4)
 
+// Used as a default for StateStackEntry initialization in some cases. Should
+// not be put on the state stack.
+CARBON_PARSE_STATE(Invalid)
+
 // Handles an index expression:
 //
 // a[0]
@@ -1039,7 +1043,8 @@ CARBON_PARSE_STATE_VARIANTS3(TypeDefinitionFinish, Class, Interface,
 //                           ^
 //   1. DeclNameAndParamsAsOptional
 //   2. TypeAfterParamsAs(Class|Interface|NamedConstraint)
-CARBON_PARSE_STATE_VARIANTS3(TypeAfterIntroducer, Class, Interface, NamedConstraint)
+CARBON_PARSE_STATE_VARIANTS3(TypeAfterIntroducer, Class, Interface,
+                             NamedConstraint)
 
 // Handles processing of a type after its optional parameters.
 //

toolchain/parse/tree.cpp

@@ -15,6 +15,11 @@
 
 namespace Carbon::Parse {
 
+auto HandleInvalid(Context& context) -> void {
+  CARBON_FATAL() << "The Invalid state shouldn't be on the stack: "
+                 << context.PopState();
+}
+
 auto Tree::Parse(Lex::TokenizedBuffer& tokens, DiagnosticConsumer& consumer,
                  llvm::raw_ostream* vlog_stream) -> Tree {
   Lex::TokenLocationTranslator translator(&tokens);