From: Chad Rosier Date: Thu, 3 Nov 2011 21:23:39 +0000 (+0000) Subject: Parse the warning options twice. The first pass sets diagnostic state, while X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=05272a659437fc6ec7fa5a7c3f3fc6eb220a6fa2;p=clang 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 --- diff --git a/lib/Frontend/Warnings.cpp b/lib/Frontend/Warnings.cpp index 8fbcd4b44d..7fcbe3acd4 100644 --- a/lib/Frontend/Warnings.cpp +++ b/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 _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; } - - // Set the warning as error flag for this specifier. - if (Diags.setDiagnosticGroupWarningAsError(Specifier, isPositive)) { - Diags.Report(isPositive ? diag::warn_unknown_warning_option : - diag::warn_unknown_negative_warning_option) - << ("-W" + Opt.str()); + + // -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; } - 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()); + + // -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; } - Specifier = Opt.substr(13); + + 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); + } - if (Specifier.empty()) { - Diags.setErrorsAsFatal(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; } - - // Set the error as fatal flag for this specifier. - if (Diags.setDiagnosticGroupErrorAsFatal(Specifier, isPositive)) { + + 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; - } - - if (Diags.setDiagnosticGroupMapping(Opt, Mapping)) { - Diags.Report(isPositive ? diag::warn_unknown_warning_option : - diag::warn_unknown_negative_warning_option) - << ("-W" + Opt.str()); } } }