Skip to content

Commit 4d6967a

Browse files
committed
[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.
1 parent 3ae0f30 commit 4d6967a

File tree

2 files changed

+61
-1
lines changed

2 files changed

+61
-1
lines changed

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "clang/AST/Type.h"
2323
#include "clang/AST/TypeLoc.h"
2424
#include "clang/Basic/IdentifierTable.h"
25+
#include "clang/Basic/OperatorKinds.h"
2526
#include "clang/Basic/SourceLocation.h"
2627
#include "clang/Basic/Specifiers.h"
2728
#include "llvm/ADT/STLExtras.h"
@@ -32,6 +33,11 @@
3233

3334
namespace clang::include_cleaner {
3435
namespace {
36+
bool isImplicitOperatorNewDelete(OverloadedOperatorKind OpKind) {
37+
return OpKind == OO_New || OpKind == OO_Delete || OpKind == OO_Array_New ||
38+
OpKind == OO_Array_Delete;
39+
}
40+
3541
using DeclCallback =
3642
llvm::function_ref<void(SourceLocation, NamedDecl &, RefType)>;
3743

@@ -158,7 +164,15 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
158164
// the container decl instead, which is preferred as it'll handle
159165
// aliases/exports properly.
160166
if (!FD->isCXXClassMember() && !llvm::isa<EnumConstantDecl>(FD)) {
161-
report(DRE->getLocation(), FD);
167+
// Global operator new/delete [] is available implicitly in every
168+
// translation unit, even without including any explicit headers. So treat
169+
// those as ambigious to not force inclusion in TUs that transitively
170+
// depend on those.
171+
RefType RT = isImplicitOperatorNewDelete(
172+
FD->getDeclName().getCXXOverloadedOperator())
173+
? RefType::Ambiguous
174+
: RefType::Explicit;
175+
report(DRE->getLocation(), FD, RT);
162176
return true;
163177
}
164178
// If the ref is without a qualifier, and is a member, ignore it. As it is

clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,52 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
397397
}
398398
}
399399

400+
// Make sure that the references to implicit operator new/delete are reported as
401+
// ambigious.
402+
TEST_F(AnalyzeTest, ImplicitOperatorNewDelete) {
403+
ExtraFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
404+
ExtraFS->addFile("header.h",
405+
/*ModificationTime=*/{},
406+
llvm::MemoryBuffer::getMemBufferCopy(guard(R"cpp(
407+
void* operator new(decltype(sizeof(int)));
408+
)cpp")));
409+
ExtraFS->addFile("wrapper.h",
410+
/*ModificationTime=*/{},
411+
llvm::MemoryBuffer::getMemBufferCopy(guard(R"cpp(
412+
#include "header.h"
413+
)cpp")));
414+
415+
// Check that header.h is not reported as missing.
416+
{
417+
Inputs.Code = R"cpp(
418+
#include "wrapper.h"
419+
void bar() {
420+
operator new(3);
421+
})cpp";
422+
TestAST AST(Inputs);
423+
std::vector<Decl *> DeclsInTU;
424+
for (auto *D : AST.context().getTranslationUnitDecl()->decls())
425+
DeclsInTU.push_back(D);
426+
auto Results = analyze(DeclsInTU, {}, PP.Includes, &PI, AST.preprocessor());
427+
EXPECT_THAT(Results.Missing, testing::IsEmpty());
428+
}
429+
430+
// Check that header.h is not reported as unused.
431+
{
432+
Inputs.Code = R"cpp(
433+
#include "header.h"
434+
void bar() {
435+
operator new(3);
436+
})cpp";
437+
TestAST AST(Inputs);
438+
std::vector<Decl *> DeclsInTU;
439+
for (auto *D : AST.context().getTranslationUnitDecl()->decls())
440+
DeclsInTU.push_back(D);
441+
auto Results = analyze(DeclsInTU, {}, PP.Includes, &PI, AST.preprocessor());
442+
EXPECT_THAT(Results.Unused, Not(testing::IsEmpty()));
443+
}
444+
}
445+
400446
TEST(FixIncludes, Basic) {
401447
llvm::StringRef Code = R"cpp(#include "d.h"
402448
#include "a.h"

0 commit comments

Comments
 (0)