sammccall updated this revision to Diff 89337. sammccall marked 7 inline comments as done. sammccall added a comment.
Address review comments. https://reviews.llvm.org/D30210 Files: include-fixer/IncludeFixer.cpp include-fixer/SymbolIndexManager.cpp include-fixer/find-all-symbols/FindAllSymbols.cpp include-fixer/find-all-symbols/SymbolInfo.cpp include-fixer/find-all-symbols/SymbolInfo.h include-fixer/find-all-symbols/SymbolReporter.h include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp test/include-fixer/Inputs/fake_yaml_db.yaml test/include-fixer/Inputs/merge/a.yaml test/include-fixer/Inputs/merge/b.yaml test/include-fixer/merge.test unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
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 @@ -19,6 +19,7 @@ #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" @@ -40,25 +41,34 @@ Symbols.push_back(Symbol); } + void reportUse(llvm::StringRef FileName, const SymbolInfo &Symbol) override { + Used.push_back(Symbol); + } + bool hasSymbol(const SymbolInfo &Symbol) const { - for (const auto &S : Symbols) { - if (S == Symbol) - return true; - } - return false; + return llvm::is_contained(Symbols, Symbol); + } + + bool hasUse(const SymbolInfo &Symbol) const { + return llvm::is_contained(Used, Symbol); } private: std::vector<SymbolInfo> Symbols; + std::vector<SymbolInfo> Used; }; class FindAllSymbolsTest : public ::testing::Test { public: bool hasSymbol(const SymbolInfo &Symbol) { return Reporter.hasSymbol(Symbol); } - bool runFindAllSymbols(StringRef Code) { + bool hasUse(const SymbolInfo &Symbol) { + return Reporter.hasUse(Symbol); + } + + bool runFindAllSymbols(StringRef HeaderCode, StringRef MainCode) { llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem( new vfs::InMemoryFileSystem); llvm::IntrusiveRefCntPtr<FileManager> Files( @@ -98,7 +108,7 @@ std::make_shared<PCHContainerOperations>()); InMemoryFileSystem->addFile(HeaderName, 0, - llvm::MemoryBuffer::getMemBuffer(Code)); + llvm::MemoryBuffer::getMemBuffer(HeaderCode)); std::string Content = "#include\"" + std::string(HeaderName) + "\"\n" @@ -118,6 +128,7 @@ SymbolInfo DirtySymbol("ExtraInternal", SymbolInfo::SymbolKind::Class, CleanHeader, 2, {}); #endif // _MSC_VER && __MINGW32__ + Content += "\n" + MainCode.str(); InMemoryFileSystem->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Content)); Invocation.run(); @@ -135,49 +146,64 @@ }; TEST_F(FindAllSymbolsTest, VariableSymbols) { - static const char Code[] = R"( + static const char Header[] = R"( extern int xargc; namespace na { static bool SSSS = false; namespace nb { const long long *XXXX; } })"; - runFindAllSymbols(Code); + static const char Main[] = R"( + auto y = &na::nb::XXXX; + int main() { if (na::SSSS) return xargc; } + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("xargc", SymbolInfo::SymbolKind::Variable, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("SSSS", SymbolInfo::SymbolKind::Variable, HeaderName, 4, {{SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("XXXX", SymbolInfo::SymbolKind::Variable, HeaderName, 5, {{SymbolInfo::ContextType::Namespace, "nb"}, {SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, ExternCSymbols) { - static const char Code[] = R"( + static const char Header[] = R"( extern "C" { int C_Func() { return 0; } struct C_struct { int Member; }; })"; - runFindAllSymbols(Code); + static const char Main[] = R"( + C_struct q() { + int(*ptr)() = C_Func; + return {0}; + } + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("C_Func", SymbolInfo::SymbolKind::Function, HeaderName, 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("C_struct", SymbolInfo::SymbolKind::Class, HeaderName, 4, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, CXXRecordSymbols) { - static const char Code[] = R"( + static const char Header[] = R"( struct Glob {}; struct A; // Not a defintion, ignored. class NOP; // Not a defintion, ignored @@ -190,26 +216,33 @@ }; }; // )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + static Glob glob; + static na::A::AAAA* a; + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("Glob", SymbolInfo::SymbolKind::Class, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("A", SymbolInfo::SymbolKind::Class, HeaderName, 6, {{SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("AAA", SymbolInfo::SymbolKind::Class, HeaderName, 7, {{SymbolInfo::ContextType::Record, "A"}, {SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_FALSE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, CXXRecordSymbolsTemplate) { - static const char Code[] = R"( + static const char Header[] = R"( template <typename T> - class T_TEMP { + struct T_TEMP { template <typename _Tp1> struct rebind { typedef T_TEMP<_Tp1> other; }; }; @@ -222,11 +255,15 @@ // Ignore specialization. template <> class Observer<int> {}; )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + extern T_TEMP<int>::rebind<char> weirdo; + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("T_TEMP", SymbolInfo::SymbolKind::Class, HeaderName, 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, DontIgnoreTemplatePartialSpecialization) { @@ -239,7 +276,7 @@ template<class T> void f() {}; template<> void f<int>() {}; )"; - runFindAllSymbols(Code); + runFindAllSymbols(Code, ""); SymbolInfo Symbol = SymbolInfo("Class", SymbolInfo::SymbolKind::Class, HeaderName, 4, {}); EXPECT_TRUE(hasSymbol(Symbol)); @@ -252,7 +289,7 @@ } TEST_F(FindAllSymbolsTest, FunctionSymbols) { - static const char Code[] = R"( + static const char Header[] = R"( namespace na { int gg(int); int f(const int &a) { int Local; static int StaticLocal; return 0; } @@ -265,91 +302,127 @@ } // namespace nb } // namespace na"; )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + int(*gg)(int) = &na::gg; + int main() { + (void)na::SSSFFF; + na::nb::fun(0); + return na::f(gg(0)); + } + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("gg", SymbolInfo::SymbolKind::Function, HeaderName, 3, {{SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("f", SymbolInfo::SymbolKind::Function, HeaderName, 4, {{SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("SSSFFF", SymbolInfo::SymbolKind::Function, HeaderName, 5, {{SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("fun", SymbolInfo::SymbolKind::Function, HeaderName, 10, {{SymbolInfo::ContextType::Namespace, "nb"}, {SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, NamespaceTest) { - static const char Code[] = R"( + static const char Header[] = R"( int X1; namespace { int X2; } namespace { namespace { int X3; } } namespace { namespace nb { int X4; } } namespace na { inline namespace __1 { int X5; } } )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + using namespace nb; + int main() { + X1 = X2; + X3 = X4; + (void)na::X5; + } + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("X1", SymbolInfo::SymbolKind::Variable, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("X2", SymbolInfo::SymbolKind::Variable, HeaderName, 3, {{SymbolInfo::ContextType::Namespace, ""}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("X3", SymbolInfo::SymbolKind::Variable, HeaderName, 4, {{SymbolInfo::ContextType::Namespace, ""}, {SymbolInfo::ContextType::Namespace, ""}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("X4", SymbolInfo::SymbolKind::Variable, HeaderName, 5, {{SymbolInfo::ContextType::Namespace, "nb"}, {SymbolInfo::ContextType::Namespace, ""}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("X5", SymbolInfo::SymbolKind::Variable, HeaderName, 6, {{SymbolInfo::ContextType::Namespace, "na"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, DecayedTypeTest) { - static const char Code[] = "void DecayedFunc(int x[], int y[10]) {}"; - runFindAllSymbols(Code); + static const char Header[] = "void DecayedFunc(int x[], int y[10]) {}"; + static const char Main[] = R"(int main() { DecayedFunc(nullptr, nullptr); })"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo( "DecayedFunc", SymbolInfo::SymbolKind::Function, HeaderName, 1, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, CTypedefTest) { - static const char Code[] = R"( + static const char Header[] = R"( typedef unsigned size_t_; typedef struct { int x; } X; using XX = X; )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + size_t_ f; + template<typename T> struct vector{}; + vector<X> list; + void foo(const XX&){} + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("size_t_", SymbolInfo::SymbolKind::TypedefName, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("X", SymbolInfo::SymbolKind::TypedefName, HeaderName, 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("XX", SymbolInfo::SymbolKind::TypedefName, HeaderName, 4, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, EnumTest) { - static const char Code[] = R"( + static const char Header[] = R"( enum Glob_E { G1, G2 }; enum class Altitude { high='h', low='l'}; enum { A1, A2 }; @@ -359,42 +432,58 @@ }; enum DECL : int; )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + static auto flags = G1 | G2; + static auto alt = Altitude::high; + static auto nested = A::X1; + extern DECL whatever; + static auto flags2 = A1 | A2; + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("Glob_E", SymbolInfo::SymbolKind::EnumDecl, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); Symbol = SymbolInfo("G1", SymbolInfo::SymbolKind::EnumConstantDecl, HeaderName, 2, {{SymbolInfo::ContextType::EnumDecl, "Glob_E"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("G2", SymbolInfo::SymbolKind::EnumConstantDecl, HeaderName, 2, {{SymbolInfo::ContextType::EnumDecl, "Glob_E"}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("Altitude", SymbolInfo::SymbolKind::EnumDecl, HeaderName, 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("high", SymbolInfo::SymbolKind::EnumConstantDecl, HeaderName, 3, {{SymbolInfo::ContextType::EnumDecl, "Altitude"}}); EXPECT_FALSE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); Symbol = SymbolInfo("A1", SymbolInfo::SymbolKind::EnumConstantDecl, HeaderName, 4, {{SymbolInfo::ContextType::EnumDecl, ""}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("A2", SymbolInfo::SymbolKind::EnumConstantDecl, HeaderName, 4, {{SymbolInfo::ContextType::EnumDecl, ""}}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); Symbol = SymbolInfo("", SymbolInfo::SymbolKind::EnumDecl, HeaderName, 4, {}); EXPECT_FALSE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); Symbol = SymbolInfo("A_ENUM", SymbolInfo::SymbolKind::EnumDecl, HeaderName, 7, {{SymbolInfo::ContextType::Record, "A"}}); EXPECT_FALSE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); Symbol = SymbolInfo("X1", SymbolInfo::SymbolKind::EnumDecl, HeaderName, 7, {{SymbolInfo::ContextType::EnumDecl, "A_ENUM"}, @@ -407,63 +496,79 @@ } TEST_F(FindAllSymbolsTest, IWYUPrivatePragmaTest) { - static const char Code[] = R"( + static const char Header[] = R"( // IWYU pragma: private, include "bar.h" struct Bar { }; )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + Bar bar; + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("Bar", SymbolInfo::SymbolKind::Class, "bar.h", 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_TRUE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, MacroTest) { - static const char Code[] = R"( + static const char Header[] = R"( #define X #define Y 1 #define MAX(X, Y) ((X) > (Y) ? (X) : (Y)) )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + int main() { return MAX(0,Y); } + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("X", SymbolInfo::SymbolKind::Macro, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); // FIXME: Macro uses not yet implemented. Symbol = SymbolInfo("Y", SymbolInfo::SymbolKind::Macro, HeaderName, 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); Symbol = SymbolInfo("MAX", SymbolInfo::SymbolKind::Macro, HeaderName, 4, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, MacroTestWithIWYU) { - static const char Code[] = R"( + static const char Header[] = R"( // IWYU pragma: private, include "bar.h" #define X #define Y 1 #define MAX(X, Y) ((X) > (Y) ? (X) : (Y)) )"; - runFindAllSymbols(Code); + static const char Main[] = R"( + int main() { return MAX(0,Y); } + )"; + runFindAllSymbols(Header, Main); SymbolInfo Symbol = SymbolInfo("X", SymbolInfo::SymbolKind::Macro, "bar.h", 3, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); // FIXME: Macro uses yet implemented. Symbol = SymbolInfo("Y", SymbolInfo::SymbolKind::Macro, "bar.h", 4, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); Symbol = SymbolInfo("MAX", SymbolInfo::SymbolKind::Macro, "bar.h", 5, {}); EXPECT_TRUE(hasSymbol(Symbol)); + EXPECT_FALSE(hasUse(Symbol)); } TEST_F(FindAllSymbolsTest, NoFriendTest) { - static const char Code[] = R"( + static const char Header[] = R"( class WorstFriend { friend void Friend(); friend class BestFriend; }; )"; - runFindAllSymbols(Code); + runFindAllSymbols(Header, ""); SymbolInfo Symbol = SymbolInfo("WorstFriend", SymbolInfo::SymbolKind::Class, HeaderName, 2, {}); EXPECT_TRUE(hasSymbol(Symbol)); Index: test/include-fixer/merge.test =================================================================== --- test/include-fixer/merge.test +++ test/include-fixer/merge.test @@ -10,6 +10,7 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 2 ... --- Name: bar @@ -20,6 +21,7 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 0 ... --- Name: foo @@ -30,4 +32,5 @@ LineNumber: 1 Type: Class NumOccurrences: 2 +NumUses: 3 ... Index: test/include-fixer/Inputs/merge/b.yaml =================================================================== --- test/include-fixer/Inputs/merge/b.yaml +++ test/include-fixer/Inputs/merge/b.yaml @@ -7,6 +7,7 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 2 ... --- Name: bar @@ -17,4 +18,5 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 0 ... Index: test/include-fixer/Inputs/merge/a.yaml =================================================================== --- test/include-fixer/Inputs/merge/a.yaml +++ test/include-fixer/Inputs/merge/a.yaml @@ -7,6 +7,7 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 1 ... --- Name: bar @@ -17,4 +18,5 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 2 ... Index: test/include-fixer/Inputs/fake_yaml_db.yaml =================================================================== --- test/include-fixer/Inputs/fake_yaml_db.yaml +++ test/include-fixer/Inputs/fake_yaml_db.yaml @@ -9,6 +9,7 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 0 --- Name: bar Contexts: @@ -20,6 +21,7 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 0 --- Name: bar Contexts: @@ -31,6 +33,7 @@ LineNumber: 2 Type: Class NumOccurrences: 3 +NumUses: 0 --- Name: bar Contexts: @@ -42,13 +45,15 @@ LineNumber: 1 Type: Class NumOccurrences: 3 +NumUses: 0 --- Name: b Contexts: FilePath: var.h LineNumber: 1 Type: Variable NumOccurrences: 1 +NumUses: 0 --- Name: bar Contexts: @@ -58,3 +63,4 @@ LineNumber: 1 Type: Class NumOccurrences: 1 +NumUses: 0 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 @@ -78,22 +78,31 @@ } void reportSymbol(StringRef FileName, const SymbolInfo &Symbol) override { - Symbols[FileName].insert(Symbol); + Symbols[FileName].insert(Symbol).first->NumOccurrences = 1; + } + + void reportUse(StringRef FileName, const SymbolInfo &Symbol) override { + Symbols[FileName].insert(Symbol).first->NumUses = 1; } private: std::map<std::string, std::set<SymbolInfo>> Symbols; }; bool Merge(llvm::StringRef MergeDir, llvm::StringRef OutputFile) { std::error_code EC; - std::map<SymbolInfo, int> SymbolToNumOccurrences; + std::set<SymbolInfo> CountedSymbols; std::mutex SymbolMutex; auto AddSymbols = [&](ArrayRef<SymbolInfo> Symbols) { // Synchronize set accesses. std::unique_lock<std::mutex> LockGuard(SymbolMutex); - for (const auto &Symbol : Symbols) - ++SymbolToNumOccurrences[Symbol]; + for (const auto &Symbol : Symbols) { + auto I = CountedSymbols.insert(Symbol); + if (!I.second) { + I.first->NumOccurrences += Symbol.NumOccurrences; + I.first->NumUses += Symbol.NumUses; + } + } }; // Load all symbol files in MergeDir. @@ -124,14 +133,7 @@ << '\n'; return false; } - std::set<SymbolInfo> Result; - for (const auto &Entry : SymbolToNumOccurrences) { - const auto &Symbol = Entry.first; - Result.insert(SymbolInfo(Symbol.getName(), Symbol.getSymbolKind(), - Symbol.getFilePath(), Symbol.getLineNumber(), - Symbol.getContexts(), Entry.second)); - } - WriteSymbolInfosToStream(OS, Result); + WriteSymbolInfosToStream(OS, CountedSymbols); return true; } Index: include-fixer/find-all-symbols/SymbolReporter.h =================================================================== --- include-fixer/find-all-symbols/SymbolReporter.h +++ include-fixer/find-all-symbols/SymbolReporter.h @@ -22,6 +22,8 @@ virtual void reportSymbol(llvm::StringRef FileName, const SymbolInfo &Symbol) = 0; + virtual void reportUse(llvm::StringRef FileName, + const SymbolInfo &Symbol) = 0; }; } // namespace find_all_symbols Index: include-fixer/find-all-symbols/SymbolInfo.h =================================================================== --- include-fixer/find-all-symbols/SymbolInfo.h +++ include-fixer/find-all-symbols/SymbolInfo.h @@ -52,7 +52,7 @@ SymbolInfo(llvm::StringRef Name, SymbolKind Type, llvm::StringRef FilePath, int LineNumber, const std::vector<Context> &Contexts, - unsigned NumOccurrences = 0); + unsigned NumOccurrences = 0, unsigned NumUses = 0); void SetFilePath(llvm::StringRef Path) { FilePath = Path; } @@ -76,8 +76,15 @@ /// \brief Get a 1-based line number of the symbol's declaration. int getLineNumber() const { return LineNumber; } - /// \brief The number of times this symbol was found during an indexing run. - unsigned getNumOccurrences() const { return NumOccurrences; } + // Ranking signals are mutable to allow updating when the SymbolInfo is a map + // key. They do not affect ordering or equality. + /// \brief The number of times this symbol was found during an indexing + /// run. Populated by the reducer and used to rank results. + mutable unsigned NumOccurrences; + + /// \brief The number of times this symbol was used during an indexing run. + /// Populated by the reducer and used to rank results. + mutable unsigned NumUses; bool operator<(const SymbolInfo &Symbol) const; @@ -110,10 +117,6 @@ /// \brief The 1-based line number of of the symbol's declaration. int LineNumber; - - /// \brief The number of times this symbol was found during an indexing - /// run. Populated by the reducer and used to rank results. - unsigned NumOccurrences; }; /// \brief Write SymbolInfos to a stream (YAML format). Index: include-fixer/find-all-symbols/SymbolInfo.cpp =================================================================== --- include-fixer/find-all-symbols/SymbolInfo.cpp +++ include-fixer/find-all-symbols/SymbolInfo.cpp @@ -34,6 +34,7 @@ io.mapRequired("LineNumber", Symbol.LineNumber); io.mapRequired("Type", Symbol.Type); io.mapRequired("NumOccurrences", Symbol.NumOccurrences); + io.mapRequired("NumUses", Symbol.NumUses); } }; @@ -74,9 +75,10 @@ SymbolInfo::SymbolInfo(llvm::StringRef Name, SymbolKind Type, llvm::StringRef FilePath, int LineNumber, const std::vector<Context> &Contexts, - unsigned NumOccurrences) + unsigned NumOccurrences, unsigned NumUses) : Name(Name), Type(Type), FilePath(FilePath), Contexts(Contexts), - LineNumber(LineNumber), NumOccurrences(NumOccurrences) {} + NumOccurrences(NumOccurrences), NumUses(NumUses), LineNumber(LineNumber){ +} bool SymbolInfo::operator==(const SymbolInfo &Symbol) const { return std::tie(Name, Type, FilePath, LineNumber, Contexts) == Index: include-fixer/find-all-symbols/FindAllSymbols.cpp =================================================================== --- include-fixer/find-all-symbols/FindAllSymbols.cpp +++ include-fixer/find-all-symbols/FindAllSymbols.cpp @@ -154,64 +154,84 @@ // The float parameter of the function pointer has an empty name, and its // declaration context is an anonymous namespace; therefore, it won't be // filtered out by our matchers above. - MatchFinder->addMatcher(varDecl(CommonFilter, - anyOf(ExternCMatcher, CCMatcher), - unless(parmVarDecl())) - .bind("decl"), - this); + auto Vars = varDecl(CommonFilter, anyOf(ExternCMatcher, CCMatcher), + unless(parmVarDecl())); // Matchers for C-style record declarations in extern "C" {...}. - MatchFinder->addMatcher( - recordDecl(CommonFilter, ExternCMatcher, isDefinition()).bind("decl"), - this); - + auto CRecords = recordDecl(CommonFilter, ExternCMatcher, isDefinition()); // Matchers for C++ record declarations. - auto CxxRecordDecl = - cxxRecordDecl(CommonFilter, CCMatcher, isDefinition()); - MatchFinder->addMatcher(CxxRecordDecl.bind("decl"), this); + auto CXXRecords = cxxRecordDecl(CommonFilter, CCMatcher, isDefinition()); // Matchers for function declarations. // We want to exclude friend declaration, but the `DeclContext` of a friend // function declaration is not the class in which it is declared, so we need // to explicitly check if the parent is a `friendDecl`. - MatchFinder->addMatcher(functionDecl(CommonFilter, - unless(hasParent(friendDecl())), - anyOf(ExternCMatcher, CCMatcher)) - .bind("decl"), - this); + auto Functions = functionDecl(CommonFilter, unless(hasParent(friendDecl())), + anyOf(ExternCMatcher, CCMatcher)); // Matcher for typedef and type alias declarations. // - // typedef and type alias can come from C-style headers and C++ heaeders. - // For C-style header, `DeclContxet` can be either `TranslationUnitDecl` + // typedef and type alias can come from C-style headers and C++ headers. + // For C-style headers, `DeclContxet` can be either `TranslationUnitDecl` // or `LinkageSpecDecl`. - // For C++ header, `DeclContext ` can be one of `TranslationUnitDecl`, - // `NamespaceDecl`. + // For C++ headers, `DeclContext ` can be either `TranslationUnitDecl` + // or `NamespaceDecl`. // With the following context matcher, we can match `typedefNameDecl` from - // both C-style header and C++ header (except for those in classes). + // both C-style headers and C++ headers (except for those in classes). // "cc_matchers" are not included since template-related matchers are not // applicable on `TypedefNameDecl`. - MatchFinder->addMatcher( + auto Typedefs = typedefNameDecl(CommonFilter, anyOf(HasNSOrTUCtxMatcher, - hasDeclContext(linkageSpecDecl()))) - .bind("decl"), - this); + hasDeclContext(linkageSpecDecl()))); // Matchers for enum declarations. - MatchFinder->addMatcher(enumDecl(CommonFilter, isDefinition(), - anyOf(HasNSOrTUCtxMatcher, ExternCMatcher)) - .bind("decl"), - this); + auto Enums = enumDecl(CommonFilter, isDefinition(), + anyOf(HasNSOrTUCtxMatcher, ExternCMatcher)); // Matchers for enum constant declarations. // We only match the enum constants in non-scoped enum declarations which are // inside toplevel translation unit or a namespace. + auto EnumConstants = enumConstantDecl( + CommonFilter, unless(isInScopedEnum()), + anyOf(hasDeclContext(enumDecl(HasNSOrTUCtxMatcher)), ExternCMatcher)); + + // Most of the time we care about all matchable decls, or all types. + auto Types = namedDecl(anyOf(CRecords, CXXRecords, Enums, Typedefs)); + auto Decls = namedDecl(anyOf(CRecords, CXXRecords, Enums, Typedefs, Vars, + EnumConstants, Functions)); + + // We want eligible decls bound to "decl"... + MatchFinder->addMatcher(Decls.bind("decl"), this); + + // ... and all uses of them bound to "use". These have many cases: + // Uses of values/functions: these generate a declRefExpr. + MatchFinder->addMatcher( + declRefExpr(isExpansionInMainFile(), to(Decls.bind("use"))), this); + // Uses of function templates: MatchFinder->addMatcher( - enumConstantDecl( - CommonFilter, - unless(isInScopedEnum()), - anyOf(hasDeclContext(enumDecl(HasNSOrTUCtxMatcher)), ExternCMatcher)) - .bind("decl"), + declRefExpr(isExpansionInMainFile(), + to(functionDecl(hasParent( + functionTemplateDecl(has(Functions.bind("use"))))))), + this); + + // Uses of most types: just look at what the typeLoc refers to. + MatchFinder->addMatcher( + typeLoc(isExpansionInMainFile(), + loc(qualType(hasDeclaration(Types.bind("use"))))), + this); + // Uses of typedefs: these are transparent to hasDeclaration, so we need to + // handle them explicitly. + MatchFinder->addMatcher( + typeLoc(isExpansionInMainFile(), + loc(typedefType(hasDeclaration(Typedefs.bind("use"))))), + this); + // Uses of class templates: + // The typeLoc names the templateSpecializationType. Its declaration is the + // ClassTemplateDecl, which contains the CXXRecordDecl we want. + MatchFinder->addMatcher( + typeLoc(isExpansionInMainFile(), + loc(templateSpecializationType(hasDeclaration( + classTemplateDecl(has(CXXRecords.bind("use"))))))), this); } @@ -221,15 +241,23 @@ return; } - const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("decl"); - assert(ND && "Matched declaration must be a NamedDecl!"); - const SourceManager *SM = Result.SourceManager; + bool IsUse; + const NamedDecl *ND; + if ((ND = Result.Nodes.getNodeAs<NamedDecl>("use"))) { + IsUse = true; + } else if((ND = Result.Nodes.getNodeAs<NamedDecl>("decl"))) { + IsUse = false; + } else { + assert(false && "Must match a NamedDecl!"); + } - llvm::Optional<SymbolInfo> Symbol = - CreateSymbolInfo(ND, *SM, Collector); - if (Symbol) - Reporter->reportSymbol( - SM->getFileEntryForID(SM->getMainFileID())->getName(), *Symbol); + const SourceManager *SM = Result.SourceManager; + llvm::Optional<SymbolInfo> Symbol = CreateSymbolInfo(ND, *SM, Collector); + if (Symbol) { + StringRef Filename = SM->getFileEntryForID(SM->getMainFileID())->getName(); + IsUse ? Reporter->reportUse(Filename, *Symbol) + : Reporter->reportSymbol(Filename, *Symbol); + } } } // namespace find_all_symbols Index: include-fixer/SymbolIndexManager.cpp =================================================================== --- include-fixer/SymbolIndexManager.cpp +++ include-fixer/SymbolIndexManager.cpp @@ -52,7 +52,7 @@ // Calculate a score from the similarity of the header the symbol is in // with the current file and the popularity of the symbol. double NewScore = similarityScore(FileName, Symbol.getFilePath()) * - (1.0 + std::log2(1 + Symbol.getNumOccurrences())); + (1.0 + std::log2(1 + Symbol.NumOccurrences)); double &S = Score[Symbol.getFilePath()]; S = std::max(S, NewScore); } Index: include-fixer/IncludeFixer.cpp =================================================================== --- include-fixer/IncludeFixer.cpp +++ include-fixer/IncludeFixer.cpp @@ -335,7 +335,7 @@ SymbolCandidates.emplace_back(Symbol.getName(), Symbol.getSymbolKind(), MinimizedFilePath, Symbol.getLineNumber(), Symbol.getContexts(), - Symbol.getNumOccurrences()); + Symbol.NumOccurrences); } return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits