Small cleanups in toolchain code (#2474)

Doing some sorting of functions / enums (generally speaking, I've been trying to keep these loosely lexically sorted for lack of a better ordering).

Also removes some code that seems to be dead, and a minor TODO comment fix.

toolchain/parser/parser.cpp

@@ -752,6 +752,16 @@ auto Parser::HandleDeclarationLoopState() -> void {
   }
 }
 
+auto Parser::HandleDeducedParameterListFinishState() -> void {
+  auto state = PopState();
+
+  CARBON_CHECK(tokens_->GetKind(*position_) == TokenKind::CloseSquareBracket())
+      << "Expected current token to be: `]`, found: "
+      << tokens_->GetKind(state.token);
+  AddNode(ParseNodeKind::DeducedParameterList(), Consume(), state.subtree_start,
+          state.has_error);
+}
+
 auto Parser::HandleDesignator(bool as_struct) -> void {
   auto state = PopState();
 
@@ -1043,16 +1053,6 @@ auto Parser::HandleFunctionIntroducerState() -> void {
   }
 }
 
-auto Parser::HandleDeducedParameterListFinishState() -> void {
-  auto state = PopState();
-
-  CARBON_CHECK(tokens_->GetKind(*position_) == TokenKind::CloseSquareBracket())
-      << "Expected current token to be: `]`, found: "
-      << tokens_->GetKind(state.token);
-  AddNode(ParseNodeKind::DeducedParameterList(), Consume(), state.subtree_start,
-          state.has_error);
-}
-
 auto Parser::HandleFunctionAfterDeducedParameterListState() -> void {
   auto state = PopState();
 
@@ -1176,6 +1176,87 @@ auto Parser::HandleFunctionDefinitionFinishState() -> void {
           state.has_error);
 }
 
+auto Parser::HandleInterfaceIntroducerState() -> void {
+  auto state = PopState();
+  CARBON_CHECK(stack_context_ == ParseContext::File)
+      << "TODO: Support nesting.";
+  stack_context_ = ParseContext::Interface;
+
+  if (!ConsumeAndAddLeafNodeIf(TokenKind::Identifier(),
+                               ParseNodeKind::DeclaredName())) {
+    CARBON_DIAGNOSTIC(ExpectedInterfaceName, Error,
+                      "Expected interface name after `interface` keyword.");
+    emitter_->Emit(*position_, ExpectedInterfaceName);
+    state.has_error = true;
+
+    // Add a name node even when it's not present because it's used for subtree
+    // bracketing on interfaces.
+    // TODO: Either fix this or normalize it, still deciding on the right
+    // approach.
+    AddLeafNode(ParseNodeKind::DeclaredName(), state.token, /*has_error=*/true);
+  }
+
+  bool parse_body = true;
+
+  if (!PositionIs(TokenKind::OpenCurlyBrace())) {
+    CARBON_DIAGNOSTIC(ExpectedInterfaceOpenCurlyBrace, Error,
+                      "Expected `{{` to start interface definition.");
+    emitter_->Emit(*position_, ExpectedInterfaceOpenCurlyBrace);
+    state.has_error = true;
+
+    SkipPastLikelyEnd(state.token);
+    parse_body = false;
+  }
+
+  state.state = ParserState::InterfaceDefinitionFinish();
+  PushState(state);
+
+  if (parse_body) {
+    PushState(ParserState::InterfaceDefinitionLoop());
+    AddLeafNode(ParseNodeKind::InterfaceBodyStart(), Consume());
+  }
+}
+
+auto Parser::HandleInterfaceDefinitionLoopState() -> void {
+  // This maintains the current state unless we're at the end of the interface
+  // definition.
+
+  switch (PositionKind()) {
+    case TokenKind::CloseCurlyBrace(): {
+      auto state = PopState();
+
+      AddNode(ParseNodeKind::InterfaceBody(), Consume(), state.subtree_start,
+              state.has_error);
+
+      break;
+    }
+    case TokenKind::Fn(): {
+      PushState(ParserState::FunctionIntroducer());
+      AddLeafNode(ParseNodeKind::FunctionIntroducer(), Consume());
+      break;
+    }
+    default: {
+      CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error,
+                        "Unrecognized declaration introducer.");
+      emitter_->Emit(*position_, UnrecognizedDeclaration);
+      if (auto semi = SkipPastLikelyEnd(*position_)) {
+        AddLeafNode(ParseNodeKind::EmptyDeclaration(), *semi,
+                    /*has_error=*/true);
+      } else {
+        ReturnErrorOnState();
+      }
+      break;
+    }
+  }
+}
+
+auto Parser::HandleInterfaceDefinitionFinishState() -> void {
+  auto state = PopState();
+  AddNode(ParseNodeKind::InterfaceDefinition(), state.token,
+          state.subtree_start, state.has_error);
+  stack_context_ = ParseContext::File;
+}
+
 auto Parser::HandlePackageState() -> void {
   auto state = PopState();
 
@@ -1358,71 +1439,6 @@ auto Parser::HandleParenExpressionFinishAsTupleState() -> void {
           state.has_error);
 }
 
-// TODO: This can possibly be merged with `HandlePattern`. Regular function
-// parameters support `addr` as well but it is not implemented yet.
-auto Parser::HandleSelfPatternState() -> void {
-  auto state = PopState();
-
-  // self `:` type
-  auto possible_self_param =
-      (PositionIs(TokenKind::SelfParameter()) &&
-       tokens_->GetKind(*(position_ + 1)) == TokenKind::Colon());
-
-  if (possible_self_param) {
-    // Ensure the finish state always follows.
-    state.state = ParserState::PatternFinish();
-
-    // Switch the context token to the colon, so that it'll be used for the root
-    // node.
-    state.token = *(position_ + 1);
-    PushState(state);
-    PushStateForExpression(PrecedenceGroup::ForType());
-    AddLeafNode(ParseNodeKind::SelfDeducedParameter(), *position_);
-    position_ += 2;
-    return;
-  }
-
-  // addr self `:` type
-  auto possible_addr_self_param =
-      (PositionIs(TokenKind::Addr()) &&
-       tokens_->GetKind(*(position_ + 1)) == TokenKind::SelfParameter() &&
-       tokens_->GetKind(*(position_ + 2)) == TokenKind::Colon());
-
-  if (possible_addr_self_param) {
-    // Ensure the finish state always follows.
-    state.state = ParserState::PatternAddress();
-    state.token = Consume();
-    PushState(state);
-
-    PushState(ParserState::PatternFinish());
-
-    PushStateForExpression(PrecedenceGroup::ForType());
-    // auto size = tree_->size();
-    // AddLeafNode(ParseNodeKind::Address(), *position_);
-    AddLeafNode(ParseNodeKind::SelfDeducedParameter(), *(position_ + 1));
-    // AddNode(ParseNodeKind::SelfDeducedParameterAddress(), *(position_ + 1),
-    //        size, false);
-    position_ += 2;
-    return;
-  }
-
-  CARBON_DIAGNOSTIC(ExpectedDeducedParam, Error,
-                    "Deduced parameters must be of the form: `<name>: <Type>` "
-                    "or `addr <name>: <Type>`.");
-  emitter_->Emit(*position_, ExpectedDeducedParam);
-  state.state = ParserState::PatternFinish();
-  state.has_error = true;
-
-  // Try to recover by skipping to the next `]`.
-  if (auto next_close_square_bracket =
-          FindNextOf({TokenKind::CloseSquareBracket()});
-      next_close_square_bracket) {
-    SkipTo(*next_close_square_bracket);
-  }
-
-  PushState(state);
-}
-
 auto Parser::HandlePattern(PatternKind pattern_kind) -> void {
   auto state = PopState();
 
@@ -1495,6 +1511,67 @@ auto Parser::HandlePatternAddressState() -> void {
           /*has_error=*/false);
 }
 
+// TODO: This can possibly be merged with `HandlePattern`. Regular function
+// parameters support `addr` as well but it is not implemented yet.
+auto Parser::HandleSelfPatternState() -> void {
+  auto state = PopState();
+
+  // self `:` type
+  auto possible_self_param =
+      (PositionIs(TokenKind::SelfParameter()) &&
+       tokens_->GetKind(*(position_ + 1)) == TokenKind::Colon());
+
+  if (possible_self_param) {
+    // Ensure the finish state always follows.
+    state.state = ParserState::PatternFinish();
+
+    // Switch the context token to the colon, so that it'll be used for the root
+    // node.
+    state.token = *(position_ + 1);
+    PushState(state);
+    PushStateForExpression(PrecedenceGroup::ForType());
+    AddLeafNode(ParseNodeKind::SelfDeducedParameter(), *position_);
+    position_ += 2;
+    return;
+  }
+
+  // addr self `:` type
+  auto possible_addr_self_param =
+      (PositionIs(TokenKind::Addr()) &&
+       tokens_->GetKind(*(position_ + 1)) == TokenKind::SelfParameter() &&
+       tokens_->GetKind(*(position_ + 2)) == TokenKind::Colon());
+
+  if (possible_addr_self_param) {
+    // Ensure the finish state always follows.
+    state.state = ParserState::PatternAddress();
+    state.token = Consume();
+    PushState(state);
+
+    PushState(ParserState::PatternFinish());
+
+    PushStateForExpression(PrecedenceGroup::ForType());
+    AddLeafNode(ParseNodeKind::SelfDeducedParameter(), *(position_ + 1));
+    position_ += 2;
+    return;
+  }
+
+  CARBON_DIAGNOSTIC(ExpectedDeducedParam, Error,
+                    "Deduced parameters must be of the form: `<name>: <Type>` "
+                    "or `addr <name>: <Type>`.");
+  emitter_->Emit(*position_, ExpectedDeducedParam);
+  state.state = ParserState::PatternFinish();
+  state.has_error = true;
+
+  // Try to recover by skipping to the next `]`.
+  if (auto next_close_square_bracket =
+          FindNextOf({TokenKind::CloseSquareBracket()});
+      next_close_square_bracket) {
+    SkipTo(*next_close_square_bracket);
+  }
+
+  PushState(state);
+}
+
 auto Parser::HandleStatementState() -> void {
   PopAndDiscardState();
 
@@ -1789,85 +1866,4 @@ auto Parser::HandleVarFinishAsForState() -> void {
           state.has_error);
 }
 
-auto Parser::HandleInterfaceIntroducerState() -> void {
-  auto state = PopState();
-  CARBON_CHECK(stack_context_ == ParseContext::File)
-      << "TODO: Support nesting.";
-  stack_context_ = ParseContext::Interface;
-
-  if (!ConsumeAndAddLeafNodeIf(TokenKind::Identifier(),
-                               ParseNodeKind::DeclaredName())) {
-    CARBON_DIAGNOSTIC(ExpectedInterfaceName, Error,
-                      "Expected interface name after `interface` keyword.");
-    emitter_->Emit(*position_, ExpectedInterfaceName);
-    state.has_error = true;
-
-    // Add a name node even when it's not present because it's used for subtree
-    // bracketing on interfaces.
-    // TODO: Either fix this or normalize it, still deciding on the right
-    // approach.
-    AddLeafNode(ParseNodeKind::DeclaredName(), state.token, /*has_error=*/true);
-  }
-
-  bool parse_body = true;
-
-  if (!PositionIs(TokenKind::OpenCurlyBrace())) {
-    CARBON_DIAGNOSTIC(ExpectedInterfaceOpenCurlyBrace, Error,
-                      "Expected `{{` to start interface definition.");
-    emitter_->Emit(*position_, ExpectedInterfaceOpenCurlyBrace);
-    state.has_error = true;
-
-    SkipPastLikelyEnd(state.token);
-    parse_body = false;
-  }
-
-  state.state = ParserState::InterfaceDefinitionFinish();
-  PushState(state);
-
-  if (parse_body) {
-    PushState(ParserState::InterfaceDefinitionLoop());
-    AddLeafNode(ParseNodeKind::InterfaceBodyStart(), Consume());
-  }
-}
-
-auto Parser::HandleInterfaceDefinitionLoopState() -> void {
-  // This maintains the current state unless we're at the end of the interface
-  // definition.
-
-  switch (PositionKind()) {
-    case TokenKind::CloseCurlyBrace(): {
-      auto state = PopState();
-
-      AddNode(ParseNodeKind::InterfaceBody(), Consume(), state.subtree_start,
-              state.has_error);
-
-      break;
-    }
-    case TokenKind::Fn(): {
-      PushState(ParserState::FunctionIntroducer());
-      AddLeafNode(ParseNodeKind::FunctionIntroducer(), Consume());
-      break;
-    }
-    default: {
-      CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error,
-                        "Unrecognized declaration introducer.");
-      emitter_->Emit(*position_, UnrecognizedDeclaration);
-      if (auto semi = SkipPastLikelyEnd(*position_)) {
-        AddLeafNode(ParseNodeKind::EmptyDeclaration(), *semi,
-                    /*has_error=*/true);
-      } else {
-        ReturnErrorOnState();
-      }
-      break;
-    }
-  }
-}
-
-auto Parser::HandleInterfaceDefinitionFinishState() -> void {
-  auto state = PopState();
-  AddNode(ParseNodeKind::InterfaceDefinition(), state.token,
-          state.subtree_start, state.has_error);
-  stack_context_ = ParseContext::File;
-}
-
 }  // namespace Carbon

toolchain/parser/parser_state.def

@@ -130,6 +130,12 @@ CARBON_PARSER_STATE(CodeBlockFinish)
 //   1. DeclarationLoop
 CARBON_PARSER_STATE(DeclarationLoop)
 
+// To handle parsing deduced parameter list at the end.
+//
+// Always:
+//   (state done)
+CARBON_PARSER_STATE(DeducedParameterListFinish)
+
 // Handles a designator expression, such as `.z` in `x.(y.z)`.
 //
 // Always:
@@ -369,12 +375,27 @@ CARBON_PARSER_STATE(ParenExpressionFinishAsTuple)
 CARBON_PARSER_STATE(PatternAsFunctionParameter)
 CARBON_PARSER_STATE(PatternAsVariable)
 
+// Handles `addr` in a pattern.
+//
+// Always:
+//   (state done)
+CARBON_PARSER_STATE(PatternAddress)
+
 // Finishes pattern processing.
 //
 // Always:
 //   (state done)
 CARBON_PARSER_STATE(PatternFinish)
 
+// To start parsing `self` deduced parameter.
+//
+// If valid:
+//   1. Expression
+//   2. PatternFinish
+// Else:
+//   1. PatternFinish
+CARBON_PARSER_STATE(SelfPattern)
+
 // 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.
@@ -571,21 +592,4 @@ CARBON_PARSER_STATE(VarFinishAsFor)
 //   2. FunctionAfterParameterList
 CARBON_PARSER_STATE(FunctionAfterDeducedParameterList)
 
-// To handle parsing deduced parameter list at the end.
-//
-// Always:
-//   (state done)
-CARBON_PARSER_STATE(DeducedParameterListFinish)
-
-// To start parsing `self` deduced parameter.
-//
-// If valid:
-//   1. Expression
-//   2. PatternFinish
-// Else:
-//   1. PatternFinish
-CARBON_PARSER_STATE(SelfPattern)
-
-CARBON_PARSER_STATE(PatternAddress)
-
 #undef CARBON_PARSER_STATE

toolchain/parser/testdata/generics/interface/fail_self_param_syntax.carbon

@@ -66,7 +66,7 @@ interface foo {
   // CHECK:STDERR: {{.*}}/toolchain/parser/testdata/generics/interface/fail_self_param_syntax.carbon:[[@LINE+1]]:10: Deduced parameters must be of the form: `<name>: <Type>` or `addr <name>: <Type>`.
   fn Mul[Self](b: Self) -> Self;
 
-  // TODO The recovery token is currently inserted after the `;`:
+  // TODO: The recovery token is currently inserted after the `;`:
   // ```
   // fn Div[me: Self(b: Self) -> Self;]
   // ```

toolchain/semantics/semantics_parse_tree_handler.cpp

@@ -198,6 +198,11 @@ auto SemanticsParseTreeHandler::AddIdentifier(ParseTree::Node decl_node)
   return semantics_->AddIdentifier(text);
 }
 
+auto SemanticsParseTreeHandler::HandleAddress(ParseTree::Node /*parse_node*/)
+    -> void {
+  CARBON_FATAL() << "TODO";
+}
+
 auto SemanticsParseTreeHandler::HandleBreakStatement(
     ParseTree::Node /*parse_node*/) -> void {
   CARBON_FATAL() << "TODO";
@@ -249,6 +254,16 @@ auto SemanticsParseTreeHandler::HandleDeclaredName(ParseTree::Node parse_node)
   Push(parse_node);
 }
 
+auto SemanticsParseTreeHandler::HandleDeducedParameterList(
+    ParseTree::Node /*parse_node*/) -> void {
+  CARBON_FATAL() << "TODO";
+}
+
+auto SemanticsParseTreeHandler::HandleDeducedParameterListStart(
+    ParseTree::Node /*parse_node*/) -> void {
+  CARBON_FATAL() << "TODO";
+}
+
 auto SemanticsParseTreeHandler::HandleDesignatedName(
     ParseTree::Node /*parse_node*/) -> void {
   CARBON_FATAL() << "TODO";
@@ -535,6 +550,16 @@ auto SemanticsParseTreeHandler::HandleReturnType(ParseTree::Node /*parse_node*/)
   CARBON_FATAL() << "TODO";
 }
 
+auto SemanticsParseTreeHandler::HandleSelfDeducedParameter(
+    ParseTree::Node /*parse_node*/) -> void {
+  CARBON_FATAL() << "TODO";
+}
+
+auto SemanticsParseTreeHandler::HandleSelfType(ParseTree::Node /*parse_node*/)
+    -> void {
+  CARBON_FATAL() << "TODO";
+}
+
 auto SemanticsParseTreeHandler::HandleStructComma(
     ParseTree::Node /*parse_node*/) -> void {
   CARBON_FATAL() << "TODO";
@@ -626,28 +651,4 @@ auto SemanticsParseTreeHandler::HandleWhileStatement(
   CARBON_FATAL() << "TODO";
 }
 
-auto SemanticsParseTreeHandler::HandleAddress(ParseTree::Node /*parse_node*/)
-    -> void {
-  CARBON_FATAL() << "TODO";
-}
-
-auto SemanticsParseTreeHandler::HandleSelfType(ParseTree::Node /*parse_node*/)
-    -> void {
-  CARBON_FATAL() << "TODO";
-}
-
-auto SemanticsParseTreeHandler::HandleDeducedParameterList(
-    ParseTree::Node /*parse_node*/) -> void {
-  CARBON_FATAL() << "TODO";
-}
-
-auto SemanticsParseTreeHandler::HandleSelfDeducedParameter(
-    ParseTree::Node /*parse_node*/) -> void {
-  CARBON_FATAL() << "TODO";
-}
-
-auto SemanticsParseTreeHandler::HandleDeducedParameterListStart(
-    ParseTree::Node /*parse_node*/) -> void {
-  CARBON_FATAL() << "TODO";
-}
 }  // namespace Carbon