Fix parse to use the error tracking consumer for has_errors_. (#4261)

This is how we are setting has_errors_ in other stages; this should only make parse consistent. Fixes #4259
carbon-language · Aug 28, 2024 · b72826c · b72826c
1 parent 702d0d8
commit b72826c
Show file tree

Hide file tree

Showing 13 changed files with 52 additions and 57 deletions.
diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp
@@ -69,17 +69,11 @@ Context::Context(Tree& tree, Lex::TokenizedBuffer& tokens,
 auto Context::AddLeafNode(NodeKind kind, Lex::TokenIndex token, bool has_error)
     -> void {
   tree_->node_impls_.push_back(Tree::NodeImpl(kind, has_error, token));
-  if (has_error) {
-    tree_->has_errors_ = true;
-  }
 }
 
 auto Context::AddNode(NodeKind kind, Lex::TokenIndex token, bool has_error)
     -> void {
   tree_->node_impls_.push_back(Tree::NodeImpl(kind, has_error, token));
-  if (has_error) {
-    tree_->has_errors_ = true;
-  }
 }
 
 auto Context::ReplacePlaceholderNode(int32_t position, NodeKind kind,
@@ -92,9 +86,6 @@ auto Context::ReplacePlaceholderNode(int32_t position, NodeKind kind,
   node_impl->kind = kind;
   node_impl->has_error = has_error;
   node_impl->token = token;
-  if (has_error) {
-    tree_->has_errors_ = true;
-  }
 }
 
 auto Context::ConsumeAndAddOpenParen(Lex::TokenIndex default_token,

diff --git a/toolchain/parse/parse.cpp b/toolchain/parse/parse.cpp
@@ -20,7 +20,8 @@ auto HandleInvalid(Context& context) -> void {
 auto Parse(Lex::TokenizedBuffer& tokens, DiagnosticConsumer& consumer,
            llvm::raw_ostream* vlog_stream) -> Tree {
   Lex::TokenDiagnosticConverter converter(&tokens);
-  Lex::TokenDiagnosticEmitter emitter(converter, consumer);
+  ErrorTrackingDiagnosticConsumer err_tracker(consumer);
+  Lex::TokenDiagnosticEmitter emitter(converter, err_tracker);
 
   // Delegate to the parser.
   Tree tree(tokens);
@@ -44,6 +45,7 @@ auto Parse(Lex::TokenizedBuffer& tokens, DiagnosticConsumer& consumer,
   }
 
   context.AddLeafNode(NodeKind::FileEnd, *context.position());
+  tree.set_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/.../recover_infix_uneven_space_before.carbon → ...ors/fail_infix_uneven_space_before.carbon b/.../recover_infix_uneven_space_before.carbon → ...ors/fail_infix_uneven_space_before.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_infix_uneven_space_before.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_infix_uneven_space_before.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_infix_uneven_space_before.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_infix_uneven_space_before.carbon
 
-// CHECK:STDERR: recover_infix_uneven_space_before.carbon:[[@LINE+3]]:15: ERROR: Whitespace missing after binary operator.
+// CHECK:STDERR: fail_infix_uneven_space_before.carbon:[[@LINE+3]]:15: ERROR: Whitespace missing after binary operator.
 // CHECK:STDERR: var n: i8 = n *n;
 // CHECK:STDERR:               ^
 var n: i8 = n *n;
 
-// CHECK:STDOUT: - filename: recover_infix_uneven_space_before.carbon
+// CHECK:STDOUT: - filename: fail_infix_uneven_space_before.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...ta/operators/recover_postfix_space.carbon → ...tdata/operators/fail_postfix_space.carbon b/...ta/operators/recover_postfix_space.carbon → ...tdata/operators/fail_postfix_space.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_postfix_space.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_postfix_space.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_postfix_space.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_postfix_space.carbon
 
-// CHECK:STDERR: recover_postfix_space.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
+// CHECK:STDERR: fail_postfix_space.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
 // CHECK:STDERR: var v: type = i8 *;
 // CHECK:STDERR:                  ^
 var v: type = i8 *;
 
-// CHECK:STDOUT: - filename: recover_postfix_space.carbon
+// CHECK:STDOUT: - filename: fail_postfix_space.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...recover_postfix_space_before_comma.carbon → ...rs/fail_postfix_space_before_comma.carbon b/...recover_postfix_space_before_comma.carbon → ...rs/fail_postfix_space_before_comma.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_postfix_space_before_comma.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_postfix_space_before_comma.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_postfix_space_before_comma.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_postfix_space_before_comma.carbon
 
-// CHECK:STDERR: recover_postfix_space_before_comma.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
+// CHECK:STDERR: fail_postfix_space_before_comma.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
 // CHECK:STDERR: var n: i8 = F(i8 *, 0);
 // CHECK:STDERR:                  ^
 var n: i8 = F(i8 *, 0);
 
-// CHECK:STDOUT: - filename: recover_postfix_space_before_comma.carbon
+// CHECK:STDOUT: - filename: fail_postfix_space_before_comma.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...tors/recover_postfix_space_in_call.carbon → ...erators/fail_postfix_space_in_call.carbon b/...tors/recover_postfix_space_in_call.carbon → ...erators/fail_postfix_space_in_call.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_postfix_space_in_call.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_postfix_space_in_call.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_postfix_space_in_call.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_postfix_space_in_call.carbon
 
-// CHECK:STDERR: recover_postfix_space_in_call.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
+// CHECK:STDERR: fail_postfix_space_in_call.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
 // CHECK:STDERR: var n: i8 = F(i8 *);
 // CHECK:STDERR:                  ^
 var n: i8 = F(i8 *);
 
-// CHECK:STDOUT: - filename: recover_postfix_space_in_call.carbon
+// CHECK:STDOUT: - filename: fail_postfix_space_in_call.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/.../recover_postfix_space_surrounding.carbon → ...ors/fail_postfix_space_surrounding.carbon b/.../recover_postfix_space_surrounding.carbon → ...ors/fail_postfix_space_surrounding.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_postfix_space_surrounding.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_postfix_space_surrounding.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_postfix_space_surrounding.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_postfix_space_surrounding.carbon
 
-// CHECK:STDERR: recover_postfix_space_surrounding.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
+// CHECK:STDERR: fail_postfix_space_surrounding.carbon:[[@LINE+3]]:18: ERROR: Whitespace is not allowed before this unary operator.
 // CHECK:STDERR: var v: type = i8 * ;
 // CHECK:STDERR:                  ^
 var v: type = i8 * ;
 
-// CHECK:STDOUT: - filename: recover_postfix_space_surrounding.carbon
+// CHECK:STDOUT: - filename: fail_postfix_space_surrounding.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...ta/operators/recover_prefix_repeat.carbon → ...tdata/operators/fail_prefix_repeat.carbon b/...ta/operators/recover_prefix_repeat.carbon → ...tdata/operators/fail_prefix_repeat.carbon
@@ -4,17 +4,17 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_prefix_repeat.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_prefix_repeat.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_prefix_repeat.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_prefix_repeat.carbon
 
-// CHECK:STDERR: recover_prefix_repeat.carbon:[[@LINE+3]]:17: ERROR: Parentheses are required around this unary `const` operator.
+// CHECK:STDERR: fail_prefix_repeat.carbon:[[@LINE+3]]:17: ERROR: Parentheses are required around this unary `const` operator.
 // CHECK:STDERR: fn F() -> const const i32* {
 // CHECK:STDERR:                 ^~~~~
 fn F() -> const const i32* {
 }
 
-// CHECK:STDOUT: - filename: recover_prefix_repeat.carbon
+// CHECK:STDOUT: - filename: fail_prefix_repeat.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:         {kind: 'FunctionIntroducer', text: 'fn'},

diff --git a/...ata/operators/recover_prefix_space.carbon → ...stdata/operators/fail_prefix_space.carbon b/...ata/operators/recover_prefix_space.carbon → ...stdata/operators/fail_prefix_space.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_prefix_space.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_prefix_space.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_prefix_space.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_prefix_space.carbon
 
-// CHECK:STDERR: recover_prefix_space.carbon:[[@LINE+3]]:13: ERROR: Whitespace is not allowed after this unary operator.
+// CHECK:STDERR: fail_prefix_space.carbon:[[@LINE+3]]:13: ERROR: Whitespace is not allowed after this unary operator.
 // CHECK:STDERR: var n: i8 = - n;
 // CHECK:STDERR:             ^
 var n: i8 = - n;
 
-// CHECK:STDOUT: - filename: recover_prefix_space.carbon
+// CHECK:STDOUT: - filename: fail_prefix_space.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...er_prefix_uneven_space_with_assign.carbon → ...il_prefix_uneven_space_with_assign.carbon b/...er_prefix_uneven_space_with_assign.carbon → ...il_prefix_uneven_space_with_assign.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_prefix_uneven_space_with_assign.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_prefix_uneven_space_with_assign.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_prefix_uneven_space_with_assign.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_prefix_uneven_space_with_assign.carbon
 
-// CHECK:STDERR: recover_prefix_uneven_space_with_assign.carbon:[[@LINE+3]]:12: ERROR: Whitespace is not allowed after this unary operator.
+// CHECK:STDERR: fail_prefix_uneven_space_with_assign.carbon:[[@LINE+3]]:12: ERROR: Whitespace is not allowed after this unary operator.
 // CHECK:STDERR: var n: i8 =- n;
 // CHECK:STDERR:            ^
 var n: i8 =- n;
 
-// CHECK:STDOUT: - filename: recover_prefix_uneven_space_with_assign.carbon
+// CHECK:STDOUT: - filename: fail_prefix_uneven_space_with_assign.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...tdata/operators/recover_star_minus.carbon → ...testdata/operators/fail_star_minus.carbon b/...tdata/operators/recover_star_minus.carbon → ...testdata/operators/fail_star_minus.carbon
@@ -4,21 +4,21 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_star_minus.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_star_minus.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_star_minus.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_star_minus.carbon
 
 // TODO: There are two possible fixes that would make this expression legal:
 // `n * -n` and `n* - n`. The parser doesn't realize that the first fix is
 // available because it has already accepted that first part of the expression,
 // so it is recovering by using the second option, but the diagnostic should
 // ideally offer (or consider) both fixes as alternatives.
-// CHECK:STDERR: recover_star_minus.carbon:[[@LINE+3]]:16: ERROR: Whitespace missing after binary operator.
+// CHECK:STDERR: fail_star_minus.carbon:[[@LINE+3]]:16: ERROR: Whitespace missing after binary operator.
 // CHECK:STDERR: var n: i8 = n* -n;
 // CHECK:STDERR:                ^
 var n: i8 = n* -n;
 
-// CHECK:STDOUT: - filename: recover_star_minus.carbon
+// CHECK:STDOUT: - filename: fail_star_minus.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/...stdata/operators/recover_star_star.carbon → .../testdata/operators/fail_star_star.carbon b/...stdata/operators/recover_star_star.carbon → .../testdata/operators/fail_star_star.carbon
@@ -4,16 +4,16 @@
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/recover_star_star.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/operators/fail_star_star.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/recover_star_star.carbon
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/operators/fail_star_star.carbon
 
-// CHECK:STDERR: recover_star_star.carbon:[[@LINE+3]]:16: ERROR: Whitespace missing after binary operator.
+// CHECK:STDERR: fail_star_star.carbon:[[@LINE+3]]:16: ERROR: Whitespace missing after binary operator.
 // CHECK:STDERR: var n: i8 = n* *p;
 // CHECK:STDERR:                ^
 var n: i8 = n* *p;
 
-// CHECK:STDOUT: - filename: recover_star_star.carbon
+// CHECK:STDOUT: - filename: fail_star_star.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:       {kind: 'VariableIntroducer', text: 'var'},

diff --git a/toolchain/parse/tree.h b/toolchain/parse/tree.h
@@ -103,9 +103,10 @@ class Tree : public Printable<Tree> {
     node_impls_.reserve(tokens_->expected_parse_tree_size());
   }
 
-  // Tests whether there are any errors in the parse tree.
   auto has_errors() const -> bool { return has_errors_; }
 
+  auto set_has_errors(bool has_errors) -> void { has_errors_ = has_errors; }
+
   // Returns the number of nodes in this parse tree.
   auto size() const -> int { return node_impls_.size(); }
 
@@ -242,14 +243,15 @@ class Tree : public Printable<Tree> {
 
   Lex::TokenizedBuffer* tokens_;
 
-  // Indicates if any errors were encountered while parsing.
+  // True if any lowering-blocking issues were encountered while parsing. Trees
+  // are expected to still be structurally valid for checking.
   //
   // This doesn't indicate how much of the tree is structurally accurate with
-  // respect to the grammar. That can be identified by looking at the `HasError`
-  // flag for a given node (see above for details). This simply indicates that
-  // some errors were encountered somewhere. A key implication is that when this
-  // is true we do *not* have the expected 1:1 mapping between tokens and parsed
-  // nodes as some tokens may have been skipped.
+  // respect to the grammar. That can be identified by looking at
+  // `node_has_error` (see above for details). This simply indicates that some
+  // errors were encountered somewhere. A key implication is that when this is
+  // true we do *not* enforce the expected 1:1 mapping between tokens and parsed
+  // nodes, because some tokens may have been skipped.
   bool has_errors_ = false;
 
   std::optional<PackagingDecl> packaging_decl_;