Start avoiding parse diagnostics on error tokens

An invalid parse due to an error token isn't likely a great diagnostic as it will already have been diagnosed by the lexer. A common case to start handling that is when the parser encounters an invalid token when expecting an expression. This removes a number of unhelpful diagnostics after the lexer has done a good job diagnosing. This also means that there may be parse tree errors that aren't diagnosed when there are lexer-diagnosed errors, so track that.
carbon-language · Oct 20, 2024 · 5fbdd2a · 5fbdd2a
1 parent c6d1893
commit 5fbdd2a
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 31 deletions.
diff --git a/toolchain/check/testdata/basics/type_literals.carbon b/toolchain/check/testdata/basics/type_literals.carbon
@@ -39,10 +39,6 @@ var test_i15: i15;
 // CHECK:STDERR:
 var test_i1000000000: i1000000000;
 // TODO: This diagnostic is not very good.
-// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+12]]:33: error(ExpectedExpr): expected expression
-// CHECK:STDERR: var test_i10000000000000000000: i10000000000000000000;
-// CHECK:STDERR:                                 ^~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
 // CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+8]]:33: error(SemanticsTodo): semantics TODO: `HandleInvalidParse`
 // CHECK:STDERR: var test_i10000000000000000000: i10000000000000000000;
 // CHECK:STDERR:                                 ^~~~~~~~~~~~~~~~~~~~~
@@ -84,10 +80,6 @@ var test_u15: u15;
 // CHECK:STDERR:
 var test_u1000000000: u1000000000;
 // TODO: This diagnostic is not very good.
-// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+12]]:33: error(ExpectedExpr): expected expression
-// CHECK:STDERR: var test_u10000000000000000000: u10000000000000000000;
-// CHECK:STDERR:                                 ^~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
 // CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+8]]:33: error(SemanticsTodo): semantics TODO: `HandleInvalidParse`
 // CHECK:STDERR: var test_u10000000000000000000: u10000000000000000000;
 // CHECK:STDERR:                                 ^~~~~~~~~~~~~~~~~~~~~

diff --git a/toolchain/parse/handle_expr.cpp b/toolchain/parse/handle_expr.cpp
@@ -208,11 +208,16 @@ auto HandleExprInPostfix(Context& context) -> void {
       break;
     }
     default: {
+      CARBON_DIAGNOSTIC(ExpectedExpr, Error, "expected expression");
+      context.emitter().Emit(*context.position(), ExpectedExpr);
+
+      // Fallthrough to the error token case -- we don't need to diagnose those.
+      [[fallthrough]];
+    }
+    case Lex::TokenKind::Error: {
       // Add a node to keep the parse tree balanced.
       context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
                           /*has_error=*/true);
-      CARBON_DIAGNOSTIC(ExpectedExpr, Error, "expected expression");
-      context.emitter().Emit(*context.position(), ExpectedExpr);
       context.ReturnErrorOnState();
       break;
     }

diff --git a/toolchain/parse/parse.cpp b/toolchain/parse/parse.cpp
@@ -45,7 +45,10 @@ auto Parse(Lex::TokenizedBuffer& tokens, DiagnosticConsumer& consumer,
   }
 
   context.AddLeafNode(NodeKind::FileEnd, *context.position());
-  tree.set_has_errors(err_tracker.seen_error());
+
+  // Mark the tree as potentially having errors if there were errors coming in
+  // from the tokenized buffer or we diagnosed new errors.
+  tree.set_has_errors(tokens.has_errors() || err_tracker.seen_error());
 
   if (auto verify = tree.Verify(); !verify.ok()) {
     // TODO: This is temporarily printing to stderr directly during development.

diff --git a/toolchain/parse/testdata/array/fail_require_close_bracket.carbon b/toolchain/parse/testdata/array/fail_require_close_bracket.carbon
@@ -9,11 +9,7 @@
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/array/fail_require_close_bracket.carbon
 // TODO: It should emit only one error message.
 
-// CHECK:STDERR: fail_require_close_bracket.carbon:[[@LINE+7]]:8: error(UnmatchedOpening): opening symbol without a corresponding closing symbol
-// CHECK:STDERR: var x: [i32;;
-// CHECK:STDERR:        ^
-// CHECK:STDERR:
-// CHECK:STDERR: fail_require_close_bracket.carbon:[[@LINE+3]]:8: error(ExpectedExpr): expected expression
+// CHECK:STDERR: fail_require_close_bracket.carbon:[[@LINE+3]]:8: error(UnmatchedOpening): opening symbol without a corresponding closing symbol
 // CHECK:STDERR: var x: [i32;;
 // CHECK:STDERR:        ^
 var x: [i32;;

diff --git a/toolchain/parse/testdata/array/fail_syntax.carbon b/toolchain/parse/testdata/array/fail_syntax.carbon
@@ -38,19 +38,11 @@ fn X() -> [:];
 
 // --- fail_unlexed_expr.carbon
 
-// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+20]]:9: error(UnrecognizedCharacters): encountered unrecognized characters while parsing
+// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+12]]:9: error(UnrecognizedCharacters): encountered unrecognized characters while parsing
 // CHECK:STDERR: var y: [`];
 // CHECK:STDERR:         ^
 // CHECK:STDERR:
-// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+16]]:9: error(ExpectedExpr): expected expression
-// CHECK:STDERR: var y: [`];
-// CHECK:STDERR:         ^
-// CHECK:STDERR:
-// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+12]]:9: error(ExpectedArraySemi): expected `;` in array type
-// CHECK:STDERR: var y: [`];
-// CHECK:STDERR:         ^
-// CHECK:STDERR:
-// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+8]]:9: error(ExpectedExpr): expected expression
+// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+8]]:9: error(ExpectedArraySemi): expected `;` in array type
 // CHECK:STDERR: var y: [`];
 // CHECK:STDERR:         ^
 // CHECK:STDERR:
@@ -62,11 +54,7 @@ var y: [`];
 
 // --- fail_no_close_bracket.carbon
 
-// CHECK:STDERR: fail_no_close_bracket.carbon:[[@LINE+8]]:8: error(UnmatchedOpening): opening symbol without a corresponding closing symbol
-// CHECK:STDERR: var x: [i32;;
-// CHECK:STDERR:        ^
-// CHECK:STDERR:
-// CHECK:STDERR: fail_no_close_bracket.carbon:[[@LINE+4]]:8: error(ExpectedExpr): expected expression
+// CHECK:STDERR: fail_no_close_bracket.carbon:[[@LINE+4]]:8: error(UnmatchedOpening): opening symbol without a corresponding closing symbol
 // CHECK:STDERR: var x: [i32;;
 // CHECK:STDERR:        ^
 // CHECK:STDERR: