From ca3e2f48ee232cb42760f5e7f27681540272baef Mon Sep 17 00:00:00 2001 From: mlangkabel Date: Tue, 24 Apr 2018 13:25:06 +0200 Subject: [PATCH] logic: removed C/C++ "unparsed" check from indexer Cxx AstVisitor re-visits previously or simultaneously indexed files because the "visible" content may have changed due to other preprocessor defines etc. --- src/lib/CMakeLists.txt | 2 - .../interprocess/InterprocessIndexer.cpp | 9 +-- .../InterprocessIndexingStatusManager.cpp | 81 ------------------- .../InterprocessIndexingStatusManager.h | 4 - src/lib/utility/file/FileRegister.cpp | 25 +----- src/lib/utility/file/FileRegister.h | 9 +-- .../utility/file/FileRegisterStateData.cpp | 71 ---------------- src/lib/utility/file/FileRegisterStateData.h | 36 --------- src/lib_cxx/data/indexer/IndexerCxx.h | 1 - .../data/parser/cxx/CommentHandler.cpp | 2 +- src/lib_cxx/data/parser/cxx/CxxAstVisitor.cpp | 42 ---------- .../cxx/CxxAstVisitorComponentIndexer.cpp | 14 +--- .../data/parser/cxx/PreprocessorCallbacks.cpp | 19 ++--- src/lib_java/data/indexer/IndexerJava.cpp | 1 - src/test/CxxIndexSampleProjectsTestSuite.h | 1 - src/test/JavaIndexSampleProjectsTestSuite.h | 1 - src/test/helper/TestFileRegister.cpp | 7 +- src/test/helper/TestFileRegister.h | 2 - 18 files changed, 13 insertions(+), 314 deletions(-) delete mode 100644 src/lib/utility/file/FileRegisterStateData.cpp delete mode 100644 src/lib/utility/file/FileRegisterStateData.h diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt index 84e7e39e..3995e93d 100644 --- a/src/lib/CMakeLists.txt +++ b/src/lib/CMakeLists.txt @@ -358,8 +358,6 @@ add_files( utility/file/FilePathFilter.h utility/file/FileRegister.cpp utility/file/FileRegister.h - utility/file/FileRegisterStateData.cpp - utility/file/FileRegisterStateData.h utility/file/FileSystem.cpp utility/file/FileSystem.h utility/file/FileTree.cpp diff --git a/src/lib/data/indexer/interprocess/InterprocessIndexer.cpp b/src/lib/data/indexer/interprocess/InterprocessIndexer.cpp index 75780d8e..01b3e992 100644 --- a/src/lib/data/indexer/interprocess/InterprocessIndexer.cpp +++ b/src/lib/data/indexer/interprocess/InterprocessIndexer.cpp @@ -1,7 +1,6 @@ #include "InterprocessIndexer.h" #include "utility/file/FileRegister.h" -#include "utility/file/FileRegisterStateData.h" #include "utility/logging/logging.h" #include "data/indexer/IndexerCommand.h" @@ -50,19 +49,13 @@ void InterprocessIndexer::work() LOG_INFO_STREAM(<< m_processId << " updating indexer status with currently indexed filepath"); m_interprocessIndexingStatusManager.startIndexingSourceFile(indexerCommand->getSourceFilePath()); - FileRegisterStateData data; - data.setIndexedFiles(m_interprocessIndexingStatusManager.getIndexedFiles()); - std::shared_ptr fileRegister = std::make_shared( - data, indexerCommand->getSourceFilePath(), indexerCommand->getIndexedPaths(), indexerCommand->getExcludeFilters() + indexerCommand->getSourceFilePath(), indexerCommand->getIndexedPaths(), indexerCommand->getExcludeFilters() ); LOG_INFO_STREAM(<< m_processId << " starting to index current file"); std::shared_ptr result = indexer->index(indexerCommand, fileRegister); - LOG_INFO_STREAM(<< m_processId << " finished indexing current file, updating indexer status"); - m_interprocessIndexingStatusManager.addIndexedFiles(fileRegister->getStateData().getIndexedFiles()); - LOG_INFO_STREAM(<< m_processId << " pushing index to shared memory"); m_interprocessIntermediateStorageManager.pushIntermediateStorage(result); diff --git a/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.cpp b/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.cpp index e73b4b30..36060364 100644 --- a/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.cpp +++ b/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.cpp @@ -8,7 +8,6 @@ const char* InterprocessIndexingStatusManager::s_sharedMemoryNamePrefix = "ists_ const char* InterprocessIndexingStatusManager::s_indexingFilesKeyName = "indexing_files"; const char* InterprocessIndexingStatusManager::s_currentFilesKeyName = "current_files"; const char* InterprocessIndexingStatusManager::s_crashedFilesKeyName = "crashed_files"; -const char* InterprocessIndexingStatusManager::s_indexedFilesKeyName = "indexed_files"; const char* InterprocessIndexingStatusManager::s_finishedProcessIdsKeyName = "finished_process_ids"; InterprocessIndexingStatusManager::InterprocessIndexingStatusManager(const std::string& instanceUuid, Id processId, bool isOwner) @@ -163,83 +162,3 @@ std::vector InterprocessIndexingStatusManager::getCrashedSourceFilePat return crashedFiles; } - -std::set InterprocessIndexingStatusManager::getIndexedFiles() -{ - std::set result; - - SharedMemory::ScopedAccess access(&m_sharedMemory); - - SharedMemory::Vector* files = - access.accessValueWithAllocator>(s_indexedFilesKeyName); - if (!files) - { - return result; - } - - for (auto& file : *files) - { - result.insert(FilePath(utility::decodeFromUtf8(file.c_str()))); - } - - return result; -} - -void InterprocessIndexingStatusManager::addIndexedFiles(std::set filePaths) -{ - const unsigned int overestimationMultiplier = 3; - - SharedMemory::ScopedAccess access(&m_sharedMemory); - - SharedMemory::Vector* indexedFiles = - access.accessValueWithAllocator>(s_indexedFilesKeyName); - if (!indexedFiles) - { - return; - } - - std::set oldFiles; - for (auto& indexedFile : *indexedFiles) - { - oldFiles.insert(indexedFile.c_str()); - } - - std::set newFiles; - for (const FilePath& filePath : filePaths) - { - if (oldFiles.find(utility::encodeToUtf8(filePath.wstr())) == oldFiles.end()) - { - newFiles.insert(utility::encodeToUtf8(filePath.wstr())); - } - } - - size_t estimatedSize = 262144; - for (auto& newFile : newFiles) - { - estimatedSize += sizeof(SharedMemory::String) + newFile.size(); - } - estimatedSize *= overestimationMultiplier; - - while (access.getFreeMemorySize() < estimatedSize) - { - LOG_INFO_STREAM( - << "grow memory - est: " << estimatedSize << " size: " << access.getMemorySize() - << " free: " << access.getFreeMemorySize() << " alloc: " << (access.getMemorySize())); - access.growMemory(access.getMemorySize()); - - LOG_INFO("growing memory succeeded"); - - indexedFiles = access.accessValueWithAllocator>(s_indexedFilesKeyName); - if (!indexedFiles) - { - return; - } - } - - for (const std::string& newFile: newFiles) - { - indexedFiles->push_back(SharedMemory::String(newFile.c_str(), access.getAllocator())); - } - - LOG_INFO(access.logString()); -} diff --git a/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.h b/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.h index 794cbf2f..1e236691 100644 --- a/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.h +++ b/src/lib/data/indexer/interprocess/InterprocessIndexingStatusManager.h @@ -21,16 +21,12 @@ public: std::vector getCurrentlyIndexedSourceFilePaths(); std::vector getCrashedSourceFilePaths(); - std::set getIndexedFiles(); - void addIndexedFiles(std::set filePaths); - private: static const char* s_sharedMemoryNamePrefix; static const char* s_indexingFilesKeyName; static const char* s_currentFilesKeyName; static const char* s_crashedFilesKeyName; - static const char* s_indexedFilesKeyName; static const char* s_finishedProcessIdsKeyName; }; diff --git a/src/lib/utility/file/FileRegister.cpp b/src/lib/utility/file/FileRegister.cpp index 103e7c0b..b697c280 100644 --- a/src/lib/utility/file/FileRegister.cpp +++ b/src/lib/utility/file/FileRegister.cpp @@ -4,13 +4,11 @@ #include "utility/file/FilePathFilter.h" FileRegister::FileRegister( - const FileRegisterStateData& stateData, const FilePath& currentPath, const std::set& indexedPaths, const std::set& excludeFilters ) - : m_stateData(stateData) - , m_currentPath(currentPath) + : m_currentPath(currentPath) , m_indexedPaths(indexedPaths) , m_excludeFilters(excludeFilters) , m_hasFilePathCache( @@ -64,31 +62,10 @@ FileRegister::FileRegister( { } - FileRegister::~FileRegister() { } -const FileRegisterStateData& FileRegister::getStateData() const -{ - return m_stateData; -} - -void FileRegister::markFileIndexing(const FilePath& filePath) -{ - m_stateData.markFileIndexing(filePath); -} - -void FileRegister::markIndexingFilesIndexed() -{ - m_stateData.markIndexingFilesIndexed(); -} - -bool FileRegister::fileIsIndexed(const FilePath& filePath) const -{ - return m_stateData.fileIsIndexed(filePath); -} - bool FileRegister::hasFilePath(const FilePath& filePath) const { return m_hasFilePathCache.getValue(filePath.wstr()); diff --git a/src/lib/utility/file/FileRegister.h b/src/lib/utility/file/FileRegister.h index a752486e..21bd0521 100644 --- a/src/lib/utility/file/FileRegister.h +++ b/src/lib/utility/file/FileRegister.h @@ -3,7 +3,7 @@ #include -#include "utility/file/FileRegisterStateData.h" +#include "utility/file/FilePath.h" #include "utility/UnorderedCache.h" class FilePathFilter; @@ -12,22 +12,15 @@ class FileRegister { public: FileRegister( - const FileRegisterStateData& stateData, const FilePath& currentPath, const std::set& indexedPaths, const std::set& excludeFilters ); virtual ~FileRegister(); - const FileRegisterStateData& getStateData() const; - - void markFileIndexing(const FilePath& filePath); - void markIndexingFilesIndexed(); - virtual bool fileIsIndexed(const FilePath& filePath) const; virtual bool hasFilePath(const FilePath& filePath) const; private: - FileRegisterStateData m_stateData; const FilePath& m_currentPath; const std::set m_indexedPaths; const std::set m_excludeFilters; diff --git a/src/lib/utility/file/FileRegisterStateData.cpp b/src/lib/utility/file/FileRegisterStateData.cpp deleted file mode 100644 index f2e64a65..00000000 --- a/src/lib/utility/file/FileRegisterStateData.cpp +++ /dev/null @@ -1,71 +0,0 @@ -#include "utility/file/FileRegisterStateData.h" - -#include "utility/file/FilePath.h" - -FileRegisterStateData::FileRegisterStateData() -{ -} - -FileRegisterStateData::FileRegisterStateData(const FileRegisterStateData& o) -{ - this->inject(o); -} - -void FileRegisterStateData::inject(const FileRegisterStateData& o) -{ - for (const auto& it: o.m_filePaths) - { - if (it.second == STATE_INDEXED) - { - m_filePaths[it.first] = STATE_INDEXED; - } - } -} - -void FileRegisterStateData::markFileIndexing(const FilePath& filePath) -{ - m_filePaths[filePath] = STATE_INDEXING; -} - -void FileRegisterStateData::markIndexingFilesIndexed() -{ - for (auto& it: m_filePaths) - { - if (it.second == STATE_INDEXING) - { - it.second = STATE_INDEXED; - } - } -} - -bool FileRegisterStateData::fileIsIndexed(const FilePath& filePath) const -{ - auto it = m_filePaths.find(filePath); - if (it != m_filePaths.end()) - { - return it->second == STATE_INDEXED; - } - - return false; -} - -void FileRegisterStateData::setIndexedFiles(const std::set& filePaths) -{ - for (auto& path : filePaths) - { - m_filePaths[path] = STATE_INDEXED; - } -} - -std::set FileRegisterStateData::getIndexedFiles() const -{ - std::set paths; - for (auto& it : m_filePaths) - { - if (it.second == STATE_INDEXED) - { - paths.insert(it.first); - } - } - return paths; -} diff --git a/src/lib/utility/file/FileRegisterStateData.h b/src/lib/utility/file/FileRegisterStateData.h deleted file mode 100644 index 4b739bf3..00000000 --- a/src/lib/utility/file/FileRegisterStateData.h +++ /dev/null @@ -1,36 +0,0 @@ -#ifndef FILE_REGISTER_STATE_DATA_H -#define FILE_REGISTER_STATE_DATA_H - -#include -#include -#include - -#include "utility/file/FilePath.h" - -class FileRegisterStateData -{ -public: - FileRegisterStateData(); - FileRegisterStateData(const FileRegisterStateData& o); - - void inject(const FileRegisterStateData& o); - - void markFileIndexing(const FilePath& filePath); - void markIndexingFilesIndexed(); - bool fileIsIndexed(const FilePath& filePath) const; - - void setIndexedFiles(const std::set& filePaths); - std::set getIndexedFiles() const; - -private: - enum IndexingState - { - STATE_NON_INDEXED, - STATE_INDEXING, - STATE_INDEXED - }; - - std::map m_filePaths; -}; - -#endif // FILE_REGISTER_STATE_DATA_H diff --git a/src/lib_cxx/data/indexer/IndexerCxx.h b/src/lib_cxx/data/indexer/IndexerCxx.h index b508e806..13371ea1 100644 --- a/src/lib_cxx/data/indexer/IndexerCxx.h +++ b/src/lib_cxx/data/indexer/IndexerCxx.h @@ -41,7 +41,6 @@ std::shared_ptr IndexerCxx: else { storage->setFilesWithErrorsIncomplete(); - fileRegister->markIndexingFilesIndexed(); } if (IndexerBase::interrupted()) diff --git a/src/lib_cxx/data/parser/cxx/CommentHandler.cpp b/src/lib_cxx/data/parser/cxx/CommentHandler.cpp index ad106ed0..99a3561a 100644 --- a/src/lib_cxx/data/parser/cxx/CommentHandler.cpp +++ b/src/lib_cxx/data/parser/cxx/CommentHandler.cpp @@ -36,7 +36,7 @@ bool CommentHandler::HandleComment(clang::Preprocessor& preprocessor, clang::Sou if (fileEntry != nullptr && fileEntry->isValid()) { FilePath filePath = m_canonicalFilePathCache->getCanonicalFilePath(fileEntry); - if (m_fileRegister->hasFilePath(filePath) && !m_fileRegister->fileIsIndexed(filePath)) + if (m_fileRegister->hasFilePath(filePath)) { m_client->recordComment(ParseLocation( filePath, diff --git a/src/lib_cxx/data/parser/cxx/CxxAstVisitor.cpp b/src/lib_cxx/data/parser/cxx/CxxAstVisitor.cpp index a397948a..c492e843 100644 --- a/src/lib_cxx/data/parser/cxx/CxxAstVisitor.cpp +++ b/src/lib_cxx/data/parser/cxx/CxxAstVisitor.cpp @@ -789,48 +789,6 @@ ParseLocation CxxAstVisitor::getParseLocation(const clang::SourceRange& sourceRa return parseLocation; } -bool CxxAstVisitor::isLocatedInUnparsedProjectFile(clang::SourceLocation loc) -{ - const clang::SourceManager& sourceManager = m_astContext->getSourceManager(); - - clang::FileID fileId; - if (loc.isValid()) - { - if (sourceManager.isWrittenInMainFile(loc)) - { - return true; - } - - fileId = sourceManager.getFileID(loc); - } - - if (fileId.isValid()) - { - auto it = m_inUnparsedProjectFileMap.find(fileId); - if (it != m_inUnparsedProjectFileMap.end()) - { - return it->second; - } - - bool ret = false; - const clang::FileEntry* fileEntry = sourceManager.getFileEntryForID(fileId); - if (fileEntry != nullptr && fileEntry->isValid()) - { - FilePath filePath = getCanonicalFilePathCache()->getCanonicalFilePath(fileEntry); - - if (m_fileRegister->hasFilePath(filePath)) - { - ret = !(m_fileRegister->fileIsIndexed(filePath)); - } - } - - m_inUnparsedProjectFileMap[fileId] = ret; - return ret; - } - - return false; -} - bool CxxAstVisitor::isLocatedInProjectFile(clang::SourceLocation loc) { const clang::SourceManager& sourceManager = m_astContext->getSourceManager(); diff --git a/src/lib_cxx/data/parser/cxx/CxxAstVisitorComponentIndexer.cpp b/src/lib_cxx/data/parser/cxx/CxxAstVisitorComponentIndexer.cpp index 0908a0f9..b3f59ec4 100644 --- a/src/lib_cxx/data/parser/cxx/CxxAstVisitorComponentIndexer.cpp +++ b/src/lib_cxx/data/parser/cxx/CxxAstVisitorComponentIndexer.cpp @@ -801,9 +801,7 @@ bool CxxAstVisitorComponentIndexer::shouldVisitDecl(const clang::Decl* decl) loc = decl->getLocation(); } - bool declIsImplicit = utility::isImplicit(decl); - if ((declIsImplicit && getAstVisitor()->isLocatedInProjectFile(loc)) || - (!declIsImplicit && getAstVisitor()->isLocatedInUnparsedProjectFile(loc))) + if (getAstVisitor()->isLocatedInProjectFile(loc)) { return true; } @@ -813,22 +811,16 @@ bool CxxAstVisitorComponentIndexer::shouldVisitDecl(const clang::Decl* decl) bool CxxAstVisitorComponentIndexer::shouldVisitReference(const clang::SourceLocation& referenceLocation, const clang::Decl* contextDecl) { - bool declIsImplicit = true; // default value is "true" to make sure that everything that should be visited gets visited. - if (contextDecl) - { - declIsImplicit = utility::isImplicit(contextDecl); - } - clang::SourceLocation loc = m_astContext->getSourceManager().getExpansionLoc(referenceLocation); if (loc.isInvalid()) { loc = referenceLocation; } - if ((declIsImplicit && getAstVisitor()->isLocatedInProjectFile(loc)) || - (!declIsImplicit && getAstVisitor()->isLocatedInUnparsedProjectFile(loc))) + if (getAstVisitor()->isLocatedInProjectFile(loc)) { return true; } + return false; } diff --git a/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp b/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp index a05e8c8c..fc3fc7e5 100644 --- a/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp +++ b/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp @@ -35,20 +35,11 @@ void PreprocessorCallbacks::FileChanged( if (fileEntry != nullptr && fileEntry->isValid()) { m_currentPath = m_canonicalFilePathCache->getCanonicalFilePath(fileEntry); - } - if (!m_currentPath.empty()) - { - bool hasFilePath = m_fileRegister->hasFilePath(m_currentPath); - - m_client->recordFile(FileSystem::getFileInfoForPath(m_currentPath), hasFilePath); // todo: fix for tests - - if (hasFilePath && !m_fileRegister->fileIsIndexed(m_currentPath)) + if (!m_currentPath.empty()) { - if (reason == EnterFile) - { - m_fileRegister->markFileIndexing(m_currentPath); - } + bool hasFilePath = m_fileRegister->hasFilePath(m_currentPath); + m_client->recordFile(FileSystem::getFileInfoForPath(m_currentPath), hasFilePath); // todo: fix for tests } } } @@ -75,7 +66,7 @@ void PreprocessorCallbacks::InclusionDirective( void PreprocessorCallbacks::MacroDefined(const clang::Token& macroNameToken, const clang::MacroDirective* macroDirective) { - if (!m_currentPath.empty() && m_fileRegister->hasFilePath(m_currentPath) && !m_fileRegister->fileIsIndexed(m_currentPath) /*TODO: remove this last check if indexed isn't important anymore*/) + if (!m_currentPath.empty() && m_fileRegister->hasFilePath(m_currentPath)) { // ignore builtin macros if (m_sourceManager.getSpellingLoc(macroNameToken.getLocation()).printToString(m_sourceManager)[0] == '<') @@ -128,7 +119,7 @@ void PreprocessorCallbacks::MacroExpands( void PreprocessorCallbacks::onMacroUsage(const clang::Token& macroNameToken) { - if (!m_currentPath.empty() && m_fileRegister->hasFilePath(m_currentPath) && !m_fileRegister->fileIsIndexed(m_currentPath) /*TODO: remove this last check if indexed isn't important anymore*/ && isLocatedInProjectFile(macroNameToken.getLocation())) + if (!m_currentPath.empty() && m_fileRegister->hasFilePath(m_currentPath) && isLocatedInProjectFile(macroNameToken.getLocation())) { const ParseLocation loc = getParseLocation(macroNameToken); diff --git a/src/lib_java/data/indexer/IndexerJava.cpp b/src/lib_java/data/indexer/IndexerJava.cpp index 424ef03e..95e6e92d 100644 --- a/src/lib_java/data/indexer/IndexerJava.cpp +++ b/src/lib_java/data/indexer/IndexerJava.cpp @@ -32,7 +32,6 @@ std::shared_ptr IndexerJava::doIndex( else { storage->setFilesWithErrorsIncomplete(); - fileRegister->markIndexingFilesIndexed(); } if (interrupted()) diff --git a/src/test/CxxIndexSampleProjectsTestSuite.h b/src/test/CxxIndexSampleProjectsTestSuite.h index 91eae25c..bdc6c879 100644 --- a/src/test/CxxIndexSampleProjectsTestSuite.h +++ b/src/test/CxxIndexSampleProjectsTestSuite.h @@ -127,7 +127,6 @@ private: const FilePath workingDirectory(L"."); std::shared_ptr fileRegister = std::make_shared( - FileRegisterStateData(), sourceFilePath, indexedPaths, excludedFilters diff --git a/src/test/JavaIndexSampleProjectsTestSuite.h b/src/test/JavaIndexSampleProjectsTestSuite.h index d660bea6..3ec8cae7 100644 --- a/src/test/JavaIndexSampleProjectsTestSuite.h +++ b/src/test/JavaIndexSampleProjectsTestSuite.h @@ -251,7 +251,6 @@ private: std::set excludeFilters = { }; std::shared_ptr fileRegister = std::make_shared( - FileRegisterStateData(), sourceFilePath, indexedPaths, excludeFilters diff --git a/src/test/helper/TestFileRegister.cpp b/src/test/helper/TestFileRegister.cpp index b4c949a4..709f5c4e 100644 --- a/src/test/helper/TestFileRegister.cpp +++ b/src/test/helper/TestFileRegister.cpp @@ -3,7 +3,7 @@ #include "utility/file/FilePathFilter.h" TestFileRegister::TestFileRegister() - : FileRegister(FileRegisterStateData(), FilePath(), std::set(), { FilePathFilter(L"") }) + : FileRegister(FilePath(), std::set(), { FilePathFilter(L"") }) { } @@ -11,11 +11,6 @@ TestFileRegister::~TestFileRegister() { } -bool TestFileRegister::fileIsIndexed(const FilePath& filePath) const -{ - return false; -} - bool TestFileRegister::hasFilePath(const FilePath& filePath) const { return true; diff --git a/src/test/helper/TestFileRegister.h b/src/test/helper/TestFileRegister.h index 371cdfbf..980ebf71 100644 --- a/src/test/helper/TestFileRegister.h +++ b/src/test/helper/TestFileRegister.h @@ -9,8 +9,6 @@ class TestFileRegister public: TestFileRegister(); virtual ~TestFileRegister(); - - virtual bool fileIsIndexed(const FilePath& filePath) const; virtual bool hasFilePath(const FilePath& filePath) const; };