From 39e78bd3f641d47ce764d6a1d2f05ce95abb500d Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Mon, 13 Aug 2018 16:56:41 +0200 Subject: [PATCH] Clang: Don't index already indexed symbols We don't index system headers any more and introduce a first step to decrease double indexing. For that we introduces the SourcesManager which so far tells you only if a file was already indexed for a certain time stamp. Change-Id: Icde54465693ca84a622764c595635cac365c0111 Reviewed-by: Ivan Donchevskii --- .../collectmacrospreprocessorcallbacks.h | 17 +++++++++-- .../collectmacrossourcefilecallbacks.cpp | 3 +- .../source/collectmacrossourcefilecallbacks.h | 9 ++++-- .../source/indexdataconsumer.cpp | 10 ++++++- .../source/indexdataconsumer.h | 5 ++-- .../source/symbolscollector.cpp | 4 +-- .../source/symbolscollector.h | 2 ++ .../source/symbolsvisitorbase.h | 28 +++++++++++++++-- .../symbolscollector_unmodified_header.h | 11 +++++++ .../data/symbolscollector_unmodified.cpp | 5 ++++ .../data/symbolscollector_unmodified2.cpp | 5 ++++ tests/unit/unittest/symbolscollector-test.cpp | 30 +++++++++++++++++++ 12 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 tests/unit/unittest/data/include/symbolscollector_unmodified_header.h create mode 100644 tests/unit/unittest/data/symbolscollector_unmodified.cpp create mode 100644 tests/unit/unittest/data/symbolscollector_unmodified2.cpp diff --git a/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h b/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h index 60969be8f6e..529b4a18b1d 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h +++ b/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h @@ -30,6 +30,7 @@ #include "sourcedependency.h" #include "sourcelocationsutils.h" #include "sourcelocationentry.h" +#include "sourcesmanager.h" #include "symbolentry.h" #include "usedmacro.h" @@ -54,8 +55,9 @@ public: SourceDependencies &sourceDependencies, FilePathCachingInterface &filePathCache, const clang::SourceManager &sourceManager, - std::shared_ptr &&preprocessor) - : SymbolsVisitorBase(filePathCache, &sourceManager), + std::shared_ptr &&preprocessor, + SourcesManager &sourcesManager) + : SymbolsVisitorBase(filePathCache, &sourceManager, sourcesManager), m_preprocessor(std::move(preprocessor)), m_sourceDependencies(sourceDependencies), m_symbolEntries(symbolEntries), @@ -165,6 +167,7 @@ public: { filterOutHeaderGuards(); mergeUsedMacros(); + m_sourcesManager.updateModifiedTimeStamps(); } void filterOutHeaderGuards() @@ -205,6 +208,9 @@ public: void addUsedMacro(const clang::Token ¯oNameToken, const clang::MacroDefinition ¯oDefinition) { + if (isInSystemHeader(macroNameToken.getLocation())) + return; + clang::MacroInfo *macroInfo = macroDefinition.getMacroInfo(); UsedMacro usedMacro{macroNameToken.getIdentifierInfo()->getName(), filePathId(macroNameToken.getLocation())}; @@ -229,12 +235,17 @@ public: return nullptr; } + bool isInSystemHeader(clang::SourceLocation sourceLocation) const + { + return m_sourceManager->isInSystemHeader(sourceLocation); + } + void addMacroAsSymbol(const clang::Token ¯oNameToken, const clang::MacroInfo *macroInfo, SourceLocationKind symbolType) { clang::SourceLocation sourceLocation = macroNameToken.getLocation(); - if (macroInfo && sourceLocation.isFileID()) { + if (macroInfo && sourceLocation.isFileID() && !isInSystemHeader(sourceLocation)) { FilePathId fileId = filePathId(sourceLocation); if (fileId.isValid()) { auto macroName = macroNameToken.getIdentifierInfo()->getName(); diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp index 6ca01f2d913..d96fb05026c 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp @@ -42,7 +42,8 @@ bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance m_sourceDependencies, m_filePathCache, compilerInstance.getSourceManager(), - compilerInstance.getPreprocessorPtr()); + compilerInstance.getPreprocessorPtr(), + m_sourcesManager); compilerInstance.getPreprocessorPtr()->addPPCallbacks(std::move(callbacks)); diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h index be2ab518568..81c689186bd 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h @@ -37,15 +37,19 @@ namespace ClangBackEnd { +class SourcesManager; + class CollectMacrosSourceFileCallbacks : public clang::tooling::SourceFileCallbacks { public: CollectMacrosSourceFileCallbacks(SymbolEntries &symbolEntries, SourceLocationEntries &sourceLocationEntries, - FilePathCachingInterface &filePathCache) + FilePathCachingInterface &filePathCache, + SourcesManager &sourcesManager) : m_symbolEntries(symbolEntries), m_sourceLocationEntries(sourceLocationEntries), - m_filePathCache(filePathCache) + m_filePathCache(filePathCache), + m_sourcesManager(sourcesManager) { } @@ -91,6 +95,7 @@ private: SymbolEntries &m_symbolEntries; SourceLocationEntries &m_sourceLocationEntries; FilePathCachingInterface &m_filePathCache; + SourcesManager &m_sourcesManager; }; } // namespace ClangBackEnd diff --git a/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp b/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp index 008dd749a49..555ec6f021a 100644 --- a/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp +++ b/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp @@ -98,6 +98,12 @@ SymbolKindAndTags symbolKindAndTags(const clang::Decl *declaration) static IndexingDeclVisitor visitor; return visitor.Visit(declaration); } + +bool isContextIndependentDeclaration(const clang::Decl *declaration) +{ + return clang::dyn_cast(declaration) + || clang::dyn_cast(declaration); +} } bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration, @@ -107,12 +113,14 @@ bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration, unsigned offset, IndexDataConsumer::ASTNodeInfo astNodeInfo) { - const auto *namedDeclaration = clang::dyn_cast(declaration); if (namedDeclaration) { if (!namedDeclaration->getIdentifier()) return true; + if (alreadyParsed(fileId) && isContextIndependentDeclaration(declaration)) + return true; + SymbolIndex globalId = toSymbolIndex(declaration->getCanonicalDecl()); clang::SourceLocation sourceLocation = m_sourceManager->getLocForStartOfFile(fileId).getLocWithOffset(offset); diff --git a/src/tools/clangrefactoringbackend/source/indexdataconsumer.h b/src/tools/clangrefactoringbackend/source/indexdataconsumer.h index 5801f0956b4..45bc2c3be51 100644 --- a/src/tools/clangrefactoringbackend/source/indexdataconsumer.h +++ b/src/tools/clangrefactoringbackend/source/indexdataconsumer.h @@ -42,8 +42,9 @@ class IndexDataConsumer : public clang::index::IndexDataConsumer, public: IndexDataConsumer(SymbolEntries &symbolEntries, SourceLocationEntries &sourceLocationEntries, - FilePathCachingInterface &filePathCache) - : SymbolsVisitorBase(filePathCache, nullptr), + FilePathCachingInterface &filePathCache, + SourcesManager &sourcesManager) + : SymbolsVisitorBase(filePathCache, nullptr, sourcesManager), m_symbolEntries(symbolEntries), m_sourceLocationEntries(sourceLocationEntries) {} diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp index fae3d4e94a1..0e041ea7ba7 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp @@ -30,9 +30,9 @@ namespace ClangBackEnd { SymbolsCollector::SymbolsCollector(FilePathCachingInterface &filePathCache) - : m_indexDataConsumer(std::make_shared(m_symbolEntries, m_sourceLocationEntries, filePathCache)), + : m_indexDataConsumer(std::make_shared(m_symbolEntries, m_sourceLocationEntries, filePathCache, m_sourcesManager)), m_collectSymbolsAction(m_indexDataConsumer), - m_collectMacrosSourceFileCallbacks(m_symbolEntries, m_sourceLocationEntries, filePathCache), + m_collectMacrosSourceFileCallbacks(m_symbolEntries, m_sourceLocationEntries, filePathCache, m_sourcesManager), m_filePathCache(filePathCache) { } diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.h b/src/tools/clangrefactoringbackend/source/symbolscollector.h index 2f9891ee5aa..59d552f0551 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.h +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.h @@ -28,6 +28,7 @@ #include "clangtool.h" #include "collectmacrossourcefilecallbacks.h" #include "collectsymbolsaction.h" +#include "sourcesmanager.h" #include "symbolscollectorinterface.h" #include @@ -62,6 +63,7 @@ private: std::shared_ptr m_indexDataConsumer; CollectSymbolsAction m_collectSymbolsAction; CollectMacrosSourceFileCallbacks m_collectMacrosSourceFileCallbacks; + SourcesManager m_sourcesManager; FilePathCachingInterface &m_filePathCache; }; diff --git a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h index dc3d0ff2328..e96d1121cb2 100644 --- a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h +++ b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h @@ -26,6 +26,8 @@ #pragma once #include "sourcelocationsutils.h" +#include "sourcesmanager.h" +#include "symbolentry.h" #include @@ -43,9 +45,11 @@ class SymbolsVisitorBase { public: SymbolsVisitorBase(FilePathCachingInterface &filePathCache, - const clang::SourceManager *sourceManager) + const clang::SourceManager *sourceManager, + SourcesManager &sourcesManager) : m_filePathCache(filePathCache), - m_sourceManager(sourceManager) + m_sourceManager(sourceManager), + m_sourcesManager(sourcesManager) {} FilePathId filePathId(clang::SourceLocation sourceLocation) @@ -56,6 +60,19 @@ public: return filePathId(fileEntry); } + bool alreadyParsed(clang::FileID fileId) + { + const clang::FileEntry *fileEntry = m_sourceManager->getFileEntryForID(fileId); + + return m_sourcesManager.alreadyParsed(filePathId(fileEntry), + fileEntry->getModificationTime()); + } + + bool alreadyParsed(clang::SourceLocation sourceLocation) + { + return alreadyParsed(m_sourceManager->getFileID(sourceLocation)); + } + FilePathId filePathId(const clang::FileEntry *fileEntry) { if (fileEntry) { @@ -128,10 +145,17 @@ public: m_sourceManager = sourceManager; } + bool isInSystemHeader(clang::FileID fileId) const + { + return clang::SrcMgr::isSystem( + m_sourceManager->getSLocEntry(fileId).getFile().getFileCharacteristic()); + } + protected: std::vector m_filePathIndices; FilePathCachingInterface &m_filePathCache; const clang::SourceManager *m_sourceManager = nullptr; + SourcesManager &m_sourcesManager; }; } // namespace ClangBackend diff --git a/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h b/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h new file mode 100644 index 00000000000..307871268aa --- /dev/null +++ b/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h @@ -0,0 +1,11 @@ +#pragma once + +#define HEADER_DEFINE + +void HeaderFunction(); + +class Class +{ +public: + int Member = 20; +}; diff --git a/tests/unit/unittest/data/symbolscollector_unmodified.cpp b/tests/unit/unittest/data/symbolscollector_unmodified.cpp new file mode 100644 index 00000000000..ef1bee84c6f --- /dev/null +++ b/tests/unit/unittest/data/symbolscollector_unmodified.cpp @@ -0,0 +1,5 @@ +#include + +#define MAINFILE_DEFINE + +void MainFileFunction(); diff --git a/tests/unit/unittest/data/symbolscollector_unmodified2.cpp b/tests/unit/unittest/data/symbolscollector_unmodified2.cpp new file mode 100644 index 00000000000..ef1bee84c6f --- /dev/null +++ b/tests/unit/unittest/data/symbolscollector_unmodified2.cpp @@ -0,0 +1,5 @@ +#include + +#define MAINFILE_DEFINE + +void MainFileFunction(); diff --git a/tests/unit/unittest/symbolscollector-test.cpp b/tests/unit/unittest/symbolscollector-test.cpp index ec2535a9a17..a07b61914ce 100644 --- a/tests/unit/unittest/symbolscollector-test.cpp +++ b/tests/unit/unittest/symbolscollector-test.cpp @@ -707,4 +707,34 @@ TEST_F(SymbolsCollector, IsVariableSymbol) HasSymbolKind(SymbolKind::Variable)))); } + +TEST_F(SymbolsCollector, DontIndexUnmodifiedHeaderFiles) +{ + collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}}); + collector.collectSymbols(); + collector.clear(); + collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified2.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + AllOf( + Not(Contains(HasSymbolName("HeaderFunction"))), + Not(Contains(HasSymbolName("Class"))), + Not(Contains(HasSymbolName("Member"))))); +} + +TEST_F(SymbolsCollector, DontIndexSystemIncudes) +{ + collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified.cpp")}, {"cc", "-isystem", {TESTDATA_DIR, "/include"}}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + AllOf( + Not(Contains(HasSymbolName("HeaderFunction"))), + Not(Contains(HasSymbolName("Class"))), + Not(Contains(HasSymbolName("Member"))), + Not(Contains(HasSymbolName("HEADER_DEFINE"))))); +} }