-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[include-cleaner] Add special mappings for operator new/delete #123027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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>.
@llvm/pr-subscribers-clang-tools-extra Author: kadir çetinkaya (kadircet) ChangesOur stdlib mappings are based on names, hence they can't handle special Global operator new/delete show up often enough in practice to create Full diff: https://github.com/llvm/llvm-project/pull/123027.diff 2 Files Affected:
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(
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
abandoning in favor of #125199 |
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 .