Skip to content

Commit

Permalink
[toolchain] Implement #623: require braces.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zygoloid
zygoloid committed Jul 31, 2021
1 parent 3a3a905 commit eff57d9
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 64 deletions.
206 changes: 152 additions & 54 deletions toolchain/parser/parse_tree_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -673,15 +631,55 @@ 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"
" if (b)\n"
" 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,
Expand All @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -786,36 +855,64 @@ 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,
MatchParseTreeNodes(
{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"
Expand All @@ -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"), ":",
Expand Down
34 changes: 25 additions & 9 deletions toolchain/parser/parser_impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion toolchain/parser/parser_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit eff57d9

Please sign in to comment.