Allow --Wdisable to take precedence over --Werror for warning mes…

…sages (p4lang#4894) * Allow --Wdisable to take precedence over --Werror for warning messages Signed-off-by: Kyle Cripps <kyle@pensando.io> * Add const qualifier Signed-off-by: Kyle Cripps <kyle@pensando.io> * Make sure to still prevent errors that are also warnings from being demoted Signed-off-by: Kyle Cripps <kyle@pensando.io> * appease cpplint Signed-off-by: Kyle Cripps <kyle@pensando.io> * appease cpplint Signed-off-by: Kyle Cripps <kyle@pensando.io> * Replace all ::warning(ErrorType::ERR_* with ErrorType::WARN_* Signed-off-by: Kyle Cripps <kyle@pensando.io> * ERR_DUPLICATE -> WARN_DUPLICATE Signed-off-by: Kyle Cripps <kyle@pensando.io> * invert logic Signed-off-by: Kyle Cripps <kyle@pensando.io> --------- Signed-off-by: Kyle Cripps <kyle@pensando.io>
linuxrocks123 · Sep 18, 2024 · f365f6b · f365f6b
1 parent 0bd4404
commit f365f6b
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 52 deletions.
diff --git a/frontends/common/parser_options.h b/frontends/common/parser_options.h
@@ -144,12 +144,6 @@ class P4CContext : public BaseCompileContext {
  errorReporter().setDefaultWarningDiagnosticAction(action);
  }
 
- /// @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) final {
- return errorReporter().getDiagnosticAction(diagnostic, defaultAction);
- }
-
  /// Set the action to take for the given diagnostic.
  void setDiagnosticAction(std::string_view diagnostic, DiagnosticAction action) {
  errorReporter().setDiagnosticAction(diagnostic, action);

diff --git a/frontends/p4/fromv1.0/programStructure.cpp b/frontends/p4/fromv1.0/programStructure.cpp
@@ -314,7 +314,7 @@ void ProgramStructure::createStructures() {
  if (metadataInstances.count(type_name)) continue;
  auto h = headers.get(it.first->name);
  if (h != nullptr)
- ::P4::warning(ErrorType::ERR_DUPLICATE,
+ ::P4::warning(ErrorType::WARN_DUPLICATE,
  "%1%: header and metadata instances %2% with the same name", it.first, h);
  auto ht = type->to<IR::Type_StructLike>();
  auto path = new IR::Path(type_name);

diff --git a/lib/compile_context.cpp b/lib/compile_context.cpp
@@ -76,9 +76,4 @@ DiagnosticAction BaseCompileContext::getDefaultErrorDiagnosticAction() {
  return DiagnosticAction::Error;
 }
 
-DiagnosticAction BaseCompileContext::getDiagnosticAction(cstring /* diagnostic */,
- DiagnosticAction defaultAction) {
- return defaultAction;
-}
-
 } // namespace P4
diff --git a/lib/compile_context.h b/lib/compile_context.h
@@ -105,11 +105,6 @@ class BaseCompileContext : public ICompileContext {
  /// @return the default diagnostic action for calls to `::P4::error()`.
  virtual DiagnosticAction getDefaultErrorDiagnosticAction();
 
- /// @return the diagnostic action to use for @diagnosticName, or
- /// @defaultAction if no diagnostic action was found.
- virtual DiagnosticAction getDiagnosticAction(cstring diagnostic,
- DiagnosticAction defaultAction);
-
  private:
  /// Error and warning tracking facilities for this compilation context.
  ErrorReporter errorReporterInstance;

diff --git a/lib/error.h b/lib/error.h
@@ -167,28 +167,6 @@ void info(const int kind, const char *format, Args &&...args) {
  context.errorReporter().diagnose(action, kind, format, "", std::forward<Args>(args)...);
 }
 
-/**
- * Trigger a diagnostic message.
- *
- * @param defaultAction The action (warn, error, etc.) to take if no action
- * was specified for this diagnostic on the command line
- * or via a pragma.
- * @param diagnosticName A human-readable name for the diagnostic. This should
- * generally use only lower-case letters and underscores
- * so the diagnostic name is a valid P4 identifier.
- * @param format A format for the diagnostic message, using the same style as
- * '::P4::warning' or '::P4::error'.
- * @param suffix A message that is appended at the end.
- */
-template <typename... Args>
-inline void diagnose(DiagnosticAction defaultAction, const char *diagnosticName, const char *format,
- const char *suffix, Args &&...args) {
- auto &context = BaseCompileContext::get();
- auto action = context.getDiagnosticAction(cstring(diagnosticName), defaultAction);
- context.errorReporter().diagnose(action, diagnosticName, format, suffix,
- std::forward<Args>(args)...);
-}
-
 } // namespace P4
 
 #endif /* LIB_ERROR_H_ */
diff --git a/lib/error_catalog.cpp b/lib/error_catalog.cpp
@@ -69,6 +69,8 @@ const int ErrorType::WARN_INVALID_HEADER = 1020;
 const int ErrorType::WARN_DUPLICATE_PRIORITIES = 1021;
 const int ErrorType::WARN_ENTRIES_OUT_OF_ORDER = 1022;
 const int ErrorType::WARN_MULTI_HDR_EXTRACT = 1023;
+const int ErrorType::WARN_EXPRESSION = 1024;
+const int ErrorType::WARN_DUPLICATE = 1025;
 
 // ------ Info messages -----------
 const int ErrorType::INFO_INFERRED = WARN_MAX + 1;
@@ -120,6 +122,8 @@ std::map<int, cstring> ErrorCatalog::errorCatalog = {
  {ErrorType::WARN_DUPLICATE_PRIORITIES, "duplicate_priorities"_cs},
  {ErrorType::WARN_ENTRIES_OUT_OF_ORDER, "entries_out_of_priority_order"_cs},
  {ErrorType::WARN_MULTI_HDR_EXTRACT, "multi_header_extract"_cs},
+ {ErrorType::WARN_EXPRESSION, "expr"_cs},
+ {ErrorType::WARN_DUPLICATE, "duplicate"_cs},
 
  // Info messages
  {ErrorType::INFO_INFERRED, "inferred"_cs},

diff --git a/lib/error_catalog.h b/lib/error_catalog.h
@@ -83,6 +83,8 @@ class ErrorType {
  static const int WARN_DUPLICATE_PRIORITIES; // two entries with the same priority
  static const int WARN_ENTRIES_OUT_OF_ORDER; // entries with priorities out of order
  static const int WARN_MULTI_HDR_EXTRACT; // same header may be extracted more than once
+ static const int WARN_EXPRESSION; // expression related warnings
+ static const int WARN_DUPLICATE; // duplicate objects
  // Backends should extend this class with additional warnings in the range 1500-2141.
  static const int WARN_MIN_BACKEND = 1500; // first allowed backend warning code
  static const int WARN_MAX = 2141; // last allowed warning code
@@ -133,6 +135,10 @@ class ErrorCatalog {
  return "--unknown--"_cs;
  }
 
+ bool isError(int errorCode) {
+ return errorCode >= ErrorType::LEGACY_ERROR && errorCode <= ErrorType::ERR_MAX;
+ }
+
  /// return true if the given diagnostic can _only_ be an error; false otherwise
  bool isError(std::string_view name) {
  cstring lookup(name);
@@ -141,8 +147,7 @@ class ErrorCatalog {
  bool error = false;
  for (const auto &pair : errorCatalog) {
  if (pair.second == lookup) {
- if (pair.first < ErrorType::LEGACY_ERROR || pair.first > ErrorType::ERR_MAX)
- return false;
+ if (!isError(pair.first)) return false;
  error = true;
  }
  }

diff --git a/lib/error_reporter.h b/lib/error_reporter.h
@@ -73,7 +73,7 @@ class ErrorReporter {
  /// and source info.
  /// If the error has been reported, return true. Otherwise, insert add the error to the
  /// list of seen errors, and return false.
- bool error_reported(int err, Util::SourceInfo source) {
+ bool error_reported(int err, const Util::SourceInfo source) {
  if (!source.isValid()) return false;
  auto p = errorTracker.emplace(err, source);
  return !p.second; // if insertion took place, then we have not seen the error.
@@ -114,7 +114,7 @@ class ErrorReporter {
  if (!node || error_reported(errorCode, node->getSourceInfo())) return;
 
  if (cstring name = get_error_name(errorCode))
- diagnose(getDiagnosticAction(name, action), name, format, suffix, node,
+ diagnose(getDiagnosticAction(errorCode, name, action), name, format, suffix, node,
  std::forward<Args>(args)...);
  else
  diagnose(action, nullptr, format, suffix, node, std::forward<Args>(args)...);
@@ -124,7 +124,7 @@ class ErrorReporter {
  void diagnose(DiagnosticAction action, const int errorCode, const char *format,
  const char *suffix, Args &&...args) {
  if (cstring name = get_error_name(errorCode))
- diagnose(getDiagnosticAction(name, action), name, format, suffix,
+ diagnose(getDiagnosticAction(errorCode, name, action), name, format, suffix,
  std::forward<Args>(args)...);
  else
  diagnose(action, nullptr, format, suffix, std::forward<Args>(args)...);
@@ -225,9 +225,10 @@ 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) {
+ DiagnosticAction getDiagnosticAction(int errorCode, cstring diagnostic,
+ DiagnosticAction defaultAction) {
  // Actions for errors can never be overridden.
- if (defaultAction == DiagnosticAction::Error) return defaultAction;
+ if (ErrorCatalog::getCatalog().isError(errorCode)) return DiagnosticAction::Error;
  auto it = diagnosticActions.find(diagnostic);
  if (it != diagnosticActions.end()) return it->second;
  // if we're dealing with warnings and they have been globally modified

diff --git a/midend/parserUnroll.cpp b/midend/parserUnroll.cpp
@@ -174,7 +174,7 @@ class ParserStateRewriter : public Transform {
  auto *value = ev.evaluate(expression->right, false);
  if (!value->is<SymbolicInteger>()) return expression;
  if (!value->to<SymbolicInteger>()->isKnown()) {
- ::P4::warning(ErrorType::ERR_INVALID,
+ ::P4::warning(ErrorType::WARN_INVALID,
  "Uninitialized value prevents loop unrolling:\n%1%", expression->right);
  wasError = true;
  return expression;
@@ -183,7 +183,7 @@ class ParserStateRewriter : public Transform {
  newExpression->right = res;
  if (!res->fitsInt64()) {
  // we need to leave expression as is.
- ::P4::warning(ErrorType::ERR_EXPRESSION, "Index can't be concretized : %1%",
+ ::P4::warning(ErrorType::WARN_EXPRESSION, "Index can't be concretized : %1%",
  expression);
  return expression;
  }
@@ -393,7 +393,7 @@ class ParserSymbolicInterpreter {
 
  if (value == nullptr) value = factory->create(type, true);
  if (value && value->is<SymbolicError>()) {
- ::P4::warning(ErrorType::ERR_EXPRESSION, "%1%: %2%", d,
+ ::P4::warning(ErrorType::WARN_EXPRESSION, "%1%: %2%", d,
  value->to<SymbolicError>()->message());
  return nullptr;
  }
@@ -447,7 +447,7 @@ class ParserSymbolicInterpreter {
 
  if (!stateClone)
  // errors in the original state are signalled
- ::P4::warning(ErrorType::ERR_EXPRESSION, "%1%: error %2% will be triggered\n%3%",
+ ::P4::warning(ErrorType::WARN_EXPRESSION, "%1%: error %2% will be triggered\n%3%",
  exc->errorPosition, exc->message(), stateChain(state));
  // else this error will occur in a clone of the state produced
  // by unrolling - if the state is reached. So we don't give an error.
@@ -682,7 +682,7 @@ class ParserSymbolicInterpreter {
  }
  }
  if (equStackVariableMap(crt->statesIndexes, state->statesIndexes)) {
- ::P4::warning(ErrorType::ERR_INVALID,
+ ::P4::warning(ErrorType::WARN_INVALID,
  "Parser cycle can't be unrolled, because ParserUnroll can't "
  "detect the number of loop iterations:\n%1%",
  stateChain(state));
@@ -694,7 +694,7 @@ class ParserSymbolicInterpreter {
  // If no header validity has changed we can't really unroll
  if (!headerValidityChange(crt->before, state->before)) {
  if (equStackVariableMap(crt->statesIndexes, state->statesIndexes)) {
- ::P4::warning(ErrorType::ERR_INVALID,
+ ::P4::warning(ErrorType::WARN_INVALID,
  "Parser cycle can't be unrolled, because ParserUnroll can't "
  "detect the number of loop iterations:\n%1%",
  stateChain(state));