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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: kadir çetinkaya (kadircet)

<details>
<summary>Changes</summary>

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>.


---
Full diff: https://github.com/llvm/llvm-project/pull/123027.diff


2 Files Affected:

- (modified) clang-tools-extra/include-cleaner/lib/FindHeaders.cpp (+16-1) 
- (modified) clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp (+45) 


``````````diff
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(

``````````

</details>


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