From 048224bef1e22fe2cdd4eb82853dfb900619f216 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 31 Jan 2018 13:53:57 +0100 Subject: [PATCH] Clang: Collect source dependencies It is quite easy because we track the include directives in the preprocessor callbacks. Change-Id: I2d7bd67b31f50c0d8d4a46c57e83dffa0c558dc7 Reviewed-by: Ivan Donchevskii --- .../collectmacrospreprocessorcallbacks.h | 30 +++++++++++++------ .../collectmacrossourcefilecallbacks.cpp | 1 + .../source/collectmacrossourcefilecallbacks.h | 7 +++++ .../source/symbolscollector.cpp | 5 ++++ .../source/symbolscollector.h | 1 + .../source/symbolscollectorinterface.h | 2 ++ .../source/symbolsvisitorbase.h | 22 +++++++++----- .../unittest/data/symbolscollector_header3.h | 3 ++ .../unittest/data/symbolscollector_main2.cpp | 5 ++++ .../unit/unittest/gtest-creator-printing.cpp | 10 +++++++ tests/unit/unittest/gtest-creator-printing.h | 2 ++ tests/unit/unittest/mocksymbolscollector.h | 3 ++ tests/unit/unittest/symbolscollector-test.cpp | 18 +++++++++++ 13 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 tests/unit/unittest/data/symbolscollector_header3.h create mode 100644 tests/unit/unittest/data/symbolscollector_main2.cpp diff --git a/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h b/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h index 1e9003990f8..639b107371d 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h +++ b/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h @@ -27,6 +27,7 @@ #include "fileinformation.h" #include "symbolsvisitorbase.h" +#include "sourcedependency.h" #include "sourcelocationsutils.h" #include "sourcelocationentry.h" #include "symbolentry.h" @@ -50,6 +51,7 @@ public: FilePathIds &sourceFiles, UsedMacros &usedMacros, FileInformations &fileInformations, + SourceDependencies &sourceDependencies, FilePathCachingInterface &filePathCache, const clang::SourceManager &sourceManager, std::shared_ptr &&preprocessor) @@ -59,7 +61,8 @@ public: m_sourceLocationEntries(sourceLocationEntries), m_sourceFiles(sourceFiles), m_usedMacros(usedMacros), - m_fileInformations(fileInformations) + m_fileInformations(fileInformations), + m_sourceDependencies(sourceDependencies) { } @@ -73,14 +76,15 @@ public: const clang::FileEntry *fileEntry = m_sourceManager.getFileEntryForID( m_sourceManager.getFileID(sourceLocation)); if (fileEntry) { - m_fileInformations.emplace_back(filePathId(sourceLocation), + m_fileInformations.emplace_back(filePathId(fileEntry), fileEntry->getSize(), fileEntry->getModificationTime()); + addSourceFile(fileEntry); } } } - void InclusionDirective(clang::SourceLocation /*hashLocation*/, + void InclusionDirective(clang::SourceLocation hashLocation, const clang::Token &/*includeToken*/, llvm::StringRef /*fileName*/, bool /*isAngled*/, @@ -91,7 +95,7 @@ public: const clang::Module * /*imported*/) override { if (!m_skipInclude && file) - addSourceFile(file); + addSourceDependency(file, hashLocation); m_skipInclude = false; } @@ -268,18 +272,26 @@ public: void addSourceFile(const clang::FileEntry *file) { - auto filePathId = m_filePathCache.filePathId( - FilePath::fromNativeFilePath(absolutePath(file->getName()))); + auto id = filePathId(file); - auto found = std::find(m_sourceFiles.begin(), m_sourceFiles.end(), filePathId); + auto found = std::find(m_sourceFiles.begin(), m_sourceFiles.end(), id); - if (found == m_sourceFiles.end() || *found != filePathId) - m_sourceFiles.insert(found, filePathId); + if (found == m_sourceFiles.end() || *found != id) + m_sourceFiles.insert(found, id); + } + + void addSourceDependency(const clang::FileEntry *file, clang::SourceLocation includeLocation) + { + auto includeFilePathId = filePathId(includeLocation); + auto includedFilePathId = filePathId(file); + + m_sourceDependencies.emplace_back(includeFilePathId, includedFilePathId); } private: UsedMacros m_maybeUsedMacros; std::shared_ptr m_preprocessor; + SourceDependencies &m_sourceDependencies; SymbolEntries &m_symbolEntries; SourceLocationEntries &m_sourceLocationEntries; FilePathIds &m_sourceFiles; diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp index ddd28c59da4..bfe045ef675 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp @@ -39,6 +39,7 @@ bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance m_sourceFiles, m_usedMacros, m_fileInformations, + m_sourceDependencies, m_filePathCache, compilerInstance.getSourceManager(), compilerInstance.getPreprocessorPtr()); diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h index b824f157d78..c977496f1d1 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h @@ -26,6 +26,7 @@ #pragma once #include "fileinformation.h" +#include "sourcedependency.h" #include "sourcelocationentry.h" #include "symbolentry.h" #include "usedmacro.h" @@ -77,10 +78,16 @@ public: return m_fileInformations; } + const SourceDependencies &sourceDependencies() const + { + return m_sourceDependencies; + } + private: FilePathIds m_sourceFiles; UsedMacros m_usedMacros; FileInformations m_fileInformations; + SourceDependencies m_sourceDependencies; SymbolEntries &m_symbolEntries; SourceLocationEntries &m_sourceLocationEntries; FilePathCachingInterface &m_filePathCache; diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp index 789bd3c743d..048da174d2c 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp @@ -87,4 +87,9 @@ const FileInformations &SymbolsCollector::fileInformations() const return m_collectMacrosSourceFileCallbacks.fileInformations(); } +const SourceDependencies &SymbolsCollector::sourceDependencies() const +{ + return m_collectMacrosSourceFileCallbacks.sourceDependencies(); +} + } // namespace ClangBackEnd diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.h b/src/tools/clangrefactoringbackend/source/symbolscollector.h index e4a996370d4..ef8099d8cc9 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.h +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.h @@ -53,6 +53,7 @@ public: const FilePathIds &sourceFiles() const override; const UsedMacros &usedMacros() const override; const FileInformations &fileInformations() const override; + const SourceDependencies &sourceDependencies() const override; private: ClangTool m_clangTool; diff --git a/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h b/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h index 00a78921cea..cad8d4128da 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h +++ b/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h @@ -27,6 +27,7 @@ #include "fileinformation.h" #include "symbolentry.h" +#include "sourcedependency.h" #include "sourcelocationentry.h" #include "usedmacro.h" @@ -56,6 +57,7 @@ public: virtual const FilePathIds &sourceFiles() const = 0; virtual const UsedMacros &usedMacros() const = 0; virtual const FileInformations &fileInformations() const = 0; + virtual const SourceDependencies &sourceDependencies() const = 0; }; } // namespace ClangBackEnd diff --git a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h index 9c5b701dfba..6dfc969fadf 100644 --- a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h +++ b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h @@ -50,22 +50,30 @@ public: FilePathId filePathId(clang::SourceLocation sourceLocation) { - uint clangFileId = m_sourceManager.getFileID(sourceLocation).getHashValue(); + clang::FileID clangFileId = m_sourceManager.getFileID(sourceLocation); + const clang::FileEntry *fileEntry = m_sourceManager.getFileEntryForID(clangFileId); - auto found = m_filePathIndices.find(clangFileId); + return filePathId(fileEntry); + } - if (found != m_filePathIndices.end()) - return found->second; + FilePathId filePathId(const clang::FileEntry *fileEntry) + { + if (fileEntry) { + uint fileHash = fileEntry->getUID(); - auto filePath = m_sourceManager.getFilename(sourceLocation); + auto found = m_filePathIndices.find(fileHash); - if (filePath.size() > 0) { + if (found != m_filePathIndices.end()) + return found->second; + + auto filePath = fileEntry->getName(); FilePathId filePathId = m_filePathCache.filePathId(FilePath::fromNativeFilePath(absolutePath(filePath))); - m_filePathIndices.emplace(clangFileId, filePathId); + m_filePathIndices.emplace(fileHash, filePathId); return filePathId; } + return {}; } diff --git a/tests/unit/unittest/data/symbolscollector_header3.h b/tests/unit/unittest/data/symbolscollector_header3.h new file mode 100644 index 00000000000..bacb59d24a7 --- /dev/null +++ b/tests/unit/unittest/data/symbolscollector_header3.h @@ -0,0 +1,3 @@ +#pragma once + +#include "symbolscollector_header2.h" diff --git a/tests/unit/unittest/data/symbolscollector_main2.cpp b/tests/unit/unittest/data/symbolscollector_main2.cpp new file mode 100644 index 00000000000..71ea20e2b65 --- /dev/null +++ b/tests/unit/unittest/data/symbolscollector_main2.cpp @@ -0,0 +1,5 @@ +#include "symbolscollector_header1.h" +#include "symbolscollector_header3.h" + +void function(); + diff --git a/tests/unit/unittest/gtest-creator-printing.cpp b/tests/unit/unittest/gtest-creator-printing.cpp index b1c61006fbc..66109d8b3f9 100644 --- a/tests/unit/unittest/gtest-creator-printing.cpp +++ b/tests/unit/unittest/gtest-creator-printing.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -838,6 +839,15 @@ std::ostream &operator<<(std::ostream &out, const FileInformation &fileInformati << ")"; } +std::ostream &operator<<(std::ostream &out, const SourceDependency &sourceDependency) +{ + return out << "(" + << sourceDependency.filePathId + << ", " + << sourceDependency.dependencyFilePathId + << ")"; +} + void PrintTo(const FilePath &filePath, ::std::ostream *os) { *os << filePath; diff --git a/tests/unit/unittest/gtest-creator-printing.h b/tests/unit/unittest/gtest-creator-printing.h index 3761f00443e..f2e00b9f9e5 100644 --- a/tests/unit/unittest/gtest-creator-printing.h +++ b/tests/unit/unittest/gtest-creator-printing.h @@ -134,6 +134,7 @@ class ToolTipInfo; class ProjectPartEntry; class UsedMacro; class FileInformation; +class SourceDependency; std::ostream &operator<<(std::ostream &out, const SourceLocationEntry &entry); std::ostream &operator<<(std::ostream &out, const IdPaths &idPaths); @@ -198,6 +199,7 @@ std::ostream &operator<<(std::ostream &out, const NativeFilePathView &nativeFile std::ostream &operator<<(std::ostream &out, const ProjectPartEntry &projectPartEntry); std::ostream &operator<<(std::ostream &out, const UsedMacro &usedMacro); std::ostream &operator<<(std::ostream &out, const FileInformation &fileInformation); +std::ostream &operator<<(std::ostream &out, const SourceDependency &sourceDependency); void PrintTo(const FilePath &filePath, ::std::ostream *os); void PrintTo(const FilePathView &filePathView, ::std::ostream *os); diff --git a/tests/unit/unittest/mocksymbolscollector.h b/tests/unit/unittest/mocksymbolscollector.h index cb5e28024b2..f0bf4a6555f 100644 --- a/tests/unit/unittest/mocksymbolscollector.h +++ b/tests/unit/unittest/mocksymbolscollector.h @@ -59,4 +59,7 @@ public: MOCK_CONST_METHOD0(fileInformations, const ClangBackEnd::FileInformations &()); + + MOCK_CONST_METHOD0(sourceDependencies, + const ClangBackEnd::SourceDependencies &()); }; diff --git a/tests/unit/unittest/symbolscollector-test.cpp b/tests/unit/unittest/symbolscollector-test.cpp index fb545d2411a..acf0da694ad 100644 --- a/tests/unit/unittest/symbolscollector-test.cpp +++ b/tests/unit/unittest/symbolscollector-test.cpp @@ -50,6 +50,7 @@ using ClangBackEnd::FilePath; using ClangBackEnd::FilePathId; using ClangBackEnd::FilePathCaching; using ClangBackEnd::V2::FileContainers; +using ClangBackEnd::SourceDependency; using ClangBackEnd::SourceLocationEntry; using ClangBackEnd::SymbolEntry; using ClangBackEnd::SymbolType; @@ -563,4 +564,21 @@ TEST_F(SymbolsCollector, CollectFileInformations) fileInformation(TESTDATA_DIR "/symbolscollector_header2.h"))); } +TEST_F(SymbolsCollector, CollectSourceDependencies) +{ + auto mainFileId = filePathId(TESTDATA_DIR "/symbolscollector_main2.cpp"); + auto header1FileId = filePathId(TESTDATA_DIR "/symbolscollector_header1.h"); + auto header2FileId = filePathId(TESTDATA_DIR "/symbolscollector_header2.h"); + auto header3FileId = filePathId(TESTDATA_DIR "/symbolscollector_header3.h"); + collector.addFiles({mainFileId}, {"cc", "-I" TESTDATA_DIR}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.sourceDependencies(), + UnorderedElementsAre(SourceDependency(mainFileId, header1FileId), + SourceDependency(mainFileId, header3FileId), + SourceDependency(header3FileId, header2FileId), + SourceDependency(header1FileId, header2FileId))); +} + }