From fe2ceb9b5d7c1d173d8d9831a29547e221ebdaa3 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 8 Mar 2021 16:17:55 +0000 Subject: [PATCH] reader/wgsl: Remove ParserImpl::diagnostics() Put all errors straight into the ProgramBuilder::Diagnostics() Fixes a TODO. Kills an assert(). Bug: chromium:1185569 Change-Id: I4e6f3b06106c3cfe75cf2bcdfc56b14ad73e81d9 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44046 Commit-Queue: Ben Clayton Reviewed-by: James Price --- src/reader/wgsl/parser.cc | 6 +----- src/reader/wgsl/parser_impl.cc | 6 ++---- src/reader/wgsl/parser_impl.h | 11 ++-------- src/reader/wgsl/parser_impl_error_msg_test.cc | 20 +++++++++---------- .../wgsl/parser_impl_error_resync_test.cc | 18 ++++++++--------- 5 files changed, 24 insertions(+), 37 deletions(-) diff --git a/src/reader/wgsl/parser.cc b/src/reader/wgsl/parser.cc index 04728ed53f..fee6e1d71e 100644 --- a/src/reader/wgsl/parser.cc +++ b/src/reader/wgsl/parser.cc @@ -25,11 +25,7 @@ namespace wgsl { Program Parse(Source::File const* file) { ParserImpl parser(file); parser.Parse(); - ProgramBuilder builder = std::move(parser.builder()); - // TODO(bclayton): Remove ParserImpl::diagnostics() and put all diagnostic - // into the builder. - builder.Diagnostics().add(parser.diagnostics()); - return Program(std::move(builder)); + return Program(std::move(parser.builder())); } } // namespace wgsl diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 80d47704cc..fee71dadeb 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -241,7 +241,7 @@ ParserImpl::Failure::Errored ParserImpl::add_error(const Token& t, ParserImpl::Failure::Errored ParserImpl::add_error(const Source& source, const std::string& err) { if (silence_errors_ == 0) { - diags_.add_error(err, source); + builder_.Diagnostics().add_error(err, source); } return Failure::kErrored; } @@ -293,14 +293,12 @@ void ParserImpl::translation_unit() { break; } expect_global_decl(); - if (diags_.error_count() >= max_errors_) { + if (builder_.Diagnostics().error_count() >= max_errors_) { add_error(Source{{}, p.source().file_path}, "stopping after " + std::to_string(max_errors_) + " errors"); break; } } - - assert(builder_.IsValid()); } // global_decl diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index 2573a0cc1a..e6008a12b3 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -293,20 +293,14 @@ class ParserImpl { size_t get_max_errors() const { return max_errors_; } /// @returns true if an error was encountered. - bool has_error() const { return diags_.contains_errors(); } + bool has_error() const { return builder_.Diagnostics().contains_errors(); } /// @returns the parser error string std::string error() const { diag::Formatter formatter{{false, false, false, false}}; - return formatter.format(diags_); + return formatter.format(builder_.Diagnostics()); } - /// @returns the diagnostic messages - const diag::List& diagnostics() const { return diags_; } - - /// @returns the diagnostic messages - diag::List& diagnostics() { return diags_; } - /// @returns the Program. The program builder in the parser will be reset /// after this. Program program() { return Program(std::move(builder_)); } @@ -827,7 +821,6 @@ class ParserImpl { return builder_.create(std::forward(args)...); } - diag::List diags_; std::unique_ptr lexer_; std::deque token_queue_; bool synchronized_ = true; diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index 1497c8c1ab..e5ffb302f7 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -27,16 +27,16 @@ const diag::Formatter::Style formatter_style{ class ParserImplErrorTest : public ParserImplTest {}; -#define EXPECT(SOURCE, EXPECTED) \ - do { \ - std::string source = SOURCE; \ - std::string expected = EXPECTED; \ - auto p = parser(source); \ - p->set_max_errors(5); \ - EXPECT_EQ(false, p->Parse()); \ - EXPECT_EQ(true, p->diagnostics().contains_errors()); \ - EXPECT_EQ(expected, \ - diag::Formatter(formatter_style).format(p->diagnostics())); \ +#define EXPECT(SOURCE, EXPECTED) \ + do { \ + std::string source = SOURCE; \ + std::string expected = EXPECTED; \ + auto p = parser(source); \ + p->set_max_errors(5); \ + EXPECT_EQ(false, p->Parse()); \ + auto diagnostics = p->builder().Diagnostics(); \ + EXPECT_EQ(true, diagnostics.contains_errors()); \ + EXPECT_EQ(expected, diag::Formatter(formatter_style).format(diagnostics)); \ } while (false) TEST_F(ParserImplErrorTest, AdditiveInvalidExpr) { diff --git a/src/reader/wgsl/parser_impl_error_resync_test.cc b/src/reader/wgsl/parser_impl_error_resync_test.cc index be671af000..010b5455b9 100644 --- a/src/reader/wgsl/parser_impl_error_resync_test.cc +++ b/src/reader/wgsl/parser_impl_error_resync_test.cc @@ -27,15 +27,15 @@ const diag::Formatter::Style formatter_style{ class ParserImplErrorResyncTest : public ParserImplTest {}; -#define EXPECT(SOURCE, EXPECTED) \ - do { \ - std::string source = SOURCE; \ - std::string expected = EXPECTED; \ - auto p = parser(source); \ - EXPECT_EQ(false, p->Parse()); \ - EXPECT_EQ(true, p->diagnostics().contains_errors()); \ - EXPECT_EQ(expected, \ - diag::Formatter(formatter_style).format(p->diagnostics())); \ +#define EXPECT(SOURCE, EXPECTED) \ + do { \ + std::string source = SOURCE; \ + std::string expected = EXPECTED; \ + auto p = parser(source); \ + EXPECT_EQ(false, p->Parse()); \ + auto diagnostics = p->builder().Diagnostics(); \ + EXPECT_EQ(true, diagnostics.contains_errors()); \ + EXPECT_EQ(expected, diag::Formatter(formatter_style).format(diagnostics)); \ } while (false) TEST_F(ParserImplErrorResyncTest, BadFunctionDecls) {