[toolchain] Implement #623: require braces.

For error recovery, allow the braces around the controlled statement of
an `if`, `while`, or similar to be omitted. Remove support for nested
code blocks as this conflicts with struct literal syntax.

toolchain/parser/parse_tree_test.cpp

@@ -354,28 +354,6 @@ TEST_F(ParseTreeTest, BasicFunctionDefinition) {
                          MatchFileEnd()}));
 }
 
-TEST_F(ParseTreeTest, FunctionDefinitionWithNestedBlocks) {
-  TokenizedBuffer tokens = GetTokenizedBuffer(
-      "fn F() {\n"
-      "  {\n"
-      "    {{}}\n"
-      "  }\n"
-      "}");
-  ParseTree tree = ParseTree::Parse(tokens, consumer);
-  EXPECT_FALSE(tree.HasErrors());
-  EXPECT_THAT(
-      tree, MatchParseTreeNodes(
-                {MatchFunctionDeclaration(
-                     MatchDeclaredName("F"), MatchParameters(),
-                     MatchCodeBlock(
-                         MatchCodeBlock(
-                             MatchCodeBlock(MatchCodeBlock(MatchCodeBlockEnd()),
-                                            MatchCodeBlockEnd()),
-                             MatchCodeBlockEnd()),
-                         MatchCodeBlockEnd())),
-                 MatchFileEnd()}));
-}
-
 TEST_F(ParseTreeTest, FunctionDefinitionWithIdenifierInStatements) {
   TokenizedBuffer tokens = GetTokenizedBuffer(
       "fn F() {\n"
@@ -393,26 +371,6 @@ TEST_F(ParseTreeTest, FunctionDefinitionWithIdenifierInStatements) {
                          MatchFileEnd()}));
 }
 
-TEST_F(ParseTreeTest, FunctionDefinitionWithIdenifierInNestedBlock) {
-  TokenizedBuffer tokens = GetTokenizedBuffer(
-      "fn F() {\n"
-      "  {bar}\n"
-      "}");
-  ParseTree tree = ParseTree::Parse(tokens, consumer);
-  // Note: this might become valid depending on the expression syntax. This test
-  // shouldn't be taken as a sign it should remain invalid.
-  EXPECT_TRUE(tree.HasErrors());
-  EXPECT_THAT(tree,
-              MatchParseTreeNodes(
-                  {MatchFunctionDeclaration(
-                       MatchDeclaredName("F"), MatchParameters(),
-                       MatchCodeBlock(
-                           MatchCodeBlock(HasError, MatchNameReference("bar"),
-                                          MatchCodeBlockEnd()),
-                           MatchCodeBlockEnd())),
-                   MatchFileEnd()}));
-}
-
 TEST_F(ParseTreeTest, FunctionDefinitionWithFunctionCall) {
   TokenizedBuffer tokens = GetTokenizedBuffer(
       "fn F() {\n"
@@ -673,6 +631,43 @@ TEST_F(ParseTreeTest, VariableDeclarations) {
 }
 
 TEST_F(ParseTreeTest, IfNoElse) {
+  TokenizedBuffer tokens = GetTokenizedBuffer(
+      "fn F() {\n"
+      "  if (a) {\n"
+      "    if (b) {\n"
+      "      if (c) {\n"
+      "        d;\n"
+      "      }\n"
+      "    }\n"
+      "  }\n"
+      "}");
+  ErrorTrackingDiagnosticConsumer error_tracker(consumer);
+  ParseTree tree = ParseTree::Parse(tokens, error_tracker);
+  EXPECT_FALSE(tree.HasErrors());
+  EXPECT_FALSE(error_tracker.SeenError());
+
+  EXPECT_THAT(
+      tree,
+      MatchParseTreeNodes(
+          {MatchFunctionWithBody(MatchIfStatement(
+               MatchCondition(MatchNameReference("a"), MatchConditionEnd()),
+               MatchCodeBlock(
+                   MatchIfStatement(
+                       MatchCondition(MatchNameReference("b"),
+                                      MatchConditionEnd()),
+                       MatchCodeBlock(
+                           MatchIfStatement(
+                               MatchCondition(MatchNameReference("c"),
+                                              MatchConditionEnd()),
+                               MatchCodeBlock(MatchExpressionStatement(
+                                                  MatchNameReference("d")),
+                                              MatchCodeBlockEnd())),
+                           MatchCodeBlockEnd())),
+                   MatchCodeBlockEnd()))),
+           MatchFileEnd()}));
+}
+
+TEST_F(ParseTreeTest, IfNoElseUnbraced) {
   TokenizedBuffer tokens = GetTokenizedBuffer(
       "fn F() {\n"
       "  if (a)\n"
@@ -680,8 +675,11 @@ TEST_F(ParseTreeTest, IfNoElse) {
       "      if (c)\n"
       "        d;\n"
       "}");
-  ParseTree tree = ParseTree::Parse(tokens, consumer);
+  ErrorTrackingDiagnosticConsumer error_tracker(consumer);
+  ParseTree tree = ParseTree::Parse(tokens, error_tracker);
+  // The missing braces are invalid, but we should be able to recover.
   EXPECT_FALSE(tree.HasErrors());
+  EXPECT_TRUE(error_tracker.SeenError());
 
   EXPECT_THAT(
       tree,
@@ -698,6 +696,74 @@ TEST_F(ParseTreeTest, IfNoElse) {
 }
 
 TEST_F(ParseTreeTest, IfElse) {
+  TokenizedBuffer tokens = GetTokenizedBuffer(
+      "fn F() {\n"
+      "  if (a) {\n"
+      "    if (b) {\n"
+      "      c;\n"
+      "    } else {\n"
+      "      d;\n"
+      "    }\n"
+      "  } else {\n"
+      "    e;\n"
+      "  }\n"
+      "  if (x) { G(1); }\n"
+      "  else if (x) { G(2); }\n"
+      "  else { G(3); }\n"
+      "}");
+  ErrorTrackingDiagnosticConsumer error_tracker(consumer);
+  ParseTree tree = ParseTree::Parse(tokens, error_tracker);
+  EXPECT_FALSE(tree.HasErrors());
+  EXPECT_FALSE(error_tracker.SeenError());
+
+  EXPECT_THAT(
+      tree,
+      MatchParseTreeNodes(
+          {MatchFunctionWithBody(
+               MatchIfStatement(
+                   MatchCondition(MatchNameReference("a"), MatchConditionEnd()),
+                   MatchCodeBlock(
+                       MatchIfStatement(
+                           MatchCondition(MatchNameReference("b"),
+                                          MatchConditionEnd()),
+                           MatchCodeBlock(MatchExpressionStatement(
+                                              MatchNameReference("c")),
+                                          MatchCodeBlockEnd()),
+                           MatchIfStatementElse(),
+                           MatchCodeBlock(MatchExpressionStatement(
+                                              MatchNameReference("d")),
+                                          MatchCodeBlockEnd())),
+                       MatchCodeBlockEnd()),
+                   MatchIfStatementElse(),
+                   MatchCodeBlock(
+                       MatchExpressionStatement(MatchNameReference("e")),
+                       MatchCodeBlockEnd())),
+               MatchIfStatement(
+                   MatchCondition(MatchNameReference("x"), MatchConditionEnd()),
+                   MatchCodeBlock(
+                       MatchExpressionStatement(MatchCallExpression(
+                           MatchNameReference("G"), MatchLiteral("1"),
+                           MatchCallExpressionEnd())),
+                       MatchCodeBlockEnd()),
+                   MatchIfStatementElse(),
+                   MatchIfStatement(
+                       MatchCondition(MatchNameReference("x"),
+                                      MatchConditionEnd()),
+                       MatchCodeBlock(
+                           MatchExpressionStatement(MatchCallExpression(
+                               MatchNameReference("G"), MatchLiteral("2"),
+                               MatchCallExpressionEnd())),
+                           MatchCodeBlockEnd()),
+                       MatchIfStatementElse(),
+                       MatchCodeBlock(
+                           MatchExpressionStatement(MatchCallExpression(
+                               MatchNameReference("G"), MatchLiteral("3"),
+                               MatchCallExpressionEnd())),
+                           MatchCodeBlockEnd())))),
+           MatchFileEnd()}));
+}
+
+TEST_F(ParseTreeTest, IfElseUnbraced) {
   TokenizedBuffer tokens = GetTokenizedBuffer(
       "fn F() {\n"
       "  if (a)\n"
@@ -711,8 +777,11 @@ TEST_F(ParseTreeTest, IfElse) {
       "  else if (x) { G(2); }\n"
       "  else { G(3); }\n"
       "}");
-  ParseTree tree = ParseTree::Parse(tokens, consumer);
+  ErrorTrackingDiagnosticConsumer error_tracker(consumer);
+  ParseTree tree = ParseTree::Parse(tokens, error_tracker);
+  // The missing braces are invalid, but we should be able to recover.
   EXPECT_FALSE(tree.HasErrors());
+  EXPECT_TRUE(error_tracker.SeenError());
 
   EXPECT_THAT(
       tree,
@@ -786,13 +855,17 @@ TEST_F(ParseTreeTest, WhileBreakContinue) {
   TokenizedBuffer tokens = GetTokenizedBuffer(
       "fn F() {\n"
       "  while (a) {\n"
-      "    if (b)\n"
+      "    if (b) {\n"
       "      break;\n"
-      "    if (c)\n"
+      "    }\n"
+      "    if (c) {\n"
       "      continue;\n"
+      "    }\n"
       "}");
-  ParseTree tree = ParseTree::Parse(tokens, consumer);
+  ErrorTrackingDiagnosticConsumer error_tracker(consumer);
+  ParseTree tree = ParseTree::Parse(tokens, error_tracker);
   EXPECT_FALSE(tree.HasErrors());
+  EXPECT_FALSE(error_tracker.SeenError());
 
   EXPECT_THAT(
       tree,
@@ -800,22 +873,46 @@ TEST_F(ParseTreeTest, WhileBreakContinue) {
           {MatchFunctionWithBody(MatchWhileStatement(
                MatchCondition(MatchNameReference("a"), MatchConditionEnd()),
                MatchCodeBlock(
-                   MatchIfStatement(MatchCondition(MatchNameReference("b"),
-                                                   MatchConditionEnd()),
-                                    MatchBreakStatement(MatchStatementEnd())),
                    MatchIfStatement(
-                       MatchCondition(MatchNameReference("c"),
+                       MatchCondition(MatchNameReference("b"),
                                       MatchConditionEnd()),
-                       MatchContinueStatement(MatchStatementEnd())),
+                       MatchCodeBlock(MatchBreakStatement(MatchStatementEnd()),
+                                      MatchCodeBlockEnd())),
+                   MatchIfStatement(MatchCondition(MatchNameReference("c"),
+                                                   MatchConditionEnd()),
+                                    MatchCodeBlock(MatchContinueStatement(
+                                                       MatchStatementEnd()),
+                                                   MatchCodeBlockEnd())),
                    MatchCodeBlockEnd()))),
            MatchFileEnd()}));
 }
 
+TEST_F(ParseTreeTest, WhileUnbraced) {
+  TokenizedBuffer tokens = GetTokenizedBuffer(
+      "fn F() {\n"
+      "  while (a) \n"
+      "    break;\n"
+      "}");
+  ErrorTrackingDiagnosticConsumer error_tracker(consumer);
+  ParseTree tree = ParseTree::Parse(tokens, error_tracker);
+  EXPECT_FALSE(tree.HasErrors());
+  EXPECT_TRUE(error_tracker.SeenError());
+
+  EXPECT_THAT(
+      tree,
+      MatchParseTreeNodes(
+          {MatchFunctionWithBody(MatchWhileStatement(
+               MatchCondition(MatchNameReference("a"), MatchConditionEnd()),
+               MatchBreakStatement(MatchStatementEnd()))),
+           MatchFileEnd()}));
+}
+
 TEST_F(ParseTreeTest, Return) {
   TokenizedBuffer tokens = GetTokenizedBuffer(
       "fn F() {\n"
-      "  if (c)\n"
+      "  if (c) {\n"
       "    return;\n"
+      "  }\n"
       "}\n"
       "fn G(x: Int) -> Int {\n"
       "  return x;\n"
@@ -828,7 +925,8 @@ TEST_F(ParseTreeTest, Return) {
       MatchParseTreeNodes(
           {MatchFunctionWithBody(MatchIfStatement(
                MatchCondition(MatchNameReference("c"), MatchConditionEnd()),
-               MatchReturnStatement(MatchStatementEnd()))),
+               MatchCodeBlock(MatchReturnStatement(MatchStatementEnd()),
+                              MatchCodeBlockEnd()))),
            MatchFunctionDeclaration(
                MatchDeclaredName(),
                MatchParameters(MatchPatternBinding(MatchDeclaredName("x"), ":",

toolchain/parser/parser_impl.cpp

@@ -60,6 +60,11 @@ struct UnrecognizedDeclaration : SimpleDiagnostic<UnrecognizedDeclaration> {
       "Unrecognized declaration introducer.";
 };
 
+struct ExpectedCodeBlock : SimpleDiagnostic<ExpectedCodeBlock> {
+  static constexpr llvm::StringLiteral ShortName = "syntax-error";
+  static constexpr llvm::StringLiteral Message = "Expected braced code block.";
+};
+
 struct ExpectedExpression : SimpleDiagnostic<ExpectedExpression> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message = "Expected expression.";
@@ -480,8 +485,16 @@ auto ParseTree::Parser::ParseFunctionSignature() -> bool {
   return params.hasValue();
 }
 
-auto ParseTree::Parser::ParseCodeBlock() -> Node {
-  TokenizedBuffer::Token open_curly = Consume(TokenKind::OpenCurlyBrace());
+auto ParseTree::Parser::ParseCodeBlock() -> llvm::Optional<Node> {
+  llvm::Optional<TokenizedBuffer::Token> maybe_open_curly =
+      ConsumeIf(TokenKind::OpenCurlyBrace());
+  if (!maybe_open_curly) {
+    // Recover by parsing a single statement.
+    emitter.EmitError<ExpectedCodeBlock>(*position);
+    return ParseStatement();
+  }
+  TokenizedBuffer::Token open_curly = *maybe_open_curly;
+
   auto start = GetSubtreeStartPosition();
 
   bool has_errors = false;
@@ -549,7 +562,9 @@ auto ParseTree::Parser::ParseFunctionDeclaration() -> Node {
 
   // See if we should parse a definition which is represented as a code block.
   if (NextTokenIs(TokenKind::OpenCurlyBrace())) {
-    ParseCodeBlock();
+    if (!ParseCodeBlock()) {
+      return add_error_function_node();
+    }
   } else if (!ConsumeAndAddLeafNodeIf(TokenKind::Semi(),
                                       ParseNodeKind::DeclarationEnd())) {
     emitter.EmitError<ExpectedFunctionBodyOrSemi>(*position);
@@ -978,11 +993,15 @@ auto ParseTree::Parser::ParseIfStatement() -> llvm::Optional<Node> {
   auto start = GetSubtreeStartPosition();
   auto if_token = Consume(TokenKind::IfKeyword());
   auto cond = ParseParenCondition(TokenKind::IfKeyword());
-  auto then_case = ParseStatement();
+  auto then_case = ParseCodeBlock();
   bool else_has_errors = false;
   if (ConsumeAndAddLeafNodeIf(TokenKind::ElseKeyword(),
                               ParseNodeKind::IfStatementElse())) {
-    else_has_errors = !ParseStatement();
+    // 'else if' is permitted as a special case.
+    if (NextTokenIs(TokenKind::IfKeyword()))
+      else_has_errors = !ParseIfStatement();
+    else
+      else_has_errors = !ParseCodeBlock();
   }
   return AddNode(ParseNodeKind::IfStatement(), if_token, start,
                  /*has_error=*/!cond || !then_case || else_has_errors);
@@ -992,7 +1011,7 @@ auto ParseTree::Parser::ParseWhileStatement() -> llvm::Optional<Node> {
   auto start = GetSubtreeStartPosition();
   auto while_token = Consume(TokenKind::WhileKeyword());
   auto cond = ParseParenCondition(TokenKind::WhileKeyword());
-  auto body = ParseStatement();
+  auto body = ParseCodeBlock();
   return AddNode(ParseNodeKind::WhileStatement(), while_token, start,
                  /*has_error=*/!cond || !body);
 }
@@ -1046,9 +1065,6 @@ auto ParseTree::Parser::ParseStatement() -> llvm::Optional<Node> {
       return ParseKeywordStatement(ParseNodeKind::ReturnStatement(),
                                    KeywordStatementArgument::Optional);
 
-    case TokenKind::OpenCurlyBrace():
-      return ParseCodeBlock();
-
     default:
       // A statement with no introducer token can only be an expression
       // statement.

toolchain/parser/parser_impl.h

@@ -153,7 +153,7 @@ class ParseTree::Parser {
   //
   // These can form the definition for a function or be nested within a function
   // definition. These contain variable declarations and statements.
-  auto ParseCodeBlock() -> Node;
+  auto ParseCodeBlock() -> llvm::Optional<Node>;
 
   // Parses a function declaration with an optional definition. Returns the
   // function parse node which is based on the `fn` introducer keyword.