[llvm-branch-commits] [clang] Clang multi-match behavior of WarningSuppressionMappings (PR #162237)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Oct 7 08:26:36 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Vitaly Buka (vitalybuka)
<details>
<summary>Changes</summary>
"The longest wins" is confusing.
E.g. why here `test/` should win?
```
src:*test/*
src:*lld/*=emit
```
For long suppression files length relationship can be hard to track.
Easier to understand rule is "the last wins".
We changed sanitizers to this approach way and
would be nice to have consistency hare.
Also consistency allows us avoid exposing
internals of SpecialCaseList in future.
---
Full diff: https://github.com/llvm/llvm-project/pull/162237.diff
5 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/docs/WarningSuppressionMappings.rst (+2-2)
- (modified) clang/include/clang/Basic/Diagnostic.h (+1-1)
- (modified) clang/lib/Basic/Diagnostic.cpp (+15-38)
- (modified) clang/unittests/Basic/DiagnosticTest.cpp (+3-5)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 390e0fa7a9a2b..84ad1ae64f06b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -69,6 +69,8 @@ Potentially Breaking Changes
call the member ``operator delete`` instead of the expected global
delete operator. The old behavior is retained under ``-fclang-abi-compat=21``
flag.
+- Clang warning suppressions file, ``--warning-suppression-mappings=``, now will
+ use the last matching entry instead of the longest one.
C/C++ Language Potentially Breaking Changes
-------------------------------------------
diff --git a/clang/docs/WarningSuppressionMappings.rst b/clang/docs/WarningSuppressionMappings.rst
index d96341ac6e563..d8af856f64ef0 100644
--- a/clang/docs/WarningSuppressionMappings.rst
+++ b/clang/docs/WarningSuppressionMappings.rst
@@ -63,7 +63,7 @@ Format
Warning suppression mappings uses the same format as
:doc:`SanitizerSpecialCaseList`.
-Sections describe which diagnostic group's behaviour to change, e.g.
+Sections describe which diagnostic group's behavior to change, e.g.
``[unused]``. When a diagnostic is matched by multiple sections, the latest
section takes precedence.
@@ -76,7 +76,7 @@ Source files are matched against these globs either:
- as paths relative to the current working directory
- as absolute paths.
-When a source file matches multiple globs in a section, the longest one takes
+When a source file matches multiple globs in a section, the last one takes
precedence.
.. code-block:: bash
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index e540040ddc524..bd3165f64f4a0 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -971,7 +971,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
/// 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
+ /// can be used to disable suppression. THe last glob that matches a filepath
/// takes precedence. For example:
/// [unused]
/// src:clang/*
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 2b89370a42d1b..b6dad86c038d3 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -517,12 +517,6 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList {
const SourceManager &SM) 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<Matcher> &CategoriesToMatchers,
- StringRef FilePath) const;
-
llvm::DenseMap<diag::kind, const Section *> DiagToSection;
};
} // namespace
@@ -584,43 +578,26 @@ void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
SourceLocation DiagLoc,
const SourceManager &SM) const {
+ PresumedLoc PLoc = SM.getPresumedLoc(DiagLoc);
+ if (!PLoc.isValid())
+ return false;
const Section *DiagSection = DiagToSection.lookup(DiagId);
if (!DiagSection)
return false;
- const SectionEntries &EntityTypeToCategories = DiagSection->Entries;
- auto SrcEntriesIt = EntityTypeToCategories.find("src");
- if (SrcEntriesIt == EntityTypeToCategories.end())
+
+ StringRef F = llvm::sys::path::remove_leading_dotslash(PLoc.getFilename());
+
+ unsigned SuppressLineNo =
+ llvm::SpecialCaseList::inSectionBlame(DiagSection->Entries, "src", F, "");
+ if (!SuppressLineNo)
return false;
- const llvm::StringMap<llvm::SpecialCaseList::Matcher> &CategoriesToMatchers =
- SrcEntriesIt->getValue();
- // We also use presumed locations here to improve reproducibility for
- // preprocessed inputs.
- if (PresumedLoc PLoc = SM.getPresumedLoc(DiagLoc); PLoc.isValid())
- return globsMatches(
- CategoriesToMatchers,
- llvm::sys::path::remove_leading_dotslash(PLoc.getFilename()));
- return false;
-}
-bool WarningsSpecialCaseList::globsMatches(
- const llvm::StringMap<Matcher> &CategoriesToMatchers,
- StringRef FilePath) const {
- StringRef LongestMatch;
- bool LongestIsPositive = false;
- for (const auto &Entry : CategoriesToMatchers) {
- StringRef Category = Entry.getKey();
- const llvm::SpecialCaseList::Matcher &Matcher = Entry.getValue();
- bool IsPositive = Category != "emit";
- for (const auto &Glob : Matcher.Globs) {
- if (Glob->Name.size() < LongestMatch.size())
- continue;
- if (!Glob->Pattern.match(FilePath))
- continue;
- LongestMatch = Glob->Name;
- LongestIsPositive = IsPositive;
- }
- }
- return LongestIsPositive;
+ unsigned EmitLineNo = llvm::SpecialCaseList::inSectionBlame(
+ DiagSection->Entries, "src", F, "emit");
+ if (!EmitLineNo)
+ return true;
+
+ return SuppressLineNo > EmitLineNo;
}
bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind DiagId,
diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index de090864e5095..5492146f40fa9 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -294,7 +294,7 @@ TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
locForFile("foo.cpp")));
}
-TEST_F(SuppressionMappingTest, LongestMatchWins) {
+TEST_F(SuppressionMappingTest, LastMatchWins) {
llvm::StringLiteral SuppressionMappingFile = R"(
[unused]
src:*clang/*
@@ -327,10 +327,8 @@ TEST_F(SuppressionMappingTest, LongShortMatch) {
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
locForFile("test/t1.cpp")));
-
- // FIXME: This is confusing.
- EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
- locForFile("lld/test/t2.cpp")));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ locForFile("lld/test/t2.cpp")));
}
TEST_F(SuppressionMappingTest, ShortLongMatch) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/162237
More information about the llvm-branch-commits
mailing list