Parse the warning options twice. The first pass sets diagnostic state, while

the second pass reports warnings/errors.  This has the effect that we follow 
the more canonical "last option wins" paradigm when there are conflicting 
options.
rdar://10383776

Previously, we parsed the warning options in order.  This caused non-intuitive
behavior:

1) clang test.c -Wnosuchwarning -Wno-unknown-warning-option
Before:
warning: unknown warning option '-Wnosuchwarning' [-Wunknown-warning-option]
1 warning generated.
After:
[0 warning generated.]

2) clang test.c -Wnosuchwarning -Werror=unknown-warning-option
Before:
warning: unknown warning option '-Wnosuchwarning' [-Wunknown-warning-option]
1 warning generated.
After:
error: unknown warning option '-Wnosuchwarning' [-Werror,-Wunknown-warning-option]

3) clang test.c -Werror=unknown-warning-option -Wnosuchwarning -Wno-error=unknown-warning-option -Wnosuchwarning
Before:
error: unknown warning option '-Wnosuchwarning' [-Werror,-Wunknown-warning-option]
warning: unknown warning option '-Wnosuchwarning' [-Wunknown-warning-option]
After:
warning: unknown warning option '-Wnosuchwarning' [-Wunknown-warning-option]
warning: unknown warning option '-Wnosuchwarning' [-Wunknown-warning-option]
2 warnings generated.

4) clang test.c -Werror=unknown-warning-option -Wnosuchwarning -Wno-error=unknown-warning-option -Wno-unknown-warning-option -Wnosuchwarning
Before:
error: unknown warning option '-Wnosuchwarning' [-Werror,-Wunknown-warning-option]

After:
[0 warning generated.]

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@143657 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Frontend/Warnings.cpp

@@ -54,92 +54,114 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
   else
     Diags.setExtensionHandlingBehavior(DiagnosticsEngine::Ext_Ignore);
 
-  for (unsigned i = 0, e = Opts.Warnings.size(); i != e; ++i) {
-    StringRef Opt = Opts.Warnings[i];
+  llvm::SmallVector<diag::kind, 10> _Diags;
+  const llvm::IntrusiveRefCntPtr< DiagnosticIDs > DiagIDs =
+    Diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are 
+  // conflicting options.
+  for (unsigned Report = 0, ReportEnd = 2; Report != ReportEnd; ++Report) {
+    bool SetDiagnostic = (Report == 0);
+    for (unsigned i = 0, e = Opts.Warnings.size(); i != e; ++i) {
+      StringRef Opt = Opts.Warnings[i];
 
-    // Check to see if this warning starts with "no-", if so, this is a negative
-    // form of the option.
-    bool isPositive = true;
-    if (Opt.startswith("no-")) {
-      isPositive = false;
-      Opt = Opt.substr(3);
-    }
-
-    // Figure out how this option affects the warning.  If -Wfoo, map the
-    // diagnostic to a warning, if -Wno-foo, map it to ignore.
-    diag::Mapping Mapping = isPositive ? diag::MAP_WARNING : diag::MAP_IGNORE;
-
-    // -Wsystem-headers is a special case, not driven by the option table.  It
-    // cannot be controlled with -Werror.
-    if (Opt == "system-headers") {
-      Diags.setSuppressSystemWarnings(!isPositive);
-      continue;
-    }
-    
-    // -Weverything is a special case as well.  It implicitly enables all
-    // warnings, including ones not explicitly in a warning group.
-    if (Opt == "everything") {
-      Diags.setEnableAllWarnings(true);
-      continue;
-    }
-
-    // -Werror/-Wno-error is a special case, not controlled by the option table.
-    // It also has the "specifier" form of -Werror=foo and -Werror-foo.
-    if (Opt.startswith("error")) {
-      StringRef Specifier;
-      if (Opt.size() > 5) {  // Specifier must be present.
-        if ((Opt[5] != '=' && Opt[5] != '-') || Opt.size() == 6) {
-          Diags.Report(diag::warn_unknown_warning_specifier)
-            << "-Werror" << ("-W" + Opt.str());
-          continue;
-        }
-        Specifier = Opt.substr(6);
+      // Check to see if this warning starts with "no-", if so, this is a
+      // negative form of the option.
+      bool isPositive = true;
+      if (Opt.startswith("no-")) {
+        isPositive = false;
+        Opt = Opt.substr(3);
       }
 
-      if (Specifier.empty()) {
-        Diags.setWarningsAsErrors(isPositive);
+      // Figure out how this option affects the warning.  If -Wfoo, map the
+      // diagnostic to a warning, if -Wno-foo, map it to ignore.
+      diag::Mapping Mapping = isPositive ? diag::MAP_WARNING : diag::MAP_IGNORE;
+      
+      // -Wsystem-headers is a special case, not driven by the option table.  It
+      // cannot be controlled with -Werror.
+      if (Opt == "system-headers") {
+        if (SetDiagnostic)
+          Diags.setSuppressSystemWarnings(!isPositive);
         continue;
       }
+      
+      // -Weverything is a special case as well.  It implicitly enables all
+      // warnings, including ones not explicitly in a warning group.
+      if (Opt == "everything") {
+        if (SetDiagnostic)
+          Diags.setEnableAllWarnings(true);
+        continue;
+      }
+      
+      // -Werror/-Wno-error is a special case, not controlled by the option 
+      // table. It also has the "specifier" form of -Werror=foo and -Werror-foo.
+      if (Opt.startswith("error")) {
+        StringRef Specifier;
+        if (Opt.size() > 5) {  // Specifier must be present.
+          if ((Opt[5] != '=' && Opt[5] != '-') || Opt.size() == 6) {
+            if (Report)
+              Diags.Report(diag::warn_unknown_warning_specifier)
+                << "-Werror" << ("-W" + Opt.str());
+            continue;
+          }
+          Specifier = Opt.substr(6);
+        }
+        
+        if (Specifier.empty()) {
+          if (SetDiagnostic)
+            Diags.setWarningsAsErrors(isPositive);
+          continue;
+        }
+        
+        if (SetDiagnostic) {
+          // Set the warning as error flag for this specifier.
+          Diags.setDiagnosticGroupWarningAsError(Specifier, isPositive);
+        } else if (DiagIDs->getDiagnosticsInGroup(Specifier, _Diags)) {
+          Diags.Report(isPositive ? diag::warn_unknown_warning_option :
+                       diag::warn_unknown_negative_warning_option)
+            << ("-W" + Opt.str());
+        }
+        continue;
+      }
+      
+      // -Wfatal-errors is yet another special case.
+      if (Opt.startswith("fatal-errors")) {
+        StringRef Specifier;
+        if (Opt.size() != 12) {
+          if ((Opt[12] != '=' && Opt[12] != '-') || Opt.size() == 13) {
+            if (Report)
+              Diags.Report(diag::warn_unknown_warning_specifier)
+                << "-Wfatal-errors" << ("-W" + Opt.str());
+            continue;
+          }
+          Specifier = Opt.substr(13);
+        }
 
-      // Set the warning as error flag for this specifier.
-      if (Diags.setDiagnosticGroupWarningAsError(Specifier, isPositive)) {
+        if (Specifier.empty()) {
+          if (SetDiagnostic)
+            Diags.setErrorsAsFatal(isPositive);
+          continue;
+        }
+        
+        if (SetDiagnostic) {
+          // Set the error as fatal flag for this specifier.
+          Diags.setDiagnosticGroupErrorAsFatal(Specifier, isPositive);
+        } else if (DiagIDs->getDiagnosticsInGroup(Specifier, _Diags)) {
+          Diags.Report(isPositive ? diag::warn_unknown_warning_option :
+                       diag::warn_unknown_negative_warning_option)
+            << ("-W" + Opt.str());
+        }
+        continue;
+      }
+      
+      if (Report && DiagIDs->getDiagnosticsInGroup(Opt, _Diags)) {
         Diags.Report(isPositive ? diag::warn_unknown_warning_option :
                      diag::warn_unknown_negative_warning_option)
           << ("-W" + Opt.str());
+      } else {
+        Diags.setDiagnosticGroupMapping(Opt, Mapping);
       }
-      continue;
-    }
-
-    // -Wfatal-errors is yet another special case.
-    if (Opt.startswith("fatal-errors")) {
-      StringRef Specifier;
-      if (Opt.size() != 12) {
-        if ((Opt[12] != '=' && Opt[12] != '-') || Opt.size() == 13) {
-          Diags.Report(diag::warn_unknown_warning_specifier)
-            << "-Wfatal-errors" << ("-W" + Opt.str());
-          continue;
-        }
-        Specifier = Opt.substr(13);
-      }
-
-      if (Specifier.empty()) {
-        Diags.setErrorsAsFatal(isPositive);
-        continue;
-      }
-
-      // Set the error as fatal flag for this specifier.
-      if (Diags.setDiagnosticGroupErrorAsFatal(Specifier, isPositive)) {
-        Diags.Report(isPositive ? diag::warn_unknown_warning_option :
-                     diag::warn_unknown_negative_warning_option)
-          << ("-W" + Opt.str());
-      }
-      continue;
-    }
-
-    if (Diags.setDiagnosticGroupMapping(Opt, Mapping)) {
-      Diags.Report(isPositive ? diag::warn_unknown_warning_option :
-                   diag::warn_unknown_negative_warning_option)
-          << ("-W" + Opt.str());
     }
   }
 }