Skip to content

Commit ab090e9

Browse files
committed
[include-cleaner] Make handling of enum constants similar to members
We were treating enum constants more like regular decls, which results in ignoring type aliases/exports. This patch brings the handling to be closer to member-like decls, with one caveat. When we encounter reference to an enum constant we still report an explicit reference to the particular enum constant, as otherwise we might not see any references to the enum itself. Also drops implicit references from qualified names to containers, as we already have explicit references from the qualifier to relevant container. Differential Revision: https://reviews.llvm.org/D158515
1 parent 779353e commit ab090e9

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,22 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
126126
}
127127

128128
bool VisitDeclRefExpr(DeclRefExpr *DRE) {
129-
// Static class members are handled here, as they don't produce MemberExprs.
130-
if (DRE->getFoundDecl()->isCXXClassMember()) {
131-
if (auto *Qual = DRE->getQualifier())
132-
report(DRE->getLocation(), Qual->getAsRecordDecl(), RefType::Implicit);
133-
} else {
134-
report(DRE->getLocation(), DRE->getFoundDecl());
129+
auto *FD = DRE->getFoundDecl();
130+
// For refs to non-meber-like decls, use the found decl.
131+
// For member-like decls, we should have a reference from the qualifier to
132+
// the container decl instead, which is preferred as it'll handle
133+
// aliases/exports properly.
134+
if (!FD->isCXXClassMember() && !llvm::isa<EnumConstantDecl>(FD)) {
135+
report(DRE->getLocation(), FD);
136+
return true;
135137
}
138+
// If the ref is without a qualifier, and is a member, ignore it. As it is
139+
// available in current context due to some other construct (e.g. base
140+
// specifiers, using decls) that has to spell the name explicitly.
141+
// If it's an enum constant, it must be due to prior decl. Report references
142+
// to it instead.
143+
if (llvm::isa<EnumConstantDecl>(FD) && !DRE->hasQualifier())
144+
report(DRE->getLocation(), FD);
136145
return true;
137146
}
138147

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ std::vector<Decl::Kind> testWalk(llvm::StringRef TargetCode,
5050
Inputs.ExtraFiles["target.h"] = Target.code().str();
5151
Inputs.ExtraArgs.push_back("-include");
5252
Inputs.ExtraArgs.push_back("target.h");
53-
Inputs.ExtraArgs.push_back("-std=c++17");
53+
Inputs.ExtraArgs.push_back("-std=c++20");
5454
TestAST AST(Inputs);
5555
const auto &SM = AST.sourceManager();
5656

@@ -114,7 +114,7 @@ TEST(WalkAST, DeclRef) {
114114
testWalk("int $explicit^x;", "int y = ^x;");
115115
testWalk("int $explicit^foo();", "int y = ^foo();");
116116
testWalk("namespace ns { int $explicit^x; }", "int y = ns::^x;");
117-
testWalk("struct $implicit^S { static int x; };", "int y = S::^x;");
117+
testWalk("struct S { static int x; };", "int y = S::^x;");
118118
// Canonical declaration only.
119119
testWalk("extern int $explicit^x; int x;", "int y = ^x;");
120120
// Return type of `foo` isn't used.
@@ -309,6 +309,14 @@ TEST(WalkAST, Alias) {
309309
namespace ns { using ::$explicit^Foo; }
310310
template<> struct ns::Foo<int> {};)cpp",
311311
"ns::^Foo<int> x;");
312+
testWalk(R"cpp(
313+
namespace ns { enum class foo { bar }; }
314+
using ns::foo;)cpp",
315+
"auto x = foo::^bar;");
316+
testWalk(R"cpp(
317+
namespace ns { enum foo { bar }; }
318+
using ns::foo::$explicit^bar;)cpp",
319+
"auto x = ^bar;");
312320
}
313321

314322
TEST(WalkAST, Using) {
@@ -338,6 +346,8 @@ TEST(WalkAST, Using) {
338346
}
339347
using ns::$explicit^Y;)cpp",
340348
"^Y<int> x;");
349+
testWalk("namespace ns { enum E {A}; } using enum ns::$explicit^E;",
350+
"auto x = ^A;");
341351
}
342352

343353
TEST(WalkAST, Namespaces) {
@@ -399,10 +409,10 @@ TEST(WalkAST, NestedTypes) {
399409
}
400410

401411
TEST(WalkAST, MemberExprs) {
402-
testWalk("struct $implicit^S { static int f; };", "void foo() { S::^f; }");
403-
testWalk("struct B { static int f; }; struct $implicit^S : B {};",
412+
testWalk("struct S { static int f; };", "void foo() { S::^f; }");
413+
testWalk("struct B { static int f; }; struct S : B {};",
404414
"void foo() { S::^f; }");
405-
testWalk("struct B { static void f(); }; struct $implicit^S : B {};",
415+
testWalk("struct B { static void f(); }; struct S : B {};",
406416
"void foo() { S::^f; }");
407417
testWalk("struct B { static void f(); }; ",
408418
"struct S : B { void foo() { ^f(); } };");

0 commit comments

Comments
 (0)