[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 7 05:16:40 PST 2024


================
@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
   // Make sure an empty warning can round-trip with \c StoredDiagnostic.
   Diags.Report(CaptureConsumer.StoredDiags.front());
 }
+
+class SuppressionMappingTest : public testing::Test {
+public:
+  SuppressionMappingTest() {
+    Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+  }
+
+protected:
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS =
+      llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions};
+
+  std::vector<StoredDiagnostic> takeDiags() {
+    return std::move(CaptureConsumer.StoredDiags);
+  }
+
+private:
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+    std::vector<StoredDiagnostic> StoredDiags;
+
+    void HandleDiagnostic(DiagnosticsEngine::Level level,
+                          const Diagnostic &Info) override {
+      StoredDiags.push_back(StoredDiagnostic(level, Info));
+    }
+  };
+  CaptureDiagnosticConsumer CaptureConsumer;
+};
+
+MATCHER_P(WithMessage, Msg, "has diagnostic message") {
+  return arg.getMessage() == Msg;
+}
+
+TEST_F(SuppressionMappingTest, MissingMappingFile) {
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+                               "no such file or directory: 'foo.txt'")));
+}
+
+TEST_F(SuppressionMappingTest, MalformedFile) {
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  FS->addFile("foo.txt", /*ModificationTime=*/{},
+              llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt"));
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage(
+                               "failed to process suppression mapping file "
+                               "'foo.txt': malformed line 1: 'asdf'")));
+}
+
+TEST_F(SuppressionMappingTest, UnknownDiagName) {
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  FS->addFile("foo.txt", /*ModificationTime=*/{},
+              llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]"));
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(takeDiags(),
+              testing::ElementsAre(WithMessage(
+                  "unknown warning option 'non-existing-warning'")));
+}
+
+TEST_F(SuppressionMappingTest, SuppressesGroup) {
+  llvm::StringLiteral SuppressionMappingFile = R"txt(
+  [unused]
+  src:*
+  )txt";
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  FS->addFile("foo.txt", /*ModificationTime=*/{},
+              llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+  EXPECT_TRUE(
+      Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+  EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, ExclusionsWork) {
+  llvm::StringLiteral SuppressionMappingFile = R"txt(
+  [unused]
+  src:*
+  src:*foo.cpp=emit
+  )txt";
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  FS->addFile("foo.txt", /*ModificationTime=*/{},
+              llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+  EXPECT_TRUE(
+      Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp"));
+  EXPECT_FALSE(
+      Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp"));
+}
+
+TEST_F(SuppressionMappingTest, LongestMatchWins) {
+  llvm::StringLiteral SuppressionMappingFile = R"txt(
+  [unused]
+  src:*clang/*
+  src:*clang/lib/Sema/*=emit
+  src:*clang/lib/Sema/foo*
+  )txt";
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  FS->addFile("foo.txt", /*ModificationTime=*/{},
+              llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(takeDiags(), testing::IsEmpty());
+
+  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"));
+}
+
+TEST_F(SuppressionMappingTest, IsIgnored) {
----------------
kadircet wrote:

i modified `DiagnosticIDs::getDiagnosticSeverity` and it is an implementation detail of the public `isIgnored` API. hence yes, this patch changes the observable behavior of `isIgnored`.

> I think most of the tests should be for isSuppressedViaMapping() and we should have a few tests for ProcessWarningOptions() just to make the new specific logic there works (covering DiagnosticSuppressionMappingsFile empty, getBufferForFile() failing (with and without ReportDiags) and success.
> I think it should also be clear from the test name which function (what API) is tested.

I believe that's what all the previous tests in this file is doing already;

> we should have a few tests for ProcessWarningOptions() just to make the new specific logic there works (covering DiagnosticSuppressionMappingsFile empty, getBufferForFile() failing (with and without ReportDiags) and success.
- MissingMappingFile
- MalformedFile
- UnknownDiagNAme

> I think most of the tests should be for isSuppressedViaMapping()

Tests that make sure "behavior" of suppression mapping is WAI, with names explaining the behaviours:
- SuppressesGroup
- EmitCategoryIsExcluded
- LongestMatchWins
- IsIgnored

`IsIgnored` tests behavior in presence of presumedlocs and diagnostic-pragmas. hence it needs to test it through a source-location based API.

https://github.com/llvm/llvm-project/pull/112517


More information about the cfe-commits mailing list