hokein updated this revision to Diff 483493.
hokein marked 3 inline comments as done.
hokein added a comment.
address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140095/new/
https://reviews.llvm.org/D140095
Files:
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
clang/include/clang/AST/Type.h
clang/lib/AST/Type.cpp
Index: clang/lib/AST/Type.cpp
===================================================================
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -524,6 +524,10 @@
return getAsSugar<TypedefType>(this);
}
+template <> const UsingType *Type::getAs() const {
+ return getAsSugar<UsingType>(this);
+}
+
template <> const TemplateSpecializationType *Type::getAs() const {
return getAsSugar<TemplateSpecializationType>(this);
}
Index: clang/include/clang/AST/Type.h
===================================================================
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2592,6 +2592,7 @@
/// This will check for a TypedefType by removing any existing sugar
/// until it reaches a TypedefType or a non-sugared type.
template <> const TypedefType *Type::getAs() const;
+template <> const UsingType *Type::getAs() const;
/// This will check for a TemplateSpecializationType by removing any
/// existing sugar until it reaches a TemplateSpecializationType or a
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
@@ -210,6 +210,25 @@
};
struct Foo {};)cpp",
"void test(unique_ptr<Foo> &V) { V.^release(); }");
+ // Respect the sugar type (typedef, using-type).
+ testWalk(R"cpp(
+ namespace ns { struct Foo { int a; }; }
+ using $explicit^Bar = ns::Foo;)cpp",
+ "void test(Bar b) { b.^a; }");
+ testWalk(R"cpp(
+ namespace ns { struct Foo { int a; }; }
+ using ns::$explicit^Foo;)cpp",
+ "void test(Foo b) { b.^a; }");
+ testWalk(R"cpp(
+ namespace ns { struct Foo { int a; }; }
+ namespace ns2 { using Bar = ns::Foo; }
+ using ns2::$explicit^Bar;
+ )cpp",
+ "void test(Bar b) { b.^a; }");
+ testWalk(R"cpp(
+ namespace ns { template<typename> struct Foo { int a; }; }
+ using ns::$explicit^Foo;)cpp",
+ "void k(Foo<int> b) { b.^a; }");
}
TEST(WalkAST, ConstructExprs) {
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
@@ -27,16 +27,6 @@
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,
RefType RT = RefType::Explicit) {
if (!ND || Loc.isInvalid())
@@ -44,10 +34,33 @@
Callback(Loc, *cast<NamedDecl>(ND->getCanonicalDecl()), RT);
}
- NamedDecl *resolveType(QualType Type) {
- if (Type->isPointerType())
- Type = Type->getPointeeType();
- return Type->getAsRecordDecl();
+ NamedDecl *resolveTemplateName(TemplateName TN) {
+ // For using-templates, only mark the alias.
+ if (auto *USD = TN.getAsUsingShadowDecl())
+ return USD;
+ return TN.getAsTemplateDecl();
+ }
+ NamedDecl *getMemberProvider(QualType Base) {
+ if (Base->isPointerType())
+ Base = Base->getPointeeType();
+ // Unwrap the sugar ElaboratedType.
+ if (const auto *ElTy = dyn_cast<ElaboratedType>(Base))
+ Base = ElTy->getNamedType();
+
+ if (const auto *TT = dyn_cast<TypedefType>(Base))
+ return TT->getDecl();
+ if (const auto *UT = dyn_cast<UsingType>(Base))
+ return UT->getFoundDecl();
+ // A heuristic: to resolve a template type to **only** its template name.
+ // We're only using this method for the base type of MemberExpr, in general
+ // the template provides the member, and the critical case `unique_ptr<Foo>`
+ // is supported (the base type is a Foo*).
+ //
+ // There are some exceptions that this heuristic could fail (dependent base,
+ // dependent typealias), but we believe these are rare.
+ if (const auto *TST = dyn_cast<TemplateSpecializationType>(Base))
+ return resolveTemplateName(TST->getTemplateName());
+ return Base->getAsRecordDecl();
}
public:
@@ -59,12 +72,16 @@
}
bool VisitMemberExpr(MemberExpr *E) {
- // A member expr implies a usage of the class type
- // (e.g., to prevent inserting a header of base class when using base
- // members from a derived object).
+ // Reporting a usage of the member decl would cause issues (e.g. force
+ // including the base class for inherited members). Instead, we report a
+ // usage of the base type of the MemberExpr, so that e.g. code
+ // `returnFoo().bar` can keep #include "foo.h" (rather than inserting
+ // "bar.h" for the underlying base type `Bar`).
+ //
// FIXME: support dependent types, e.g., "std::vector<T>().size()".
QualType Type = E->getBase()->IgnoreImpCasts()->getType();
- report(E->getMemberLoc(), resolveType(Type));
+ // FIXME: this should report as implicit reference.
+ report(E->getMemberLoc(), getMemberProvider(Type));
return true;
}
@@ -126,15 +143,17 @@
bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
// FIXME: Handle explicit specializations.
- return handleTemplateName(TL.getTemplateNameLoc(),
- TL.getTypePtr()->getTemplateName());
+ report(TL.getTemplateNameLoc(),
+ resolveTemplateName(TL.getTypePtr()->getTemplateName()));
+ return true;
}
bool VisitDeducedTemplateSpecializationTypeLoc(
DeducedTemplateSpecializationTypeLoc TL) {
// FIXME: Handle specializations.
- return handleTemplateName(TL.getTemplateNameLoc(),
- TL.getTypePtr()->getTemplateName());
+ report(TL.getTemplateNameLoc(),
+ resolveTemplateName(TL.getTypePtr()->getTemplateName()));
+ return true;
}
bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &TL) {
@@ -142,9 +161,11 @@
// 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());
+ Arg.getKind() == TemplateArgument::TemplateExpansion) {
+ report(TL.getLocation(),
+ resolveTemplateName(Arg.getAsTemplateOrTemplatePattern()));
+ return true;
+ }
return RecursiveASTVisitor::TraverseTemplateArgumentLoc(TL);
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
