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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
