diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp index d0d107e45e2..d96fb05026c 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp @@ -45,8 +45,6 @@ bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance compilerInstance.getPreprocessorPtr(), m_sourcesManager); - compilerInstance.getLangOpts().DelayedTemplateParsing = false; - compilerInstance.getPreprocessorPtr()->SetSuppressIncludeNotFoundError(true); compilerInstance.getPreprocessorPtr()->addPPCallbacks(std::move(callbacks)); return true; diff --git a/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h b/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h index d90a08ded97..0d79bf24a36 100644 --- a/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h +++ b/src/tools/clangrefactoringbackend/source/collectsymbolsaction.h @@ -34,6 +34,7 @@ #include +#include #include #include @@ -52,19 +53,22 @@ public: std::unique_ptr newASTConsumer(clang::CompilerInstance &compilerInstance, llvm::StringRef inFile); private: - class WrappedIndexAction : public clang::WrapperFrontendAction + class WrappedIndexAction final : public clang::WrapperFrontendAction { public: - WrappedIndexAction(std::shared_ptr m_indexDataConsumer, + WrappedIndexAction(std::shared_ptr indexDataConsumer, clang::index::IndexingOptions indexingOptions) : clang::WrapperFrontendAction( - clang::index::createIndexingAction(m_indexDataConsumer, indexingOptions, nullptr)) + clang::index::createIndexingAction(indexDataConsumer, indexingOptions, nullptr)) {} std::unique_ptr CreateASTConsumer(clang::CompilerInstance &compilerInstance, llvm::StringRef inFile) override { + compilerInstance.getPreprocessor().SetSuppressIncludeNotFoundError(true); + compilerInstance.getLangOpts().DelayedTemplateParsing = false; + return WrapperFrontendAction::CreateASTConsumer(compilerInstance, inFile); } }; diff --git a/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp b/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp index 2c190c6a101..35cc0df219e 100644 --- a/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp +++ b/src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp @@ -124,33 +124,23 @@ bool IndexDataConsumer::skipSymbol(clang::FileID fileId, clang::index::SymbolRol return isParsedDeclaration || isParsedReference; } -bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration, - clang::index::SymbolRoleSet symbolRoles, - llvm::ArrayRef symbolRelations, -#if LLVM_VERSION_MAJOR >= 7 - clang::SourceLocation sourceLocation, -#else - clang::FileID fileId, - unsigned offset, -#endif - IndexDataConsumer::ASTNodeInfo astNodeInfo) +bool IndexDataConsumer::handleDeclOccurence( + const clang::Decl *declaration, + clang::index::SymbolRoleSet symbolRoles, + llvm::ArrayRef /*symbolRelations*/, + clang::SourceLocation sourceLocation, + IndexDataConsumer::ASTNodeInfo /*astNodeInfo*/) { const auto *namedDeclaration = clang::dyn_cast(declaration); if (namedDeclaration) { if (!namedDeclaration->getIdentifier()) return true; -#if LLVM_VERSION_MAJOR >= 7 if (skipSymbol(m_sourceManager->getFileID(sourceLocation), symbolRoles)) -#else - if (skipSymbol(fileId, symbolRoles)) -#endif + return true; SymbolIndex globalId = toSymbolIndex(declaration->getCanonicalDecl()); -#if LLVM_VERSION_MAJOR < 7 - clang::SourceLocation sourceLocation = m_sourceManager->getLocForStartOfFile(fileId).getLocWithOffset(offset); -#endif auto found = m_symbolEntries.find(globalId); if (found == m_symbolEntries.end()) { diff --git a/src/tools/clangrefactoringbackend/source/indexdataconsumer.h b/src/tools/clangrefactoringbackend/source/indexdataconsumer.h index 8524455574c..6db595391c3 100644 --- a/src/tools/clangrefactoringbackend/source/indexdataconsumer.h +++ b/src/tools/clangrefactoringbackend/source/indexdataconsumer.h @@ -55,12 +55,7 @@ public: bool handleDeclOccurence(const clang::Decl *declaration, clang::index::SymbolRoleSet symbolRoles, llvm::ArrayRef symbolRelations, -#if LLVM_VERSION_MAJOR >= 7 clang::SourceLocation sourceLocation, -#else - clang::FileID fileId, - unsigned offset, -#endif ASTNodeInfo astNodeInfo) override; private: diff --git a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp index d3d0f4a505e..2b4e90cf5a4 100644 --- a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp @@ -117,27 +117,32 @@ void SymbolIndexer::updateProjectPart(ProjectPartContainer &&projectPart) std::vector symbolIndexerTask; symbolIndexerTask.reserve(projectPart.sourcePathIds.size()); for (FilePathId sourcePathId : projectPart.sourcePathIds) { - auto indexing = [projectPartId, arguments = commandLineBuilder.commandLine, sourcePathId, this]( - SymbolsCollectorInterface &symbolsCollector) { + auto indexing = [projectPartId, + arguments = commandLineBuilder.commandLine, + sourcePathId, + this](SymbolsCollectorInterface &symbolsCollector) { symbolsCollector.setFile(sourcePathId, arguments); - symbolsCollector.collectSymbols(); + bool success = symbolsCollector.collectSymbols(); - Sqlite::ImmediateTransaction transaction{m_transactionInterface}; + if (success) { + Sqlite::ImmediateTransaction transaction{m_transactionInterface}; - m_symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), - symbolsCollector.sourceLocations()); + m_symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), + symbolsCollector.sourceLocations()); - m_symbolStorage.updateProjectPartSources(projectPartId, symbolsCollector.sourceFiles()); + m_symbolStorage.updateProjectPartSources(projectPartId, + symbolsCollector.sourceFiles()); - m_buildDependencyStorage.insertOrUpdateUsedMacros(symbolsCollector.usedMacros()); + m_buildDependencyStorage.insertOrUpdateUsedMacros(symbolsCollector.usedMacros()); - m_buildDependencyStorage.insertFileStatuses(symbolsCollector.fileStatuses()); + m_buildDependencyStorage.insertFileStatuses(symbolsCollector.fileStatuses()); - m_buildDependencyStorage.insertOrUpdateSourceDependencies( - symbolsCollector.sourceDependencies()); + m_buildDependencyStorage.insertOrUpdateSourceDependencies( + symbolsCollector.sourceDependencies()); - transaction.commit(); + transaction.commit(); + } }; symbolIndexerTask.emplace_back(sourcePathId, projectPartId, std::move(indexing)); @@ -185,27 +190,31 @@ void SymbolIndexer::updateChangedPath(FilePathId filePathId, CommandLineBuilder builder{artefact, artefact.toolChainArguments, InputFileType::Source, {}, {}, pchPath}; - auto indexing = [projectPartId = artefact.projectPartId, arguments=builder.commandLine, filePathId, this]( - SymbolsCollectorInterface &symbolsCollector) { + auto indexing = [projectPartId = artefact.projectPartId, + arguments = builder.commandLine, + filePathId, + this](SymbolsCollectorInterface &symbolsCollector) { symbolsCollector.setFile(filePathId, arguments); - symbolsCollector.collectSymbols(); + bool success = symbolsCollector.collectSymbols(); - Sqlite::ImmediateTransaction transaction{m_transactionInterface}; + if (success) { + Sqlite::ImmediateTransaction transaction{m_transactionInterface}; - m_symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), - symbolsCollector.sourceLocations()); + m_symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), + symbolsCollector.sourceLocations()); - m_symbolStorage.updateProjectPartSources(projectPartId, symbolsCollector.sourceFiles()); + m_symbolStorage.updateProjectPartSources(projectPartId, symbolsCollector.sourceFiles()); - m_buildDependencyStorage.insertOrUpdateUsedMacros(symbolsCollector.usedMacros()); + m_buildDependencyStorage.insertOrUpdateUsedMacros(symbolsCollector.usedMacros()); - m_buildDependencyStorage.insertFileStatuses(symbolsCollector.fileStatuses()); + m_buildDependencyStorage.insertFileStatuses(symbolsCollector.fileStatuses()); - m_buildDependencyStorage.insertOrUpdateSourceDependencies( - symbolsCollector.sourceDependencies()); + m_buildDependencyStorage.insertOrUpdateSourceDependencies( + symbolsCollector.sourceDependencies()); - transaction.commit(); + transaction.commit(); + } }; symbolIndexerTask.emplace_back(filePathId, optionalArtefact->projectPartId, std::move(indexing)); diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp index afa01de2e9c..2669a3d8767 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.cpp @@ -121,12 +121,14 @@ newFrontendActionFactory(Factory *consumerFactory, new FrontendActionFactoryAdapter(consumerFactory, sourceFileCallbacks)); } -void SymbolsCollector::collectSymbols() +bool SymbolsCollector::collectSymbols() { auto tool = m_clangTool.createTool(); - tool.run(ClangBackEnd::newFrontendActionFactory(&m_collectSymbolsAction, - &m_collectMacrosSourceFileCallbacks).get()); + auto actionFactory = ClangBackEnd::newFrontendActionFactory(&m_collectSymbolsAction, + &m_collectMacrosSourceFileCallbacks); + + return tool.run(actionFactory.get()) != 1; } void SymbolsCollector::doInMainThreadAfterFinished() diff --git a/src/tools/clangrefactoringbackend/source/symbolscollector.h b/src/tools/clangrefactoringbackend/source/symbolscollector.h index beb610d1901..b37647e9694 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollector.h +++ b/src/tools/clangrefactoringbackend/source/symbolscollector.h @@ -52,7 +52,7 @@ public: void clear() override; - void collectSymbols() override; + bool collectSymbols() override; void doInMainThreadAfterFinished() override; diff --git a/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h b/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h index 0b7e00509df..94b31423718 100644 --- a/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h +++ b/src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h @@ -44,7 +44,7 @@ class SymbolsCollectorInterface : public ProcessorInterface { public: virtual void setFile(FilePathId filePathId, const Utils::SmallStringVector &arguments) = 0; - virtual void collectSymbols() = 0; + virtual bool collectSymbols() = 0; virtual const SymbolEntries &symbols() const = 0; virtual const SourceLocationEntries &sourceLocations() const = 0; diff --git a/tests/unit/unittest/symbolindexer-test.cpp b/tests/unit/unittest/symbolindexer-test.cpp index 0db32d658c7..0e4e2072cdb 100644 --- a/tests/unit/unittest/symbolindexer-test.cpp +++ b/tests/unit/unittest/symbolindexer-test.cpp @@ -114,6 +114,7 @@ protected: ON_CALL(mockCollector, sourceDependencies()).WillByDefault(ReturnRef(sourceDependencies)); ON_CALL(mockSymbolStorage, fetchProjectPartArtefact(A())).WillByDefault(Return(artefact)); ON_CALL(mockBuildDependenciesStorage, fetchLowestLastModifiedTime(A())).WillByDefault(Return(-1)); + ON_CALL(mockCollector, collectSymbols()).WillByDefault(Return(true)); mockCollector.setIsUsed(false); @@ -590,6 +591,61 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrderWithProjectPartArtifact) indexer.updateProjectParts({projectPart1}); } +TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrderButGetsAnErrorForCollectingSymbols) +{ + InSequence s; + + EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()); + EXPECT_CALL(mockSymbolStorage, + fetchProjectPartArtefact(TypedEq(projectPart1.projectPartId))) + .WillOnce(Return(nullArtefact)); + EXPECT_CALL(mockSymbolStorage, + insertOrUpdateProjectPart(Eq(projectPart1.projectPartId), + Eq(projectPart1.toolChainArguments), + Eq(projectPart1.compilerMacros), + Eq(projectPart1.systemIncludeSearchPaths), + Eq(projectPart1.projectIncludeSearchPaths), + Eq(Utils::Language::Cxx), + Eq(Utils::LanguageVersion::CXX14), + Eq(Utils::LanguageExtension::None))) + .WillOnce(Return(12)); + EXPECT_CALL(mockSymbolStorage, fetchPrecompiledHeader(Eq(12))); + EXPECT_CALL(mockBuildDependenciesStorage, fetchLowestLastModifiedTime(Eq(main1PathId))).Times(0); + EXPECT_CALL(mockSqliteTransactionBackend, commit()); + EXPECT_CALL(mockCollector, + setFile(main1PathId, + ElementsAre("clang++", + "-Wno-pragma-once-outside-header", + "-x", + "c++", + "-std=c++14", + "-nostdinc", + "-nostdinc++", + "-DBAR=1", + "-DFOO=1", + "-I", + "/project/includes", + "-I", + "/other/project/includes", + "-isystem", + TESTDATA_DIR, + "-isystem", + "/other/includes", + "-isystem", + "/includes"))); + EXPECT_CALL(mockCollector, collectSymbols()).WillOnce(Return(false)); + EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()).Times(0); + EXPECT_CALL(mockSymbolStorage, addSymbolsAndSourceLocations(symbolEntries, sourceLocations)).Times(0); + EXPECT_CALL(mockSymbolStorage, updateProjectPartSources(TypedEq(12), Eq(sourceFileIds))).Times(0); + EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateUsedMacros(Eq(usedMacros))).Times(0); + EXPECT_CALL(mockBuildDependenciesStorage, insertFileStatuses(Eq(fileStatus))).Times(0); + EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateSourceDependencies(Eq(sourceDependencies))) + .Times(0); + EXPECT_CALL(mockSqliteTransactionBackend, commit()).Times(0); + + indexer.updateProjectParts({projectPart1}); +} + TEST_F(SymbolIndexer, CallSetNotifier) { EXPECT_CALL(mockPathWatcher, setNotifier(_)); @@ -667,6 +723,50 @@ TEST_F(SymbolIndexer, HandleEmptyOptionalArtifactInUpdateChangedPath) indexer.pathsChanged({sourceFileIds[0]}); } +TEST_F(SymbolIndexer, UpdateChangedPathCallsInOrderButGetsAnErrorForCollectingSymbols) +{ + InSequence s; + + EXPECT_CALL(mockSqliteTransactionBackend, deferredBegin()); + EXPECT_CALL(mockSymbolStorage, fetchProjectPartArtefact(TypedEq(sourceFileIds[0]))) + .WillOnce(Return(artefact)); + EXPECT_CALL(mockSymbolStorage, fetchPrecompiledHeader(Eq(artefact.projectPartId))); + EXPECT_CALL(mockSqliteTransactionBackend, commit()); + EXPECT_CALL(mockCollector, + setFile(Eq(sourceFileIds[0]), + ElementsAre("clang++", + "-DFOO", + "-x", + "c++", + "-std=c++14", + "-nostdinc", + "-nostdinc++", + "-DBAR=1", + "-DFOO=1", + "-I", + "/project/includes", + "-I", + "/other/project/includes", + "-isystem", + TESTDATA_DIR, + "-isystem", + "/other/includes", + "-isystem", + "/includes"))); + EXPECT_CALL(mockCollector, collectSymbols()).WillOnce(Return(false)); + EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()).Times(0); + EXPECT_CALL(mockSymbolStorage, addSymbolsAndSourceLocations(symbolEntries, sourceLocations)).Times(0); + EXPECT_CALL(mockSymbolStorage, updateProjectPartSources(artefact.projectPartId, Eq(sourceFileIds))) + .Times(0); + EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateUsedMacros(Eq(usedMacros))).Times(0); + EXPECT_CALL(mockBuildDependenciesStorage, insertFileStatuses(Eq(fileStatus))).Times(0); + EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateSourceDependencies(Eq(sourceDependencies))) + .Times(0); + EXPECT_CALL(mockSqliteTransactionBackend, commit()).Times(0); + + indexer.pathsChanged({sourceFileIds[0]}); +} + TEST_F(SymbolIndexer, UpdateChangedPathIsUsingPrecompiledHeader) { ON_CALL(mockSymbolStorage, fetchProjectPartArtefact(TypedEq(sourceFileIds[0])))