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

[kadir çetinkaya via cfe-commits]

https://github.com/kadircet created https://github.com/llvm/llvm-project/pull/125199

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


>From 4d6967a12f6793a27ed39fd7a7c1cc32fa001a09 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Fri, 31 Jan 2025 11:17:27 +0100
Subject: [PATCH] [include-cleaner] Dont report explicit refs for global
 operator new/delete

These are available for all translations implicitly. We shouldn't report
explicit refs and enforce includes.
---
 .../include-cleaner/lib/WalkAST.cpp           | 16 ++++++-
 .../unittests/AnalysisTest.cpp                | 46 +++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index aae3eda519ffdc..e3686a29d4367d 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 isImplicitOperatorNewDelete(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 = isImplicitOperatorNewDelete(
+                       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..21797e1c6825ac 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -397,6 +397,52 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
   }
 }
 
+// Make sure that the references to implicit operator new/delete are reported as
+// ambigious.
+TEST_F(AnalyzeTest, ImplicitOperatorNewDelete) {
+  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")));
+
+  // Check that header.h is not reported as missing.
+  {
+    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());
+  }
+
+  // Check that header.h is not reported as unused.
+  {
+    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, Not(testing::IsEmpty()));
+  }
+}
+
 TEST(FixIncludes, Basic) {
   llvm::StringRef Code = R"cpp(#include "d.h"
 #include "a.h"