Skip to content

[clang][NFC] Change suppression mapping interfaces to use SourceLocation #118960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang/include/clang/Basic/Diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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.
///
Expand Down
24 changes: 17 additions & 7 deletions clang/lib/Basic/Diagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -614,8 +622,10 @@ 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) {
Expand Down
11 changes: 2 additions & 9 deletions clang/lib/Basic/DiagnosticIDs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
40 changes: 25 additions & 15 deletions clang/unittests/Basic/DiagnosticTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -20,6 +21,7 @@
#include "llvm/Support/VirtualFileSystem.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <memory>
#include <optional>
#include <vector>

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -255,9 +267,9 @@ TEST_F(SuppressionMappingTest, SuppressesGroup) {
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
EXPECT_THAT(diags(), IsEmpty());

EXPECT_TRUE(
Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
SourceLocation FooLoc = locForFile("foo.cpp");
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function, FooLoc));
EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, FooLoc));
}

TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
Expand All @@ -271,10 +283,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) {
Expand All @@ -289,12 +301,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) {
Expand All @@ -308,9 +320,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 =
Expand Down
Loading