diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h --- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h +++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h @@ -29,6 +29,19 @@ class NamedDecl; namespace include_cleaner { +/// Indicates the relation between the reference and the target. +enum class RefType { + /// Target is explicit from the reference, e.g. function call. + Explicit, + /// Target isn't spelled, e.g. default constructor call. + Implicit, + /// Target is not directly pointed, but implied by the reference, e.g. unused + /// overloads brought in by a using decl. + /// This allows a policy in which the responsibility of including headers can + /// be shifted between the user code vs the library. + Related, +}; + /// Traverses part of the AST from \p Root, finding uses of symbols. /// /// Each use is reported to the callback: @@ -36,10 +49,13 @@ /// the primary location of the AST node found under Root. /// - the NamedDecl is the symbol referenced. It is canonical, rather than e.g. /// the redecl actually found by lookup. +/// - the RefType describes the relation between the SourceLocation and the +/// NamedDecl. /// /// walkAST is typically called once per top-level declaration in the file /// being analyzed, in order to find all references within it. -void walkAST(Decl &Root, llvm::function_ref); +void walkAST(Decl &Root, + llvm::function_ref); } // namespace include_cleaner } // namespace clang diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp --- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp +++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp @@ -22,7 +22,8 @@ namespace clang { namespace include_cleaner { namespace { -using DeclCallback = llvm::function_ref; +using DeclCallback = + llvm::function_ref; class ASTWalker : public RecursiveASTVisitor { DeclCallback Callback; @@ -30,54 +31,60 @@ bool handleTemplateName(SourceLocation Loc, TemplateName TN) { // For using-templates, only mark the alias. if (auto *USD = TN.getAsUsingShadowDecl()) { - report(Loc, USD); + report(Loc, USD, RefType::Explicit); return true; } - report(Loc, TN.getAsTemplateDecl()); + report(Loc, TN.getAsTemplateDecl(), RefType::Explicit); return true; } - void report(SourceLocation Loc, NamedDecl *ND) { + void report(SourceLocation Loc, NamedDecl *ND, RefType RT) { if (!ND || Loc.isInvalid()) return; - Callback(Loc, *cast(ND->getCanonicalDecl())); + Callback(Loc, *cast(ND->getCanonicalDecl()), RT); } public: ASTWalker(DeclCallback Callback) : Callback(Callback) {} bool VisitDeclRefExpr(DeclRefExpr *DRE) { - report(DRE->getLocation(), DRE->getFoundDecl()); + report(DRE->getLocation(), DRE->getFoundDecl(), RefType::Explicit); return true; } bool VisitMemberExpr(MemberExpr *E) { - report(E->getMemberLoc(), E->getFoundDecl().getDecl()); + report(E->getMemberLoc(), E->getFoundDecl().getDecl(), RefType::Explicit); return true; } bool VisitCXXConstructExpr(CXXConstructExpr *E) { - report(E->getLocation(), E->getConstructor()); + report(E->getLocation(), E->getConstructor(), + E->getParenOrBraceRange().isValid() ? RefType::Explicit + : RefType::Implicit); return true; } bool VisitOverloadExpr(OverloadExpr *E) { - // Mark all candidates as used when overload is not resolved. - llvm::for_each(E->decls(), - [this, E](NamedDecl *D) { report(E->getNameLoc(), D); }); + llvm::for_each(E->decls(), [this, E](NamedDecl *D) { + report(E->getNameLoc(), D, RefType::Related); + }); return true; } bool VisitUsingDecl(UsingDecl *UD) { - for (const auto *Shadow : UD->shadows()) - report(UD->getLocation(), Shadow->getTargetDecl()); + for (const auto *Shadow : UD->shadows()) { + auto *TD = Shadow->getTargetDecl(); + report(UD->getLocation(), TD, + (TD->isUsed() || TD->isReferenced()) ? RefType::Explicit + : RefType::Related); + } return true; } bool VisitFunctionDecl(FunctionDecl *FD) { // Mark declaration from definition as it needs type-checking. if (FD->isThisDeclarationADefinition()) - report(FD->getLocation(), FD); + report(FD->getLocation(), FD, RefType::Explicit); return true; } @@ -85,23 +92,23 @@ // Definition of an enum with an underlying type references declaration for // type-checking purposes. if (D->isThisDeclarationADefinition() && D->getIntegerTypeSourceInfo()) - report(D->getLocation(), D); + report(D->getLocation(), D, RefType::Explicit); return true; } // TypeLoc visitors. bool VisitUsingTypeLoc(UsingTypeLoc TL) { - report(TL.getNameLoc(), TL.getFoundDecl()); + report(TL.getNameLoc(), TL.getFoundDecl(), RefType::Explicit); return true; } bool VisitTagTypeLoc(TagTypeLoc TTL) { - report(TTL.getNameLoc(), TTL.getDecl()); + report(TTL.getNameLoc(), TTL.getDecl(), RefType::Explicit); return true; } bool VisitTypedefTypeLoc(TypedefTypeLoc TTL) { - report(TTL.getNameLoc(), TTL.getTypedefNameDecl()); + report(TTL.getNameLoc(), TTL.getTypedefNameDecl(), RefType::Explicit); return true; } diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp @@ -1,16 +1,35 @@ #include "AnalysisInternal.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" #include "clang/Basic/FileManager.h" #include "clang/Frontend/TextDiagnostic.h" #include "clang/Testing/TestAST.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Testing/Support/Annotations.h" #include "gtest/gtest.h" +#include +#include +#include +#include namespace clang { namespace include_cleaner { namespace { +llvm::StringLiteral to_string(RefType RT) { + switch (RT) { + case RefType::Explicit: + return "explicit"; + case RefType::Implicit: + return "implicit"; + case RefType::Related: + return "related"; + } + llvm_unreachable("Unexpected RefType"); +} + // Specifies a test of which symbols are referenced by a piece of code. // // Example: @@ -38,20 +57,21 @@ llvm::cantFail(AST.fileManager().getFileRef("target.h"))); // Perform the walk, and capture the offsets of the referenced targets. - std::vector ReferencedOffsets; + std::unordered_map> ReferencedOffsets; for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) { if (ReferencingFile != SM.getDecomposedExpansionLoc(D->getLocation()).first) continue; - walkAST(*D, [&](SourceLocation Loc, NamedDecl &ND) { + walkAST(*D, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) { if (SM.getFileLoc(Loc) != ReferencingLoc) return; auto NDLoc = SM.getDecomposedLoc(SM.getFileLoc(ND.getLocation())); if (NDLoc.first != TargetFile) return; - ReferencedOffsets.push_back(NDLoc.second); + ReferencedOffsets[RT].push_back(NDLoc.second); }); } - llvm::sort(ReferencedOffsets); + for (auto &Entry : ReferencedOffsets) + llvm::sort(Entry.second); // Compare results to the expected points. // For each difference, show the target point in context, like a diagnostic. @@ -61,17 +81,22 @@ DiagOpts->ShowLevel = 0; DiagOpts->ShowNoteIncludeStack = 0; TextDiagnostic Diag(DiagOS, AST.context().getLangOpts(), DiagOpts); - auto DiagnosePoint = [&](const char *Message, unsigned Offset) { + auto DiagnosePoint = [&](llvm::StringRef Message, unsigned Offset) { Diag.emitDiagnostic( FullSourceLoc(SM.getComposedLoc(TargetFile, Offset), SM), DiagnosticsEngine::Note, Message, {}, {}); }; - for (auto Expected : Target.points()) - if (!llvm::is_contained(ReferencedOffsets, Expected)) - DiagnosePoint("location not marked used", Expected); - for (auto Actual : ReferencedOffsets) - if (!llvm::is_contained(Target.points(), Actual)) - DiagnosePoint("location unexpectedly used", Actual); + for (auto RT : {RefType::Explicit, RefType::Implicit, RefType::Related}) { + auto RTStr = to_string(RT); + for (auto Expected : Target.points(RTStr)) + if (!llvm::is_contained(ReferencedOffsets[RT], Expected)) + DiagnosePoint(("location not marked used with type " + RTStr).str(), + Expected); + for (auto Actual : ReferencedOffsets[RT]) + if (!llvm::is_contained(Target.points(RTStr), Actual)) + DiagnosePoint(("location unexpectedly used with type " + RTStr).str(), + Actual); + } // If there were any differences, we print the entire referencing code once. if (!DiagBuf.empty()) @@ -79,35 +104,39 @@ } TEST(WalkAST, DeclRef) { - testWalk("int ^x;", "int y = ^x;"); - testWalk("int ^foo();", "int y = ^foo();"); - testWalk("namespace ns { int ^x; }", "int y = ns::^x;"); - testWalk("struct S { static int ^x; };", "int y = S::^x;"); + testWalk("int $explicit^x;", "int y = ^x;"); + testWalk("int $explicit^foo();", "int y = ^foo();"); + testWalk("namespace ns { int $explicit^x; }", "int y = ns::^x;"); + testWalk("struct S { static int $explicit^x; };", "int y = S::^x;"); // Canonical declaration only. - testWalk("extern int ^x; int x;", "int y = ^x;"); + testWalk("extern int $explicit^x; int x;", "int y = ^x;"); // Return type of `foo` isn't used. - testWalk("struct S{}; S ^foo();", "auto bar() { return ^foo(); }"); + testWalk("struct S{}; S $explicit^foo();", "auto bar() { return ^foo(); }"); } TEST(WalkAST, TagType) { - testWalk("struct ^S {};", "^S *y;"); - testWalk("enum ^E {};", "^E *y;"); - testWalk("struct ^S { static int x; };", "int y = ^S::x;"); + testWalk("struct $explicit^S {};", "^S *y;"); + testWalk("enum $explicit^E {};", "^E *y;"); + testWalk("struct $explicit^S { static int x; };", "int y = ^S::x;"); } TEST(WalkAST, Alias) { testWalk(R"cpp( namespace ns { int x; } - using ns::^x; + using ns::$explicit^x; )cpp", "int y = ^x;"); - testWalk("using ^foo = int;", "^foo x;"); - testWalk("struct S {}; using ^foo = S;", "^foo x;"); + testWalk("using $explicit^foo = int;", "^foo x;"); + testWalk("struct S {}; using $explicit^foo = S;", "^foo x;"); } TEST(WalkAST, Using) { - testWalk("namespace ns { void ^x(); void ^x(int); }", "using ns::^x;"); - testWalk("namespace ns { struct S; } using ns::^S;", "^S *s;"); + testWalk(R"cpp( + namespace ns { + void $explicit^x(); void $related^x(int); void $related^x(char); + })cpp", + "using ns::^x; void foo() { x(); }"); + testWalk("namespace ns { struct S; } using ns::$explicit^S;", "^S *s;"); } TEST(WalkAST, Namespaces) { @@ -115,51 +144,53 @@ } TEST(WalkAST, TemplateNames) { - testWalk("template struct ^S {};", "^S s;"); + testWalk("template struct $explicit^S {};", "^S s;"); // FIXME: Template decl has the wrong primary location for type-alias template // decls. testWalk(R"cpp( template struct S {}; - template ^using foo = S;)cpp", + template $explicit^using foo = S;)cpp", "^foo x;"); testWalk(R"cpp( namespace ns {template struct S {}; } - using ns::^S;)cpp", + using ns::$explicit^S;)cpp", "^S x;"); - testWalk("template struct ^S {};", + testWalk("template struct $explicit^S {};", R"cpp( template