Skip to content

Commit

Permalink
[include-cleaner] Make handling of enum constants similar to members
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kadircet committed Aug 28, 2023
1 parent 779353e commit ab090e9
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 11 deletions.
21 changes: 15 additions & 6 deletions clang-tools-extra/include-cleaner/lib/WalkAST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,22 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
}

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

Expand Down
20 changes: 15 additions & 5 deletions clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ std::vector<Decl::Kind> testWalk(llvm::StringRef TargetCode,
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");
Inputs.ExtraArgs.push_back("-std=c++20");
TestAST AST(Inputs);
const auto &SM = AST.sourceManager();

Expand Down Expand Up @@ -114,7 +114,7 @@ TEST(WalkAST, DeclRef) {
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 $implicit^S { static int x; };", "int y = S::^x;");
testWalk("struct S { static int x; };", "int y = S::^x;");
// Canonical declaration only.
testWalk("extern int $explicit^x; int x;", "int y = ^x;");
// Return type of `foo` isn't used.
Expand Down Expand Up @@ -309,6 +309,14 @@ TEST(WalkAST, Alias) {
namespace ns { using ::$explicit^Foo; }
template<> struct ns::Foo<int> {};)cpp",
"ns::^Foo<int> x;");
testWalk(R"cpp(
namespace ns { enum class foo { bar }; }
using ns::foo;)cpp",
"auto x = foo::^bar;");
testWalk(R"cpp(
namespace ns { enum foo { bar }; }
using ns::foo::$explicit^bar;)cpp",
"auto x = ^bar;");
}

TEST(WalkAST, Using) {
Expand Down Expand Up @@ -338,6 +346,8 @@ TEST(WalkAST, Using) {
}
using ns::$explicit^Y;)cpp",
"^Y<int> x;");
testWalk("namespace ns { enum E {A}; } using enum ns::$explicit^E;",
"auto x = ^A;");
}

TEST(WalkAST, Namespaces) {
Expand Down Expand Up @@ -399,10 +409,10 @@ TEST(WalkAST, NestedTypes) {
}

TEST(WalkAST, MemberExprs) {
testWalk("struct $implicit^S { static int f; };", "void foo() { S::^f; }");
testWalk("struct B { static int f; }; struct $implicit^S : B {};",
testWalk("struct S { static int f; };", "void foo() { S::^f; }");
testWalk("struct B { static int f; }; struct S : B {};",
"void foo() { S::^f; }");
testWalk("struct B { static void f(); }; struct $implicit^S : B {};",
testWalk("struct B { static void f(); }; struct S : B {};",
"void foo() { S::^f; }");
testWalk("struct B { static void f(); }; ",
"struct S : B { void foo() { ^f(); } };");
Expand Down

0 comments on commit ab090e9

Please sign in to comment.