Bläddra i källkod

Improve parsing of invalid expressions. (#2726)

This addresses crashes for infix operator expressions, but the approach should more generally yield balanced parsed trees.
Jon Ross-Perkins 3 år sedan
förälder
incheckning
a905cdea30

+ 3 - 0
toolchain/parser/parse_node_kind.def

@@ -43,6 +43,9 @@
 // The end of the file.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(FileEnd, 0)
 
+// An invalid expression. Used to balance the parse tree. Always has an error.
+CARBON_PARSE_NODE_KIND_CHILD_COUNT(InvalidExpression, 0)
+
 // An empty declaration, such as `;`.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(EmptyDeclaration, 0)
 

+ 3 - 0
toolchain/parser/parser.cpp

@@ -959,6 +959,9 @@ auto Parser::HandleExpressionInPostfixState() -> void {
       break;
     }
     default: {
+      // Add a node to keep the parse tree balanced.
+      AddLeafNode(ParseNodeKind::InvalidExpression, *position_,
+                  /*has_error=*/true);
       CARBON_DIAGNOSTIC(ExpectedExpression, Error, "Expected expression.");
       emitter_->Emit(*position_, ExpectedExpression);
       ReturnErrorOnState();

+ 7 - 5
toolchain/parser/testdata/if/fail_errors.carbon

@@ -17,10 +17,11 @@
 // CHECK:STDOUT:     {kind: 'CodeBlock', text: '}', subtree_size: 2},
 // CHECK:STDOUT:   {kind: 'IfStatement', text: 'if', subtree_size: 6},
 // CHECK:STDOUT:       {kind: 'IfConditionStart', text: '('},
-// CHECK:STDOUT:     {kind: 'IfCondition', text: ')', has_error: yes, subtree_size: 2},
+// CHECK:STDOUT:       {kind: 'InvalidExpression', text: ')', has_error: yes},
+// CHECK:STDOUT:     {kind: 'IfCondition', text: ')', has_error: yes, subtree_size: 3},
 // CHECK:STDOUT:       {kind: 'CodeBlockStart', text: '{'},
 // CHECK:STDOUT:     {kind: 'CodeBlock', text: '}', subtree_size: 2},
-// CHECK:STDOUT:   {kind: 'IfStatement', text: 'if', subtree_size: 5},
+// CHECK:STDOUT:   {kind: 'IfStatement', text: 'if', subtree_size: 6},
 // CHECK:STDOUT:       {kind: 'IfConditionStart', text: '('},
 // CHECK:STDOUT:       {kind: 'NameReference', text: 'b'},
 // CHECK:STDOUT:     {kind: 'IfCondition', text: ')', has_error: yes, subtree_size: 3},
@@ -31,9 +32,10 @@
 // CHECK:STDOUT:       {kind: 'NameReference', text: 'd'},
 // CHECK:STDOUT:     {kind: 'IfCondition', text: ')', subtree_size: 3},
 // CHECK:STDOUT:       {kind: 'CodeBlockStart', text: '}', has_error: yes},
-// CHECK:STDOUT:     {kind: 'CodeBlock', text: '}', has_error: yes, subtree_size: 2},
-// CHECK:STDOUT:   {kind: 'IfStatement', text: 'if', subtree_size: 6},
-// CHECK:STDOUT: {kind: 'FunctionDefinition', text: '}', subtree_size: 29},
+// CHECK:STDOUT:       {kind: 'InvalidExpression', text: '}', has_error: yes},
+// CHECK:STDOUT:     {kind: 'CodeBlock', text: '}', has_error: yes, subtree_size: 3},
+// CHECK:STDOUT:   {kind: 'IfStatement', text: 'if', subtree_size: 7},
+// CHECK:STDOUT: {kind: 'FunctionDefinition', text: '}', subtree_size: 31},
 // CHECK:STDOUT: {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 

+ 44 - 0
toolchain/parser/testdata/operators/fail_invalid_infix.carbon

@@ -0,0 +1,44 @@
+// 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
+//
+// AUTOUPDATE
+// RUN: %{not} %{carbon-run-parser}
+// CHECK:STDOUT: [
+// CHECK:STDOUT:   {kind: 'VariableIntroducer', text: 'var'},
+// CHECK:STDOUT:     {kind: 'DeclaredName', text: 'a'},
+// CHECK:STDOUT:     {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:   {kind: 'VariableInitializer', text: '='},
+// CHECK:STDOUT:     {kind: 'NameReference', text: 'n'},
+// CHECK:STDOUT:     {kind: 'InvalidExpression', text: ';', has_error: yes},
+// CHECK:STDOUT:   {kind: 'InfixOperator', text: '==', has_error: yes, subtree_size: 3},
+// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 9},
+// CHECK:STDOUT:   {kind: 'VariableIntroducer', text: 'var'},
+// CHECK:STDOUT:     {kind: 'DeclaredName', text: 'b'},
+// CHECK:STDOUT:     {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:   {kind: 'VariableInitializer', text: '='},
+// CHECK:STDOUT:     {kind: 'InvalidExpression', text: '==', has_error: yes},
+// CHECK:STDOUT:     {kind: 'NameReference', text: 'n'},
+// CHECK:STDOUT:   {kind: 'InfixOperator', text: '==', has_error: yes, subtree_size: 3},
+// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 9},
+// CHECK:STDOUT:   {kind: 'VariableIntroducer', text: 'var'},
+// CHECK:STDOUT:     {kind: 'DeclaredName', text: 'c'},
+// CHECK:STDOUT:     {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:   {kind: 'VariableInitializer', text: '='},
+// CHECK:STDOUT:     {kind: 'InvalidExpression', text: '==', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidExpression', text: ';', has_error: yes},
+// CHECK:STDOUT:   {kind: 'InfixOperator', text: '==', has_error: yes, subtree_size: 3},
+// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 9},
+// CHECK:STDOUT: {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT: ]
+
+// CHECK:STDERR: {{.*}}/toolchain/parser/testdata/operators/fail_invalid_infix.carbon:[[@LINE+1]]:19: Expected expression.
+var a: i32 = n == ;
+// CHECK:STDERR: {{.*}}/toolchain/parser/testdata/operators/fail_invalid_infix.carbon:[[@LINE+1]]:14: Expected expression.
+var b: i32 = == n;
+// CHECK:STDERR: {{.*}}/toolchain/parser/testdata/operators/fail_invalid_infix.carbon:[[@LINE+2]]:14: Expected expression.
+// CHECK:STDERR: {{.*}}/toolchain/parser/testdata/operators/fail_invalid_infix.carbon:[[@LINE+1]]:17: Expected expression.
+var c: i32 = == ;

+ 3 - 2
toolchain/parser/testdata/return/fail_no_semi.carbon

@@ -11,8 +11,9 @@
 // CHECK:STDOUT:     {kind: 'ParameterList', text: ')', subtree_size: 2},
 // CHECK:STDOUT:   {kind: 'FunctionDefinitionStart', text: '{', subtree_size: 5},
 // CHECK:STDOUT:     {kind: 'ReturnStatementStart', text: 'return'},
-// CHECK:STDOUT:   {kind: 'ReturnStatement', text: 'return', has_error: yes, subtree_size: 2},
-// CHECK:STDOUT: {kind: 'FunctionDefinition', text: '}', subtree_size: 8},
+// CHECK:STDOUT:     {kind: 'InvalidExpression', text: '}', has_error: yes},
+// CHECK:STDOUT:   {kind: 'ReturnStatement', text: 'return', has_error: yes, subtree_size: 3},
+// CHECK:STDOUT: {kind: 'FunctionDefinition', text: '}', subtree_size: 9},
 // CHECK:STDOUT: {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 

+ 4 - 3
toolchain/parser/testdata/struct/fail_missing_type.carbon

@@ -10,13 +10,14 @@
 // CHECK:STDOUT:       {kind: 'StructLiteralOrStructTypeLiteralStart', text: '{'},
 // CHECK:STDOUT:         {kind: 'DesignatedName', text: 'a'},
 // CHECK:STDOUT:       {kind: 'StructFieldDesignator', text: '.', subtree_size: 2},
+// CHECK:STDOUT:       {kind: 'InvalidExpression', text: '}', has_error: yes},
 // CHECK:STDOUT:       {kind: 'StructFieldUnknown', text: ':', has_error: yes},
-// CHECK:STDOUT:     {kind: 'StructTypeLiteral', text: '}', subtree_size: 5},
-// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 7},
+// CHECK:STDOUT:     {kind: 'StructTypeLiteral', text: '}', subtree_size: 6},
+// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 8},
 // CHECK:STDOUT:   {kind: 'VariableInitializer', text: '='},
 // CHECK:STDOUT:     {kind: 'StructLiteralOrStructTypeLiteralStart', text: '{'},
 // CHECK:STDOUT:   {kind: 'StructLiteral', text: '}', subtree_size: 2},
-// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 12},
+// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 13},
 // CHECK:STDOUT: {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 

+ 4 - 3
toolchain/parser/testdata/struct/fail_missing_value.carbon

@@ -10,13 +10,14 @@
 // CHECK:STDOUT:       {kind: 'StructLiteralOrStructTypeLiteralStart', text: '{'},
 // CHECK:STDOUT:         {kind: 'DesignatedName', text: 'a'},
 // CHECK:STDOUT:       {kind: 'StructFieldDesignator', text: '.', subtree_size: 2},
+// CHECK:STDOUT:       {kind: 'InvalidExpression', text: '}', has_error: yes},
 // CHECK:STDOUT:       {kind: 'StructFieldUnknown', text: '=', has_error: yes},
-// CHECK:STDOUT:     {kind: 'StructLiteral', text: '}', subtree_size: 5},
-// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 7},
+// CHECK:STDOUT:     {kind: 'StructLiteral', text: '}', subtree_size: 6},
+// CHECK:STDOUT:   {kind: 'PatternBinding', text: ':', subtree_size: 8},
 // CHECK:STDOUT:   {kind: 'VariableInitializer', text: '='},
 // CHECK:STDOUT:     {kind: 'StructLiteralOrStructTypeLiteralStart', text: '{'},
 // CHECK:STDOUT:   {kind: 'StructLiteral', text: '}', subtree_size: 2},
-// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 12},
+// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 13},
 // CHECK:STDOUT: {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 

+ 6 - 0
toolchain/semantics/semantics_parse_tree_handler.cpp

@@ -732,6 +732,12 @@ auto SemanticsParseTreeHandler::HandleInterfaceIntroducer(
   return false;
 }
 
+auto SemanticsParseTreeHandler::HandleInvalidExpression(
+    ParseTree::Node parse_node) -> bool {
+  emitter_->Emit(parse_node, SemanticsTodo, "HandleInvalidExpression");
+  return false;
+}
+
 auto SemanticsParseTreeHandler::HandleLiteral(ParseTree::Node parse_node)
     -> bool {
   auto token = parse_tree_->node_token(parse_node);