[clang] [Clang] Silently ignore unknown warnings in `--warning-suppression-mappings` (PR #124141)

[James Y Knight via cfe-commits]

================
@@ -547,11 +547,9 @@ void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
     StringRef DiagGroup = SectionEntry->getKey();
     if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(
             WarningFlavor, DiagGroup, GroupDiags)) {
-      StringRef Suggestion =
-          DiagnosticIDs::getNearestOption(WarningFlavor, DiagGroup);
-      Diags.Report(diag::warn_unknown_diag_option)
-          << static_cast<unsigned>(WarningFlavor) << DiagGroup
-          << !Suggestion.empty() << Suggestion;
+      // If a diagnostic group name is unknown, simply ignore the
----------------
jyknight wrote:

I realize it was intentional to emit unknown-warning diagnostics, but I really think we should not. Yes, that could catch a typo in the suppressions file, but you could also typo the filenames in the same way without notice, so I don't find that terribly persuasive as a use-case. Because this is a suppression file, the primary notice that you've made a mistake is that the diagnostic you expected to suppress did not get suppressed.

On the other hand, not emitting unknown-warning diagnostic is particularly useful here, as it allows the suppressions file for a given codebase to be easily used across multiple compilers. It allows users to easily write a file which suppresses some instance of a newly-introduced diagnostic, yet doesn't complain when the compiler doesn't yet support the suppressed-diagnostic. Notably, on the version which doesn't recognize the flag, generally the suppression is not _necessary_, since it doesn't know how to emit the diagnostic anyways.

And there's not the ability to guard with `#if __has_warning` like you can if adding a suppression via editing the source file.

> make sure we do respect rest of the warning options?

But, yes, if we _do_ continue to emit an unknown-warning diagnostic, it needs to honor -Werror and -Wno-unknown-warning-option.

> in addition to that, we should propagate ReportDiags into DiagnosticsEngine::setDiagSuppressionMapping and not emit warnings when it's set to false (to prevent double emitting in driver & cc1).

Why parse the suppression file in the driver? Even if we do check ReportDiags, isn't it just wasted work to read it at all?

> I am happy to do these changes myself, as they're more involved then what's in this PR currently.

I'd be happy to have you do so! (Even if a bit less happy than going with the simpler proposal, here).

https://github.com/llvm/llvm-project/pull/124141