================
@@ -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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
