tint: Use std::string_view for diagnostics

And also prevent diagnostics (not just errors) in the WGSL parser when resyncing.

Change-Id: I54f433b7581a5ff1f4ec288121e23a4dc5880092
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/124220
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>

src/tint/diagnostic/diagnostic.h

@@ -141,7 +141,7 @@ class List {
     /// @param system the system raising the note message
     /// @param note_msg the note message
     /// @param source the source of the note diagnostic
-    void add_note(System system, const std::string& note_msg, const Source& source) {
+    void add_note(System system, std::string_view note_msg, const Source& source) {
         diag::Diagnostic note{};
         note.severity = diag::Severity::Note;
         note.system = system;
@@ -154,7 +154,7 @@ class List {
     /// @param system the system raising the warning message
     /// @param warning_msg the warning message
     /// @param source the source of the warning diagnostic
-    void add_warning(System system, const std::string& warning_msg, const Source& source) {
+    void add_warning(System system, std::string_view warning_msg, const Source& source) {
         diag::Diagnostic warning{};
         warning.severity = diag::Severity::Warning;
         warning.system = system;
@@ -166,11 +166,11 @@ class List {
     /// adds the error message without a source to the end of this list.
     /// @param system the system raising the error message
     /// @param err_msg the error message
-    void add_error(System system, std::string err_msg) {
+    void add_error(System system, std::string_view err_msg) {
         diag::Diagnostic error{};
         error.severity = diag::Severity::Error;
         error.system = system;
-        error.message = std::move(err_msg);
+        error.message = err_msg;
         add(std::move(error));
     }
 
@@ -178,12 +178,12 @@ class List {
     /// @param system the system raising the error message
     /// @param err_msg the error message
     /// @param source the source of the error diagnostic
-    void add_error(System system, std::string err_msg, const Source& source) {
+    void add_error(System system, std::string_view err_msg, const Source& source) {
         diag::Diagnostic error{};
         error.severity = diag::Severity::Error;
         error.system = system;
         error.source = source;
-        error.message = std::move(err_msg);
+        error.message = err_msg;
         add(std::move(error));
     }
 
@@ -193,13 +193,16 @@ class List {
     /// @param code the error code
     /// @param err_msg the error message
     /// @param source the source of the error diagnostic
-    void add_error(System system, const char* code, std::string err_msg, const Source& source) {
+    void add_error(System system,
+                   const char* code,
+                   std::string_view err_msg,
+                   const Source& source) {
         diag::Diagnostic error{};
         error.code = code;
         error.severity = diag::Severity::Error;
         error.system = system;
         error.source = source;
-        error.message = std::move(err_msg);
+        error.message = err_msg;
         add(std::move(error));
     }
 
@@ -209,7 +212,7 @@ class List {
     /// @param source the source of the internal compiler error
     /// @param file the Source::File owned by this diagnostic
     void add_ice(System system,
-                 const std::string& err_msg,
+                 std::string_view err_msg,
                  const Source& source,
                  std::shared_ptr<Source::File> file) {
         diag::Diagnostic ice{};

src/tint/reader/wgsl/parser_impl.cc

@@ -213,30 +213,41 @@ ParserImpl::~ParserImpl() = default;
 ParserImpl::Failure::Errored ParserImpl::add_error(const Source& source,
                                                    std::string_view err,
                                                    std::string_view use) {
-    utils::StringStream msg;
+    if (silence_diags_ == 0) {
-    msg << err;
+        utils::StringStream msg;
-    if (!use.empty()) {
+        msg << err;
-        msg << " for " << use;
+        if (!use.empty()) {
+            msg << " for " << use;
+        }
+        add_error(source, msg.str());
     }
-    add_error(source, msg.str());
     return Failure::kErrored;
 }
 
-ParserImpl::Failure::Errored ParserImpl::add_error(const Token& t, const std::string& err) {
+ParserImpl::Failure::Errored ParserImpl::add_error(const Token& t, std::string_view err) {
     add_error(t.source(), err);
     return Failure::kErrored;
 }
 
-ParserImpl::Failure::Errored ParserImpl::add_error(const Source& source, const std::string& err) {
+ParserImpl::Failure::Errored ParserImpl::add_error(const Source& source, std::string_view err) {
-    if (silence_errors_ == 0) {
+    if (silence_diags_ == 0) {
         builder_.Diagnostics().add_error(diag::System::Reader, err, source);
     }
     return Failure::kErrored;
 }
 
-void ParserImpl::deprecated(const Source& source, const std::string& msg) {
+void ParserImpl::add_note(const Source& source, std::string_view err) {
-    builder_.Diagnostics().add_warning(diag::System::Reader,
+    if (silence_diags_ == 0) {
-                                       "use of deprecated language feature: " + msg, source);
+        builder_.Diagnostics().add_note(diag::System::Reader, err, source);
+    }
+}
+
+void ParserImpl::deprecated(const Source& source, std::string_view msg) {
+    if (silence_diags_ == 0) {
+        builder_.Diagnostics().add_warning(
+            diag::System::Reader, "use of deprecated language feature: " + std::string(msg),
+            source);
+    }
 }
 
 const Token& ParserImpl::next() {
@@ -605,7 +616,7 @@ Maybe<Void> ParserImpl::global_decl() {
 
     // We have a statement outside of a function?
     auto& t = peek();
-    auto stat = without_error([&] { return statement(); });
+    auto stat = without_diag([&] { return statement(); });
     if (stat.matched) {
         // Attempt to jump to the next '}' - the function might have just been
         // missing an opening line.
@@ -3372,10 +3383,10 @@ bool ParserImpl::handle_error(const Token& t) {
 }
 
 template <typename F, typename T>
-T ParserImpl::without_error(F&& body) {
+T ParserImpl::without_diag(F&& body) {
-    silence_errors_++;
+    silence_diags_++;
     auto result = body();
-    silence_errors_--;
+    silence_diags_--;
     return result;
 }
 

src/tint/reader/wgsl/parser_impl.h

@@ -348,7 +348,7 @@ class ParserImpl {
     /// @param msg the error message
     /// @return `Failure::Errored::kError` so that you can combine an add_error()
     /// call and return on the same line.
-    Failure::Errored add_error(const Token& t, const std::string& msg);
+    Failure::Errored add_error(const Token& t, std::string_view msg);
     /// Appends an error raised when parsing `use` at `t` with the message
     /// `msg`
     /// @param source the source to associate the error with
@@ -363,12 +363,16 @@ class ParserImpl {
     /// @param msg the error message
     /// @return `Failure::Errored::kError` so that you can combine an add_error()
     /// call and return on the same line.
-    Failure::Errored add_error(const Source& source, const std::string& msg);
+    Failure::Errored add_error(const Source& source, std::string_view msg);
+    /// Appends a note at `source` with the message `msg`
+    /// @param source the source to associate the error with
+    /// @param msg the note message
+    void add_note(const Source& source, std::string_view msg);
     /// Appends a deprecated-language-feature warning at `source` with the message
     /// `msg`
     /// @param source the source to associate the error with
     /// @param msg the warning message
-    void deprecated(const Source& source, const std::string& msg);
+    void deprecated(const Source& source, std::string_view msg);
     /// Parses the `translation_unit` grammar element
     void translation_unit();
     /// Parses the `global_directive` grammar element, erroring on parse failure.
@@ -805,14 +809,13 @@ class ParserImpl {
         return synchronized_ && builder_.Diagnostics().error_count() < max_errors_;
     }
 
-    /// without_error() calls the function `func` muting any grammatical errors
+    /// without_diag() calls the function `func` muting any diagnostics found while executing the
-    /// found while executing the function. This can be used as a best-effort to
+    /// function. This can be used to silence spew when attempting to resynchronize the parser.
-    /// produce a meaningful error message when the parser is out of sync.
     /// @param func a function or lambda with the signature: `Expect<Result>()` or
     /// `Maybe<Result>()`.
     /// @return the value returned by `func`
     template <typename F, typename T = ReturnType<F>>
-    T without_error(F&& func);
+    T without_diag(F&& func);
 
     /// Reports an error if the attribute list `list` is not empty.
     /// Used to ensure that all attributes are consumed.
@@ -856,7 +859,7 @@ class ParserImpl {
     bool synchronized_ = true;
     uint32_t parse_depth_ = 0;
     std::vector<Token::Type> sync_tokens_;
-    int silence_errors_ = 0;
+    int silence_diags_ = 0;
     ProgramBuilder builder_;
     size_t max_errors_ = 25;
 };