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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits