[PATCH] D15095: Accept "-Weverything" in pragma clang diagnostic ...

[Sunil Srivastava via cfe-commits]

Sunil_Srivastava added a comment.

Please see the comment about getAllDiagnostics


================
Comment at: lib/Basic/Diagnostic.cpp:251-257
@@ -250,2 +250,9 @@
                                             SourceLocation Loc) {
+  // Special handling for pragma clang diagnostic ... "-Weverything"
+  // There is no formal group named "everything", so there has to be a special
+  // case for it.
+  if (Group == "everything") {
+   setSeverityForAll(Flavor, Map, Loc);
+   return false;
+  }
   // Get the diagnostics in this group.
----------------
rsmith wrote:
> If you want to handle this at the `DiagnosticsEngine` level, please do so consistently: teach `getDiagnosticsInGroup` about this special case too, and remove the now-redundant code in `clang::ProcessWarningOptions`.
> 
> This is not currently setting the `EnableAllWarnings` flag correctly on the `DiagnosticsEngine`.
Hi Richard, There is a problem in teaching getDiagnosticsInGroup this special case. 

getDiagnosticsInGroup can get the list from getAllDiagnostics, but that list will contain disgnostics that can not be downgraded (or those for which isBuiltinWarningOrExtension() is false).

Back in setSeverityForGroup, it is safe to call setSeverityForAll, because it checks isBuiltinWarningOrExtension before calling setSeverity, but the loop in setSeverityForGroup itself does not have this check. So a simplistic getAllDiagnostics for "everything" leads to an assertion failure "Cannot map errors into warnings!" in setSeverity.

In fact, ProcessWarningOptions has the same issue because it also calls setSeverityForGroup.

So the options are:
1) add isBuiltinWarning... test in the loop in setSeverityForGroup, similar to setSeverityForAll
2) have some variant of getAllDiagnostics that returns a trimmed down list.

Please advise.

The other point about EnableAllWarnings: I agree.

================
Comment at: test/Frontend/Peverything.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+#pragma clang diagnostic error "-Weverything" 
----------------
rsmith wrote:
> This test belongs in **test/Preprocessor/pragma_diagnostic.c**.
OK. And I will add push pop tests as well


http://reviews.llvm.org/D15095