[clang-tools-extra] fcb1234 - [include-cleaner] Dont report explicit refs for global operator new/delete (#125199)

via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 31 05:01:48 PST 2025


Author: kadir çetinkaya
Date: 2025-01-31T14:01:45+01:00
New Revision: fcb12346dd656b6ce02971c3530b2680e3a332ac

URL: https://github.com/llvm/llvm-project/commit/fcb12346dd656b6ce02971c3530b2680e3a332ac
DIFF: https://github.com/llvm/llvm-project/commit/fcb12346dd656b6ce02971c3530b2680e3a332ac.diff

LOG: [include-cleaner] Dont report explicit refs for global operator new/delete (#125199)

These are available for all translations implicitly. We shouldn't report
explicit refs and enforce includes.

Added: 
    

Modified: 
    clang-tools-extra/include-cleaner/lib/WalkAST.cpp
    clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index aae3eda519ffdc..7a140c991925c8 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
@@ -32,6 +33,11 @@
 
 namespace clang::include_cleaner {
 namespace {
+bool isOperatorNewDelete(OverloadedOperatorKind OpKind) {
+  return OpKind == OO_New || OpKind == OO_Delete || OpKind == OO_Array_New ||
+         OpKind == OO_Array_Delete;
+}
+
 using DeclCallback =
     llvm::function_ref<void(SourceLocation, NamedDecl &, RefType)>;
 
@@ -158,7 +164,15 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
     // the container decl instead, which is preferred as it'll handle
     // aliases/exports properly.
     if (!FD->isCXXClassMember() && !llvm::isa<EnumConstantDecl>(FD)) {
-      report(DRE->getLocation(), FD);
+      // Global operator new/delete [] is available implicitly in every
+      // translation unit, even without including any explicit headers. So treat
+      // those as ambigious to not force inclusion in TUs that transitively
+      // depend on those.
+      RefType RT =
+          isOperatorNewDelete(FD->getDeclName().getCXXOverloadedOperator())
+              ? RefType::Ambiguous
+              : RefType::Explicit;
+      report(DRE->getLocation(), FD, RT);
       return true;
     }
     // If the ref is without a qualifier, and is a member, ignore it. As it is

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index d2d137a0dfb42a..74321c312cb718 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -397,6 +397,55 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
   }
 }
 
+// Make sure that the references to implicit operator new/delete are reported as
+// ambigious.
+TEST_F(AnalyzeTest, ImplicitOperatorNewDeleteNotMissing) {
+  ExtraFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  ExtraFS->addFile("header.h",
+                   /*ModificationTime=*/{},
+                   llvm::MemoryBuffer::getMemBufferCopy(guard(R"cpp(
+  void* operator new(decltype(sizeof(int)));
+  )cpp")));
+  ExtraFS->addFile("wrapper.h",
+                   /*ModificationTime=*/{},
+                   llvm::MemoryBuffer::getMemBufferCopy(guard(R"cpp(
+  #include "header.h"
+  )cpp")));
+
+  Inputs.Code = R"cpp(
+      #include "wrapper.h"
+      void bar() {
+        operator new(3);
+      })cpp";
+  TestAST AST(Inputs);
+  std::vector<Decl *> DeclsInTU;
+  for (auto *D : AST.context().getTranslationUnitDecl()->decls())
+    DeclsInTU.push_back(D);
+  auto Results = analyze(DeclsInTU, {}, PP.Includes, &PI, AST.preprocessor());
+  EXPECT_THAT(Results.Missing, testing::IsEmpty());
+}
+
+TEST_F(AnalyzeTest, ImplicitOperatorNewDeleteNotUnused) {
+  ExtraFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  ExtraFS->addFile("header.h",
+                   /*ModificationTime=*/{},
+                   llvm::MemoryBuffer::getMemBufferCopy(guard(R"cpp(
+  void* operator new(decltype(sizeof(int)));
+  )cpp")));
+
+  Inputs.Code = R"cpp(
+      #include "header.h"
+      void bar() {
+        operator new(3);
+      })cpp";
+  TestAST AST(Inputs);
+  std::vector<Decl *> DeclsInTU;
+  for (auto *D : AST.context().getTranslationUnitDecl()->decls())
+    DeclsInTU.push_back(D);
+  auto Results = analyze(DeclsInTU, {}, PP.Includes, &PI, AST.preprocessor());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
+}
+
 TEST(FixIncludes, Basic) {
   llvm::StringRef Code = R"cpp(#include "d.h"
 #include "a.h"


        


More information about the cfe-commits mailing list