[clang] [llvm] [NFC][SpecialCaseList] Use BumpPtrAllocator to own strings (PR #162304)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 8 15:27:21 PDT 2025
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/162304
>From fd2dad1fcdea9ac27ed91bed44cee5385326c5cf Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Tue, 7 Oct 2025 08:26:01 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20change?=
=?UTF-8?q?s=20to=20main=20this=20commit=20is=20based=20on?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.6
[skip ci]
---
clang/docs/ReleaseNotes.rst | 2 +
clang/docs/WarningSuppressionMappings.rst | 4 +-
clang/include/clang/Basic/Diagnostic.h | 2 +-
clang/lib/Basic/Diagnostic.cpp | 53 ++++--------
clang/lib/Basic/SanitizerSpecialCaseList.cpp | 2 +-
clang/unittests/Basic/DiagnosticTest.cpp | 8 +-
llvm/include/llvm/Support/SpecialCaseList.h | 44 +++++++---
llvm/lib/Support/SpecialCaseList.cpp | 91 +++++++++++++-------
8 files changed, 115 insertions(+), 91 deletions(-)
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/lib/Basic/SanitizerSpecialCaseList.cpp b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
index f7bc1d5545d75..582c2557d8aa7 100644
--- a/clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -42,7 +42,7 @@ void SanitizerSpecialCaseList::createSanitizerSections() {
SanitizerMask Mask;
#define SANITIZER(NAME, ID) \
- if (S.SectionMatcher->match(NAME)) \
+ if (S.SectionMatcher.match(NAME)) \
Mask |= SanitizerKind::ID;
#define SANITIZER_GROUP(NAME, ID, ALIAS) SANITIZER(NAME, ID)
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) {
diff --git a/llvm/include/llvm/Support/SpecialCaseList.h b/llvm/include/llvm/Support/SpecialCaseList.h
index 22a62eac9e01a..eff36569fcab7 100644
--- a/llvm/include/llvm/Support/SpecialCaseList.h
+++ b/llvm/include/llvm/Support/SpecialCaseList.h
@@ -19,6 +19,7 @@
#include <memory>
#include <string>
#include <utility>
+#include <variant>
#include <vector>
namespace llvm {
@@ -118,15 +119,20 @@ class SpecialCaseList {
SpecialCaseList(SpecialCaseList const &) = delete;
SpecialCaseList &operator=(SpecialCaseList const &) = delete;
- /// Represents a set of globs and their line numbers
- class Matcher {
+ // Lagacy v1 matcher.
+ class RegexMatcher {
+ public:
+ LLVM_ABI unsigned match(StringRef Query) const;
+ LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber);
+ std::vector<std::pair<std::unique_ptr<Regex>, unsigned>> RegExes;
+ };
+
+ class GlobMatcher {
public:
- LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber,
- bool UseRegex);
// Returns the line number in the source file that this query matches to.
// Returns zero if no match is found.
LLVM_ABI unsigned match(StringRef Query) const;
-
+ LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber);
struct Glob {
std::string Name;
unsigned LineNo;
@@ -137,17 +143,33 @@ class SpecialCaseList {
Glob() = default;
};
- std::vector<std::unique_ptr<Matcher::Glob>> Globs;
- std::vector<std::pair<std::unique_ptr<Regex>, unsigned>> RegExes;
+ std::vector<std::unique_ptr<GlobMatcher::Glob>> Globs;
+ };
+
+ /// Represents a set of globs and their line numbers
+ class Matcher {
+ public:
+ LLVM_ABI explicit Matcher(bool UseGlobs);
+ // Returns the line number in the source file that this query matches to.
+ // Returns zero if no match is found.
+ LLVM_ABI unsigned match(StringRef Query) const;
+
+ private:
+ friend class SpecialCaseList;
+ LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber);
+
+ std::variant<RegexMatcher, GlobMatcher> M;
};
using SectionEntries = StringMap<StringMap<Matcher>>;
struct Section {
- Section(StringRef Str, unsigned FileIdx)
- : SectionStr(Str), FileIdx(FileIdx) {};
+ Section(StringRef Str, unsigned FileIdx, bool UseGlobs)
+ : SectionMatcher(UseGlobs), SectionStr(Str), FileIdx(FileIdx) {};
+
+ Section(Section &&) = default;
- std::unique_ptr<Matcher> SectionMatcher = std::make_unique<Matcher>();
+ Matcher SectionMatcher;
SectionEntries Entries;
std::string SectionStr;
unsigned FileIdx;
@@ -157,7 +179,7 @@ class SpecialCaseList {
LLVM_ABI Expected<Section *> addSection(StringRef SectionStr,
unsigned FileIdx, unsigned LineNo,
- bool UseGlobs = true);
+ bool UseGlobs);
/// Parses just-constructed SpecialCaseList entries from a memory buffer.
LLVM_ABI bool parse(unsigned FileIdx, const MemoryBuffer *MB,
diff --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp
index 8d4e043bc1c9f..71f7b9aa65796 100644
--- a/llvm/lib/Support/SpecialCaseList.cpp
+++ b/llvm/lib/Support/SpecialCaseList.cpp
@@ -25,36 +25,48 @@
namespace llvm {
-Error SpecialCaseList::Matcher::insert(StringRef Pattern, unsigned LineNumber,
- bool UseGlobs) {
- if (Pattern.empty())
+Error SpecialCaseList::RegexMatcher::insert(StringRef Pattern,
+ unsigned LineNumber) {
+ if (Pattern.empty()) {
return createStringError(errc::invalid_argument,
- Twine("Supplied ") +
- (UseGlobs ? "glob" : "regex") + " was blank");
-
- if (!UseGlobs) {
- // Replace * with .*
- auto Regexp = Pattern.str();
- for (size_t pos = 0; (pos = Regexp.find('*', pos)) != std::string::npos;
- pos += strlen(".*")) {
- Regexp.replace(pos, strlen("*"), ".*");
- }
+ "Supplied regex was blank");
+ }
+
+ // Replace * with .*
+ auto Regexp = Pattern.str();
+ for (size_t pos = 0; (pos = Regexp.find('*', pos)) != std::string::npos;
+ pos += strlen(".*")) {
+ Regexp.replace(pos, strlen("*"), ".*");
+ }
+
+ Regexp = (Twine("^(") + StringRef(Regexp) + ")$").str();
+
+ // Check that the regexp is valid.
+ Regex CheckRE(Regexp);
+ std::string REError;
+ if (!CheckRE.isValid(REError))
+ return createStringError(errc::invalid_argument, REError);
- Regexp = (Twine("^(") + StringRef(Regexp) + ")$").str();
+ RegExes.emplace_back(
+ std::make_pair(std::make_unique<Regex>(std::move(CheckRE)), LineNumber));
- // Check that the regexp is valid.
- Regex CheckRE(Regexp);
- std::string REError;
- if (!CheckRE.isValid(REError))
- return createStringError(errc::invalid_argument, REError);
+ return Error::success();
+}
- RegExes.emplace_back(std::make_pair(
- std::make_unique<Regex>(std::move(CheckRE)), LineNumber));
+unsigned SpecialCaseList::RegexMatcher::match(StringRef Query) const {
+ for (const auto &[Regex, LineNumber] : reverse(RegExes))
+ if (Regex->match(Query))
+ return LineNumber;
+ return 0;
+}
- return Error::success();
+Error SpecialCaseList::GlobMatcher::insert(StringRef Pattern,
+ unsigned LineNumber) {
+ if (Pattern.empty()) {
+ return createStringError(errc::invalid_argument, "Supplied glob was blank");
}
- auto Glob = std::make_unique<Matcher::Glob>();
+ auto Glob = std::make_unique<GlobMatcher::Glob>();
Glob->Name = Pattern.str();
Glob->LineNo = LineNumber;
// We must be sure to use the string in `Glob` rather than the provided
@@ -66,16 +78,28 @@ Error SpecialCaseList::Matcher::insert(StringRef Pattern, unsigned LineNumber,
return Error::success();
}
-unsigned SpecialCaseList::Matcher::match(StringRef Query) const {
+unsigned SpecialCaseList::GlobMatcher::match(StringRef Query) const {
for (const auto &Glob : reverse(Globs))
if (Glob->Pattern.match(Query))
return Glob->LineNo;
- for (const auto &[Regex, LineNumber] : reverse(RegExes))
- if (Regex->match(Query))
- return LineNumber;
return 0;
}
+SpecialCaseList::Matcher::Matcher(bool UseGlobs) {
+ if (UseGlobs)
+ M.emplace<GlobMatcher>();
+ else
+ M.emplace<RegexMatcher>();
+}
+
+unsigned SpecialCaseList::Matcher::match(StringRef Query) const {
+ return std::visit([&](auto &V) { return V.match(Query); }, M);
+}
+
+Error SpecialCaseList::Matcher::insert(StringRef Pattern, unsigned LineNumber) {
+ return std::visit([&](auto &V) { return V.insert(Pattern, LineNumber); }, M);
+}
+
// TODO: Refactor this to return Expected<...>
std::unique_ptr<SpecialCaseList>
SpecialCaseList::create(const std::vector<std::string> &Paths,
@@ -132,10 +156,10 @@ bool SpecialCaseList::createInternal(const MemoryBuffer *MB,
Expected<SpecialCaseList::Section *>
SpecialCaseList::addSection(StringRef SectionStr, unsigned FileNo,
unsigned LineNo, bool UseGlobs) {
- Sections.emplace_back(SectionStr, FileNo);
+ Sections.emplace_back(SectionStr, FileNo, UseGlobs);
auto &Section = Sections.back();
- if (auto Err = Section.SectionMatcher->insert(SectionStr, LineNo, UseGlobs)) {
+ if (auto Err = Section.SectionMatcher.insert(SectionStr, LineNo)) {
return createStringError(errc::invalid_argument,
"malformed section at line " + Twine(LineNo) +
": '" + SectionStr +
@@ -148,7 +172,7 @@ SpecialCaseList::addSection(StringRef SectionStr, unsigned FileNo,
bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
std::string &Error) {
Section *CurrentSection;
- if (auto Err = addSection("*", FileIdx, 1).moveInto(CurrentSection)) {
+ if (auto Err = addSection("*", FileIdx, 1, true).moveInto(CurrentSection)) {
Error = toString(std::move(Err));
return false;
}
@@ -194,8 +218,9 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
}
auto [Pattern, Category] = Postfix.split("=");
- auto &Entry = CurrentSection->Entries[Prefix][Category];
- if (auto Err = Entry.insert(Pattern, LineNo, UseGlobs)) {
+ auto [It, _] =
+ CurrentSection->Entries[Prefix].try_emplace(Category, UseGlobs);
+ if (auto Err = It->second.insert(Pattern, LineNo)) {
Error =
(Twine("malformed ") + (UseGlobs ? "glob" : "regex") + " in line " +
Twine(LineNo) + ": '" + Pattern + "': " + toString(std::move(Err)))
@@ -218,7 +243,7 @@ std::pair<unsigned, unsigned>
SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
StringRef Query, StringRef Category) const {
for (const auto &S : reverse(Sections)) {
- if (S.SectionMatcher->match(Section)) {
+ if (S.SectionMatcher.match(Section)) {
unsigned Blame = inSectionBlame(S.Entries, Prefix, Query, Category);
if (Blame)
return {S.FileIdx, Blame};
More information about the llvm-commits
mailing list