kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang-tools-extra.
This brings IncludeCleaner's reference discovery from AST to the parity
with current implementation in clangd. Some highlights:
- Handling of MemberExprs, only the member declaration is marked as referenced
and not the container, unlike clangd.
- Constructor calls, only the constructor and not the container, unlike clangd.
- All the possible candidates for unresolved overloads, same as clangd.
- All the shadow decls for using-decls, same as clangd.
- Declarations for definitions of enums with an underlying type and functions,
same as clangd.
- Using typelocs, using templatenames and typedefs only reference the found
decl, same as clangd.
- Template specializations only reference the primary template, not the
explicit specializations, to be fixed.
- Expr types aren't marked as used, unlike clangd.
Going forward, we can consider having signals to indicate type of a
reference (e.g. `implicit` signal for type of an expr) so that the
applications can perform a filtering based on their needs.
At the moment the biggest discrepancy is around type of exprs, i.e. not
marking containers for member/constructor accesses. I believe this is
the right model since the declaration of the member and the container
should be available in a single file (modulo macros).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132110
Files:
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -25,6 +25,7 @@
Inputs.ExtraFiles["target.h"] = Target.code().str();
Inputs.ExtraArgs.push_back("-include");
Inputs.ExtraArgs.push_back("target.h");
+ Inputs.ExtraArgs.push_back("-std=c++17");
TestAST AST(Inputs);
const auto &SM = AST.sourceManager();
@@ -84,6 +85,8 @@
testWalk("struct S { static int ^x; };", "int y = S::^x;");
// Canonical declaration only.
testWalk("extern int ^x; int x;", "int y = ^x;");
+ // Return type of `foo` isn't used.
+ testWalk("struct S{}; S ^foo();", "auto bar() { return ^foo(); }");
}
TEST(WalkAST, TagType) {
@@ -98,11 +101,64 @@
using ns::^x;
)cpp",
"int y = ^x;");
+ testWalk("using ^foo = int;", "^foo x;");
+ testWalk("struct S {}; using ^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;");
+}
+
+TEST(WalkAST, Namespaces) {
+ testWalk("namespace ns { void x(); }", "using namespace ^ns;");
+}
+
+TEST(WalkAST, TemplateNames) {
+ testWalk("template<typename> struct ^S {};", "^S<int> s;");
+ // FIXME: Template decl has the wrong primary location for type-alias template
+ // decls.
testWalk(R"cpp(
- namespace ns { struct S; } // Not used
- using ns::S; // FIXME: S should be used
- )cpp",
- "^S *x;");
+ template <typename> struct S {};
+ template <typename T> ^using foo = S<T>;)cpp",
+ "^foo<int> x;");
+ testWalk(R"cpp(
+ namespace ns {template <typename> struct S {}; }
+ using ns::^S;)cpp",
+ "^S<int> x;");
+ testWalk("template<typename> struct ^S {};",
+ R"cpp(
+ template <template <typename> typename> struct X {};
+ X<^S> x;)cpp");
+ testWalk("template<typename T> struct ^S { S(T); };", "^S s(42);");
+ // Should we mark the specialization instead?
+ testWalk("template<typename> struct ^S {}; template <> struct S<int> {};",
+ "^S<int> s;");
+}
+
+TEST(WalkAST, MemberExprs) {
+ testWalk("struct S { void ^foo(); };", "void foo() { S{}.^foo(); }");
+}
+
+TEST(WalkAST, ConstructExprs) {
+ testWalk("struct ^S {};", "S ^t;");
+ testWalk("struct S { ^S(int); };", "S ^t(42);");
+}
+
+TEST(WalkAST, Functions) {
+ // Definition uses declaration, not the other way around.
+ testWalk("void ^foo();", "void ^foo() {}");
+ testWalk("void foo() {}", "void ^foo();");
+
+ // Unresolved calls marks all the overloads.
+ testWalk("void ^foo(int); void ^foo(char);",
+ "template <typename T> void bar() { ^foo(T{}); }");
+}
+
+TEST(WalkAST, Enums) {
+ testWalk("enum E { ^A = 42, B = 43 };", "int e = ^A;");
+ testWalk("enum class ^E : int;", "enum class ^E : int {};");
+ testWalk("enum class E : int {};", "enum class ^E : int ;");
}
} // namespace
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -7,7 +7,17 @@
//===----------------------------------------------------------------------===//
#include "AnalysisInternal.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TemplateName.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
namespace clang {
namespace include_cleaner {
@@ -17,6 +27,16 @@
class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
DeclCallback Callback;
+ bool handleTemplateName(SourceLocation Loc, TemplateName TN) {
+ // For using-templates, only mark the alias.
+ if (auto *USD = TN.getAsUsingShadowDecl()) {
+ report(Loc, USD);
+ return true;
+ }
+ report(Loc, TN.getAsTemplateDecl());
+ return true;
+ }
+
void report(SourceLocation Loc, NamedDecl *ND) {
if (!ND || Loc.isInvalid())
return;
@@ -26,15 +46,89 @@
public:
ASTWalker(DeclCallback Callback) : Callback(Callback) {}
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ report(DRE->getLocation(), DRE->getFoundDecl());
+ return true;
+ }
+
+ bool VisitMemberExpr(MemberExpr *E) {
+ auto *MD = E->getMemberDecl();
+ report(E->getMemberLoc(), MD);
+ return true;
+ }
+
+ bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+ report(E->getLocation(), E->getConstructor());
+ 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); });
+ return true;
+ }
+
+ bool VisitUsingDecl(UsingDecl *UD) {
+ for (const auto *Shadow : UD->shadows())
+ report(UD->getLocation(), Shadow->getTargetDecl());
+ return true;
+ }
+
+ bool VisitFunctionDecl(FunctionDecl *FD) {
+ // Only mark the declaration from a definition.
+ if (FD->isThisDeclarationADefinition())
+ report(FD->getLocation(), FD);
+ return true;
+ }
+
+ bool VisitEnumDecl(EnumDecl *D) {
+ // Definition of an enum with an underlying type references declaration for
+ // type-checking purposes.
+ if (D->isThisDeclarationADefinition() && D->getIntegerTypeSourceInfo())
+ report(D->getLocation(), D);
+ return true;
+ }
+
+ // TypeLoc visitors.
+ bool VisitUsingTypeLoc(UsingTypeLoc TL) {
+ report(TL.getNameLoc(), TL.getFoundDecl());
+ return true;
+ }
+
bool VisitTagTypeLoc(TagTypeLoc TTL) {
report(TTL.getNameLoc(), TTL.getDecl());
return true;
}
- bool VisitDeclRefExpr(DeclRefExpr *DRE) {
- report(DRE->getLocation(), DRE->getFoundDecl());
+ bool VisitTypedefTypeLoc(TypedefTypeLoc TTL) {
+ report(TTL.getNameLoc(), TTL.getTypedefNameDecl());
return true;
}
+
+ bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
+ // FIXME: Handle specializations.
+ return handleTemplateName(TL.getTemplateNameLoc(),
+ TL.getTypePtr()->getTemplateName());
+ }
+
+ bool VisitDeducedTemplateSpecializationTypeLoc(
+ DeducedTemplateSpecializationTypeLoc TL) {
+ // FIXME: Handle specializations.
+ return handleTemplateName(TL.getTemplateNameLoc(),
+ TL.getTypePtr()->getTemplateName());
+ }
+
+ bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &TL) {
+ auto &Arg = TL.getArgument();
+ // Template-template parameters require special attention, as there's no
+ // TemplateNameLoc.
+ if (Arg.getKind() == TemplateArgument::Template ||
+ Arg.getKind() == TemplateArgument::TemplateExpansion)
+ return handleTemplateName(TL.getLocation(),
+ Arg.getAsTemplateOrTemplatePattern());
+ return RecursiveASTVisitor::TraverseTemplateArgumentLoc(TL);
+ }
};
} // namespace
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
