Adding more function support to the Parser2 rewrite (#2375)

Also does `if` support, discussed refactorings, like PopState/PushState instead of edits.

With these changes, I'm to where I can start talking about what's still missing:

```
//toolchain/parser/testdata:basics/fail_invalid_designators.carbon.test  FAILED in 0.7s
//toolchain/parser/testdata:basics/fail_paren_match_regression.carbon.test FAILED in 0.6s
//toolchain/parser/testdata:basics/function_call.carbon.test             FAILED in 0.7s
//toolchain/parser/testdata:basics/package.carbon.test                   FAILED in 0.7s
//toolchain/parser/testdata:basics/structs.carbon.test                   FAILED in 0.7s
//toolchain/parser/testdata:basics/tuples.carbon.test                    FAILED in 0.6s
//toolchain/parser/testdata:basics/var.carbon.test                       FAILED in 0.7s
//toolchain/parser/testdata:for/fail_colon_instead_of_in.carbon.test     FAILED in 0.6s
//toolchain/parser/testdata:for/fail_missing_in.carbon.test              FAILED in 0.6s
//toolchain/parser/testdata:for/fail_missing_var.carbon.test             FAILED in 0.7s
//toolchain/parser/testdata:for/nested.carbon.test                       FAILED in 0.6s
//toolchain/parser/testdata:for/simple.carbon.test                       FAILED in 0.6s
//toolchain/parser/testdata:function/definition/with_params.carbon.test  FAILED in 0.7s
//toolchain/parser/testdata:operators/associative.carbon.test            FAILED in 0.6s
//toolchain/parser/testdata:operators/fail_missing_precedence_and_or.carbon.test FAILED in 0.6s
//toolchain/parser/testdata:operators/fail_missing_precedence_or_and.carbon.test FAILED in 0.6s
//toolchain/parser/testdata:operators/fail_variety.carbon.test           FAILED in 0.6s
//toolchain/parser/testdata:operators/fixity.carbon.test                 FAILED in 0.9s
//toolchain/parser/testdata:operators/missing_precedence_not.carbon.test FAILED in 0.6s
//toolchain/parser/testdata:operators/postfix_unary.carbon.test          FAILED in 0.7s
//toolchain/parser/testdata:operators/prefix_unary.carbon.test           FAILED in 0.7s
//toolchain/parser/testdata:while/basic.carbon.test                      FAILED in 0.6s
//toolchain/parser/testdata:while/fail_unbraced.carbon.test              FAILED in 0.6s
```

This does modify a couple `if` tests to not test so much expression syntax -- that just seems like unrelated syntax.

toolchain/parser/BUILD

@@ -17,6 +17,18 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "parse_node_kind_test",
+    size = "small",
+    srcs = ["parse_node_kind_test.cpp"],
+    deps = [
+        ":parse_node_kind",
+        "//common:gtest_main",
+        "@com_google_googletest//:gtest",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
 cc_library(
     name = "parser_state",
     srcs = ["parser_state.cpp"],
@@ -29,11 +41,11 @@ cc_library(
 )
 
 cc_test(
-    name = "parse_node_kind_test",
+    name = "parser_state_test",
     size = "small",
-    srcs = ["parse_node_kind_test.cpp"],
+    srcs = ["parser_state_test.cpp"],
     deps = [
-        ":parse_node_kind",
+        ":parser_state",
         "//common:gtest_main",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Support",

toolchain/parser/parser2.cpp

@@ -27,13 +27,23 @@ class Parser2::PrettyStackTraceParseState : public llvm::PrettyStackTraceEntry {
     output << "Parser stack:\n";
     for (int i = 0; i < static_cast<int>(parser_->state_stack_.size()); ++i) {
       const auto& entry = parser_->state_stack_[i];
-      output << "\t" << i << ".\t" << entry.state << " @ " << entry.start_token
-             << ":" << parser_->tokens_.GetKind(entry.start_token).Name()
-             << "\n";
+      output << "\t" << i << ".\t" << entry.state;
+      Print(output, entry.token);
     }
+    output << "\tabort\tposition_";
+    Print(output, *parser_->position_);
   }
 
  private:
+  auto Print(llvm::raw_ostream& output, TokenizedBuffer::Token token) const
+      -> void {
+    auto line = parser_->tokens_.GetLine(token);
+    output << " @ " << parser_->tokens_.GetLineNumber(line) << ":"
+           << parser_->tokens_.GetColumnNumber(token) << ":"
+           << " token " << token << " : "
+           << parser_->tokens_.GetKind(token).Name() << "\n";
+  }
+
   const Parser2* parser_;
 };
 
@@ -70,6 +80,22 @@ auto Parser2::AddNode(ParseNodeKind kind, TokenizedBuffer::Token token,
   }
 }
 
+auto Parser2::ConsumeAndAddCloseParen(TokenizedBuffer::Token open_paren,
+                                      ParseNodeKind close_kind) -> bool {
+  if (ConsumeAndAddLeafNodeIf(TokenKind::CloseParen(), close_kind)) {
+    return true;
+  }
+
+  // TODO: Include the location of the matching open_paren in the diagnostic.
+  CARBON_DIAGNOSTIC(ExpectedCloseParen, Error, "Unexpected tokens before `)`.");
+  emitter_.Emit(*position_, ExpectedCloseParen);
+
+  SkipTo(tokens_.GetMatchedClosingToken(open_paren));
+  AddLeafNode(close_kind, *position_);
+  ++position_;
+  return false;
+}
+
 auto Parser2::ConsumeAndAddLeafNodeIf(TokenKind token_kind,
                                       ParseNodeKind node_kind) -> bool {
   auto token = ConsumeIf(token_kind);
@@ -91,10 +117,34 @@ auto Parser2::ConsumeIf(TokenKind kind)
   return token;
 }
 
+auto Parser2::FindNextOf(std::initializer_list<TokenKind> desired_kinds)
+    -> llvm::Optional<TokenizedBuffer::Token> {
+  auto new_position = position_;
+  while (true) {
+    TokenizedBuffer::Token token = *new_position;
+    TokenKind kind = tokens_.GetKind(token);
+    if (kind.IsOneOf(desired_kinds)) {
+      return token;
+    }
+
+    // Step to the next token at the current bracketing level.
+    if (kind.IsClosingSymbol() || kind == TokenKind::EndOfFile()) {
+      // There are no more tokens at this level.
+      return llvm::None;
+    } else if (kind.IsOpeningSymbol()) {
+      new_position =
+          TokenizedBuffer::TokenIterator(tokens_.GetMatchedClosingToken(token));
+      // Advance past the closing token.
+      ++new_position;
+    } else {
+      ++new_position;
+    }
+  }
+}
+
 auto Parser2::Parse() -> void {
-#ifndef NDEBUG
+  // Traces state_stack_. This runs even in opt because it's low overhead.
   PrettyStackTraceParseState pretty_stack(this);
-#endif
 
   PushState(ParserState::Declaration());
   while (!state_stack_.empty()) {
@@ -122,7 +172,9 @@ auto Parser2::SkipMatchingGroup() -> bool {
 
 auto Parser2::SkipPastLikelyEnd(TokenizedBuffer::Token skip_root)
     -> llvm::Optional<TokenizedBuffer::Token> {
-  CARBON_CHECK(position_ < end_);
+  if (position_ == end_) {
+    return llvm::None;
+  }
 
   TokenizedBuffer::Line root_line = tokens_.GetLine(skip_root);
   int root_line_indent = tokens_.GetIndentColumnNumber(root_line);
@@ -166,56 +218,160 @@ auto Parser2::SkipPastLikelyEnd(TokenizedBuffer::Token skip_root)
 }
 
 auto Parser2::SkipTo(TokenizedBuffer::Token t) -> void {
-  CARBON_CHECK(t > *position_) << "Tried to skip backwards.";
+  CARBON_CHECK(t >= *position_) << "Tried to skip backwards from " << position_
+                                << " to " << TokenizedBuffer::TokenIterator(t);
   position_ = TokenizedBuffer::TokenIterator(t);
   CARBON_CHECK(position_ != end_) << "Skipped past EOF.";
 }
 
-auto Parser2::HandleDeclarationState() -> void {
-  do {
-    switch (auto token_kind = PositionKind()) {
-      case TokenKind::EndOfFile(): {
-        state_stack_.pop_back();
-        return;
-      }
-      case TokenKind::Fn(): {
-        PushState(ParserState::FunctionIntroducer());
-        AddLeafNode(ParseNodeKind::FunctionIntroducer(), *position_);
-        ++position_;
-        return;
-      }
-      case TokenKind::Semi(): {
-        AddLeafNode(ParseNodeKind::EmptyDeclaration(), *position_);
-        ++position_;
-        break;
-      }
-      default: {
-        CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error,
-                          "Unrecognized declaration introducer.");
-        emitter_.Emit(*position_, UnrecognizedDeclaration);
-        tree_.has_errors_ = true;
-        if (auto semi = SkipPastLikelyEnd(*position_)) {
-          AddLeafNode(ParseNodeKind::EmptyDeclaration(), *semi,
-                      /*has_error=*/true);
-        }
-        break;
-      }
-    }
-  } while (position_ < end_);
+auto Parser2::HandleCodeBlock() -> void {
+  PushState(ParserState::CodeBlockFinish());
+  if (ConsumeAndAddLeafNodeIf(TokenKind::OpenCurlyBrace(),
+                              ParseNodeKind::CodeBlockStart())) {
+    PushState(ParserState::StatementScope());
+  } else {
+    AddLeafNode(ParseNodeKind::CodeBlockStart(), *position_,
+                /*has_error=*/true);
+
+    // Recover by parsing a single statement.
+    CARBON_DIAGNOSTIC(ExpectedCodeBlock, Error, "Expected braced code block.");
+    emitter_.Emit(*position_, ExpectedCodeBlock);
+
+    HandleStatement(PositionKind());
+  }
 }
 
-auto Parser2::HandleFunctionError(bool skip_past_likely_end) -> void {
-  auto token = state_stack_.back().start_token;
-  if (skip_past_likely_end && SkipPastLikelyEnd(token)) {
-    token = *position_;
+auto Parser2::HandleCodeBlockFinishState() -> void {
+  auto state = PopState();
+
+  // If the block started with an open curly, this is a close curly.
+  if (tokens_.GetKind(state.token) == TokenKind::OpenCurlyBrace()) {
+    AddNode(ParseNodeKind::CodeBlock(), *position_, state.subtree_start,
+            state.has_error);
+    ++position_;
+  } else {
+    AddNode(ParseNodeKind::CodeBlock(), state.token, state.subtree_start,
+            /*has_error=*/true);
   }
-  AddNode(ParseNodeKind::FunctionDeclaration(), token,
-          state_stack_.back().subtree_start,
+}
+
+auto Parser2::HandleDeclarationState() -> void {
+  // This maintains the current state unless we're at the end of the file.
+
+  switch (PositionKind()) {
+    case TokenKind::EndOfFile(): {
+      PopAndDiscardState();
+      break;
+    }
+    case TokenKind::Fn(): {
+      PushState(ParserState::FunctionIntroducer());
+      AddLeafNode(ParseNodeKind::FunctionIntroducer(), *position_);
+      ++position_;
+      break;
+    }
+    case TokenKind::Semi(): {
+      AddLeafNode(ParseNodeKind::EmptyDeclaration(), *position_);
+      ++position_;
+      break;
+    }
+    default: {
+      CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error,
+                        "Unrecognized declaration introducer.");
+      emitter_.Emit(*position_, UnrecognizedDeclaration);
+      tree_.has_errors_ = true;
+      if (auto semi = SkipPastLikelyEnd(*position_)) {
+        AddLeafNode(ParseNodeKind::EmptyDeclaration(), *semi,
+                    /*has_error=*/true);
+      }
+      break;
+    }
+  }
+}
+
+auto Parser2::HandleExpressionFormPrimary() -> void {
+  // TODO: Handle OpenParen and OpenCurlyBrace.
+  switch (PositionKind()) {
+    case TokenKind::Identifier():
+      AddLeafNode(ParseNodeKind::NameReference(), *position_);
+      break;
+
+    case TokenKind::IntegerLiteral():
+    case TokenKind::RealLiteral():
+    case TokenKind::StringLiteral():
+    case TokenKind::IntegerTypeLiteral():
+    case TokenKind::UnsignedIntegerTypeLiteral():
+    case TokenKind::FloatingPointTypeLiteral():
+      AddLeafNode(ParseNodeKind::Literal(), *position_);
+      break;
+
+    default:
+      CARBON_DIAGNOSTIC(ExpectedExpression, Error, "Expected expression.");
+      emitter_.Emit(*position_, ExpectedExpression);
+      ReturnErrorOnState();
+      return;
+  }
+  ++position_;
+}
+
+auto Parser2::HandleExpressionState() -> void {
+  // TODO: This is temporary, we should need this state. If not, maybe add an
+  // overload that uses pop_back instead of pop_back_val.
+  auto state = PopState();
+  (void)state;
+
+  HandleExpressionFormPrimary();
+}
+
+auto Parser2::HandleExpressionForTypeState() -> void {
+  // TODO: This is temporary, we should need this state. If not, maybe add an
+  // overload that uses pop_back instead of pop_back_val.
+  auto state = PopState();
+  (void)state;
+
+  HandleExpressionFormPrimary();
+}
+
+auto Parser2::HandleExpressionStatementFinishState() -> void {
+  auto state = PopState();
+
+  if (auto semi = ConsumeIf(TokenKind::Semi())) {
+    AddNode(ParseNodeKind::ExpressionStatement(), *semi, state.subtree_start,
+            state.has_error);
+    return;
+  }
+
+  if (!state.has_error) {
+    CARBON_DIAGNOSTIC(ExpectedSemiAfterExpression, Error,
+                      "Expected `;` after expression.");
+    emitter_.Emit(*position_, ExpectedSemiAfterExpression);
+  }
+
+  if (auto semi_token = SkipPastLikelyEnd(state.token)) {
+    AddNode(ParseNodeKind::ExpressionStatement(), *semi_token,
+            state.subtree_start,
+            /*has_error=*/true);
+    return;
+  }
+
+  // Found junk not even followed by a `;`, no node to add.
+  ReturnErrorOnState();
+}
+
+auto Parser2::HandleFunctionError(StateStackEntry state,
+                                  bool skip_past_likely_end) -> void {
+  auto token = state.token;
+  if (skip_past_likely_end) {
+    if (auto semi = SkipPastLikelyEnd(token)) {
+      token = *semi;
+    }
+  }
+  AddNode(ParseNodeKind::FunctionDeclaration(), token, state.subtree_start,
           /*has_error=*/true);
-  state_stack_.pop_back();
 }
 
 auto Parser2::HandleFunctionIntroducerState() -> void {
+  auto state = PopState();
+
   if (!ConsumeAndAddLeafNodeIf(TokenKind::Identifier(),
                                ParseNodeKind::DeclaredName())) {
     CARBON_DIAGNOSTIC(ExpectedFunctionName, Error,
@@ -224,7 +380,7 @@ auto Parser2::HandleFunctionIntroducerState() -> void {
     // TODO: We could change the lexer to allow us to synthesize certain
     // kinds of tokens and try to "recover" here, but unclear that this is
     // really useful.
-    HandleFunctionError(true);
+    HandleFunctionError(state, true);
     return;
   }
 
@@ -232,57 +388,77 @@ auto Parser2::HandleFunctionIntroducerState() -> void {
     CARBON_DIAGNOSTIC(ExpectedFunctionParams, Error,
                       "Expected `(` after function name.");
     emitter_.Emit(*position_, ExpectedFunctionParams);
-    HandleFunctionError(true);
+    HandleFunctionError(state, true);
     return;
   }
 
   // Parse the parameter list as its own subtree; once that pops, resume
   // function parsing.
-  state_stack_.back().state = ParserState::FunctionParameterListDone();
-  PushState(ParserState::FunctionParameterList());
-  // Advance past the open parenthesis before continuing.
-  // TODO: When swapping () start/end, this should AddNode the open before
+  state.state = ParserState::FunctionAfterParameterList();
+  PushState(state);
+  // TODO: When swapping () start/end, this should AddLeafNode the open before
   // continuing.
+  PushState(ParserState::FunctionParameterListFinish());
+  // Advance past the open paren.
   ++position_;
-}
-
-auto Parser2::HandleFunctionParameterListState() -> void {
-  // TODO: Handle non-empty lists.
-  if (!PositionIs(TokenKind::CloseParen())) {
-    CARBON_DIAGNOSTIC(ExpectedFunctionParams, Error,
-                      "Expected `(` after function name.");
-    emitter_.Emit(*position_, ExpectedFunctionParams);
-    SkipTo(tokens_.GetMatchedClosingToken(state_stack_.back().start_token));
-    AddLeafNode(ParseNodeKind::ParameterListEnd(), *position_,
-                /*has_error=*/true);
-    AddNode(ParseNodeKind::ParameterList(), state_stack_.back().start_token,
-            state_stack_.back().subtree_start);
-    ++position_;
-    return;
+  if (PositionKind() != TokenKind::CloseParen()) {
+    PushState(ParserState::PatternForFunctionParameter());
   }
-  AddLeafNode(ParseNodeKind::ParameterListEnd(), *position_);
-  AddNode(ParseNodeKind::ParameterList(), state_stack_.back().start_token,
-          state_stack_.back().subtree_start);
-  ++position_;
-  state_stack_.pop_back();
 }
 
-auto Parser2::HandleFunctionParameterListDoneState() -> void {
-  switch (auto token_kind = PositionKind()) {
+auto Parser2::HandleFunctionParameterListFinishState() -> void {
+  auto state = PopState();
+
+  CARBON_CHECK(PositionKind() == TokenKind::CloseParen())
+      << PositionKind().Name();
+  AddLeafNode(ParseNodeKind::ParameterListEnd(), *position_);
+  AddNode(ParseNodeKind::ParameterList(), state.token, state.subtree_start,
+          state.has_error);
+  ++position_;
+}
+
+auto Parser2::HandleFunctionAfterParameterListState() -> void {
+  auto state = PopState();
+
+  // Regardless of whether there's a return type, we'll finish the signature.
+  state.state = ParserState::FunctionSignatureFinish();
+  PushState(state);
+
+  // If there is a return type, parse the expression before adding the return
+  // type nod.e
+  if (PositionIs(TokenKind::MinusGreater())) {
+    PushState(ParserState::FunctionReturnTypeFinish());
+    ++position_;
+    PushState(ParserState::ExpressionForType());
+  }
+}
+
+auto Parser2::HandleFunctionReturnTypeFinishState() -> void {
+  auto state = PopState();
+
+  AddNode(ParseNodeKind::ReturnType(), state.token, state.subtree_start,
+          state.has_error);
+}
+
+auto Parser2::HandleFunctionSignatureFinishState() -> void {
+  auto state = PopState();
+
+  switch (PositionKind()) {
     case TokenKind::Semi(): {
       AddNode(ParseNodeKind::FunctionDeclaration(), *position_,
-              state_stack_.back().subtree_start);
+              state.subtree_start, state.has_error);
       ++position_;
-      state_stack_.pop_back();
       break;
     }
-    // TODO: OpenCurlyBrace is a definition.
     case TokenKind::OpenCurlyBrace(): {
-      CARBON_DIAGNOSTIC(
-          ExpectedFunctionBodyOrSemi, Error,
-          "Expected function definition or `;` after function declaration.");
-      emitter_.Emit(*position_, ExpectedFunctionBodyOrSemi);
-      HandleFunctionError(true);
+      AddNode(ParseNodeKind::FunctionDefinitionStart(), *position_,
+              state.subtree_start, state.has_error);
+      ++position_;
+      // Any error is recorded on the FunctionDefinitionStart.
+      state.has_error = false;
+      state.state = ParserState::FunctionDefinitionFinish();
+      PushState(state);
+      PushState(ParserState::StatementScope());
       break;
     }
     default: {
@@ -291,11 +467,266 @@ auto Parser2::HandleFunctionParameterListDoneState() -> void {
           "Expected function definition or `;` after function declaration.");
       emitter_.Emit(*position_, ExpectedFunctionBodyOrSemi);
       // Only need to skip if we've not already found a new line.
-      HandleFunctionError(tokens_.GetLine(*position_) ==
-                          tokens_.GetLine(state_stack_.back().start_token));
+      bool skip_past_likely_end =
+          tokens_.GetLine(*position_) == tokens_.GetLine(state.token);
+      HandleFunctionError(state, skip_past_likely_end);
       break;
     }
   }
 }
 
+auto Parser2::HandleFunctionDefinitionFinishState() -> void {
+  auto state = PopState();
+  AddNode(ParseNodeKind::FunctionDefinition(), *position_, state.subtree_start,
+          state.has_error);
+  ++position_;
+}
+
+auto Parser2::HandlePatternStart(PatternKind pattern_kind) -> void {
+  auto state = PopState();
+
+  // Ensure the finish state always follows.
+  switch (pattern_kind) {
+    case PatternKind::Parameter: {
+      state.state = ParserState::PatternForFunctionParameterFinish();
+      break;
+    }
+    case PatternKind::Variable: {
+      CARBON_FATAL() << "TODO";
+      break;
+    }
+  }
+
+  // Handle an invalid pattern introducer.
+  if (!PositionIs(TokenKind::Identifier()) ||
+      tokens_.GetKind(*(position_ + 1)) != TokenKind::Colon()) {
+    switch (pattern_kind) {
+      case PatternKind::Parameter: {
+        CARBON_DIAGNOSTIC(ExpectedParameterName, Error,
+                          "Expected parameter declaration.");
+        emitter_.Emit(*position_, ExpectedParameterName);
+        break;
+      }
+      case PatternKind::Variable: {
+        CARBON_DIAGNOSTIC(ExpectedVariableName, Error,
+                          "Expected pattern in `var` declaration.");
+        emitter_.Emit(*position_, ExpectedVariableName);
+        break;
+      }
+    }
+    state.has_error = true;
+    PushState(state);
+    return;
+  }
+
+  // Switch the context token to the colon, so that it'll be used for the root
+  // node.
+  state.token = *(position_ + 1);
+  PushState(state);
+  PushState(ParserState::ExpressionForType());
+  AddLeafNode(ParseNodeKind::DeclaredName(), *position_);
+  position_ += 2;
+}
+
+auto Parser2::HandleKeywordStatementFinish(TokenKind token_kind,
+                                           ParseNodeKind node_kind) -> void {
+  auto state = PopState();
+
+  auto semi =
+      ConsumeAndAddLeafNodeIf(TokenKind::Semi(), ParseNodeKind::StatementEnd());
+  if (!semi) {
+    CARBON_DIAGNOSTIC(ExpectedSemiAfter, Error, "Expected `;` after `{0}`.",
+                      TokenKind);
+    emitter_.Emit(*position_, ExpectedSemiAfter, token_kind);
+    if (auto semi_token = SkipPastLikelyEnd(state.token)) {
+      AddLeafNode(ParseNodeKind::StatementEnd(), *semi_token,
+                  /*has_error=*/true);
+    }
+  }
+  AddNode(node_kind, state.token, state.subtree_start, state.has_error);
+}
+
+auto Parser2::HandleKeywordStatementFinishForReturnState() -> void {
+  HandleKeywordStatementFinish(TokenKind::Return(),
+                               ParseNodeKind::ReturnStatement());
+}
+
+auto Parser2::HandleParenConditionState() -> void {
+  auto state = PopState();
+
+  auto open_paren = ConsumeIf(TokenKind::OpenParen());
+  if (open_paren) {
+    state.token = *open_paren;
+  } else {
+    CARBON_DIAGNOSTIC(ExpectedParenAfter, Error, "Expected `(` after `{0}`.",
+                      TokenKind);
+    emitter_.Emit(*position_, ExpectedParenAfter, tokens_.GetKind(state.token));
+  }
+
+  // TODO: This should be adding a ConditionStart here instead of ConditionEnd
+  // later, so this does state modification instead of a simpler push.
+  state.state = ParserState::ParenConditionFinish();
+  PushState(state);
+  PushState(ParserState::Expression());
+}
+
+auto Parser2::HandleParenConditionFinishState() -> void {
+  auto state = PopState();
+
+  if (tokens_.GetKind(state.token) != TokenKind::OpenParen()) {
+    // Don't expect a matching closing paren if there wasn't an opening paren.
+    // TODO: Should probably push nodes on this state in order to have the
+    // condition wrapped, but it wasn't before, so not doing it for consistency.
+    ReturnErrorOnState();
+    return;
+  }
+
+  bool close_paren =
+      ConsumeAndAddCloseParen(state.token, ParseNodeKind::ConditionEnd());
+
+  return AddNode(ParseNodeKind::Condition(), state.token, state.subtree_start,
+                 /*has_error=*/state.has_error || !close_paren);
+}
+
+auto Parser2::HandlePatternForFunctionParameterState() -> void {
+  HandlePatternStart(PatternKind::Parameter);
+}
+
+auto Parser2::HandlePatternForFunctionParameterFinishState() -> void {
+  auto state = PopState();
+
+  // If an error was encountered, propagate it without adding a node.
+  bool has_error = state.has_error;
+  if (has_error) {
+    ReturnErrorOnState();
+  } else {
+    // TODO: may need to mark has_error if !type.
+    AddNode(ParseNodeKind::PatternBinding(), state.token, state.subtree_start,
+            state.has_error);
+  }
+
+  // Handle tokens following a parameter.
+  switch (PositionKind()) {
+    case TokenKind::CloseParen(): {
+      // Done with the parameter list.
+      return;
+    }
+    case TokenKind::Comma(): {
+      // Comma handling is after the switch.
+      break;
+    }
+    default: {
+      // Don't error twice for the same issue.
+      if (!has_error) {
+        CARBON_DIAGNOSTIC(UnexpectedTokenAfterListElement, Error,
+                          "Expected `,` or `)`.");
+        emitter_.Emit(*position_, UnexpectedTokenAfterListElement);
+        ReturnErrorOnState();
+      }
+
+      // Recover from the invalid token.
+      auto end_of_element =
+          FindNextOf({TokenKind::Comma(), TokenKind::CloseParen()});
+      // The lexer guarantees that parentheses are balanced.
+      CARBON_CHECK(end_of_element) << "missing matching `)` for `(`";
+      SkipTo(*end_of_element);
+      if (PositionKind() == TokenKind::CloseParen()) {
+        // Done with the parameter list.
+        return;
+      }
+      // Comma handling is after the switch.
+      break;
+    }
+  }
+
+  // We are guaranteed to now be at a comma.
+  AddLeafNode(ParseNodeKind::ParameterListComma(), *position_);
+  ++position_;
+  if (PositionKind() != TokenKind::CloseParen()) {
+    PushState(ParserState::PatternForFunctionParameter());
+  }
+}
+
+auto Parser2::HandleStatementIf() -> void {
+  PushState(ParserState::StatementIfConditionFinish());
+  PushState(ParserState::ParenCondition());
+  ++position_;
+}
+
+auto Parser2::HandleStatementIfConditionFinishState() -> void {
+  auto state = PopState();
+
+  state.state = ParserState::StatementIfThenBlockFinish();
+  PushState(state);
+  HandleCodeBlock();
+}
+
+auto Parser2::HandleStatementIfThenBlockFinishState() -> void {
+  auto state = PopState();
+
+  if (ConsumeAndAddLeafNodeIf(TokenKind::Else(),
+                              ParseNodeKind::IfStatementElse())) {
+    state.state = ParserState::StatementIfElseBlockFinish();
+    PushState(state);
+    // `else if` is permitted as a special case.
+    if (PositionIs(TokenKind::If())) {
+      HandleStatementIf();
+    } else {
+      HandleCodeBlock();
+    }
+  } else {
+    AddNode(ParseNodeKind::IfStatement(), state.token, state.subtree_start,
+            state.has_error);
+  }
+}
+
+auto Parser2::HandleStatementIfElseBlockFinishState() -> void {
+  auto state = PopState();
+  AddNode(ParseNodeKind::IfStatement(), state.token, state.subtree_start,
+          state.has_error);
+}
+
+auto Parser2::HandleStatement(TokenKind token_kind) -> void {
+  switch (token_kind) {
+    case TokenKind::If(): {
+      HandleStatementIf();
+      break;
+    }
+    case TokenKind::Return(): {
+      auto return_token = *position_;
+      if (tokens_.GetKind(*(position_ + 1)) == TokenKind::Semi()) {
+        int subtree_start = tree_.size();
+        AddLeafNode(ParseNodeKind::StatementEnd(), *(position_ + 1));
+        AddNode(ParseNodeKind::ReturnStatement(), return_token, subtree_start,
+                /*has_error=*/false);
+        position_ += 2;
+      } else {
+        PushState(ParserState::KeywordStatementFinishForReturn());
+        ++position_;
+        PushState(ParserState::Expression());
+      }
+      break;
+    }
+    default: {
+      PushState(ParserState::ExpressionStatementFinish());
+      PushState(ParserState::Expression());
+      break;
+    }
+  }
+}
+
+auto Parser2::HandleStatementScopeState() -> void {
+  // This maintains the current state until we're at the end of the scope.
+
+  auto token_kind = PositionKind();
+  if (token_kind == TokenKind::CloseCurlyBrace()) {
+    auto state = PopState();
+    if (state.has_error) {
+      ReturnErrorOnState();
+    }
+  } else {
+    HandleStatement(token_kind);
+  }
+}
+
 }  // namespace Carbon

toolchain/parser/parser2.h

@@ -28,16 +28,29 @@ class Parser2 {
   }
 
  private:
+  // Supported kinds of patterns for HandlePattern.
+  enum class PatternKind { Parameter, Variable };
+
+  // Helper class for tracing state_stack_ on crashes.
   class PrettyStackTraceParseState;
 
   // Used to track state on state_stack_.
   struct StateStackEntry {
+    StateStackEntry(ParserState state, TokenizedBuffer::Token token,
+                    int32_t subtree_start)
+        : state(state), token(token), subtree_start(subtree_start) {}
+
     // The state.
     ParserState state;
-    // The token indicating the start of a tracked subtree.
-    TokenizedBuffer::Token start_token;
+    // 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
+    // will typically be used for the subtree's root node.
+    TokenizedBuffer::Token token;
     // The offset within the ParseTree of the subtree start.
     int32_t subtree_start;
+    // Set to true  to indicate that an error was found, and that contextual
+    // error recovery may be needed.
+    bool has_error = false;
   };
 
   Parser2(ParseTree& tree, TokenizedBuffer& tokens,
@@ -54,7 +67,13 @@ class Parser2 {
                    bool has_error = false) -> void;
 
   auto AddNode(ParseNodeKind kind, TokenizedBuffer::Token token,
-               int subtree_start, bool has_error = false) -> void;
+               int subtree_start, bool has_error) -> void;
+
+  // Parses a close paren token corresponding to the given open paren token,
+  // possibly skipping forward and diagnosing if necessary. Creates a parse node
+  // of the specified kind if successful.
+  auto ConsumeAndAddCloseParen(TokenizedBuffer::Token open_paren,
+                               ParseNodeKind close_kind) -> bool;
 
   // Composes `ConsumeIf` and `AddLeafNode`, returning false when ConsumeIf
   // fails.
@@ -65,6 +84,11 @@ class Parser2 {
   // advances to the next position. Otherwise returns an empty optional.
   auto ConsumeIf(TokenKind kind) -> llvm::Optional<TokenizedBuffer::Token>;
 
+  // Find the next token of any of the given kinds at the current bracketing
+  // level.
+  auto FindNextOf(std::initializer_list<TokenKind> desired_kinds)
+      -> llvm::Optional<TokenizedBuffer::Token>;
+
   // Gets the kind of the next token to be consumed.
   auto PositionKind() const -> TokenKind { return tokens_.GetKind(*position_); }
 
@@ -99,15 +123,67 @@ class Parser2 {
   // Skip forward to the given token. Verifies that it is actually forward.
   auto SkipTo(TokenizedBuffer::Token t) -> void;
 
+  // Pushes a new state with the current position for context.
   auto PushState(ParserState state) -> void {
-    state_stack_.push_back({state, *position_, tree_.size()});
+    PushState(StateStackEntry(state, *position_, tree_.size()));
   }
 
+  // Pushes a new state with the token for context.
+  auto PushState(ParserState state, TokenizedBuffer::Token token) -> void {
+    PushState(StateStackEntry(state, token, tree_.size()));
+  }
+
+  // Pushes a constructed state onto the stack.
+  auto PushState(StateStackEntry state) -> void {
+    state_stack_.push_back(state);
+  }
+
+  // Pops the state and keeps the value for inspection.
+  auto PopState() -> StateStackEntry { return state_stack_.pop_back_val(); }
+
+  // Pops the state and discards it.
+  auto PopAndDiscardState() -> void { state_stack_.pop_back(); }
+
+  // Propagates an error up the state stack, to the parent state.
+  auto ReturnErrorOnState() -> void { state_stack_.back().has_error = true; }
+
+  // Parses a primary expression, which is either a terminal portion of an
+  // expression tree, such as an identifier or literal, or a parenthesized
+  // expression.
+  auto HandleExpressionFormPrimary() -> void;
+
   // When handling errors before the start of the definition, treat it as a
   // declaration. Recover to a semicolon when it makes sense as a possible
   // function end, otherwise use the fn token for the error.
-  auto HandleFunctionError(bool skip_past_likely_end) -> void;
+  auto HandleFunctionError(StateStackEntry state, bool skip_past_likely_end)
+      -> void;
 
+  // Handles a code block in the context of a statement scope.
+  auto HandleCodeBlock() -> void;
+
+  // Handles parsing of a function parameter list, including commas and the
+  // close paren.
+  auto HandleFunctionParameterList(bool is_start) -> void;
+
+  // Handles the `;` after a keyword statement.
+  auto HandleKeywordStatementFinish(TokenKind token_kind,
+                                    ParseNodeKind node_kind) -> void;
+
+  // Handles the start of a pattern.
+  // If the start of the pattern is invalid, it's the responsibility of the
+  // outside context to advance past the pattern.
+  auto HandlePatternStart(PatternKind pattern_kind) -> void;
+
+  // Handles a single statement. While typically within a statement block, this
+  // can also be used for error recovery where we expect a statement block and
+  // are missing braces.
+  auto HandleStatement(TokenKind token_kind) -> void;
+
+  // Handles a `if` statement at the start `if` token.
+  auto HandleStatementIf() -> void;
+
+  // `clang-format` has a bug with spacing around `->` returns in macros. See
+  // https://bugs.llvm.org/show_bug.cgi?id=48320 for details.
 #define CARBON_PARSER_STATE(Name) auto Handle##Name##State()->void;
 #include "toolchain/parser/parser_state.def"
 

toolchain/parser/parser_state.def

@@ -13,9 +13,75 @@
 #error "Must define the x-macro to use this file."
 #endif
 
+// Handles processing at the `}` on a typical code block, after a statement
+// scope is done.
+CARBON_PARSER_STATE(CodeBlockFinish)
+
+// Handles processing of a declaration scope. Things like fn, class, interface,
+// and so on.
 CARBON_PARSER_STATE(Declaration)
+
+// Handles processing of an expression.
+CARBON_PARSER_STATE(Expression)
+
+// As with Expression, but specifically uses the Type precedence grouping.
+CARBON_PARSER_STATE(ExpressionForType)
+
+// Handles the `;` for an expression statement, which is different from most
+// keyword statements.
+CARBON_PARSER_STATE(ExpressionStatementFinish)
+
+// Handles processing of a function's `fn <name>(`, and enqueues parameter list
+// handling.
 CARBON_PARSER_STATE(FunctionIntroducer)
-CARBON_PARSER_STATE(FunctionParameterList)
-CARBON_PARSER_STATE(FunctionParameterListDone)
+
+// Handles processing of a function's parameter list `)`.
+CARBON_PARSER_STATE(FunctionParameterListFinish)
+
+// Handles processing of a function's syntax after `)`, primarily the
+// possibility a `->` return type is there. Always enqueues signature finish
+// handling.
+CARBON_PARSER_STATE(FunctionAfterParameterList)
+
+// Finishes a function return type.
+CARBON_PARSER_STATE(FunctionReturnTypeFinish)
+
+// Finishes a function signature. If it's a declaration, the function is done;
+// otherwise, this also starts definition processing.
+CARBON_PARSER_STATE(FunctionSignatureFinish)
+
+// Finishes a function definition.
+CARBON_PARSER_STATE(FunctionDefinitionFinish)
+
+// StatementFinish variants are just handling the `;` expectation.
+CARBON_PARSER_STATE(KeywordStatementFinishForReturn)
+
+// Handles the processing of a `(condition)` up through the expression.
+CARBON_PARSER_STATE(ParenCondition)
+
+// Finishes the processing of a `(condition)` after the expression.
+CARBON_PARSER_STATE(ParenConditionFinish)
+
+// Handles `if` processing between the condition and start of the first code
+// block.
+CARBON_PARSER_STATE(StatementIfConditionFinish)
+
+// Handles `if` processing after the end of the first code block, with the
+// optional `else`.
+CARBON_PARSER_STATE(StatementIfThenBlockFinish)
+
+// Handles `if` processing after a provided `else` code block.
+CARBON_PARSER_STATE(StatementIfElseBlockFinish)
+
+// Handles pattern parsing for a function parameter, enqueuing type expression
+// processing. Proceeds to the matching Finish state when done.
+CARBON_PARSER_STATE(PatternForFunctionParameter)
+
+// Finishes function parameter processing, including `,`. If there are more
+// parameters, enqueues another parameter processing state.
+CARBON_PARSER_STATE(PatternForFunctionParameterFinish)
+
+// Handles processing of statements within a scope.
+CARBON_PARSER_STATE(StatementScope)
 
 #undef CARBON_PARSER_STATE

toolchain/parser/parser_state_test.cpp

@@ -0,0 +1,23 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "toolchain/parser/parser_state.h"
+
+#include <gtest/gtest.h>
+
+#include <cstring>
+
+#include "llvm/ADT/StringRef.h"
+
+namespace Carbon::Testing {
+namespace {
+
+// Not much to test here, so just verify that the API compiles and returns the
+// data in the `.def` file.
+#define CARBON_PARSER_STATE(Name) \
+  TEST(ParserState, Name) { EXPECT_EQ(#Name, ParserState::Name().name()); }
+#include "toolchain/parser/parser_state.def"
+
+}  // namespace
+}  // namespace Carbon::Testing

toolchain/parser/testdata/if/else.carbon

@@ -5,7 +5,7 @@
 // AUTOUPDATE
 // RUN: %{carbon-run-parser}
 // CHECK:STDOUT: [
-// CHECK:STDOUT: {node_index: 60, kind: 'FunctionDefinition', text: '}', subtree_size: 61, children: [
+// CHECK:STDOUT: {node_index: 51, kind: 'FunctionDefinition', text: '}', subtree_size: 52, children: [
 // CHECK:STDOUT:   {node_index: 4, kind: 'FunctionDefinitionStart', text: '{', subtree_size: 5, children: [
 // CHECK:STDOUT:     {node_index: 0, kind: 'FunctionIntroducer', text: 'fn'},
 // CHECK:STDOUT:     {node_index: 1, kind: 'DeclaredName', text: 'F'},
@@ -35,38 +35,29 @@
 // CHECK:STDOUT:       {node_index: 24, kind: 'CodeBlockStart', text: '{'},
 // CHECK:STDOUT:       {node_index: 26, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
 // CHECK:STDOUT:         {node_index: 25, kind: 'NameReference', text: 'e'}]}]}]},
-// CHECK:STDOUT:   {node_index: 59, kind: 'IfStatement', text: 'if', subtree_size: 31, children: [
+// CHECK:STDOUT:   {node_index: 50, kind: 'IfStatement', text: 'if', subtree_size: 22, children: [
 // CHECK:STDOUT:     {node_index: 31, kind: 'Condition', text: '(', subtree_size: 3, children: [
 // CHECK:STDOUT:       {node_index: 29, kind: 'NameReference', text: 'x'},
 // CHECK:STDOUT:       {node_index: 30, kind: 'ConditionEnd', text: ')'}]},
-// CHECK:STDOUT:     {node_index: 38, kind: 'CodeBlock', text: '}', subtree_size: 7, children: [
+// CHECK:STDOUT:     {node_index: 35, kind: 'CodeBlock', text: '}', subtree_size: 4, children: [
 // CHECK:STDOUT:       {node_index: 32, kind: 'CodeBlockStart', text: '{'},
-// CHECK:STDOUT:       {node_index: 37, kind: 'ExpressionStatement', text: ';', subtree_size: 5, children: [
-// CHECK:STDOUT:         {node_index: 36, kind: 'CallExpression', text: '(', subtree_size: 4, children: [
-// CHECK:STDOUT:           {node_index: 33, kind: 'NameReference', text: 'G'},
-// CHECK:STDOUT:           {node_index: 34, kind: 'Literal', text: '1'},
-// CHECK:STDOUT:           {node_index: 35, kind: 'CallExpressionEnd', text: ')'}]}]}]},
-// CHECK:STDOUT:     {node_index: 39, kind: 'IfStatementElse', text: 'else'},
-// CHECK:STDOUT:     {node_index: 58, kind: 'IfStatement', text: 'if', subtree_size: 19, children: [
-// CHECK:STDOUT:       {node_index: 42, kind: 'Condition', text: '(', subtree_size: 3, children: [
-// CHECK:STDOUT:         {node_index: 40, kind: 'NameReference', text: 'x'},
-// CHECK:STDOUT:         {node_index: 41, kind: 'ConditionEnd', text: ')'}]},
-// CHECK:STDOUT:       {node_index: 49, kind: 'CodeBlock', text: '}', subtree_size: 7, children: [
-// CHECK:STDOUT:         {node_index: 43, kind: 'CodeBlockStart', text: '{'},
-// CHECK:STDOUT:         {node_index: 48, kind: 'ExpressionStatement', text: ';', subtree_size: 5, children: [
-// CHECK:STDOUT:           {node_index: 47, kind: 'CallExpression', text: '(', subtree_size: 4, children: [
-// CHECK:STDOUT:             {node_index: 44, kind: 'NameReference', text: 'G'},
-// CHECK:STDOUT:             {node_index: 45, kind: 'Literal', text: '2'},
-// CHECK:STDOUT:             {node_index: 46, kind: 'CallExpressionEnd', text: ')'}]}]}]},
-// CHECK:STDOUT:       {node_index: 50, kind: 'IfStatementElse', text: 'else'},
-// CHECK:STDOUT:       {node_index: 57, kind: 'CodeBlock', text: '}', subtree_size: 7, children: [
-// CHECK:STDOUT:         {node_index: 51, kind: 'CodeBlockStart', text: '{'},
-// CHECK:STDOUT:         {node_index: 56, kind: 'ExpressionStatement', text: ';', subtree_size: 5, children: [
-// CHECK:STDOUT:           {node_index: 55, kind: 'CallExpression', text: '(', subtree_size: 4, children: [
-// CHECK:STDOUT:             {node_index: 52, kind: 'NameReference', text: 'G'},
-// CHECK:STDOUT:             {node_index: 53, kind: 'Literal', text: '3'},
-// CHECK:STDOUT:             {node_index: 54, kind: 'CallExpressionEnd', text: ')'}]}]}]}]}]}]},
-// CHECK:STDOUT: {node_index: 61, kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:       {node_index: 34, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
+// CHECK:STDOUT:         {node_index: 33, kind: 'NameReference', text: 'f'}]}]},
+// CHECK:STDOUT:     {node_index: 36, kind: 'IfStatementElse', text: 'else'},
+// CHECK:STDOUT:     {node_index: 49, kind: 'IfStatement', text: 'if', subtree_size: 13, children: [
+// CHECK:STDOUT:       {node_index: 39, kind: 'Condition', text: '(', subtree_size: 3, children: [
+// CHECK:STDOUT:         {node_index: 37, kind: 'NameReference', text: 'x'},
+// CHECK:STDOUT:         {node_index: 38, kind: 'ConditionEnd', text: ')'}]},
+// CHECK:STDOUT:       {node_index: 43, kind: 'CodeBlock', text: '}', subtree_size: 4, children: [
+// CHECK:STDOUT:         {node_index: 40, kind: 'CodeBlockStart', text: '{'},
+// CHECK:STDOUT:         {node_index: 42, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
+// CHECK:STDOUT:           {node_index: 41, kind: 'NameReference', text: 'g'}]}]},
+// CHECK:STDOUT:       {node_index: 44, kind: 'IfStatementElse', text: 'else'},
+// CHECK:STDOUT:       {node_index: 48, kind: 'CodeBlock', text: '}', subtree_size: 4, children: [
+// CHECK:STDOUT:         {node_index: 45, kind: 'CodeBlockStart', text: '{'},
+// CHECK:STDOUT:         {node_index: 47, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
+// CHECK:STDOUT:           {node_index: 46, kind: 'NameReference', text: 'h'}]}]}]}]}]},
+// CHECK:STDOUT: {node_index: 52, kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 
 fn F() {
@@ -79,7 +70,7 @@ fn F() {
   } else {
     e;
   }
-  if (x) { G(1); }
-  else if (x) { G(2); }
-  else { G(3); }
+  if (x) { f; }
+  else if (x) { g; }
+  else { h; }
 }

toolchain/parser/testdata/if/fail_else_unbraced.carbon

@@ -5,7 +5,7 @@
 // AUTOUPDATE
 // RUN: %{not} %{carbon-run-parser}
 // CHECK:STDOUT: [
-// CHECK:STDOUT: {node_index: 60, kind: 'FunctionDefinition', text: '}', subtree_size: 61, children: [
+// CHECK:STDOUT: {node_index: 51, kind: 'FunctionDefinition', text: '}', subtree_size: 52, children: [
 // CHECK:STDOUT:   {node_index: 4, kind: 'FunctionDefinitionStart', text: '{', subtree_size: 5, children: [
 // CHECK:STDOUT:     {node_index: 0, kind: 'FunctionIntroducer', text: 'fn'},
 // CHECK:STDOUT:     {node_index: 1, kind: 'DeclaredName', text: 'F'},
@@ -35,38 +35,29 @@
 // CHECK:STDOUT:       {node_index: 24, kind: 'CodeBlockStart', text: 'e', has_error: yes},
 // CHECK:STDOUT:       {node_index: 26, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
 // CHECK:STDOUT:         {node_index: 25, kind: 'NameReference', text: 'e'}]}]}]},
-// CHECK:STDOUT:   {node_index: 59, kind: 'IfStatement', text: 'if', subtree_size: 31, children: [
+// CHECK:STDOUT:   {node_index: 50, kind: 'IfStatement', text: 'if', subtree_size: 22, children: [
 // CHECK:STDOUT:     {node_index: 31, kind: 'Condition', text: '(', subtree_size: 3, children: [
 // CHECK:STDOUT:       {node_index: 29, kind: 'NameReference', text: 'x'},
 // CHECK:STDOUT:       {node_index: 30, kind: 'ConditionEnd', text: ')'}]},
-// CHECK:STDOUT:     {node_index: 38, kind: 'CodeBlock', text: '}', subtree_size: 7, children: [
+// CHECK:STDOUT:     {node_index: 35, kind: 'CodeBlock', text: '}', subtree_size: 4, children: [
 // CHECK:STDOUT:       {node_index: 32, kind: 'CodeBlockStart', text: '{'},
-// CHECK:STDOUT:       {node_index: 37, kind: 'ExpressionStatement', text: ';', subtree_size: 5, children: [
-// CHECK:STDOUT:         {node_index: 36, kind: 'CallExpression', text: '(', subtree_size: 4, children: [
-// CHECK:STDOUT:           {node_index: 33, kind: 'NameReference', text: 'G'},
-// CHECK:STDOUT:           {node_index: 34, kind: 'Literal', text: '1'},
-// CHECK:STDOUT:           {node_index: 35, kind: 'CallExpressionEnd', text: ')'}]}]}]},
-// CHECK:STDOUT:     {node_index: 39, kind: 'IfStatementElse', text: 'else'},
-// CHECK:STDOUT:     {node_index: 58, kind: 'IfStatement', text: 'if', subtree_size: 19, children: [
-// CHECK:STDOUT:       {node_index: 42, kind: 'Condition', text: '(', subtree_size: 3, children: [
-// CHECK:STDOUT:         {node_index: 40, kind: 'NameReference', text: 'x'},
-// CHECK:STDOUT:         {node_index: 41, kind: 'ConditionEnd', text: ')'}]},
-// CHECK:STDOUT:       {node_index: 49, kind: 'CodeBlock', text: '}', subtree_size: 7, children: [
-// CHECK:STDOUT:         {node_index: 43, kind: 'CodeBlockStart', text: '{'},
-// CHECK:STDOUT:         {node_index: 48, kind: 'ExpressionStatement', text: ';', subtree_size: 5, children: [
-// CHECK:STDOUT:           {node_index: 47, kind: 'CallExpression', text: '(', subtree_size: 4, children: [
-// CHECK:STDOUT:             {node_index: 44, kind: 'NameReference', text: 'G'},
-// CHECK:STDOUT:             {node_index: 45, kind: 'Literal', text: '2'},
-// CHECK:STDOUT:             {node_index: 46, kind: 'CallExpressionEnd', text: ')'}]}]}]},
-// CHECK:STDOUT:       {node_index: 50, kind: 'IfStatementElse', text: 'else'},
-// CHECK:STDOUT:       {node_index: 57, kind: 'CodeBlock', text: '}', subtree_size: 7, children: [
-// CHECK:STDOUT:         {node_index: 51, kind: 'CodeBlockStart', text: '{'},
-// CHECK:STDOUT:         {node_index: 56, kind: 'ExpressionStatement', text: ';', subtree_size: 5, children: [
-// CHECK:STDOUT:           {node_index: 55, kind: 'CallExpression', text: '(', subtree_size: 4, children: [
-// CHECK:STDOUT:             {node_index: 52, kind: 'NameReference', text: 'G'},
-// CHECK:STDOUT:             {node_index: 53, kind: 'Literal', text: '3'},
-// CHECK:STDOUT:             {node_index: 54, kind: 'CallExpressionEnd', text: ')'}]}]}]}]}]}]},
-// CHECK:STDOUT: {node_index: 61, kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:       {node_index: 34, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
+// CHECK:STDOUT:         {node_index: 33, kind: 'NameReference', text: 'f'}]}]},
+// CHECK:STDOUT:     {node_index: 36, kind: 'IfStatementElse', text: 'else'},
+// CHECK:STDOUT:     {node_index: 49, kind: 'IfStatement', text: 'if', subtree_size: 13, children: [
+// CHECK:STDOUT:       {node_index: 39, kind: 'Condition', text: '(', subtree_size: 3, children: [
+// CHECK:STDOUT:         {node_index: 37, kind: 'NameReference', text: 'x'},
+// CHECK:STDOUT:         {node_index: 38, kind: 'ConditionEnd', text: ')'}]},
+// CHECK:STDOUT:       {node_index: 43, kind: 'CodeBlock', text: '}', subtree_size: 4, children: [
+// CHECK:STDOUT:         {node_index: 40, kind: 'CodeBlockStart', text: '{'},
+// CHECK:STDOUT:         {node_index: 42, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
+// CHECK:STDOUT:           {node_index: 41, kind: 'NameReference', text: 'g'}]}]},
+// CHECK:STDOUT:       {node_index: 44, kind: 'IfStatementElse', text: 'else'},
+// CHECK:STDOUT:       {node_index: 48, kind: 'CodeBlock', text: '}', subtree_size: 4, children: [
+// CHECK:STDOUT:         {node_index: 45, kind: 'CodeBlockStart', text: '{'},
+// CHECK:STDOUT:         {node_index: 47, kind: 'ExpressionStatement', text: ';', subtree_size: 2, children: [
+// CHECK:STDOUT:           {node_index: 46, kind: 'NameReference', text: 'h'}]}]}]}]}]},
+// CHECK:STDOUT: {node_index: 52, kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 
 fn F() {
@@ -81,7 +72,7 @@ fn F() {
   else
     // CHECK:STDERR: {{.*}}/toolchain/parser/testdata/if/fail_else_unbraced.carbon:[[@LINE+1]]:5: Expected braced code block.
     e;
-  if (x) { G(1); }
-  else if (x) { G(2); }
-  else { G(3); }
+  if (x) { f; }
+  else if (x) { g; }
+  else { h; }
 }