Skip to content

Commit d74214c

Browse files
authored
[clang][NFC] Change suppression mapping interfaces to use SourceLocation (#118960)
This way we can delay getting a presumed location even further, only performing it for diagnostics that are mapped.
1 parent c91ba04 commit d74214c

File tree

4 files changed

+47
-33
lines changed

4 files changed

+47
-33
lines changed

clang/include/clang/Basic/Diagnostic.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,8 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
560560
ArgToStringFnTy ArgToStringFn;
561561

562562
/// Whether the diagnostic should be suppressed in FilePath.
563-
llvm::unique_function<bool(diag::kind, StringRef /*FilePath*/) const>
563+
llvm::unique_function<bool(diag::kind, SourceLocation /*DiagLoc*/,
564+
const SourceManager &) const>
564565
DiagSuppressionMapping;
565566

566567
public:
@@ -972,7 +973,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
972973
/// These take presumed locations into account, and can still be overriden by
973974
/// clang-diagnostics pragmas.
974975
void setDiagSuppressionMapping(llvm::MemoryBuffer &Input);
975-
bool isSuppressedViaMapping(diag::kind DiagId, StringRef FilePath) const;
976+
bool isSuppressedViaMapping(diag::kind DiagId, SourceLocation DiagLoc) const;
976977

977978
/// Issue the message to the client.
978979
///

clang/lib/Basic/Diagnostic.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,8 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList {
500500
// the last section take precedence in such cases.
501501
void processSections(DiagnosticsEngine &Diags);
502502

503-
bool isDiagSuppressed(diag::kind DiagId, StringRef FilePath) const;
503+
bool isDiagSuppressed(diag::kind DiagId, SourceLocation DiagLoc,
504+
const SourceManager &SM) const;
504505

505506
private:
506507
// Find the longest glob pattern that matches FilePath amongst
@@ -573,13 +574,14 @@ void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &Input) {
573574
WarningSuppressionList->processSections(*this);
574575
DiagSuppressionMapping =
575576
[WarningSuppressionList(std::move(WarningSuppressionList))](
576-
diag::kind DiagId, StringRef Path) {
577-
return WarningSuppressionList->isDiagSuppressed(DiagId, Path);
577+
diag::kind DiagId, SourceLocation DiagLoc, const SourceManager &SM) {
578+
return WarningSuppressionList->isDiagSuppressed(DiagId, DiagLoc, SM);
578579
};
579580
}
580581

581582
bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
582-
StringRef FilePath) const {
583+
SourceLocation DiagLoc,
584+
const SourceManager &SM) const {
583585
const Section *DiagSection = DiagToSection.lookup(DiagId);
584586
if (!DiagSection)
585587
return false;
@@ -589,7 +591,13 @@ bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
589591
return false;
590592
const llvm::StringMap<llvm::SpecialCaseList::Matcher> &CategoriesToMatchers =
591593
SrcEntriesIt->getValue();
592-
return globsMatches(CategoriesToMatchers, FilePath);
594+
// We also use presumed locations here to improve reproducibility for
595+
// preprocessed inputs.
596+
if (PresumedLoc PLoc = SM.getPresumedLoc(DiagLoc); PLoc.isValid())
597+
return globsMatches(
598+
CategoriesToMatchers,
599+
llvm::sys::path::remove_leading_dotslash(PLoc.getFilename()));
600+
return false;
593601
}
594602

595603
bool WarningsSpecialCaseList::globsMatches(
@@ -614,8 +622,10 @@ bool WarningsSpecialCaseList::globsMatches(
614622
}
615623

616624
bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind DiagId,
617-
StringRef FilePath) const {
618-
return DiagSuppressionMapping && DiagSuppressionMapping(DiagId, FilePath);
625+
SourceLocation DiagLoc) const {
626+
if (!hasSourceManager() || !DiagSuppressionMapping)
627+
return false;
628+
return DiagSuppressionMapping(DiagId, DiagLoc, getSourceManager());
619629
}
620630

621631
void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) {

clang/lib/Basic/DiagnosticIDs.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -601,15 +601,8 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
601601
return diag::Severity::Ignored;
602602

603603
// Clang-diagnostics pragmas always take precedence over suppression mapping.
604-
if (!Mapping.isPragma() && Diag.DiagSuppressionMapping) {
605-
// We also use presumed locations here to improve reproducibility for
606-
// preprocessed inputs.
607-
if (PresumedLoc PLoc = SM.getPresumedLoc(Loc);
608-
PLoc.isValid() && Diag.isSuppressedViaMapping(
609-
DiagID, llvm::sys::path::remove_leading_dotslash(
610-
PLoc.getFilename())))
611-
return diag::Severity::Ignored;
612-
}
604+
if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc))
605+
return diag::Severity::Ignored;
613606

614607
return Result;
615608
}

clang/unittests/Basic/DiagnosticTest.cpp

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "clang/Basic/DiagnosticLex.h"
1313
#include "clang/Basic/DiagnosticSema.h"
1414
#include "clang/Basic/FileManager.h"
15+
#include "clang/Basic/SourceLocation.h"
1516
#include "clang/Basic/SourceManager.h"
1617
#include "llvm/ADT/ArrayRef.h"
1718
#include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -20,6 +21,7 @@
2021
#include "llvm/Support/VirtualFileSystem.h"
2122
#include "gmock/gmock.h"
2223
#include "gtest/gtest.h"
24+
#include <memory>
2325
#include <optional>
2426
#include <vector>
2527

@@ -196,7 +198,17 @@ class SuppressionMappingTest : public testing::Test {
196198
return CaptureConsumer.StoredDiags;
197199
}
198200

201+
SourceLocation locForFile(llvm::StringRef FileName) {
202+
auto Buf = MemoryBuffer::getMemBuffer("", FileName);
203+
SourceManager &SM = Diags.getSourceManager();
204+
FileID FooID = SM.createFileID(std::move(Buf));
205+
return SM.getLocForStartOfFile(FooID);
206+
}
207+
199208
private:
209+
FileManager FM{{}, FS};
210+
SourceManager SM{Diags, FM};
211+
200212
class CaptureDiagnosticConsumer : public DiagnosticConsumer {
201213
public:
202214
std::vector<StoredDiagnostic> StoredDiags;
@@ -255,9 +267,9 @@ TEST_F(SuppressionMappingTest, SuppressesGroup) {
255267
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
256268
EXPECT_THAT(diags(), IsEmpty());
257269

258-
EXPECT_TRUE(
259-
Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
260-
EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
270+
SourceLocation FooLoc = locForFile("foo.cpp");
271+
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function, FooLoc));
272+
EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, FooLoc));
261273
}
262274

263275
TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
@@ -271,10 +283,10 @@ TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
271283
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
272284
EXPECT_THAT(diags(), IsEmpty());
273285

274-
EXPECT_TRUE(
275-
Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
276-
EXPECT_FALSE(
277-
Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
286+
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
287+
locForFile("bar.cpp")));
288+
EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
289+
locForFile("foo.cpp")));
278290
}
279291

280292
TEST_F(SuppressionMappingTest, LongestMatchWins) {
@@ -289,12 +301,12 @@ TEST_F(SuppressionMappingTest, LongestMatchWins) {
289301
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
290302
EXPECT_THAT(diags(), IsEmpty());
291303

304+
EXPECT_TRUE(Diags.isSuppressedViaMapping(
305+
diag::warn_unused_function, locForFile("clang/lib/Basic/foo.h")));
306+
EXPECT_FALSE(Diags.isSuppressedViaMapping(
307+
diag::warn_unused_function, locForFile("clang/lib/Sema/bar.h")));
292308
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
293-
"clang/lib/Basic/foo.h"));
294-
EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
295-
"clang/lib/Sema/bar.h"));
296-
EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
297-
"clang/lib/Sema/foo.h"));
309+
locForFile("clang/lib/Sema/foo.h")));
298310
}
299311

300312
TEST_F(SuppressionMappingTest, IsIgnored) {
@@ -308,9 +320,7 @@ TEST_F(SuppressionMappingTest, IsIgnored) {
308320
clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
309321
ASSERT_THAT(diags(), IsEmpty());
310322

311-
FileManager FM({}, FS);
312-
SourceManager SM(Diags, FM);
313-
323+
SourceManager &SM = Diags.getSourceManager();
314324
auto ClangID =
315325
SM.createFileID(llvm::MemoryBuffer::getMemBuffer("", "clang/foo.h"));
316326
auto NonClangID =

0 commit comments

Comments
 (0)