[clang-tools-extra] 0ab57bd - [include-cleaner] Fix the member-expr-access usage for sugar type.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 19 00:05:17 PST 2022
Author: Haojian Wu
Date: 2022-12-19T08:48:32+01:00
New Revision: 0ab57bdc9745bfc8147831c09ed05073f87e7040
URL: https://github.com/llvm/llvm-project/commit/0ab57bdc9745bfc8147831c09ed05073f87e7040
DIFF: https://github.com/llvm/llvm-project/commit/0ab57bdc9745bfc8147831c09ed05073f87e7040.diff
LOG: [include-cleaner] Fix the member-expr-access usage for sugar type.
Fixes https://github.com/llvm/llvm-project/issues/59533
Differential Revision: https://reviews.llvm.org/D140095
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index ff15e62efdfd5..cf2f0d7bcff63 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -27,16 +27,6 @@ using DeclCallback =
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 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
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 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
}
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 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
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 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
// 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);
}
};
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index a197c1b763ad8..cb41ebb35cd58 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -210,6 +210,25 @@ TEST(WalkAST, MemberExprs) {
};
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) {
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 54896e7378933..2407400b31315 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2594,6 +2594,7 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
/// 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
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index fe7bbcd1479dd..0b551ae9af432 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -524,6 +524,10 @@ template <> const TypedefType *Type::getAs() const {
return getAsSugar<TypedefType>(this);
}
+template <> const UsingType *Type::getAs() const {
+ return getAsSugar<UsingType>(this);
+}
+
template <> const TemplateSpecializationType *Type::getAs() const {
return getAsSugar<TemplateSpecializationType>(this);
}
More information about the cfe-commits
mailing list