[clang] Switch warning suppression multi-match rule to "last match takes precedence"

The current "longest match takes precedence" rule
for warning suppression mappings can be confusing,
especially in long suppression files where
tracking the length relationship between globs is
difficult.

For example, with the following rules, it's not
immediately obvious why the first one should
currently take precedence:

```
src:*test/*
src:*lld/*=emit
```

This commit changes the multi-match behavior so
the last match takes precedence. This rule is
easier to understand and consistent with the
approach used by sanitizers, simplifying the
mechanism by providing a uniform experience across
different tools.

This is potentially breaking, but very unlikely.
An investigation of known uses showed they do not
rely on the length.

Reviewers: thurstond, kadircet, fmayer

Pull Request: https://github.com/llvm/llvm-project/pull/162237
This commit is contained in:
Vitaly Buka 2025-10-06 16:01:34 -07:00
parent 14296285f9
commit ab08fbd92c
5 changed files with 13 additions and 17 deletions

View File

@ -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.
- Trailing null statements in GNU statement expressions are no longer
ignored by Clang; they now result in a void type. Clang previously
matched GCC's behavior, which was recently clarified to be incorrect.

View File

@ -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

View File

@ -971,7 +971,7 @@ public:
/// 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/*

View File

@ -525,8 +525,7 @@ std::unique_ptr<WarningsSpecialCaseList>
WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
std::string &Err) {
auto WarningSuppressionList = std::make_unique<WarningsSpecialCaseList>();
if (!WarningSuppressionList->createInternal(&Input, Err,
/*OrderBySize=*/true))
if (!WarningSuppressionList->createInternal(&Input, Err))
return nullptr;
return WarningSuppressionList;
}
@ -588,15 +587,12 @@ bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
StringRef F = llvm::sys::path::remove_leading_dotslash(PLoc.getFilename());
StringRef LongestSup = DiagSection->getLongestMatch("src", F, "");
if (LongestSup.empty())
unsigned LastSup = DiagSection->getLastMatch("src", F, "");
if (LastSup == 0)
return false;
StringRef LongestEmit = DiagSection->getLongestMatch("src", F, "emit");
if (LongestEmit.empty())
return true;
return LongestSup.size() > LongestEmit.size();
unsigned LastEmit = DiagSection->getLastMatch("src", F, "emit");
return LastSup > LastEmit;
}
bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind DiagId,

View File

@ -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) {