[#4365] Do not allow --Wdisable or --Wwarn to demote errors. Allo…

…w `--Winfo=diagnostic` to work for `diagnostic`s that can be both warnings and errors. (#4366)
p4lang · Feb 7, 2024 · 8bef42a · 8bef42a
1 parent 6cec6b4
commit 8bef42a
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 16 deletions.
diff --git a/frontends/common/parser_options.cpp b/frontends/common/parser_options.cpp
@@ -192,6 +192,10 @@ ParserOptions::ParserOptions() : Util::Options(defaultMessage) {
         "--Wdisable", "diagnostic",
         [](const char *diagnostic) {
             if (diagnostic) {
+                if (ErrorCatalog::getCatalog().isError(diagnostic)) {
+                    ::error(ErrorType::ERR_INVALID, "Error %1% cannot be demoted.", diagnostic);
+                    return false;
+                }
                 P4CContext::get().setDiagnosticAction(diagnostic, DiagnosticAction::Ignore);
             } else {
                 auto action = DiagnosticAction::Ignore;
@@ -207,7 +211,7 @@ ParserOptions::ParserOptions() : Util::Options(defaultMessage) {
         [](const char *diagnostic) {
             if (diagnostic) {
                 if (ErrorCatalog::getCatalog().isError(diagnostic)) {
-                    ::error(ErrorType::ERR_INVALID, "Error %1% cannot be demoted", diagnostic);
+                    ::error(ErrorType::ERR_INVALID, "Error %1% cannot be demoted.", diagnostic);
                     return false;
                 }
                 P4CContext::get().setDiagnosticAction(diagnostic, DiagnosticAction::Info);
@@ -219,6 +223,10 @@ ParserOptions::ParserOptions() : Util::Options(defaultMessage) {
         "--Wwarn", "diagnostic",
         [](const char *diagnostic) {
             if (diagnostic) {
+                if (ErrorCatalog::getCatalog().isError(diagnostic)) {
+                    ::error(ErrorType::ERR_INVALID, "Error %1% cannot be demoted.", diagnostic);
+                    return false;
+                }
                 P4CContext::get().setDiagnosticAction(diagnostic, DiagnosticAction::Warn);
             } else {
                 auto action = DiagnosticAction::Warn;

diff --git a/lib/error_catalog.h b/lib/error_catalog.h
@@ -127,21 +127,20 @@ class ErrorCatalog {
         return "--unknown--";
     }
 
-    /// retrieve the code for name
-    int getCode(cstring name) {
-        auto it =
-            std::find_if(errorCatalog.begin(), errorCatalog.end(),
-                         [name](const std::pair<int, cstring> &p) { return p.second == name; });
-        if (it != errorCatalog.end()) return it->first;
-        return -1;
-    }
-
-    /// return true if the name is an error; false otherwise
+    /// return true if the given diagnostic can _only_ be an error; false otherwise
     bool isError(cstring name) {
-        int code = getCode(name);
-        if (code == -1) return false;
-        if (code > ErrorType::ERR_MAX) return false;
-        return true;
+        // Some diagnostics might be both errors and warning/info
+        // (e.g. "invalid" -> both ERR_INVALID and WARN_INVALID).
+        bool error = false;
+        for (const auto &pair : errorCatalog) {
+            if (pair.second == name) {
+                if (pair.first < ErrorType::LEGACY_ERROR || pair.first > ErrorType::ERR_MAX)
+                    return false;
+                error = true;
+            }
+        }
+
+        return error;
     }
 
  private:

diff --git a/lib/error_reporter.h b/lib/error_reporter.h
@@ -231,10 +231,12 @@ class ErrorReporter {
     /// @return the action to take for the given diagnostic, falling back to the
     /// default action if it wasn't overridden via the command line or a pragma.
     DiagnosticAction getDiagnosticAction(cstring diagnostic, DiagnosticAction defaultAction) {
+        // Actions for errors can never be overridden.
+        if (defaultAction == DiagnosticAction::Error) return defaultAction;
         auto it = diagnosticActions.find(diagnostic);
         if (it != diagnosticActions.end()) return it->second;
         // if we're dealing with warnings and they have been globally modified
-        // (ingnored or turned into errors), then return the global default
+        // (ignored or turned into errors), then return the global default
         if (defaultAction == DiagnosticAction::Warn &&
             defaultWarningDiagnosticAction != DiagnosticAction::Warn)
             return defaultWarningDiagnosticAction;