[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -1315,6 +1319,34 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. AaronBallman wrote: That's true, but I think we want to guide users towards fixing diagnostics rather than suppressing them. Users will use any suppression mechanisms we provide regardless of how we document it, so I don't think it's harmful to imply this is more about cases where you can't modify the code. But I also don't feel strongly. WDYT? https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -1315,6 +1319,34 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts kadircet wrote: > Add comma: per-file, granular manner "per-file" was meant to describe "granular". rewrote as `per-file granularity` instead. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -23,10 +23,15 @@ // simpler because a remark can't be promoted to an error. #include "clang/Basic/AllDiagnostics.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticDriver.h" +#include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" -#include +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/SpecialCaseList.h" +#include "llvm/Support/VirtualFileSystem.h" #include -#include +#include +#include kadircet wrote: oops, I had more implementation here initially. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
kadircet wrote:
As you well know these discrepancies are coming from
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LLVM.h.
It's really hard to know/remember what's available there and what isn't.
Moreover the header only forward-declares, so at usage side one actually needs
to include the actual header from `llvm/...`.
Hence can't do this for `SpecialCaseList` but doing it for others that I can.
But I think in the long run it'd be interesting to see if people still find
this intermediate header useful or not.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -1315,6 +1319,34 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. kadircet wrote: > Reword: some headers -> some headers which cannot be modified. But this covers headers even inside the project (i.e. the incremental cleanup of a new warning). So I feel like `which cannot be modified.` adds a non-existing limitation. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -9017,3 +9017,8 @@ def wasm_opt : Flag<["--"], "wasm-opt">, Group, HelpText<"Enable the wasm-opt optimizer (default)">, MarshallingInfoNegativeFlag>; + +def warning_suppression_mappings_EQ : Joined<["--"], kadircet wrote: > This should move elsewhere in Options.td, it's currently listed under > clang-dxc options. Oh didn't notice we actually had "lexical" sections in the doc (AFAICT it doesn't have any semantic affect afterwards, flag is still only visible for cc1 and clang-driver). > Also, should this be exposed in clang-cl as well? What about flang? CC > @zmodem @rnk @MaskRay @jansvoboda11 for opinions Going to carry this discussion into RFC to make sure it doesn't get lost. https://discourse.llvm.org/t/rfc-add-support-for-controlling-diagnostics-severities-at-file-level-granularity-through-command-line/81292/21?u=kadircet https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
nikic wrote: > so > [12e3ed8](https://github.com/llvm/llvm-project/commit/12e3ed8de8c6063b15916b3faf67c8c9cd17df1f) > should resolve both lldb buildbot breakage and compile time regression in > clang. Thank you, I can confirm that this fixes the compile-time regression (or at least most of it). https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: so https://github.com/llvm/llvm-project/commit/12e3ed8de8c6063b15916b3faf67c8c9cd17df1f should resolve both lldb buildbot breakage and compile time regression in clang. that being said, I don't think https://github.com/llvm/llvm-project/commit/12e3ed8de8c6063b15916b3faf67c8c9cd17df1f is the right fix for lldb breakage. I think this change reveals an actual bug in lldb's handling of modules, but I can't repro locally. I hit a similar issue with clang-repl during development, you can find the relevant fix in https://github.com/llvm/llvm-project/pull/112517/files#diff-23c4b4ecf706e1f0b594e365095a343594e3978b47991718782e1f76064f241a. Basically I think LLDB is doing something similar, it's probably loading/building modules with one SourceManager and then it's trying to consume them using another. @Michael137, If you can provide more detailed instructions about how to set up a similar testing environment in linux (I couldn't get the std-modules bit to work), or if you can provide a detailed stack trace from assertion failure, I can try to dig deeper. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
Michael137 wrote:
Hmm this one's a bit strange. Looks like the issue is in:
```
// Clang-diagnostics pragmas always take precedence over suppression mapping.
if (!Mapping.isPragma()) {
// We also use presumed locations here to improve reproducibility for
// preprocessed inputs.
if (PresumedLoc PLoc = SM.getPresumedLoc(Loc);
PLoc.isValid() && Diag.isSuppressedViaMapping(
DiagID, llvm::sys::path::remove_leading_dotslash(
PLoc.getFilename(
return diag::Severity::Ignored;
}
```
The assert triggers inside `getPresumedLoc`. So doesn't look like it's an
inherent issue with the patch. But it seems to be breaking assumptions that
LLDB is making about these source locations
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
Michael137 wrote: > > FYI, looks like this is causing following LLDB tests to fail: > > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/15106/execution/node/97/log/ > > ``` > > Unresolved Tests (17): > > lldb-api :: > > commands/expression/import-std-module/array/TestArrayFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/iterator/TestIteratorFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/list/TestListFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py > > lldb-api :: > > commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py > > lldb-api :: > > commands/expression/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContentFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py > > lldb-api :: > > commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py > > lldb-api :: > > commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py > > ``` > > > > > > > > > > > > > > > > > > > > > > > > Example assertion: > > ``` > > Assertion failed: (0 && "Invalid SLocOffset or bad function choice"), > > function getFileIDLoaded, file SourceManager.cpp, line 867. > > PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ > > and include the crash backtrace. > > `` > > ``` > > FWIW, I am still trying to reproduce these failures. It seems to be hard on a > linux machine as tests are all disabled with a link to > https://discourse.llvm.org/t/lldb-test-failures-on-linux/80095. AFAIK only a few of these are skipped on Linux. E.g., `TestIteratorFromStdModule.py` should still reproduce. Happy to help you reproduce/narrow this down https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
nikic wrote: Though looking at that file list in more details, this also affects files outside clang, so I think it might be adding some compile-time overhead that is only visible on specific workloads. E.g. `LLVMSupport.dir/UnicodeNameToCodepointGenerated.cpp` has a 4.5% increase. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
nikic wrote: It looks like this increases time to build clang by 0.5% (see affected files here: https://llvm-compile-time-tracker.com/compare_clang.php?from=e385e0d3e71e17da0b2023f480259c95923707bd&to=41e3919ded78d8870f7c95e9181c7f7e29aa3cc4&stat=instructions%3Au&sortBy=interestingness) This usually indicates that you are including something expensive in a core header -- it would be good to check whether it can be avoided. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: > FYI, looks like this is causing following LLDB tests to fail: > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/15106/execution/node/97/log/ > > ``` > Unresolved Tests (17): > lldb-api :: > commands/expression/import-std-module/array/TestArrayFromStdModule.py > lldb-api :: > commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py > lldb-api :: > commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py > lldb-api :: > commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py > lldb-api :: > commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py > lldb-api :: > commands/expression/import-std-module/iterator/TestIteratorFromStdModule.py > lldb-api :: > commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py > lldb-api :: > commands/expression/import-std-module/list/TestListFromStdModule.py > lldb-api :: > commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py > lldb-api :: > commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py > lldb-api :: > commands/expression/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContentFromStdModule.py > lldb-api :: > commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py > lldb-api :: > commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py > lldb-api :: > commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py > lldb-api :: > commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py > lldb-api :: > commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py > lldb-api :: > commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py > ``` > > Example assertion: > > ``` > Assertion failed: (0 && "Invalid SLocOffset or bad function choice"), > function getFileIDLoaded, file SourceManager.cpp, line 867. > PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ > and include the crash backtrace. > `` > ``` FWIW, I am still trying to reproduce these failures. It seems to be hard on a linux machine as tests are all disabled with a link to https://discourse.llvm.org/t/lldb-test-failures-on-linux/80095. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -23,10 +23,15 @@ // simpler because a remark can't be promoted to an error. #include "clang/Basic/AllDiagnostics.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticDriver.h" +#include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" -#include +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/SpecialCaseList.h" +#include "llvm/Support/VirtualFileSystem.h" #include -#include +#include +#include AaronBallman wrote: Why are these being added? https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,97 @@ + +Warning suppression mappings + + +.. contents:: + :local: + +Introduction + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. + +Goal and usage +== + +Clang allows diagnostics to be configured at a translation-unit granularity. +If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers +also need to be clean. Hence turning on new warnings in large codebases requires AaronBallman wrote: Add comma: Hence, turning on https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.getKey();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.getValue().SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Error;
+ auto WarningSuppressionList = WarningsSpecialCaseList::create(Input, Error);
+ if (!WarningSuppressionList) {
+Report(diag::err_drv_malformed_warning_suppression_mapping)
AaronBallman wrote:
Can you add a FIXME comment here: it would be better to not use `Error` here as
a string but instead use `%select` in the diagnostic message; combining
arbitrary strings makes it harder when folks are localizing diagnostics.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -44,6 +44,7 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" AaronBallman wrote: Why is this being added? https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -1315,6 +1319,34 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts AaronBallman wrote: Add comma: per-file, granular manner Reword: Enabling enforcement of -> This allows enforcing https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -555,6 +560,10 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
+ /// Whether the diagnostic should be suppressed in FilePath.
+ llvm::unique_function
AaronBallman wrote:
You can drop the `llvm::` from things in this file as the prevailing style
leaves them off already.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
AaronBallman wrote:
Prevailing style in this file also drops the `llvm::` prefixes. Probably worth
taking a pass over the whole PR to verify you're following the local styles.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -1315,6 +1319,34 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. AaronBallman wrote: Remove comma: project even if there Reword: some headers -> some headers which cannot be modified. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,97 @@ + +Warning suppression mappings + + +.. contents:: + :local: + +Introduction + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. AaronBallman wrote: Same suggestions here as above. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/AaronBallman commented: > @AaronBallman merging this to main now. PLMK if you have any concerns, I am > happy to address them post-submit (or revert if need be) Sorry, there was a US holiday on Monday and I was mostly out on Friday, so that's why there was a delay in review. In the future, I would recommend waiting before landing significant changes like these unless the review is accepted by a maintainer or someone known to be familiar with that area of the code. That reduces pressure on reviewers and allows folks to be sick/take vacations/etc without things landing out from under them. As it turns out, I can no longer leave suggested changes on the PR because it was already merged (thanks, GitHub), so my review comments will be somewhat awkward. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -9017,3 +9017,8 @@ def wasm_opt : Flag<["--"], "wasm-opt">, Group, HelpText<"Enable the wasm-opt optimizer (default)">, MarshallingInfoNegativeFlag>; + +def warning_suppression_mappings_EQ : Joined<["--"], AaronBallman wrote: This should move elsewhere in Options.td, it's currently listed under `clang-dxc` options. Also, should this be exposed in clang-cl as well? What about flang? CC @zmodem @rnk @MaskRay @jansvoboda11 for opinions https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
Michael137 wrote: FYI, looks like this is causing following LLDB builds to fail: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/15106/execution/node/97/log/ ``` Unresolved Tests (17): lldb-api :: commands/expression/import-std-module/array/TestArrayFromStdModule.py lldb-api :: commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py lldb-api :: commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py lldb-api :: commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py lldb-api :: commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py lldb-api :: commands/expression/import-std-module/iterator/TestIteratorFromStdModule.py lldb-api :: commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py lldb-api :: commands/expression/import-std-module/list/TestListFromStdModule.py lldb-api :: commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py lldb-api :: commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py lldb-api :: commands/expression/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContentFromStdModule.py lldb-api :: commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py lldb-api :: commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py lldb-api :: commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py lldb-api :: commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py lldb-api :: commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py lldb-api :: commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py ``` Example assertion: ``` Assertion failed: (0 && "Invalid SLocOffset or bad function choice"), function getFileIDLoaded, file SourceManager.cpp, line 867. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. `` https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: @AaronBallman merging this to main now. PLMK if you have any concerns, I am happy to address them post-submit (or revert if need be) https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: thanks for the review! @AaronBallman i'd like to land this early next week and start testing/polishing. It'd be great if you can give some explicit LGTM. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.first();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.second.SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Error;
+ auto WarningSuppressionList = WarningsSpecialCaseList::create(Input, Error);
+ if (!WarningSuppressionList) {
+Report(diag::err_drv_malformed_warning_suppression_mapping)
+<< Input.getBufferIdentifier() << Error;
+return;
+ }
+ WarningSuppressionList->processSections(*this);
+ DiagSuppressionMapping =
+ [WarningSuppressionList(std::move(WarningSuppressionList))](
+ diag::kind DiagId, llvm::StringRef Path) {
+return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
+ };
+}
+
+bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
+ llvm::StringRef FilePath) const
{
+ const Section *DiagSection = DiagToSection.lookup(DiagId);
+ if (!DiagSection)
+return false;
+ const SectionEntries &EntityTypeToCategories = DiagSection->Entries;
+ auto SrcEntriesIt = EntityTypeToCategories.find("src");
+ if (SrcEntriesIt == EntityTypeToCategories.end())
+return false;
+ const llvm::StringMap &CategoriesToMatchers =
+ SrcEntriesIt->second;
bricknerb wrote:
I suggest using getValue() and getKey() instead of second and first throughout
the code when possible, since it's more readable.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +179,155 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ llvm::ArrayRef takeDiags() {
bricknerb wrote:
Rename to getDiags()?
Take suggests they're no longer there.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ src:*foo.cpp=emit
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
+ EXPECT_FALSE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, LongestMatchWins) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ src:*clang/lib/Sema/*=emit
+ src:*clang/lib/Sema/foo*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Basic/foo.h"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+"clang/lib/Sema/bar.h"));
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Sema/foo.h"));
+}
+
+TEST_F(SuppressionMappingTest, IsIgnored) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ Diags.getDiagnost
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
bricknerb wrote:
We might care whether we propagate this as an error, warning etc (I guess we
could change that).
In another case we use `WithMessage()` below we test
`rr_drv_malformed_warning_suppression_mapping`, which I guess we should care
about more than just the text.
For consistency and making sure we have coverage, I suggest checking all the
interesting fields, and not just the text.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,109 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &MB, std::string &Err) {
+auto SCL = std::make_unique();
+if (!SCL->createInternal(&MB, Err))
+ return nullptr;
+return SCL;
+ }
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags) {
+// Drop the default section introduced by special case list, we only
support
+// exact diagnostic group names.
+Sections.erase("*");
+// Make sure we iterate sections by their line numbers.
+std::vector *>>
+LineAndSectionEntry;
+LineAndSectionEntry.reserve(Sections.size());
+for (const auto &Entry : Sections) {
+ LineAndSectionEntry.emplace_back(
+ Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry);
+}
+llvm::sort(LineAndSectionEntry);
+static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError;
+for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+ SmallVector GroupDiags;
+ llvm::StringRef DiagGroup = SectionEntry->getKey();
+ if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(kFlavor, DiagGroup,
+ GroupDiags)) {
+StringRef Suggestion =
+DiagnosticIDs::getNearestOption(kFlavor, DiagGroup);
+Diags.Report(diag::warn_unknown_diag_option)
+<< static_cast(kFlavor) << DiagGroup
+<< !Suggestion.empty() << Suggestion;
+continue;
+ }
+ for (diag::kind D : GroupDiags)
+DiagToSection[D] = &SectionEntry->getValue();
+}
+ }
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const {
+auto Section = DiagToSection.find(DiagId);
+if (Section == DiagToSection.end())
+ return false;
+auto &DiagEntries = Section->second->Entries;
+auto SrcEntries = DiagEntries.find("src");
+if (SrcEntries == DiagEntries.end())
+ return false;
+return globsMatches(SrcEntries->second, FilePath);
+ }
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(llvm::StringMap CategoriesToMatchers,
+llvm::StringRef FilePath) const {
+llvm::StringRef LongestMatch;
+bool LongestIsPositive = false;
+for (const auto &[Category, Matcher] : CategoriesToMatchers) {
+ bool IsPositive = Category != "emit";
+ for (const auto &[Pattern, Glob] : Matcher.Globs) {
bricknerb wrote:
Performance isn't my main concern here, just API consistency.
Given that there are no exclusions in other use cases, I think no inconsistency
can happen, so we should be good.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/bricknerb approved this pull request. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.
bricknerb wrote:
Careful reading it again this is ok.
If you want to emphasize it better, you can make it a list and/or emphasize the
either.
... against these globs **either**:
* as ...
* as ...
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.first();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.second.SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Err;
+ auto WarningSuppressionList = WarningsSpecialCaseList::create(Input, Err);
+ if (!WarningSuppressionList) {
+Report(diag::err_drv_malformed_warning_suppression_mapping)
+<< Input.getBufferIdentifier() << Err;
+return;
+ }
+ WarningSuppressionList->processSections(*this);
+ DiagSuppressionMapping =
+ [WarningSuppressionList(std::move(WarningSuppressionList))](
+ diag::kind DiagId, llvm::StringRef Path) {
+return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
+ };
+}
+
+bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
+ llvm::StringRef FilePath) const
{
+ const Section *DiagSection = DiagToSection.lookup(DiagId);
+ if (!DiagSection)
+return false;
+ const SectionEntries &EntityTypeToCategories = DiagSection->Entries;
+ auto SrcEntriesIt = EntityTypeToCategories.find("src");
+ if (SrcEntriesIt == EntityTypeToCategories.end())
+return false;
+ const llvm::StringMap &CategoriesToMatchers =
+ SrcEntriesIt->second;
bricknerb wrote:
Thanks, makes sense!
Now I feel like adding this functionality :).
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
bricknerb wrote: Unit tests cover the code and Clang integration tests are useful to test a feature end-to-end. The compiler logic that handles the code until it reaches this point might break the assumptions we have in the specific code (especially if we think about future changes to the logic). https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ src:*foo.cpp=emit
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
+ EXPECT_FALSE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, LongestMatchWins) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ src:*clang/lib/Sema/*=emit
+ src:*clang/lib/Sema/foo*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Basic/foo.h"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+"clang/lib/Sema/bar.h"));
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Sema/foo.h"));
+}
+
+TEST_F(SuppressionMappingTest, IsIgnored) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ Diags.getDiagnost
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/kadircet commented: > Note that there are a few open comments from previous iterations. I tried to go over them, but github UI was ... suboptimal. So PLMK if you got some that aren't addressed in this revision as well. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -701,11 +702,10 @@ class CompilerInstance : public ModuleLoader {
/// used by some diagnostics printers (for logging purposes only).
///
/// \return The new object on success, or null on failure.
- static IntrusiveRefCntPtr
- createDiagnostics(DiagnosticOptions *Opts,
-DiagnosticConsumer *Client = nullptr,
-bool ShouldOwnClient = true,
-const CodeGenOptions *CodeGenOpts = nullptr);
+ static IntrusiveRefCntPtr createDiagnostics(
+ DiagnosticOptions *Opts, DiagnosticConsumer *Client = nullptr,
+ bool ShouldOwnClient = true, const CodeGenOptions *CodeGenOpts = nullptr,
+ IntrusiveRefCntPtr VFS = nullptr);
kadircet wrote:
i got that coming up in a future change, pending this one to land.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.
+When a source file matches multiple globs, the longest one takes precendence.
kadircet wrote:
longest glob in the latest matching section. mentioned that matching still
happens within a single section.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.first();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.second.SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Err;
+ auto WarningSuppressionList = WarningsSpecialCaseList::create(Input, Err);
+ if (!WarningSuppressionList) {
+Report(diag::err_drv_malformed_warning_suppression_mapping)
+<< Input.getBufferIdentifier() << Err;
+return;
+ }
+ WarningSuppressionList->processSections(*this);
+ DiagSuppressionMapping =
+ [WarningSuppressionList(std::move(WarningSuppressionList))](
+ diag::kind DiagId, llvm::StringRef Path) {
+return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
+ };
+}
+
+bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
+ llvm::StringRef FilePath) const
{
+ const Section *DiagSection = DiagToSection.lookup(DiagId);
+ if (!DiagSection)
+return false;
+ const SectionEntries &EntityTypeToCategories = DiagSection->Entries;
+ auto SrcEntriesIt = EntityTypeToCategories.find("src");
+ if (SrcEntriesIt == EntityTypeToCategories.end())
+return false;
+ const llvm::StringMap &CategoriesToMatchers =
+ SrcEntriesIt->second;
kadircet wrote:
lookup returns by value (because it needs to return a default-constructed
object when key doesn't exist), hence can't bind it to a reference.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
kadircet wrote:
but diagnostics usually belong to multiple groups. i think the poor choice was
saying `groups` here rather than `sections`. reworded, ptal.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: sorry i am struggling to see the value in an integration test while we can (and do) test it in a unittest. can you elaborate ? https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: sorry missed that, added docs to clang root tree https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
kadircet wrote:
it really is called a category though,
https://clang.llvm.org/docs/SanitizerSpecialCaseList.html#:~:text=a%20tool%2Dspecific-,category,-%2C%20e.g.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ src:*foo.cpp=emit
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
+ EXPECT_FALSE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, LongestMatchWins) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ src:*clang/lib/Sema/*=emit
+ src:*clang/lib/Sema/foo*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Basic/foo.h"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+"clang/lib/Sema/bar.h"));
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Sema/foo.h"));
+}
+
+TEST_F(SuppressionMappingTest, IsIgnored) {
kadircet wrote:
i modified `DiagnosticIDs::getDiagnosticSeverity` and it is an implementation
detail of the public `isIgnored` API. hence yes, this patch changes the
observ
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.
kadircet wrote:
it's the latter. i was trying to emphasize that with `either/or`, do you have
any recommendations for making it more concrete?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
kadircet wrote:
the only interesting bit in the logic is propagation of input filename in the
warning message. i don't see the value in testing rest of the shared diagnostic
infrastructure. happy to test that if you feel strongly about this.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,109 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &MB, std::string &Err) {
+auto SCL = std::make_unique();
+if (!SCL->createInternal(&MB, Err))
+ return nullptr;
+return SCL;
+ }
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags) {
+// Drop the default section introduced by special case list, we only
support
+// exact diagnostic group names.
+Sections.erase("*");
+// Make sure we iterate sections by their line numbers.
+std::vector *>>
+LineAndSectionEntry;
+LineAndSectionEntry.reserve(Sections.size());
+for (const auto &Entry : Sections) {
+ LineAndSectionEntry.emplace_back(
+ Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry);
+}
+llvm::sort(LineAndSectionEntry);
+static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError;
+for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+ SmallVector GroupDiags;
+ llvm::StringRef DiagGroup = SectionEntry->getKey();
+ if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(kFlavor, DiagGroup,
+ GroupDiags)) {
+StringRef Suggestion =
+DiagnosticIDs::getNearestOption(kFlavor, DiagGroup);
+Diags.Report(diag::warn_unknown_diag_option)
+<< static_cast(kFlavor) << DiagGroup
+<< !Suggestion.empty() << Suggestion;
+continue;
+ }
+ for (diag::kind D : GroupDiags)
+DiagToSection[D] = &SectionEntry->getValue();
+}
+ }
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const {
+auto Section = DiagToSection.find(DiagId);
+if (Section == DiagToSection.end())
+ return false;
+auto &DiagEntries = Section->second->Entries;
+auto SrcEntries = DiagEntries.find("src");
+if (SrcEntries == DiagEntries.end())
+ return false;
+return globsMatches(SrcEntries->second, FilePath);
+ }
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(llvm::StringMap CategoriesToMatchers,
+llvm::StringRef FilePath) const {
+llvm::StringRef LongestMatch;
+bool LongestIsPositive = false;
+for (const auto &[Category, Matcher] : CategoriesToMatchers) {
+ bool IsPositive = Category != "emit";
+ for (const auto &[Pattern, Glob] : Matcher.Globs) {
kadircet wrote:
> Sure, I didn't mean to suggest we should do it for other SpecialCaseList
> users.
> We can limit sorting to this use case.
I am skeptical of that being a net-positive even for
warning-suppression-mappings. In the common path, compiler doesn't emit
warnings. Hence most of the compilations will just end up sorting this list,
and never query it afterwards.
> I wonder if it is required to have a different matching behavior or we could
> match by order so that users would just put their patterns in the order they
> want them to be matched.
I'd rather give users less ways to misconfigure this infrastructure. but by the
nature of the current behavior, users can get the exact same end result by just
ensuring a subdirectory is mentioned before its parents in the list, eg:
```
foo/bar/*=emit
foo/*
```
> If in other use cases the matching is by order, and we could have a pattern
> wider than another (foo/* wider than foo/bar/*), why is this use case
> different?
other use cases only allow inclusion, they don't allow exclusions.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
bricknerb wrote:
Nit: Consider removing the "txt" as it has no effect here.
Seems like other use cases of R"()" in llvm don't set this.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ src:*foo.cpp=emit
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
+ EXPECT_FALSE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, LongestMatchWins) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ src:*clang/lib/Sema/*=emit
+ src:*clang/lib/Sema/foo*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Basic/foo.h"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+"clang/lib/Sema/bar.h"));
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Sema/foo.h"));
+}
+
+TEST_F(SuppressionMappingTest, IsIgnored) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ Diags.getDiagnost
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
bricknerb wrote:
Why move instead of returning const& ?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
bricknerb wrote:
Why do we only test the message and not also level and possibly other fields?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ src:*foo.cpp=emit
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
+ EXPECT_FALSE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, LongestMatchWins) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*clang/*
+ src:*clang/lib/Sema/*=emit
+ src:*clang/lib/Sema/foo*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Basic/foo.h"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+"clang/lib/Sema/bar.h"));
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ "clang/lib/Sema/foo.h"));
+}
+
+TEST_F(SuppressionMappingTest, IsIgnored) {
bricknerb wrote:
Did we change the logic of isIgnored that we need to add specific tests for it
here?
I think most of the tests should be for isSuppressedViaMapping() and w
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "failed to process suppression mapping file "
+ "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(),
+ testing::ElementsAre(WithMessage(
+ "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+ llvm::StringLiteral SuppressionMappingFile = R"txt(
+ [unused]
+ src:*
+ )txt";
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+ EXPECT_TRUE(
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
bricknerb wrote:
What do you mean by "Work"?
Would "Exclusion" be enough? Or "ExclusionSuccess"?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
+public:
+ SuppressionMappingTest() {
+Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+ }
+
+protected:
+ llvm::IntrusiveRefCntPtr FS =
+ llvm::makeIntrusiveRefCnt();
+ DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+ std::vector takeDiags() {
+return std::move(CaptureConsumer.StoredDiags);
+ }
+
+private:
+ class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+ public:
+std::vector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+ const Diagnostic &Info) override {
+ StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+ };
+ CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+ return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+ "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+ Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+ FS->addFile("foo.txt", /*ModificationTime=*/{},
+ llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+ clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+ EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
bricknerb wrote:
Consider adding using for these:
```
using testing::ElementsAre;
using testing::IsEmpty;
```
It seems like the common practice in Clang's unit tests.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.first();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.second.SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Err;
+ auto WarningSuppressionList = WarningsSpecialCaseList::create(Input, Err);
+ if (!WarningSuppressionList) {
+Report(diag::err_drv_malformed_warning_suppression_mapping)
+<< Input.getBufferIdentifier() << Err;
+return;
+ }
+ WarningSuppressionList->processSections(*this);
+ DiagSuppressionMapping =
+ [WarningSuppressionList(std::move(WarningSuppressionList))](
+ diag::kind DiagId, llvm::StringRef Path) {
+return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
+ };
+}
+
+bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
+ llvm::StringRef FilePath) const
{
+ const Section *DiagSection = DiagToSection.lookup(DiagId);
+ if (!DiagSection)
+return false;
+ const SectionEntries &EntityTypeToCategories = DiagSection->Entries;
+ auto SrcEntriesIt = EntityTypeToCategories.find("src");
+ if (SrcEntriesIt == EntityTypeToCategories.end())
+return false;
+ const llvm::StringMap &CategoriesToMatchers =
+ SrcEntriesIt->second;
+ return globsMatches(CategoriesToMatchers, FilePath);
+}
+
+bool WarningsSpecialCaseList::globsMatches(
+const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const {
+ llvm::StringRef LongestMatch;
+ bool LongestIsPositive = false;
+ for (const auto &Entry : CategoriesToMatchers) {
+llvm::StringRef Category = Entry.first();
+const auto &Matcher = Entry.second;
bricknerb wrote:
Consider avoiding auto here since the type isn't trivial.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@list
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.first();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.second.SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Err;
+ auto WarningSuppressionList = WarningsSpecialCaseList::create(Input, Err);
+ if (!WarningSuppressionList) {
+Report(diag::err_drv_malformed_warning_suppression_mapping)
+<< Input.getBufferIdentifier() << Err;
+return;
+ }
+ WarningSuppressionList->processSections(*this);
+ DiagSuppressionMapping =
+ [WarningSuppressionList(std::move(WarningSuppressionList))](
+ diag::kind DiagId, llvm::StringRef Path) {
+return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
+ };
+}
+
+bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
+ llvm::StringRef FilePath) const
{
+ const Section *DiagSection = DiagToSection.lookup(DiagId);
+ if (!DiagSection)
+return false;
+ const SectionEntries &EntityTypeToCategories = DiagSection->Entries;
+ auto SrcEntriesIt = EntityTypeToCategories.find("src");
+ if (SrcEntriesIt == EntityTypeToCategories.end())
+return false;
+ const llvm::StringMap &CategoriesToMatchers =
+ SrcEntriesIt->second;
bricknerb wrote:
How about replacing lines 585-589 with:
const llvm::StringMap &CategoriesToMatchers =
EntityTypeToCategories.lookup("src");
I believe this will just work.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/bricknerb edited https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/bricknerb requested changes to this pull request. Note that there are a few open comments from previous iterations. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,136 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+// FIXME: We should isolate the parser from SpecialCaseList and just use it
+// here.
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &Input, std::string &Err);
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags);
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const;
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(const llvm::StringMap &CategoriesToMatchers,
+llvm::StringRef FilePath) const;
+
+ llvm::DenseMap DiagToSection;
+};
+} // namespace
+
+std::unique_ptr
+WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
+std::string &Err) {
+ auto WarningSuppressionList = std::make_unique();
+ if (!WarningSuppressionList->createInternal(&Input, Err))
+return nullptr;
+ return WarningSuppressionList;
+}
+
+void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
+ // Drop the default section introduced by special case list, we only support
+ // exact diagnostic group names.
+ // FIXME: We should make this configurable in the parser instead.
+ Sections.erase("*");
+ // Make sure we iterate sections by their line numbers.
+ std::vector *>>
+ LineAndSectionEntry;
+ LineAndSectionEntry.reserve(Sections.size());
+ for (const auto &Entry : Sections) {
+llvm::StringRef DiagName = Entry.first();
+// Each section has a matcher with that section's name, attached to that
+// line.
+const auto &DiagSectionMatcher = Entry.second.SectionMatcher;
+unsigned DiagLine = DiagSectionMatcher->Globs.at(DiagName).second;
+LineAndSectionEntry.emplace_back(DiagLine, &Entry);
+ }
+ llvm::sort(LineAndSectionEntry);
+ static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
+ for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+SmallVector GroupDiags;
+llvm::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(WarningFlavor) << DiagGroup
+ << !Suggestion.empty() << Suggestion;
+ continue;
+}
+for (diag::kind Diag : GroupDiags)
+ // We're intentionally overwriting any previous mappings here to make
sure
+ // latest one takes precedence.
+ DiagToSection[Diag] = &SectionEntry->getValue();
+ }
+}
+
+void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
+ std::string Err;
bricknerb wrote:
It seems more common to avoid the short name.
```suggestion
std::string Error;
```
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -701,11 +702,10 @@ class CompilerInstance : public ModuleLoader {
/// used by some diagnostics printers (for logging purposes only).
///
/// \return The new object on success, or null on failure.
- static IntrusiveRefCntPtr
- createDiagnostics(DiagnosticOptions *Opts,
-DiagnosticConsumer *Client = nullptr,
-bool ShouldOwnClient = true,
-const CodeGenOptions *CodeGenOpts = nullptr);
+ static IntrusiveRefCntPtr createDiagnostics(
+ DiagnosticOptions *Opts, DiagnosticConsumer *Client = nullptr,
+ bool ShouldOwnClient = true, const CodeGenOptions *CodeGenOpts = nullptr,
+ IntrusiveRefCntPtr VFS = nullptr);
bricknerb wrote:
Optional: Given that this already has 5 parameters, consider wrapping the
params in a struct to make the calls easier.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/bricknerb edited https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/bricknerb edited https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@ + +Warning suppression mappings + + +.. contents:: + :local: + +Introduction + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. + +Goal and usage +== + +Clang allows diagnostics to be configured at a translation-unit granularity. +If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers +also need to be clean. Hence turning on new warnings in large codebases can be +difficult today. Since it requires cleaning up all the existing warnings, bricknerb wrote: RE "today": If this documentation should describe what the feature is and how to use, I wouldn't refer to "today" as if we don't have the feature. Also, instead of "difficult" I would just continue to the next sentence to say it requires cleaning ... https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,109 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &MB, std::string &Err) {
+auto SCL = std::make_unique();
+if (!SCL->createInternal(&MB, Err))
+ return nullptr;
+return SCL;
+ }
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags) {
+// Drop the default section introduced by special case list, we only
support
+// exact diagnostic group names.
+Sections.erase("*");
+// Make sure we iterate sections by their line numbers.
+std::vector *>>
+LineAndSectionEntry;
+LineAndSectionEntry.reserve(Sections.size());
+for (const auto &Entry : Sections) {
+ LineAndSectionEntry.emplace_back(
+ Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry);
+}
+llvm::sort(LineAndSectionEntry);
+static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError;
+for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+ SmallVector GroupDiags;
+ llvm::StringRef DiagGroup = SectionEntry->getKey();
+ if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(kFlavor, DiagGroup,
+ GroupDiags)) {
+StringRef Suggestion =
+DiagnosticIDs::getNearestOption(kFlavor, DiagGroup);
+Diags.Report(diag::warn_unknown_diag_option)
+<< static_cast(kFlavor) << DiagGroup
+<< !Suggestion.empty() << Suggestion;
+continue;
+ }
+ for (diag::kind D : GroupDiags)
+DiagToSection[D] = &SectionEntry->getValue();
+}
+ }
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const {
+auto Section = DiagToSection.find(DiagId);
+if (Section == DiagToSection.end())
+ return false;
+auto &DiagEntries = Section->second->Entries;
+auto SrcEntries = DiagEntries.find("src");
+if (SrcEntries == DiagEntries.end())
+ return false;
+return globsMatches(SrcEntries->second, FilePath);
+ }
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(llvm::StringMap CategoriesToMatchers,
+llvm::StringRef FilePath) const {
+llvm::StringRef LongestMatch;
+bool LongestIsPositive = false;
+for (const auto &[Category, Matcher] : CategoriesToMatchers) {
+ bool IsPositive = Category != "emit";
+ for (const auto &[Pattern, Glob] : Matcher.Globs) {
bricknerb wrote:
Sure, I didn't mean to suggest we should do it for other SpecialCaseList users.
We can limit sorting to this use case.
It's not just implementation, it's also behavior.
Two similar mapping files in two different use cases would match different
patterns.
I wonder if it is required to have a different matching behavior or we could
match by order so that users would just put their patterns in the order they
want them to be matched.
If in other use cases the matching is by order, and we could have a pattern
wider than another (foo/* wider than foo/bar/*), why is this use case different?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
bricknerb wrote: Yes, I meant as an integration test. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -599,6 +600,19 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, SM.isInSystemMacro(Loc)) return diag::Severity::Ignored; + // Apply suppression mappings if severity wasn't explicitly mapped with a + // clang-diagnostics pragma to ensure pragmas always take precedence over bricknerb wrote: Carefully reading the comment, it's clear. We could make it easier perhaps. // Clang-diagnostics pragmas always take precedence over suppression mapping. The rest of the information in the comment can be put inside the first if(). https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,109 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &MB, std::string &Err) {
+auto SCL = std::make_unique();
+if (!SCL->createInternal(&MB, Err))
+ return nullptr;
+return SCL;
+ }
+
+ // Section names refer to diagnostic groups, which cover multiple individual
+ // diagnostics. Expand diagnostic groups here to individual diagnostics.
+ // A diagnostic can have multiple diagnostic groups associated with it, we
let
+ // the last section take precedence in such cases.
+ void processSections(DiagnosticsEngine &Diags) {
+// Drop the default section introduced by special case list, we only
support
+// exact diagnostic group names.
+Sections.erase("*");
+// Make sure we iterate sections by their line numbers.
+std::vector *>>
+LineAndSectionEntry;
+LineAndSectionEntry.reserve(Sections.size());
+for (const auto &Entry : Sections) {
+ LineAndSectionEntry.emplace_back(
+ Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry);
+}
+llvm::sort(LineAndSectionEntry);
+static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError;
+for (const auto &[_, SectionEntry] : LineAndSectionEntry) {
+ SmallVector GroupDiags;
+ llvm::StringRef DiagGroup = SectionEntry->getKey();
+ if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(kFlavor, DiagGroup,
+ GroupDiags)) {
+StringRef Suggestion =
+DiagnosticIDs::getNearestOption(kFlavor, DiagGroup);
+Diags.Report(diag::warn_unknown_diag_option)
+<< static_cast(kFlavor) << DiagGroup
+<< !Suggestion.empty() << Suggestion;
+continue;
+ }
+ for (diag::kind D : GroupDiags)
+DiagToSection[D] = &SectionEntry->getValue();
+}
+ }
+
+ bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const {
+auto Section = DiagToSection.find(DiagId);
+if (Section == DiagToSection.end())
+ return false;
+auto &DiagEntries = Section->second->Entries;
+auto SrcEntries = DiagEntries.find("src");
+if (SrcEntries == DiagEntries.end())
+ return false;
+return globsMatches(SrcEntries->second, FilePath);
+ }
+
+private:
+ // Find the longest glob pattern that matches FilePath amongst
+ // CategoriesToMatchers, return true iff the match exists and belongs to a
+ // positive category.
+ bool globsMatches(llvm::StringMap CategoriesToMatchers,
+llvm::StringRef FilePath) const {
+llvm::StringRef LongestMatch;
+bool LongestIsPositive = false;
+for (const auto &[Category, Matcher] : CategoriesToMatchers) {
+ bool IsPositive = Category != "emit";
+ for (const auto &[Pattern, Glob] : Matcher.Globs) {
kadircet wrote:
> RE: "rest of the speical-case-list actually match entries in line order",
sorry I think I was confused while writing this one. I was trying to say, rest
of the SpecialCaseList users can observe which line matched first. Hence
performing a re-ordering of patterns would result in behavioral changes for
these users (I can't say if it'll be a regression or not, as I don't fully
understand their use cases).
> do you mean that the behavior here is significantly different. Would this be
> confusing?
not sure what you mean here exactly. overall we don't have a different matching
behavior in a "user-visible" manner. implementation-wise it's slightly
different than other sanitizer-list matching, as we keep searching for longest
matching glob, rather than stopping at first match. it's documented as such and
I think is intuitive for users (that the most specific entry takes precedence).
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
kadircet wrote: this is already tested in https://github.com/llvm/llvm-project/pull/112517/files#diff-a8d03c054381627f238bdb8f7914daa4abebdae705d313dda026ce36eca0fe2fR327-R332 https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +485,100 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
kadircet wrote:
also see https://discourse.llvm.org/t/refactoring-llvm-specialcaselist/82931
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -70,6 +76,16 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
else
Diags.setExtensionHandlingBehavior(diag::Severity::Ignored);
+ if (!Opts.DiagnosticSuppressionMappingsFile.empty()) {
+if (auto Buf =
kadircet wrote:
sure, i think it's a little bit unfortunate to spend so much effort in a
variable that's scoped to 2 lines.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -122,7 +122,6 @@ class SpecialCaseList {
// Returns zero if no match is found.
unsigned match(StringRef Query) const;
- private:
StringMap> Globs;
kadircet wrote:
thanks, and I'd like to address these in a follow-up refactoring, sent out
https://discourse.llvm.org/t/refactoring-llvm-specialcaselist/82931 to see what
people think about this.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -599,6 +600,19 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, SM.isInSystemMacro(Loc)) return diag::Severity::Ignored; + // Apply suppression mappings if severity wasn't explicitly mapped with a + // clang-diagnostics pragma to ensure pragmas always take precedence over kadircet wrote: pragma takes precedence and we don't suppress via the mappings. do you think the comment needs rewording? https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
kadircet wrote:
yes, see
https://github.com/llvm/llvm-project/pull/112517/files#diff-a8d03c054381627f238bdb8f7914daa4abebdae705d313dda026ce36eca0fe2fR327-R332
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
bricknerb wrote:
After seeing the below text and example, I think it's better to say that a
diagnostic "belongs to" multiple groups.
When you say "matched", I assumed you mean the diagnostic has matching global
in multiple group sections.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -122,7 +122,6 @@ class SpecialCaseList {
// Returns zero if no match is found.
unsigned match(StringRef Query) const;
- private:
StringMap> Globs;
AaronBallman wrote:
Oh, I missed the inner class context, that makes sense, thanks! It unfortunate
that we're losing encapsulation with the change, but I can hold my nose.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
bricknerb wrote:
> Sections are denoted as ``[unused]`` in this format.
Isn't "unused" only an example for a diagnostic group?
Did you mean to refer to the specific example?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
bricknerb wrote: Consider referring to pragmas. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -70,6 +76,16 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
else
Diags.setExtensionHandlingBehavior(diag::Severity::Ignored);
+ if (!Opts.DiagnosticSuppressionMappingsFile.empty()) {
+if (auto Buf =
bricknerb wrote:
[Buf](https://github.com/search?type=code&auto_enroll=true&q=repo%3Allvm%2Fllvm-project+%2Fbuf%5B%5Ef%5D.*%3D.*%5Cn%3F.*getBufferForFile%2F)
is less common, and more so if we focus on Clang code. IIUC, this is the
content of the file, FileBuffer or FileContent is more descriptive, and Buffer
is better than Buf.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
bricknerb wrote: Consider also testing the same warning belonging to multiple groups. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
bricknerb wrote:
"category" here might be confused with a diagnostic category, which is probably
not what you mean here.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
// Make sure an empty warning can round-trip with \c StoredDiagnostic.
Diags.Report(CaptureConsumer.StoredDiags.front());
}
+
+class SuppressionMappingTest : public testing::Test {
bricknerb wrote:
Do you also test pragmas here?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.
bricknerb wrote:
When you say "or" do you we try to match both or do we match in one of these
methods? I think it's worth to clarify that.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,118 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &MB, std::string &Err) {
+auto SCL = std::make_unique();
bricknerb wrote:
Replace SCL with WarningSuppressionList
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -946,6 +955,27 @@ class DiagnosticsEngine : public
RefCountedBase {
return (Level)Diags->getDiagnosticLevel(DiagID, Loc, *this);
}
+ /// Diagnostic suppression mappings can be used to suppress specific
+ /// diagnostics in specific files.
+ /// Mapping file is expected to be a special case list with sections denoting
+ /// diagnostic groups and `src` entries for globs to suppress. `emit`
category
+ /// can be used to disable suppression. Longest glob that matches a filepath
+ /// takes precendence. For example:
bricknerb wrote:
```suggestion
/// takes precedence. For example:
```
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.
+When a source file matches multiple globs, the longest one takes precendence.
bricknerb wrote:
What happens if a warning that belongs to multiple diagnostic groups is in a
source file that matches multiple globs in multiple diagnostic groups?
Would it be the longest glob in the last group or the longest glob in all
categories?
I think it's clear from the example that the warning would only trigger the
section of the latest group, but it might worth clarifying.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
bricknerb wrote:
precedence
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@ + +Warning suppression mappings + + +.. contents:: + :local: + +Introduction + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. + +Goal and usage +== + +Clang allows diagnostics to be configured at a translation-unit granularity. +If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers +also need to be clean. Hence turning on new warnings in large codebases can be +difficult today. Since it requires cleaning up all the existing warnings, +which might not be possible when some dependencies aren't in the project owner's +control or because new violations are creeping up quicker than the clean up. + +Warning suppression mappings aim to alleviate some of these concerns by making +diagnostic configuration granularity finer, at a source file level. + +To achieve this, user can create a file that lists which diagnostic groups to bricknerb wrote: Perhaps refer somewhere to what is a "diagnostic group" so a user can easily what diagnostic groups are available? https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -477,6 +486,118 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor
Flavor,
setSeverity(Diag, Map, Loc);
}
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+ static std::unique_ptr
+ create(const llvm::MemoryBuffer &MB, std::string &Err) {
bricknerb wrote:
Avoid putting the implementation in an anonymous namespace.
Per https://llvm.org/docs/CodingStandards.html#anonymous-namespaces, we should
only put the class declaration into anonymous namespace.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+ :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+ $ cat my/user/code.cpp
+ #include
+ namespace { void unused_func1(); }
+
+ $ cat foo/bar.h
+ namespace { void unused_func2(); }
+
+ $ cat suppression_mappings.txt
+ # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+ [unused]
+ src:*
+ src:*foo/*=emit
+ $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt
my/user/code.cpp
+ # prints warning: unused function 'unused_func2', but no warnings for
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
bricknerb wrote:
"th" typo.
```suggestion
Source files are matched against these globs either as paths relative to the
```
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
bricknerb wrote: Given that Test documentation build check failed, it's worth clarifying how the documentation was tested and perhaps add a few screenshots. https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/112517
From 20f1c1fea3466de38a04b5486cb05d95dbe3b96c Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Mon, 14 Oct 2024 11:20:55 +0200
Subject: [PATCH 1/7] [clang] Introduce diagnostics suppression mappings
This implements
https://discourse.llvm.org/t/rfc-add-support-for-controlling-diagnostics-severities-at-file-level-granularity-through-command-line/81292.
Users now can suppress warnings for certain headers by providing a
mapping with globs, a sample file looks like:
```
[unused]
src:*
src:*clang/*=emit
```
This will suppress warnings from `-Wunused` group in all files that
aren't under `clang/` directory. This mapping file can be passed to
clang via `--warning-suppression-mappings=foo.txt`.
At a high level, mapping file is stored in DiagnosticOptions and then
processed with rest of the warning flags when creating a
DiagnosticsEngine. This is a functor that uses SpecialCaseLists
underneath to match against globs coming from the mappings file.
This implies processing warning options now performs IO, relevant
interfaces are updated to take in a VFS, falling back to RealFileSystem
when one is not available.
---
.../ExpandModularHeadersPPCallbacks.cpp | 3 +-
clang/include/clang/Basic/Diagnostic.h| 15 +-
.../clang/Basic/DiagnosticDriverKinds.td | 3 +
clang/include/clang/Basic/DiagnosticOptions.h | 3 +
clang/include/clang/Driver/Options.td | 5 +
.../include/clang/Frontend/CompilerInstance.h | 10 +-
clang/lib/Basic/Diagnostic.cpp| 12 ++
clang/lib/Basic/DiagnosticIDs.cpp | 12 ++
clang/lib/Basic/Warnings.cpp | 107
clang/lib/Driver/ToolChains/Clang.cpp | 7 +
clang/lib/Frontend/ASTUnit.cpp| 15 +-
clang/lib/Frontend/CompilerInstance.cpp | 21 ++-
clang/lib/Frontend/CompilerInvocation.cpp | 8 +
clang/lib/Frontend/PrecompiledPreamble.cpp| 2 +-
clang/lib/Interpreter/CodeCompletion.cpp | 4 +-
clang/lib/Serialization/ASTReader.cpp | 4 +-
.../test/Misc/Inputs/suppression-mapping.txt | 4 +
.../Misc/warning-suppression-mappings.cpp | 16 ++
clang/tools/driver/cc1gen_reproducer_main.cpp | 7 +-
clang/tools/driver/driver.cpp | 7 +-
clang/unittests/Basic/DiagnosticTest.cpp | 164 ++
.../Frontend/CompilerInvocationTest.cpp | 10 ++
llvm/include/llvm/Support/SpecialCaseList.h | 1 -
23 files changed, 409 insertions(+), 31 deletions(-)
create mode 100644 clang/test/Misc/Inputs/suppression-mapping.txt
create mode 100644 clang/test/Misc/warning-suppression-mappings.cpp
diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index fef086c5a99d86..4c34f9ea122d9e 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -81,7 +81,8 @@
ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(
Diags.setSourceManager(&Sources);
// FIXME: Investigate whatever is there better way to initialize DiagEngine
// or whatever DiagEngine can be shared by multiple preprocessors
- ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());
+ ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts(),
+Compiler.getVirtualFileSystem());
LangOpts.Modules = false;
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 3b1efdb12824c7..79b7545e92d6da 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -20,11 +20,13 @@
#include "clang/Basic/Specifiers.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include
#include
#include
@@ -555,6 +557,10 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
+ /// Whether the diagnostic should be suppressed in FilePath.
+ llvm::unique_function
+ DiagSuppressionMapping;
+
public:
explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
IntrusiveRefCntPtr DiagOpts,
@@ -946,6 +952,13 @@ class DiagnosticsEngine : public
RefCountedBase {
return (Level)Diags->getDiagnosticLevel(DiagID, Loc, *this);
}
+ /// Diagnostic suppression mappings can be used to ignore diagnostics based
on
+ /// the file they occur in.
+ /// These take presumed locations into account, and can still be overriden by
+ /// clang-diagnostics pragmas.
+ void setDiagSuppressionMapping(decltype(DiagS
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -122,7 +122,6 @@ class SpecialCaseList {
// Returns zero if no match is found.
unsigned match(StringRef Query) const;
- private:
StringMap> Globs;
kadircet wrote:
these are members of `SpecialCaseList::Matcher`. `WarningsSpecialCaseList` is
just inheriting from `SpecialCaseList`. Hence it won't have visibility into
protected members of a nested class.
e.g. something like:
```cpp
$ cat a.cc
struct Foo {
class Bar {
protected:
int x;
};
};
struct Derived : public Foo {
void x() {
Bar b;
b.x = 3; // visibility error
}
};
```
`Derived` can't access `Foo::Bar::x`.
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -122,7 +122,6 @@ class SpecialCaseList {
// Returns zero if no match is found.
unsigned match(StringRef Query) const;
- private:
StringMap> Globs;
AaronBallman wrote:
The only places I see it used are within `class WarningsSpecialCaseList :
public llvm::SpecialCaseList` -- what am I missing?
https://github.com/llvm/llvm-project/pull/112517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)
@@ -1315,6 +1319,32 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings + + +Warning suppression mappings enable users to suppress Clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in some headers. + +.. code-block:: console + + $ cat mappings.txt + [unused] + src:foo/* + + $ clang --warning-suppression-mappings=mapping.txt -Wunused foo/bar.cc + # This compilation won't emit any unused findings for sources under foo/ + # directory. But it'll still complain for all the other sources, e.g: + $ cat foo/bar.cc + #include "dir/include.h" // clang flags unused declarations here. + #include "foo/include.h" // but unused warnings under this source is omitted. + + AaronBallman wrote: ```suggestion $ cat foo/bar.cc #include "dir/include.h" // Clang flags unused declarations here #include "foo/include.h" // but unused warnings from this header are omitted #include "next_to_bar_cc.h" // as are unused warnings from this header file. // Further, unused warnings in the remainder of bar.cc are also omitted. ``` https://github.com/llvm/llvm-project/pull/112517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
