[clang] [clang][NFC] Change suppression mapping interfaces to use SourceLocation (PR #118960)
kadir çetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 6 04:14:46 PST 2024
https://github.com/kadircet created https://github.com/llvm/llvm-project/pull/118960
This way we can delay getting a presumed location even further, only
performing it for diagnostics that are mapped.
>From 4c52dda9a253d643ef52f1f5f294cd1fd5bcfa76 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Fri, 6 Dec 2024 13:13:15 +0100
Subject: [PATCH] [clang][NFC] Change suppression mapping interfaces to use
SourceLocation
This way we can delay getting a presumed location even further, only
performing it for diagnostics that are mapped.
---
clang/include/clang/Basic/Diagnostic.h | 5 +--
clang/lib/Basic/Diagnostic.cpp | 23 +++++++++-----
clang/lib/Basic/DiagnosticIDs.cpp | 11 ++-----
clang/unittests/Basic/DiagnosticTest.cpp | 39 +++++++++++++++---------
4 files changed, 46 insertions(+), 32 deletions(-)
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index d271accca3d3b7..510b782e35d065 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -560,7 +560,8 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
ArgToStringFnTy ArgToStringFn;
/// Whether the diagnostic should be suppressed in FilePath.
- llvm::unique_function<bool(diag::kind, StringRef /*FilePath*/) const>
+ llvm::unique_function<bool(diag::kind, SourceLocation /*DiagLoc*/,
+ const SourceManager &) const>
DiagSuppressionMapping;
public:
@@ -972,7 +973,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
/// These take presumed locations into account, and can still be overriden by
/// clang-diagnostics pragmas.
void setDiagSuppressionMapping(llvm::MemoryBuffer &Input);
- bool isSuppressedViaMapping(diag::kind DiagId, StringRef FilePath) const;
+ bool isSuppressedViaMapping(diag::kind DiagId, SourceLocation DiagLoc) const;
/// Issue the message to the client.
///
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 2d0e358116362c..682e02222a1993 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -500,7 +500,8 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList {
// the last section take precedence in such cases.
void processSections(DiagnosticsEngine &Diags);
- bool isDiagSuppressed(diag::kind DiagId, StringRef FilePath) const;
+ bool isDiagSuppressed(diag::kind DiagId, SourceLocation DiagLoc,
+ const SourceManager &SM) const;
private:
// Find the longest glob pattern that matches FilePath amongst
@@ -573,13 +574,14 @@ void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
WarningSuppressionList->processSections(*this);
DiagSuppressionMapping =
[WarningSuppressionList(std::move(WarningSuppressionList))](
- diag::kind DiagId, StringRef Path) {
- return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
+ diag::kind DiagId, SourceLocation DiagLoc, const SourceManager &SM) {
+ return WarningSuppressionList->isDiagSuppressed(DiagId, DiagLoc, SM);
};
}
bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
- StringRef FilePath) const {
+ SourceLocation DiagLoc,
+ const SourceManager &SM) const {
const Section *DiagSection = DiagToSection.lookup(DiagId);
if (!DiagSection)
return false;
@@ -589,7 +591,13 @@ bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
return false;
const llvm::StringMap<llvm::SpecialCaseList::Matcher> &CategoriesToMatchers =
SrcEntriesIt->getValue();
- return globsMatches(CategoriesToMatchers, FilePath);
+ // 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(
@@ -614,8 +622,9 @@ bool WarningsSpecialCaseList::globsMatches(
}
bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind DiagId,
- StringRef FilePath) const {
- return DiagSuppressionMapping && DiagSuppressionMapping(DiagId, FilePath);
+ SourceLocation DiagLoc) const {
+ if (!hasSourceManager() || !DiagSuppressionMapping) return false;
+ return DiagSuppressionMapping(DiagId, DiagLoc, getSourceManager());
}
void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) {
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index 44922aa7872dbf..d77f28c80b2eb2 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -601,15 +601,8 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
return diag::Severity::Ignored;
// Clang-diagnostics pragmas always take precedence over suppression mapping.
- if (!Mapping.isPragma() && Diag.DiagSuppressionMapping) {
- // We also use presumed locations here to improve reproducibility for
- // preprocessed inputs.
- if (PresumedLoc PLoc = SM.getPresumedLoc(Loc);
- PLoc.isValid() && Diag.isSuppressedViaMapping(
- DiagID, llvm::sys::path::remove_leading_dotslash(
- PLoc.getFilename())))
- return diag::Severity::Ignored;
- }
+ if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc))
+ return diag::Severity::Ignored;
return Result;
}
diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index 36a77c7247655f..5c3d7934b242ec 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -12,6 +12,7 @@
#include "clang/Basic/DiagnosticLex.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/FileManager.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -20,6 +21,7 @@
#include "llvm/Support/VirtualFileSystem.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <memory>
#include <optional>
#include <vector>
@@ -196,7 +198,17 @@ class SuppressionMappingTest : public testing::Test {
return CaptureConsumer.StoredDiags;
}
+ SourceLocation locForFile(llvm::StringRef FileName) {
+ auto Buf = MemoryBuffer::getMemBuffer("", FileName);
+ SourceManager &SM = Diags.getSourceManager();
+ FileID FooID = SM.createFileID(std::move(Buf));
+ return SM.getLocForStartOfFile(FooID);
+ }
+
private:
+ FileManager FM{{}, FS};
+ SourceManager SM{Diags, FM};
+
class CaptureDiagnosticConsumer : public DiagnosticConsumer {
public:
std::vector<StoredDiagnostic> StoredDiags;
@@ -255,9 +267,10 @@ TEST_F(SuppressionMappingTest, SuppressesGroup) {
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
EXPECT_THAT(diags(), IsEmpty());
+ SourceLocation FooLoc = locForFile("foo.cpp");
EXPECT_TRUE(
- Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
- EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+ Diags.isSuppressedViaMapping(diag::warn_unused_function, FooLoc));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, FooLoc));
}
TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
@@ -271,10 +284,10 @@ TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
EXPECT_THAT(diags(), IsEmpty());
- EXPECT_TRUE(
- Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
- EXPECT_FALSE(
- Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ locForFile("bar.cpp")));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+ locForFile("foo.cpp")));
}
TEST_F(SuppressionMappingTest, LongestMatchWins) {
@@ -289,12 +302,12 @@ TEST_F(SuppressionMappingTest, LongestMatchWins) {
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
EXPECT_THAT(diags(), IsEmpty());
+ EXPECT_TRUE(Diags.isSuppressedViaMapping(
+ diag::warn_unused_function, locForFile("clang/lib/Basic/foo.h")));
+ EXPECT_FALSE(Diags.isSuppressedViaMapping(
+ diag::warn_unused_function, locForFile("clang/lib/Sema/bar.h")));
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
- "clang/lib/Basic/foo.h"));
- EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
- "clang/lib/Sema/bar.h"));
- EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
- "clang/lib/Sema/foo.h"));
+ locForFile("clang/lib/Sema/foo.h")));
}
TEST_F(SuppressionMappingTest, IsIgnored) {
@@ -308,9 +321,7 @@ TEST_F(SuppressionMappingTest, IsIgnored) {
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
ASSERT_THAT(diags(), IsEmpty());
- FileManager FM({}, FS);
- SourceManager SM(Diags, FM);
-
+ SourceManager &SM = Diags.getSourceManager();
auto ClangID =
SM.createFileID(llvm::MemoryBuffer::getMemBuffer("", "clang/foo.h"));
auto NonClangID =
More information about the cfe-commits
mailing list