Index: clang-tidy/misc/UnusedUsingDeclsCheck.h =================================================================== --- clang-tidy/misc/UnusedUsingDeclsCheck.h +++ clang-tidy/misc/UnusedUsingDeclsCheck.h @@ -30,6 +30,7 @@ void onEndOfTranslationUnit() override; private: + void removeFromFoundDecls(const Decl *D); llvm::DenseMap FoundDecls; llvm::DenseMap FoundRanges; }; Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp =================================================================== --- clang-tidy/misc/UnusedUsingDeclsCheck.cpp +++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp @@ -23,6 +23,7 @@ auto DeclMatcher = hasDeclaration(namedDecl().bind("used")); Finder->addMatcher(loc(recordType(DeclMatcher)), this); Finder->addMatcher(loc(templateSpecializationType(DeclMatcher)), this); + Finder->addMatcher(declRefExpr().bind("used"), this); } void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) { @@ -34,8 +35,9 @@ const auto *TargetDecl = Using->shadow_begin()->getTargetDecl()->getCanonicalDecl(); - // FIXME: Handle other target types. - if (!isa(TargetDecl) && !isa(TargetDecl)) + if (!isa(TargetDecl) && !isa(TargetDecl) && + !isa(TargetDecl) && !isa(TargetDecl) && + !isa(TargetDecl)) return; FoundDecls[TargetDecl] = Using; @@ -57,12 +59,26 @@ if (const auto *Specialization = dyn_cast(Used)) Used = Specialization->getSpecializedTemplate(); - auto I = FoundDecls.find(Used->getCanonicalDecl()); - if (I != FoundDecls.end()) - I->second = nullptr; + removeFromFoundDecls(Used->getCanonicalDecl()); + } else if (const auto *DRE = Result.Nodes.getNodeAs("used")) { + if (const auto *FD = dyn_cast(DRE->getDecl())) { + if (const auto *FDT = FD->getPrimaryTemplate()) { + removeFromFoundDecls(FDT); + } else { + removeFromFoundDecls(FD); + } + } else if (const auto *VD = dyn_cast(DRE->getDecl())) { + removeFromFoundDecls(VD); + } } } +void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) { + auto I = FoundDecls.find(D); + if (I != FoundDecls.end()) + I->second = nullptr; +} + void UnusedUsingDeclsCheck::onEndOfTranslationUnit() { for (const auto &FoundDecl : FoundDecls) { if (FoundDecl.second == nullptr) Index: include-fixer/find-all-symbols/CMakeLists.txt =================================================================== --- include-fixer/find-all-symbols/CMakeLists.txt +++ include-fixer/find-all-symbols/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(findAllSymbols FindAllSymbols.cpp + PragmaCommentHandler.cpp SymbolInfo.cpp LINK_LIBS Index: include-fixer/find-all-symbols/FindAllSymbols.h =================================================================== --- include-fixer/find-all-symbols/FindAllSymbols.h +++ include-fixer/find-all-symbols/FindAllSymbols.h @@ -12,11 +12,15 @@ #include "SymbolInfo.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/StringRef.h" +#include #include namespace clang { namespace find_all_symbols { +class HeaderMapCollector; + /// \brief FindAllSymbols collects all classes, free standing functions and /// global variables with some extra information such as the path of the header /// file, the namespaces they are contained in, the type of variables and the @@ -39,7 +43,9 @@ const SymbolInfo &Symbol) = 0; }; - explicit FindAllSymbols(ResultReporter *Reporter) : Reporter(Reporter) {} + explicit FindAllSymbols(ResultReporter *Reporter, + HeaderMapCollector *Collector) + : Reporter(Reporter), Collector(Collector) {} void registerMatchers(clang::ast_matchers::MatchFinder *MatchFinder); @@ -48,6 +54,7 @@ private: ResultReporter *const Reporter; + HeaderMapCollector *const Collector; }; } // namespace find_all_symbols Index: include-fixer/find-all-symbols/FindAllSymbols.cpp =================================================================== --- include-fixer/find-all-symbols/FindAllSymbols.cpp +++ include-fixer/find-all-symbols/FindAllSymbols.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "FindAllSymbols.h" +#include "HeaderMapCollector.h" #include "SymbolInfo.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" @@ -43,8 +44,9 @@ } } -bool SetCommonInfo(const MatchFinder::MatchResult &Result, - const NamedDecl *ND, SymbolInfo *Symbol) { +bool SetCommonInfo(const MatchFinder::MatchResult &Result, const NamedDecl *ND, + SymbolInfo *Symbol, + const HeaderMapCollector::HeaderMap &HeaderMappingTable) { SetContext(ND, Symbol); Symbol->Name = ND->getNameAsString(); @@ -64,6 +66,13 @@ if (FilePath.empty()) return false; + // Check pragma remapping header. + auto Iter = HeaderMappingTable.find(FilePath); + if (Iter != HeaderMappingTable.end()) { + Symbol->FilePath = Iter->second; + return true; + } + llvm::SmallString<128> AbsolutePath; if (llvm::sys::path::is_absolute(FilePath)) { AbsolutePath = FilePath; @@ -174,7 +183,7 @@ const SourceManager *SM = Result.SourceManager; SymbolInfo Symbol; - if (!SetCommonInfo(Result, ND, &Symbol)) + if (!SetCommonInfo(Result, ND, &Symbol, Collector->getHeaderMappingTable())) return; if (const auto *VD = llvm::dyn_cast(ND)) { Index: include-fixer/find-all-symbols/HeaderMapCollector.h =================================================================== --- /dev/null +++ include-fixer/find-all-symbols/HeaderMapCollector.h @@ -0,0 +1,38 @@ +//===-- HeaderMapCoolector.h - find all symbols------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_HEADER_MAP_COLLECTOR_H +#define LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_HEADER_MAP_COLLECTOR_H + +#include "llvm/ADT/StringMap.h" +#include + +namespace clang { +namespace find_all_symbols { + +/// \brief HeaderMappCollector collects all remapping header files. +class HeaderMapCollector { +public: + typedef llvm::StringMap HeaderMap; + + void addHeaderMapping(llvm::StringRef OrignalHeaderPath, + llvm::StringRef MappingHeaderPath) { + HeaderMappingTable[OrignalHeaderPath] = MappingHeaderPath; + }; + const HeaderMap &getHeaderMappingTable() { return HeaderMappingTable; }; + +private: + /// A string-to-string map saving the mapping relationship. + HeaderMap HeaderMappingTable; +}; + +} // namespace find_all_symbols +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_HEADER_MAP_COLLECTOR_H Index: include-fixer/find-all-symbols/PragmaCommentHandler.h =================================================================== --- /dev/null +++ include-fixer/find-all-symbols/PragmaCommentHandler.h @@ -0,0 +1,41 @@ +//===-- PragmaCommentHandler.h - find all symbols----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_PRAGMA_COMMENT_HANDLER_H +#define LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_PRAGMA_COMMENT_HANDLER_H + +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Preprocessor.h" +#include + +namespace clang { +namespace find_all_symbols { + +class HeaderMapCollector; + +/// \brief PragmaCommentHandler parses pragma comment on include files to +/// determine when we should include a different header from the header that +/// directly defines a symbol. +/// +/// Currently it only supports IWYU private pragma: +/// https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private +class PragmaCommentHandler : public clang::CommentHandler { +public: + PragmaCommentHandler(HeaderMapCollector *Collector) : Collector(Collector) {} + + bool HandleComment(Preprocessor &PP, SourceRange Range) override; + +private: + HeaderMapCollector *const Collector; +}; + +} // namespace find_all_symbols +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_PRAGMA_COMMENT_HANDLER_H Index: include-fixer/find-all-symbols/PragmaCommentHandler.cpp =================================================================== --- /dev/null +++ include-fixer/find-all-symbols/PragmaCommentHandler.cpp @@ -0,0 +1,37 @@ +//===-- PragmaCommentHandler.cpp - find all symbols -----------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "PragmaCommentHandler.h" +#include "FindAllSymbols.h" +#include "HeaderMapCollector.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/Support/Regex.h" + +namespace clang { +namespace find_all_symbols { +namespace { +const char IWYUPragma[] = "// IWYU pragma: private, include "; +} // namespace + +bool PragmaCommentHandler::HandleComment(Preprocessor &PP, SourceRange Range) { + StringRef Text = + Lexer::getSourceText(CharSourceRange::getCharRange(Range), + PP.getSourceManager(), PP.getLangOpts()); + size_t Pos = Text.find(IWYUPragma); + if (Pos == StringRef::npos) + return false; + StringRef RemappingFilePath = Text.substr(Pos + std::strlen(IWYUPragma)); + Collector->addHeaderMapping( + PP.getSourceManager().getFilename(Range.getBegin()), + RemappingFilePath.trim("\"<>")); + return false; +} + +} // namespace find_all_symbols +} // namespace clang Index: include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp =================================================================== --- include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp +++ include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp @@ -8,8 +8,14 @@ //===----------------------------------------------------------------------===// #include "FindAllSymbols.h" +#include "HeaderMapCollector.h" +#include "PragmaCommentHandler.h" #include "SymbolInfo.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/ArrayRef.h" @@ -82,6 +88,31 @@ std::map> Symbols; }; +class FindAllSymbolsAction : public clang::ASTFrontendAction { +public: + FindAllSymbolsAction() + : Reporter(), MatchFinder(), Collector(), Handler(&Collector), + Matcher(&Reporter, &Collector) { + Matcher.registerMatchers(&MatchFinder); + } + + std::unique_ptr + CreateASTConsumer(clang::CompilerInstance &Compiler, + StringRef InFile) override { + Compiler.getPreprocessor().addCommentHandler(&Handler); + return MatchFinder.newASTConsumer(); + } + + void EndSourceFileAction() override { Reporter.Write(OutputDir); } + +private: + YamlReporter Reporter; + clang::ast_matchers::MatchFinder MatchFinder; + HeaderMapCollector Collector; + PragmaCommentHandler Handler; + FindAllSymbols Matcher; +}; + bool Merge(llvm::StringRef MergeDir, llvm::StringRef OutputFile) { std::error_code EC; std::set UniqueSymbols; @@ -142,11 +173,8 @@ return 0; } - clang::find_all_symbols::YamlReporter Reporter; - clang::find_all_symbols::FindAllSymbols Matcher(&Reporter); - clang::ast_matchers::MatchFinder MatchFinder; - Matcher.registerMatchers(&MatchFinder); - Tool.run(newFrontendActionFactory(&MatchFinder).get()); - Reporter.Write(OutputDir); + Tool.run( + newFrontendActionFactory() + .get()); return 0; } Index: test/clang-tidy/misc-unused-using-decls.cpp =================================================================== --- test/clang-tidy/misc-unused-using-decls.cpp +++ test/clang-tidy/misc-unused-using-decls.cpp @@ -10,6 +10,21 @@ class D { public: static int i; }; template class E {}; template class F {}; + +D UsedInstance; +D UnusedInstance; + +int UsedFunc() { return 1; } +int UnusedFunc() { return 1; } +template int UsedTemplateFunc() { return 1; } +template int UnusedTemplateFunc() { return 1; } + +class ostream { +public: + ostream &operator<<(ostream &(*PF)(ostream &)); +}; +extern ostream cout; +ostream &endl(ostream &os); } // ----- Using declarations ----- @@ -24,6 +39,15 @@ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'E' is unused // CHECK-FIXES: {{^}}// E using n::F; +using n::UsedInstance; +using n::UsedFunc; +using n::UsedTemplateFunc; +using n::UnusedInstance; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedInstance' is unused +using n::UnusedFunc; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedFunc' is unused +using n::cout; +using n::endl; // ----- Usages ----- void f(B b); @@ -31,5 +55,9 @@ vector data; D::i = 1; F f; + UsedInstance.i; + UsedFunc(); + UsedTemplateFunc(); + cout << endl; } Index: unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp =================================================================== --- unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp +++ unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp @@ -8,11 +8,14 @@ //===----------------------------------------------------------------------===// #include "FindAllSymbols.h" +#include "HeaderMapCollector.h" +#include "PragmaCommentHandler.h" #include "SymbolInfo.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" #include "clang/Basic/VirtualFileSystem.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" @@ -62,6 +65,41 @@ std::vector Symbols; }; +class TestFindAllSymbolsAction : public clang::ASTFrontendAction { +public: + TestFindAllSymbolsAction(FindAllSymbols::ResultReporter *Reporter) + : MatchFinder(), Collector(), Handler(&Collector), + Matcher(Reporter, &Collector) { + Matcher.registerMatchers(&MatchFinder); + } + + std::unique_ptr + CreateASTConsumer(clang::CompilerInstance &Compiler, + StringRef InFile) override { + Compiler.getPreprocessor().addCommentHandler(&Handler); + return MatchFinder.newASTConsumer(); + } + +private: + ast_matchers::MatchFinder MatchFinder; + HeaderMapCollector Collector; + PragmaCommentHandler Handler; + FindAllSymbols Matcher; +}; + +class TestFindAllSymbolsActionFactory + : public clang::tooling::FrontendActionFactory { +public: + TestFindAllSymbolsActionFactory(MockReporter *Reporter) + : Reporter(Reporter) {} + clang::FrontendAction *create() override { + return new TestFindAllSymbolsAction(Reporter); + } + +private: + MockReporter *const Reporter; +}; + class FindAllSymbolsTest : public ::testing::Test { public: bool hasSymbol(const SymbolInfo &Symbol) { @@ -73,18 +111,16 @@ } bool runFindAllSymbols(StringRef Code) { - FindAllSymbols matcher(&Reporter); - clang::ast_matchers::MatchFinder MatchFinder; - matcher.registerMatchers(&MatchFinder); - llvm::IntrusiveRefCntPtr InMemoryFileSystem( new vfs::InMemoryFileSystem); llvm::IntrusiveRefCntPtr Files( new FileManager(FileSystemOptions(), InMemoryFileSystem)); std::string FileName = "symbol.cc"; - std::unique_ptr Factory = - clang::tooling::newFrontendActionFactory(&MatchFinder); + + std::unique_ptr Factory( + new TestFindAllSymbolsActionFactory(&Reporter)); + tooling::ToolInvocation Invocation( {std::string("find_all_symbols"), std::string("-fsyntax-only"), FileName}, @@ -371,5 +407,20 @@ } } +TEST_F(FindAllSymbolsTest, IWYUPrivatePragmaTest) { + static const char Code[] = R"( + // IWYU pragma: private, include "bar.h" + struct Bar { + }; + )"; + runFindAllSymbols(Code); + + { + SymbolInfo Symbol = + CreateSymbolInfo("Bar", SymbolInfo::Class, "bar.h", 3, {}); + EXPECT_TRUE(hasSymbol(Symbol)); + } +} + } // namespace find_all_symbols } // namespace clang