Require that InvalidParse nodes must have an error (#4482)

Unifies some boilerplate AddLeafNode calls for InvalidParse, and checks
has_error in AddNode.

toolchain/parse/context.cpp

@@ -39,13 +39,14 @@ Context::Context(Tree& tree, Lex::TokenizedBuffer& tokens,
 auto Context::ReplacePlaceholderNode(int32_t position, NodeKind kind,
                                      Lex::TokenIndex token, bool has_error)
     -> void {
+  CARBON_CHECK(
+      kind != NodeKind::InvalidParse && kind != NodeKind::InvalidParseSubtree,
+      "{0} shouldn't occur in Placeholder use-cases", kind);
   CARBON_CHECK(position >= 0 && position < tree_->size(),
                "position: {0} size: {1}", position, tree_->size());
   auto* node_impl = &tree_->node_impls_[position];
   CARBON_CHECK(node_impl->kind == NodeKind::Placeholder);
-  node_impl->kind = kind;
-  node_impl->has_error = has_error;
-  node_impl->token = token;
+  *node_impl = {.kind = kind, .has_error = has_error, .token = token};
 }
 
 auto Context::ConsumeAndAddOpenParen(Lex::TokenIndex default_token,

toolchain/parse/context.h

@@ -98,12 +98,22 @@ class Context {
   // Adds a node to the parse tree that has no children (a leaf).
   auto AddLeafNode(NodeKind kind, Lex::TokenIndex token, bool has_error = false)
       -> void {
-    tree_->node_impls_.push_back(Tree::NodeImpl(kind, has_error, token));
+    AddNode(kind, token, has_error);
   }
 
   // Adds a node to the parse tree that has children.
   auto AddNode(NodeKind kind, Lex::TokenIndex token, bool has_error) -> void {
-    tree_->node_impls_.push_back(Tree::NodeImpl(kind, has_error, token));
+    CARBON_CHECK(has_error || (kind != NodeKind::InvalidParse &&
+                               kind != NodeKind::InvalidParseStart &&
+                               kind != NodeKind::InvalidParseSubtree),
+                 "{0} nodes must always have an error", kind);
+    tree_->node_impls_.push_back(
+        {.kind = kind, .has_error = has_error, .token = token});
+  }
+
+  // Adds an invalid parse node.
+  auto AddInvalidParse(Lex::TokenIndex token) -> void {
+    AddNode(NodeKind::InvalidParse, token, /*has_error=*/true);
   }
 
   // Replaces the placeholder node at the indicated position with a leaf node.

toolchain/parse/handle_binding_pattern.cpp

@@ -67,9 +67,8 @@ auto HandleBindingPattern(Context& context) -> void {
     context.PushStateForExpr(PrecedenceGroup::ForType());
   } else {
     on_error(/*expected_name=*/false);
-    // Add a placeholder for the type.
-    context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                        /*has_error=*/true);
+    // Add a substitute for a type node.
+    context.AddInvalidParse(*context.position());
     context.PushState(state, State::BindingPatternFinishAsRegular);
   }
 }

toolchain/parse/handle_decl_name_and_params.cpp

@@ -30,8 +30,7 @@ auto HandleDeclNameAndParams(Context& context) -> void {
                              context.tokens().GetKind(state.token));
     }
     context.ReturnErrorOnState();
-    context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                        /*has_error=*/true);
+    context.AddInvalidParse(*context.position());
     return;
   }
 

toolchain/parse/handle_expr.cpp

@@ -190,11 +190,9 @@ auto HandleExprInPostfix(Context& context) -> void {
           context.AddLeafNode(NodeKind::IdentifierName, context.Consume(),
                               /*has_error=*/true);
         } else if (context.PositionIs(Lex::TokenKind::IntLiteral)) {
-          context.AddLeafNode(NodeKind::InvalidParse, context.Consume(),
-                              /*has_error=*/true);
+          context.AddInvalidParse(context.Consume());
         } else {
-          context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                              /*has_error=*/true);
+          context.AddInvalidParse(*context.position());
           // Indicate the error to the parent state so that it can avoid
           // producing more errors. We only do this on this path where we don't
           // consume the token after the period, where we expect further errors
@@ -215,8 +213,7 @@ auto HandleExprInPostfix(Context& context) -> void {
       }
 
       // Add a node to keep the parse tree balanced.
-      context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                          /*has_error=*/true);
+      context.AddInvalidParse(*context.position());
       context.ReturnErrorOnState();
       break;
     }

toolchain/parse/handle_if_expr.cpp

@@ -24,11 +24,9 @@ auto HandleIfExprFinishCondition(Context& context) -> void {
     if (!state.has_error) {
       context.emitter().Emit(*context.position(), ExpectedThenAfterIf);
     }
-    // Add placeholders for `IfExprThen` and final `Expr`.
-    context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                        /*has_error=*/true);
-    context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                        /*has_error=*/true);
+    // Add invalid nodes to substitute for `IfExprThen` and the final `Expr`.
+    context.AddInvalidParse(*context.position());
+    context.AddInvalidParse(*context.position());
     context.ReturnErrorOnState();
   }
 }
@@ -49,9 +47,8 @@ auto HandleIfExprFinishThen(Context& context) -> void {
     if (!state.has_error) {
       context.emitter().Emit(*context.position(), ExpectedElseAfterIf);
     }
-    // Add placeholder for the final `Expr`.
-    context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                        /*has_error=*/true);
+    // Add an invalid node to substitute for the final `Expr`.
+    context.AddInvalidParse(*context.position());
     context.ReturnErrorOnState();
   }
 }

toolchain/parse/handle_impl.cpp

@@ -38,8 +38,7 @@ auto HandleImplAfterIntroducer(Context& context) -> void {
       // If we aren't producing a node from the PatternListAsImplicit state,
       // we still need to create a node to be the child of the `ImplForall`
       // token created in the `ImplAfterForall` state.
-      context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                          /*has_error=*/true);
+      context.AddInvalidParse(*context.position());
     }
   } else {
     // One of:

toolchain/parse/handle_match.cpp

@@ -176,8 +176,7 @@ auto HandleMatchCaseAfterPattern(Context& context) -> void {
 
       context.AddLeafNode(NodeKind::MatchCaseGuardStart, *context.position(),
                           true);
-      context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                          /*has_error=*/true);
+      context.AddInvalidParse(*context.position());
       state = context.PopState();
       context.AddNode(NodeKind::MatchCaseGuard, *context.position(),
                       /*has_error=*/true);

toolchain/parse/handle_paren_condition.cpp

@@ -23,8 +23,7 @@ static auto HandleParenCondition(Context& context, NodeKind start_kind,
     // For an open curly, assume the condition was completely omitted.
     // Expression parsing would treat the { as a struct, but instead assume it's
     // a code block and just emit an invalid parse.
-    context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
-                        /*has_error=*/true);
+    context.AddInvalidParse(*context.position());
   } else {
     context.PushState(State::Expr);
   }

toolchain/parse/tree.h

@@ -199,9 +199,6 @@ class Tree : public Printable<Tree> {
   // The in-memory representation of data used for a particular node in the
   // tree.
   struct NodeImpl {
-    explicit NodeImpl(NodeKind kind, bool has_error, Lex::TokenIndex token)
-        : kind(kind), has_error(has_error), token(token) {}
-
     // The kind of this node. Note that this is only a single byte.
     NodeKind kind;
 
@@ -221,7 +218,7 @@ class Tree : public Printable<Tree> {
     // optional (and will depend on the particular parse implementation
     // strategy). The goal is that you can rely on grammar-based structural
     // invariants *until* you encounter a node with this set.
-    bool has_error = false;
+    bool has_error;
 
     // The token root of this node.
     Lex::TokenIndex token;