From 1b2eb42c5a080873935525ccabde9fc7dd43e444 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Fri, 1 Nov 2024 22:53:27 -0700 Subject: [PATCH] Start avoiding parse diagnostics on error tokens (#4431) 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. Follow-up to #4430 that almost finishes addressing its diagnostic TODO. --- .../check/testdata/basics/type_literals.carbon | 8 -------- toolchain/parse/handle_expr.cpp | 10 +++++++--- toolchain/parse/parse.cpp | 5 ++++- .../array/fail_require_close_bracket.carbon | 6 +----- .../parse/testdata/array/fail_syntax.carbon | 18 +++--------------- 5 files changed, 15 insertions(+), 32 deletions(-) diff --git a/toolchain/check/testdata/basics/type_literals.carbon b/toolchain/check/testdata/basics/type_literals.carbon index 00a0c1f37daa..41a5db08ed78 100644 --- 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: expected expression [ExpectedExpr] -// CHECK:STDERR: var test_i10000000000000000000: i10000000000000000000; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: // CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+8]]:33: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] // 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: expected expression [ExpectedExpr] -// CHECK:STDERR: var test_u10000000000000000000: u10000000000000000000; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: // CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+8]]:33: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] // CHECK:STDERR: var test_u10000000000000000000: u10000000000000000000; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ diff --git a/toolchain/parse/handle_expr.cpp b/toolchain/parse/handle_expr.cpp index 489ad8ec1603..8e22b126caf3 100644 --- a/toolchain/parse/handle_expr.cpp +++ b/toolchain/parse/handle_expr.cpp @@ -73,7 +73,7 @@ auto HandleExprInPostfix(Context& context) -> void { // Parses a primary expression, which is either a terminal portion of an // expression tree, such as an identifier or literal, or a parenthesized // expression. - switch (context.PositionKind()) { + switch (auto token_kind = context.PositionKind()) { case Lex::TokenKind::Identifier: { context.AddLeafNode(NodeKind::IdentifierNameExpr, context.Consume()); context.PushState(state); @@ -208,11 +208,15 @@ auto HandleExprInPostfix(Context& context) -> void { break; } default: { + // If not already diagnosed in the lexer, diagnose it here. + if (token_kind != Lex::TokenKind::Error) { + CARBON_DIAGNOSTIC(ExpectedExpr, Error, "expected expression"); + context.emitter().Emit(*context.position(), ExpectedExpr); + } + // 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 index d6c5642032cc..8c98ed10a5b1 100644 --- 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 index 25f8f3631768..dd6b12a5f620 100644 --- 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: opening symbol without a corresponding closing symbol [UnmatchedOpening] -// CHECK:STDERR: var x: [i32;; -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDERR: fail_require_close_bracket.carbon:[[@LINE+3]]:8: error: expected expression [ExpectedExpr] +// CHECK:STDERR: fail_require_close_bracket.carbon:[[@LINE+3]]:8: error: opening symbol without a corresponding closing symbol [UnmatchedOpening] // 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 index 0eb6bd90422f..9b3632700ed2 100644 --- 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: encountered unrecognized characters while parsing [UnrecognizedCharacters] +// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+12]]:9: error: encountered unrecognized characters while parsing [UnrecognizedCharacters] // CHECK:STDERR: var y: [`]; // CHECK:STDERR: ^ // CHECK:STDERR: -// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+16]]:9: error: expected expression [ExpectedExpr] -// CHECK:STDERR: var y: [`]; -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+12]]:9: error: expected `;` in array type [ExpectedArraySemi] -// CHECK:STDERR: var y: [`]; -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+8]]:9: error: expected expression [ExpectedExpr] +// CHECK:STDERR: fail_unlexed_expr.carbon:[[@LINE+8]]:9: error: expected `;` in array type [ExpectedArraySemi] // 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: opening symbol without a corresponding closing symbol [UnmatchedOpening] -// CHECK:STDERR: var x: [i32;; -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDERR: fail_no_close_bracket.carbon:[[@LINE+4]]:8: error: expected expression [ExpectedExpr] +// CHECK:STDERR: fail_no_close_bracket.carbon:[[@LINE+4]]:8: error: opening symbol without a corresponding closing symbol [UnmatchedOpening] // CHECK:STDERR: var x: [i32;; // CHECK:STDERR: ^ // CHECK:STDERR: