diff --git a/toolchain/parser/parse_tree_test.cpp b/toolchain/parser/parse_tree_test.cpp index ce3af3034014f..0ebeb053ea6f5 100644 --- a/toolchain/parser/parse_tree_test.cpp +++ b/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"), ":", diff --git a/toolchain/parser/parser_impl.cpp b/toolchain/parser/parser_impl.cpp index d26e8256e996a..320bee94d51ea 100644 --- a/toolchain/parser/parser_impl.cpp +++ b/toolchain/parser/parser_impl.cpp @@ -60,6 +60,11 @@ struct UnrecognizedDeclaration : SimpleDiagnostic { "Unrecognized declaration introducer."; }; +struct ExpectedCodeBlock : SimpleDiagnostic { + static constexpr llvm::StringLiteral ShortName = "syntax-error"; + static constexpr llvm::StringLiteral Message = "Expected braced code block."; +}; + struct ExpectedExpression : SimpleDiagnostic { 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 { + llvm::Optional maybe_open_curly = + ConsumeIf(TokenKind::OpenCurlyBrace()); + if (!maybe_open_curly) { + // Recover by parsing a single statement. + emitter.EmitError(*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(*position); @@ -978,11 +993,15 @@ auto ParseTree::Parser::ParseIfStatement() -> llvm::Optional { 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 { 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 { return ParseKeywordStatement(ParseNodeKind::ReturnStatement(), KeywordStatementArgument::Optional); - case TokenKind::OpenCurlyBrace(): - return ParseCodeBlock(); - default: // A statement with no introducer token can only be an expression // statement. diff --git a/toolchain/parser/parser_impl.h b/toolchain/parser/parser_impl.h index 7861fc82db095..1046a32b9cdbc 100644 --- a/toolchain/parser/parser_impl.h +++ b/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; // Parses a function declaration with an optional definition. Returns the // function parse node which is based on the `fn` introducer keyword.