[clang-tools-extra] r370760 - [clang-tidy] Fix a false positive in unused-using-decl check
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 3 07:13:00 PDT 2019
Author: hokein
Date: Tue Sep 3 07:13:00 2019
New Revision: 370760
URL: http://llvm.org/viewvc/llvm-project?rev=370760&view=rev
Log:
[clang-tidy] Fix a false positive in unused-using-decl check
The previous matcher "hasAnyTemplateArgument(templateArgument())" only
matches the first template argument, but the check wants to iterate all
template arguments. This patch fixes this.
Also some refactorings in this patch (to make the code reusable).
Modified:
clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp?rev=370760&r1=370759&r2=370760&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp Tue Sep 3 07:13:00 2019
@@ -17,6 +17,29 @@ namespace clang {
namespace tidy {
namespace misc {
+namespace {
+// FIXME: Move ASTMatcher library.
+AST_POLYMORPHIC_MATCHER_P(
+ forEachTemplateArgument,
+ AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
+ TemplateSpecializationType, FunctionDecl),
+ clang::ast_matchers::internal::Matcher<TemplateArgument>, InnerMatcher) {
+ ArrayRef<TemplateArgument> TemplateArgs =
+ clang::ast_matchers::internal::getTemplateSpecializationArgs(Node);
+ clang::ast_matchers::internal::BoundNodesTreeBuilder Result;
+ bool Matched = false;
+ for (const auto &Arg : TemplateArgs) {
+ clang::ast_matchers::internal::BoundNodesTreeBuilder ArgBuilder(*Builder);
+ if (InnerMatcher.matches(Arg, Finder, &ArgBuilder)) {
+ Matched = true;
+ Result.addMatch(ArgBuilder);
+ }
+ }
+ *Builder = std::move(Result);
+ return Matched;
+}
+} // namespace
+
// A function that helps to tell whether a TargetDecl in a UsingDecl will be
// checked. Only variable, function, function template, class template, class,
// enum declaration and enum constant declaration are considered.
@@ -37,11 +60,10 @@ void UnusedUsingDeclsCheck::registerMatc
Finder->addMatcher(callExpr(callee(unresolvedLookupExpr().bind("used"))),
this);
Finder->addMatcher(
- callExpr(hasDeclaration(functionDecl(hasAnyTemplateArgument(
- anyOf(refersToTemplate(templateName().bind("used")),
- refersToDeclaration(functionDecl().bind("used"))))))),
+ callExpr(hasDeclaration(functionDecl(
+ forEachTemplateArgument(templateArgument().bind("used"))))),
this);
- Finder->addMatcher(loc(templateSpecializationType(hasAnyTemplateArgument(
+ Finder->addMatcher(loc(templateSpecializationType(forEachTemplateArgument(
templateArgument().bind("used")))),
this);
}
@@ -80,52 +102,47 @@ void UnusedUsingDeclsCheck::check(const
Contexts.push_back(Context);
return;
}
- // Mark using declarations as used by setting FoundDecls' value to zero. As
- // the AST is walked in order, usages are only marked after a the
- // corresponding using declaration has been found.
- // FIXME: This currently doesn't look at whether the type reference is
- // actually found with the help of the using declaration.
- if (const auto *Used = Result.Nodes.getNodeAs<NamedDecl>("used")) {
+
+ // Mark a corresponding using declaration as used.
+ auto RemoveNamedDecl = [&](const NamedDecl *Used) {
+ removeFromFoundDecls(Used);
+ // Also remove variants of Used.
if (const auto *FD = dyn_cast<FunctionDecl>(Used)) {
removeFromFoundDecls(FD->getPrimaryTemplate());
} else if (const auto *Specialization =
dyn_cast<ClassTemplateSpecializationDecl>(Used)) {
- Used = Specialization->getSpecializedTemplate();
+ removeFromFoundDecls(Specialization->getSpecializedTemplate());
+ } else if (const auto *FD = dyn_cast<FunctionDecl>(Used)) {
+ if (const auto *FDT = FD->getPrimaryTemplate())
+ removeFromFoundDecls(FDT);
+ } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(Used)) {
+ if (const auto *ET = ECD->getType()->getAs<EnumType>())
+ removeFromFoundDecls(ET->getDecl());
}
- removeFromFoundDecls(Used);
+ };
+ // We rely on the fact that the clang AST is walked in order, usages are only
+ // marked after a corresponding using decl has been found.
+ if (const auto *Used = Result.Nodes.getNodeAs<NamedDecl>("used")) {
+ RemoveNamedDecl(Used);
return;
}
if (const auto *Used = Result.Nodes.getNodeAs<TemplateArgument>("used")) {
- // FIXME: Support non-type template parameters.
if (Used->getKind() == TemplateArgument::Template) {
if (const auto *TD = Used->getAsTemplate().getAsTemplateDecl())
removeFromFoundDecls(TD);
} else if (Used->getKind() == TemplateArgument::Type) {
if (auto *RD = Used->getAsType()->getAsCXXRecordDecl())
removeFromFoundDecls(RD);
+ } else if (Used->getKind() == TemplateArgument::Declaration) {
+ RemoveNamedDecl(Used->getAsDecl());
}
return;
}
- if (const auto *Used = Result.Nodes.getNodeAs<TemplateName>("used")) {
- removeFromFoundDecls(Used->getAsTemplateDecl());
- return;
- }
-
if (const auto *DRE = Result.Nodes.getNodeAs<DeclRefExpr>("used")) {
- if (const auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
- if (const auto *FDT = FD->getPrimaryTemplate())
- removeFromFoundDecls(FDT);
- else
- removeFromFoundDecls(FD);
- } else if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
- removeFromFoundDecls(VD);
- } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
- removeFromFoundDecls(ECD);
- if (const auto *ET = ECD->getType()->getAs<EnumType>())
- removeFromFoundDecls(ET->getDecl());
- }
+ RemoveNamedDecl(DRE->getDecl());
+ return;
}
// Check the uninstantiated template function usage.
if (const auto *ULE = Result.Nodes.getNodeAs<UnresolvedLookupExpr>("used")) {
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp?rev=370760&r1=370759&r2=370760&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp Tue Sep 3 07:13:00 2019
@@ -31,6 +31,8 @@ class N {};
template <int T> class P {};
const int Constant = 0;
+template <typename T> class Q {};
+
class Base {
public:
void f();
@@ -169,6 +171,8 @@ using n::N;
using n::Constant;
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'Constant' is unused
+using n::Q;
+
// ----- Usages -----
void f(B b);
void g() {
@@ -202,3 +206,8 @@ template void h(n::M<N>* t);
template <int T>
void i(n::P<T>* t) {}
template void i(n::P<Constant>* t);
+
+template <typename T, template <typename> class U> class Bar {};
+// We used to report Q unsued, because we only checked the first template
+// argument.
+Bar<int, Q> *bar;
More information about the cfe-commits
mailing list