From 9b3623933c78e4eebe9362ca1b7b7df29a185c1a Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 7 Oct 2025 08:25:54 -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 | 4 +- llvm/lib/Support/SpecialCaseList.cpp | 4 +- 8 files changed, 29 insertions(+), 50 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..c2c9271b0a37a 100644 --- a/llvm/include/llvm/Support/SpecialCaseList.h +++ b/llvm/include/llvm/Support/SpecialCaseList.h @@ -147,7 +147,9 @@ class SpecialCaseList { Section(StringRef Str, unsigned FileIdx) : SectionStr(Str), FileIdx(FileIdx) {}; - std::unique_ptr SectionMatcher = std::make_unique(); + Section(Section &&) = default; + + Matcher SectionMatcher; SectionEntries Entries; std::string SectionStr; unsigned FileIdx; diff --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp index 8d4e043bc1c9f..4b038850b62ca 100644 --- a/llvm/lib/Support/SpecialCaseList.cpp +++ b/llvm/lib/Support/SpecialCaseList.cpp @@ -135,7 +135,7 @@ SpecialCaseList::addSection(StringRef SectionStr, unsigned FileNo, Sections.emplace_back(SectionStr, FileNo); auto &Section = Sections.back(); - if (auto Err = Section.SectionMatcher->insert(SectionStr, LineNo, UseGlobs)) { + if (auto Err = Section.SectionMatcher.insert(SectionStr, LineNo, UseGlobs)) { return createStringError(errc::invalid_argument, "malformed section at line " + Twine(LineNo) + ": '" + SectionStr + @@ -218,7 +218,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};