diff --git a/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h b/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h index 59e860b7292..d90a08ded97 100644 --- a/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h +++ b/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h @@ -75,7 +75,7 @@ private: clang::index::IndexingOptions options; options.SystemSymbolFilter = clang::index::IndexingOptions::SystemSymbolFilterKind::None; - options.IndexFunctionLocals = false; + options.IndexFunctionLocals = true; return options; } diff --git a/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp b/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp index 555ec6f021a..01f5190c080 100644 --- a/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp +++ b/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp @@ -99,11 +99,29 @@ SymbolKindAndTags symbolKindAndTags(const clang::Decl *declaration) return visitor.Visit(declaration); } -bool isContextIndependentDeclaration(const clang::Decl *declaration) +bool isDeclaration(clang::index::SymbolRoleSet symbolRoles) { - return clang::dyn_cast(declaration) - || clang::dyn_cast(declaration); + using namespace clang::index; + + return symbolRoles & (uint(SymbolRole::Declaration) | uint(SymbolRole::Definition)); } + +bool isReference(clang::index::SymbolRoleSet symbolRoles) +{ + using namespace clang::index; + + return symbolRoles & (uint(SymbolRole::Reference) | uint(SymbolRole::Call)); +} + +} + +bool IndexDataConsumer::skipSymbol(clang::FileID fileId, clang::index::SymbolRoleSet symbolRoles) +{ + bool alreadyParsed = isAlreadyParsed(fileId); + bool isParsedDeclaration = alreadyParsed && isDeclaration(symbolRoles); + bool isParsedReference = alreadyParsed && !dependentFilesAreModified() && isReference(symbolRoles); + + return isParsedDeclaration || isParsedReference; } bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration, @@ -118,7 +136,7 @@ bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration, if (!namedDeclaration->getIdentifier()) return true; - if (alreadyParsed(fileId) && isContextIndependentDeclaration(declaration)) + if (skipSymbol(fileId, symbolRoles)) return true; SymbolIndex globalId = toSymbolIndex(declaration->getCanonicalDecl()); diff --git a/src/tools/clangrefactoringbackend/source/indexdataconsumer.h b/src/tools/clangrefactoringbackend/source/indexdataconsumer.h index 45bc2c3be51..1024d0f5009 100644 --- a/src/tools/clangrefactoringbackend/source/indexdataconsumer.h +++ b/src/tools/clangrefactoringbackend/source/indexdataconsumer.h @@ -59,6 +59,9 @@ public: unsigned offset, ASTNodeInfo astNodeInfo) override; +private: + bool skipSymbol(clang::FileID fileId, clang::index::SymbolRoleSet symbolRoles); + private: SymbolEntries &m_symbolEntries; SourceLocationEntries &m_sourceLocationEntries; diff --git a/src/tools/clangrefactoringbackend/source/sourcesmanager.h b/src/tools/clangrefactoringbackend/source/sourcesmanager.h index a586707c638..91d2eb61438 100644 --- a/src/tools/clangrefactoringbackend/source/sourcesmanager.h +++ b/src/tools/clangrefactoringbackend/source/sourcesmanager.h @@ -59,7 +59,7 @@ public: if (!upToDate) addOrUpdateNewEntry(filePathId, modifiedTime); - m_dependendFilesModified = m_dependendFilesModified || !upToDate; + m_dependentFilesModified = m_dependentFilesModified || !upToDate; return upToDate ; } @@ -82,17 +82,17 @@ public: m_modifiedTimeStamps = std::move(mergedModifiedTimeStamps); m_newModifiedTimeStamps.clear(); - m_dependendFilesModified = false; + m_dependentFilesModified = false; } - bool dependendFilesModified() const + bool dependentFilesModified() const { - return m_dependendFilesModified; + return m_dependentFilesModified; } - bool alreadyParsedAllDependFiles(FilePathId filePathId, std::time_t modifiedTime) + bool alreadyParsedAllDependentFiles(FilePathId filePathId, std::time_t modifiedTime) { - return alreadyParsed(filePathId, modifiedTime) && !dependendFilesModified(); + return alreadyParsed(filePathId, modifiedTime) && !dependentFilesModified(); } private: @@ -114,7 +114,7 @@ private: private: std::vector m_modifiedTimeStamps; std::vector m_newModifiedTimeStamps; - bool m_dependendFilesModified = false; + bool m_dependentFilesModified = false; }; } diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp index 0e041ea7ba7..b727834523c 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp @@ -51,6 +51,7 @@ void SymbolsCollector::addUnsavedFiles(const V2::FileContainers &unsavedFiles) void SymbolsCollector::clear() { + m_indexDataConsumer->clear(); m_collectMacrosSourceFileCallbacks.clear(); m_symbolEntries.clear(); m_sourceLocationEntries.clear(); diff --git a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h index e96d1121cb2..1e4c6dcdc3d 100644 --- a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h +++ b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h @@ -60,7 +60,12 @@ public: return filePathId(fileEntry); } - bool alreadyParsed(clang::FileID fileId) + bool dependentFilesAreModified() + { + return m_sourcesManager.dependentFilesModified(); + } + + bool isAlreadyParsed(clang::FileID fileId) { const clang::FileEntry *fileEntry = m_sourceManager->getFileEntryForID(fileId); @@ -70,7 +75,7 @@ public: bool alreadyParsed(clang::SourceLocation sourceLocation) { - return alreadyParsed(m_sourceManager->getFileID(sourceLocation)); + return isAlreadyParsed(m_sourceManager->getFileID(sourceLocation)); } FilePathId filePathId(const clang::FileEntry *fileEntry) @@ -151,6 +156,11 @@ public: m_sourceManager->getSLocEntry(fileId).getFile().getFileCharacteristic()); } + void clear() + { + m_filePathIndices.clear(); + } + protected: std::vector m_filePathIndices; FilePathCachingInterface &m_filePathCache; diff --git a/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h b/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h index 307871268aa..c4bf4e6d99f 100644 --- a/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h +++ b/tests/unit/unittest/data/include/symbolscollector_unmodified_header.h @@ -3,9 +3,20 @@ #define HEADER_DEFINE void HeaderFunction(); +void HeaderFunctionReference(); +void HeaderFunctionReferenceInMainFile(); class Class { public: int Member = 20; + int MemberReference = 10; + + void InlineHeaderMemberFunction() + { + int FunctionLocalVariable = 90; + MemberReference = 30; + HeaderFunctionReference(); + } }; + diff --git a/tests/unit/unittest/data/include/symbolscollector_unmodified_header2.h b/tests/unit/unittest/data/include/symbolscollector_unmodified_header2.h new file mode 100644 index 00000000000..7905e974193 --- /dev/null +++ b/tests/unit/unittest/data/include/symbolscollector_unmodified_header2.h @@ -0,0 +1,3 @@ +#pragma once + +void TouchHeaderFunction(); diff --git a/tests/unit/unittest/data/symbolscollector_unmodified.cpp b/tests/unit/unittest/data/symbolscollector_unmodified.cpp index ef1bee84c6f..b7fe032e558 100644 --- a/tests/unit/unittest/data/symbolscollector_unmodified.cpp +++ b/tests/unit/unittest/data/symbolscollector_unmodified.cpp @@ -2,4 +2,7 @@ #define MAINFILE_DEFINE -void MainFileFunction(); +void MainFileFunction() +{ + HeaderFunctionReferenceInMainFile(); +} diff --git a/tests/unit/unittest/data/symbolscollector_unmodified2.cpp b/tests/unit/unittest/data/symbolscollector_unmodified2.cpp index ef1bee84c6f..6127587bd36 100644 --- a/tests/unit/unittest/data/symbolscollector_unmodified2.cpp +++ b/tests/unit/unittest/data/symbolscollector_unmodified2.cpp @@ -2,4 +2,8 @@ #define MAINFILE_DEFINE -void MainFileFunction(); +void MainFileFunction() +{ + HeaderFunctionReferenceInMainFile(); +} + diff --git a/tests/unit/unittest/data/symbolscollector_unmodified3.cpp b/tests/unit/unittest/data/symbolscollector_unmodified3.cpp new file mode 100644 index 00000000000..dbaabe7352d --- /dev/null +++ b/tests/unit/unittest/data/symbolscollector_unmodified3.cpp @@ -0,0 +1,9 @@ +#include +#include + +#define MAINFILE_DEFINE + +void MainFileFunction() +{ + HeaderFunctionReferenceInMainFile(); +} diff --git a/tests/unit/unittest/sourcesmanager-test.cpp b/tests/unit/unittest/sourcesmanager-test.cpp index f969b9bc09d..9c149dac6e4 100644 --- a/tests/unit/unittest/sourcesmanager-test.cpp +++ b/tests/unit/unittest/sourcesmanager-test.cpp @@ -138,14 +138,14 @@ TEST_F(SourcesManager, TimeIsUpdated) TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterInitialization) { - ASSERT_FALSE(sources.dependendFilesModified()); + ASSERT_FALSE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AnyDependFileIsModified) { sources.alreadyParsed({1, 1}, 56); - ASSERT_TRUE(sources.dependendFilesModified()); + ASSERT_TRUE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AnyDependFileIsModifiedAfterParsingTwoTimesSameTimeStamp) @@ -153,7 +153,7 @@ TEST_F(SourcesManager, AnyDependFileIsModifiedAfterParsingTwoTimesSameTimeStamp) sources.alreadyParsed({1, 1}, 56); sources.alreadyParsed({1, 1}, 56); - ASSERT_TRUE(sources.dependendFilesModified()); + ASSERT_TRUE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterUpdate) @@ -161,7 +161,7 @@ TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterUpdate) sources.alreadyParsed({1, 1}, 56); sources.updateModifiedTimeStamps(); - ASSERT_FALSE(sources.dependendFilesModified()); + ASSERT_FALSE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterNotAlreadyPared) @@ -171,7 +171,7 @@ TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterNotAlreadyPared) sources.alreadyParsed({1, 1}, 56); - ASSERT_FALSE(sources.dependendFilesModified()); + ASSERT_FALSE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterAlreadyPared) @@ -181,7 +181,7 @@ TEST_F(SourcesManager, AnyDependFileIsNotModifiedAfterAlreadyPared) sources.alreadyParsed({1, 1}, 57); - ASSERT_TRUE(sources.dependendFilesModified()); + ASSERT_TRUE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AnyDependFileIsModifiedAfterUpdateNewTimeStamp) @@ -193,41 +193,41 @@ TEST_F(SourcesManager, AnyDependFileIsModifiedAfterUpdateNewTimeStamp) sources.alreadyParsed({1, 2}, 56); - ASSERT_TRUE(sources.dependendFilesModified()); + ASSERT_TRUE(sources.dependentFilesModified()); } TEST_F(SourcesManager, AlreadyParsedWithDependencyAfterUpdateNewTimeStamp) { - sources.alreadyParsedAllDependFiles({1, 1}, 56); - sources.alreadyParsedAllDependFiles({1, 2}, 56); + sources.alreadyParsedAllDependentFiles({1, 1}, 56); + sources.alreadyParsedAllDependentFiles({1, 2}, 56); sources.updateModifiedTimeStamps(); - sources.alreadyParsedAllDependFiles({1, 1}, 57); + sources.alreadyParsedAllDependentFiles({1, 1}, 57); - bool alreadyParsed = sources.alreadyParsedAllDependFiles({1, 2}, 56); + bool alreadyParsed = sources.alreadyParsedAllDependentFiles({1, 2}, 56); ASSERT_FALSE(alreadyParsed); } TEST_F(SourcesManager, AlreadyParsedWithDependencyAfterUpdateNewSecondTimeStamp) { - sources.alreadyParsedAllDependFiles({1, 1}, 56); - sources.alreadyParsedAllDependFiles({1, 2}, 56); + sources.alreadyParsedAllDependentFiles({1, 1}, 56); + sources.alreadyParsedAllDependentFiles({1, 2}, 56); sources.updateModifiedTimeStamps(); - sources.alreadyParsedAllDependFiles({1, 1}, 56); + sources.alreadyParsedAllDependentFiles({1, 1}, 56); - bool alreadyParsed = sources.alreadyParsedAllDependFiles({1, 2}, 57); + bool alreadyParsed = sources.alreadyParsedAllDependentFiles({1, 2}, 57); ASSERT_FALSE(alreadyParsed); } TEST_F(SourcesManager, AlreadyParsedWithDependencyAfterUpdateSameTimeStamps) { - sources.alreadyParsedAllDependFiles({1, 1}, 56); - sources.alreadyParsedAllDependFiles({1, 2}, 56); + sources.alreadyParsedAllDependentFiles({1, 1}, 56); + sources.alreadyParsedAllDependentFiles({1, 2}, 56); sources.updateModifiedTimeStamps(); - sources.alreadyParsedAllDependFiles({1, 1}, 56); + sources.alreadyParsedAllDependentFiles({1, 1}, 56); - bool alreadyParsed = sources.alreadyParsedAllDependFiles({1, 2}, 56); + bool alreadyParsed = sources.alreadyParsedAllDependentFiles({1, 2}, 56); ASSERT_TRUE(alreadyParsed); } diff --git a/tests/unit/unittest/symbolscollector-test.cpp b/tests/unit/unittest/symbolscollector-test.cpp index a07b61914ce..7b8bafe8f08 100644 --- a/tests/unit/unittest/symbolscollector-test.cpp +++ b/tests/unit/unittest/symbolscollector-test.cpp @@ -36,6 +36,12 @@ #include #include +#ifdef _WIN32 +#include +#else +#include +#endif + using testing::PrintToString; using testing::AllOf; using testing::Contains; @@ -154,6 +160,15 @@ protected: return 0; } + void touchFile(const char *filePath) + { +#ifdef _WIN32 + QFile::resize(QString::fromUtf8(filePath), QFileInfo(QString::fromUtf8(filePath)).size()); +#else + utime(filePath, nullptr); +#endif + } + protected: Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory}; ClangBackEnd::RefactoringDatabaseInitializer initializer{database}; @@ -707,8 +722,24 @@ TEST_F(SymbolsCollector, IsVariableSymbol) HasSymbolKind(SymbolKind::Variable)))); } +TEST_F(SymbolsCollector, IndexUnmodifiedHeaderFilesAtFirstRun) +{ + collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}}); -TEST_F(SymbolsCollector, DontIndexUnmodifiedHeaderFiles) + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + AllOf( + Contains(HasSymbolName("MemberReference")), + Contains(HasSymbolName("HeaderFunction")), + Contains(HasSymbolName("HeaderFunctionReference")), + Contains(HasSymbolName("Class")), + Contains(HasSymbolName("Member")), + Contains(HasSymbolName("HEADER_DEFINE")), + Contains(HasSymbolName("FunctionLocalVariable")))); +} + +TEST_F(SymbolsCollector, DontIndexUnmodifiedHeaderFilesAtSecondRun) { collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}}); collector.collectSymbols(); @@ -719,6 +750,29 @@ TEST_F(SymbolsCollector, DontIndexUnmodifiedHeaderFiles) ASSERT_THAT(collector.symbols(), AllOf( + Contains(HasSymbolName("HeaderFunctionReferenceInMainFile")), + Not(Contains(HasSymbolName("MemberReference"))), + Not(Contains(HasSymbolName("HeaderFunctionReference"))), + Not(Contains(HasSymbolName("HeaderFunction"))), + Not(Contains(HasSymbolName("Class"))), + Not(Contains(HasSymbolName("Member"))))); +} + +TEST_F(SymbolsCollector, DontIndexUnmodifiedHeaderFilesAtTouchHeader) +{ + collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified3.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}}); + collector.collectSymbols(); + collector.clear(); + touchFile(TESTDATA_DIR "/include/symbolscollector_unmodified_header2.h"); + collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified3.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + AllOf( + Contains(HasSymbolName("HeaderFunctionReferenceInMainFile")), + Not(Contains(HasSymbolName("MemberReference"))), + Not(Contains(HasSymbolName("HeaderFunctionReference"))), Not(Contains(HasSymbolName("HeaderFunction"))), Not(Contains(HasSymbolName("Class"))), Not(Contains(HasSymbolName("Member"))))); @@ -732,9 +786,13 @@ TEST_F(SymbolsCollector, DontIndexSystemIncudes) ASSERT_THAT(collector.symbols(), AllOf( + Contains(HasSymbolName("MainFileFunction")), + Contains(HasSymbolName("HeaderFunctionReferenceInMainFile")), + Not(Contains(HasSymbolName("MemberReference"))), Not(Contains(HasSymbolName("HeaderFunction"))), Not(Contains(HasSymbolName("Class"))), Not(Contains(HasSymbolName("Member"))), - Not(Contains(HasSymbolName("HEADER_DEFINE"))))); + Not(Contains(HasSymbolName("HEADER_DEFINE"))), + Not(Contains(HasSymbolName("FunctionLocalVariable"))))); } } diff --git a/tests/unit/unittest/unittest.pro b/tests/unit/unittest/unittest.pro index c52896eedf0..9feaba5f816 100644 --- a/tests/unit/unittest/unittest.pro +++ b/tests/unit/unittest/unittest.pro @@ -248,4 +248,4 @@ HEADERS += \ testclangtool.h \ } -OTHER_FILES += $$files(data/*) +OTHER_FILES += $$files(data/*) $$files(data/include/*)