From fd2dad1fcdea9ac27ed91bed44cee5385326c5cf Mon Sep 17 00:00:00 2001 From: Vitaly Buka 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 { /// 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 &CategoriesToMatchers, - StringRef FilePath) const; - llvm::DenseMap 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 &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 &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 #include #include +#include #include 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, 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> Globs; - std::vector, unsigned>> RegExes; + std::vector> 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 M; }; using SectionEntries = StringMap>; 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 SectionMatcher = std::make_unique(); + Matcher SectionMatcher; SectionEntries Entries; std::string SectionStr; unsigned FileIdx; @@ -157,7 +179,7 @@ class SpecialCaseList { LLVM_ABI Expected
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(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(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(); + auto Glob = std::make_unique(); 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(); + else + M.emplace(); +} + +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::create(const std::vector &Paths, @@ -132,10 +156,10 @@ bool SpecialCaseList::createInternal(const MemoryBuffer *MB, Expected 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 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};