[clang-tools-extra] [include-cleaner] Add special mappings for operator new/delete (PR #123027)

[kadir çetinkaya via cfe-commits]

https://github.com/kadircet updated https://github.com/llvm/llvm-project/pull/123027

>From 5339f8e303a99b8a75320b24a3a371e531fa6140 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Wed, 15 Jan 2025 10:11:09 +0100
Subject: [PATCH 1/2] [include-cleaner] Add special mappings for operator
 new/delete

Our stdlib mappings are based on names, hence they can't handle special
symbols like oprators.

Global operator new/delete show up often enough in practice to create
some user frustration, so we map these to <new>.
---
 .../include-cleaner/lib/FindHeaders.cpp       | 17 ++++++-
 .../unittests/FindHeadersTest.cpp             | 45 +++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index 7b28d1c252d715..e6a642ae8ed48a 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -16,6 +16,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/FileEntry.h"
+#include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
@@ -157,8 +158,22 @@ headersForSpecialSymbol(const Symbol &S, const SourceManager &SM,
   if (!ND)
     return std::nullopt;
   auto *II = ND->getIdentifier();
-  if (!II)
+  if (!II) {
+    // Special case global operator new/delete, these show up often enough in
+    // practice and stdlib mappings can't work with them as they're symbol-name
+    // based.
+    if (ND->getDeclContext()->isTranslationUnit()) {
+      switch (ND->getDeclName().getCXXOverloadedOperator()) {
+      case OverloadedOperatorKind::OO_New:
+      case OverloadedOperatorKind::OO_Delete:
+        return hintedHeadersForStdHeaders(
+            {tooling::stdlib::Header::named("<new>").value()}, SM, PI);
+      default:
+        break;
+      }
+    }
     return std::nullopt;
+  }
 
   // Check first for symbols that are part of our stdlib mapping. As we have
   // header names for those.
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 84e02e1d0d621b..d5b0a61ed1dfcb 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -11,11 +11,13 @@
 #include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/OperatorKinds.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
@@ -617,6 +619,49 @@ TEST_F(HeadersForSymbolTest, AmbiguousStdSymbolsUsingShadow) {
                   Header(*tooling::stdlib::Header::named("<cstdio>"))));
 }
 
+TEST_F(HeadersForSymbolTest, GlobalOperatorNewDelete) {
+  Inputs.Code = R"cpp(
+    void k() {
+      int *x;
+      // make sure operator new/delete are part of TU.
+      x = static_cast<int*>(::operator new(sizeof(int)));
+      ::operator delete(x);
+    }
+  )cpp";
+  buildAST();
+
+  // Find global new/delete operators.
+  struct Visitor : public DynamicRecursiveASTVisitor {
+    const NamedDecl *New = nullptr;
+    const NamedDecl *Delete = nullptr;
+    bool VisitNamedDecl(NamedDecl *ND) override {
+      if (!ND->getDeclContext()->isTranslationUnit())
+        return true;
+      switch (ND->getDeclName().getCXXOverloadedOperator()) {
+      case OO_New:
+        New = ND;
+        break;
+      case OO_Delete:
+        Delete = ND;
+        break;
+      default:
+        break;
+      }
+      return true;
+    }
+  };
+  Visitor V;
+  V.ShouldVisitImplicitCode = true;
+  V.TraverseDecl(AST->context().getTranslationUnitDecl());
+  ASSERT_TRUE(V.New) << "Couldn't find global new!";
+  ASSERT_TRUE(V.Delete) << "Couldn't find global delete!";
+  EXPECT_THAT(headersForSymbol(*V.New, AST->sourceManager(), &PI),
+              UnorderedElementsAre(
+                  Header(*tooling::stdlib::Header::named("<new>"))));
+  EXPECT_THAT(headersForSymbol(*V.Delete, AST->sourceManager(), &PI),
+              UnorderedElementsAre(
+                  Header(*tooling::stdlib::Header::named("<new>"))));
+}
 
 TEST_F(HeadersForSymbolTest, StandardHeaders) {
   Inputs.Code = R"cpp(

>From 31ee98aa1896fd77380749ad90e7c452f36e2ce2 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Wed, 15 Jan 2025 10:34:17 +0100
Subject: [PATCH 2/2] formatting

---
 .../include-cleaner/unittests/FindHeadersTest.cpp    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index d5b0a61ed1dfcb..5553958f1d1f29 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -655,12 +655,12 @@ TEST_F(HeadersForSymbolTest, GlobalOperatorNewDelete) {
   V.TraverseDecl(AST->context().getTranslationUnitDecl());
   ASSERT_TRUE(V.New) << "Couldn't find global new!";
   ASSERT_TRUE(V.Delete) << "Couldn't find global delete!";
-  EXPECT_THAT(headersForSymbol(*V.New, AST->sourceManager(), &PI),
-              UnorderedElementsAre(
-                  Header(*tooling::stdlib::Header::named("<new>"))));
-  EXPECT_THAT(headersForSymbol(*V.Delete, AST->sourceManager(), &PI),
-              UnorderedElementsAre(
-                  Header(*tooling::stdlib::Header::named("<new>"))));
+  EXPECT_THAT(
+      headersForSymbol(*V.New, AST->sourceManager(), &PI),
+      UnorderedElementsAre(Header(*tooling::stdlib::Header::named("<new>"))));
+  EXPECT_THAT(
+      headersForSymbol(*V.Delete, AST->sourceManager(), &PI),
+      UnorderedElementsAre(Header(*tooling::stdlib::Header::named("<new>"))));
 }
 
 TEST_F(HeadersForSymbolTest, StandardHeaders) {