Clang: Don't reparse if the macros haven't changed

We have to extend that to include paths too, which will be happen in a
follow up patch.

Change-Id: I7f8ac663ae8588e647fc6a6b5d689a629a28ef65
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/plugins/clangpchmanager/projectupdater.cpp

@@ -123,10 +123,14 @@ QStringList ProjectUpdater::compilerArguments(CppTools::ProjectPart *projectPart
 
 ClangBackEnd::CompilerMacros ProjectUpdater::createCompilerMacros(CppTools::ProjectPart *projectPart)
 {
-    return Utils::transform<ClangBackEnd::CompilerMacros>(projectPart->projectMacros,
+    auto macros =  Utils::transform<ClangBackEnd::CompilerMacros>(projectPart->projectMacros,
-                                            [] (const ProjectExplorer::Macro &macro) {
+                                                                  [] (const ProjectExplorer::Macro &macro) {
         return ClangBackEnd::CompilerMacro{macro.key, macro.value};
     });
+
+    std::sort(macros.begin(), macros.end());
+
+    return macros;
 }
 
 ClangBackEnd::V2::ProjectPartContainer ProjectUpdater::toProjectPartContainer(

src/tools/clangrefactoringbackend/source/clangrefactoringbackend-source.pri

@@ -18,6 +18,7 @@ HEADERS += \
     $$PWD/usedmacro.h \
     $$PWD/sourcedependency.h \
     $$PWD/filestatus.h \
+    $$PWD/projectpartartefactexception.h \
     $$PWD/projectpartartefact.h
 
 !isEmpty(LIBTOOLING_LIBS) {

src/tools/clangrefactoringbackend/source/storagesqlitestatementfactory.h

@@ -221,6 +221,9 @@ public:
        "SELECT compilerArguments, compilerMacros, projectPartId FROM projectParts WHERE projectPartId = (SELECT projectPartId FROM projectPartsSources WHERE sourceId = ?)",
        database
    };
+   ReadStatement getProjectPartCompilerArgumentsAndCompilerMacrosByProjectPartName{
+       "SELECT compilerArguments, compilerMacros, projectPartId FROM projectParts WHERE projectPartName = ?",
+       database
+   };
 };
-
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/symbolindexer.cpp

@@ -52,31 +52,32 @@ void SymbolIndexer::updateProjectPart(V2::ProjectPartContainer &&projectPart,
 {
     m_symbolsCollector.clear();
 
-    m_symbolsCollector.addFiles(projectPart.sourcePathIds(), projectPart.arguments());
+    if (compilerMacrosAreDifferent(projectPart)) {
+        m_symbolsCollector.addFiles(projectPart.sourcePathIds(), projectPart.arguments());
 
-    m_symbolsCollector.addUnsavedFiles(generatedFiles);
+        m_symbolsCollector.addUnsavedFiles(generatedFiles);
 
-    m_symbolsCollector.collectSymbols();
+        m_symbolsCollector.collectSymbols();
 
-    Sqlite::ImmediateTransaction transaction{m_transactionInterface};
+        Sqlite::ImmediateTransaction transaction{m_transactionInterface};
 
-    m_symbolStorage.addSymbolsAndSourceLocations(m_symbolsCollector.symbols(),
+        m_symbolStorage.addSymbolsAndSourceLocations(m_symbolsCollector.symbols(),
-                                                 m_symbolsCollector.sourceLocations());
+                                                     m_symbolsCollector.sourceLocations());
 
-    m_symbolStorage.insertOrUpdateProjectPart(projectPart.projectPartId(),
+        m_symbolStorage.insertOrUpdateProjectPart(projectPart.projectPartId(),
-                                              projectPart.arguments(),
+                                                  projectPart.arguments(),
-                                              projectPart.compilerMacros());
+                                                  projectPart.compilerMacros());
-    m_symbolStorage.updateProjectPartSources(projectPart.projectPartId(),
+        m_symbolStorage.updateProjectPartSources(projectPart.projectPartId(),
-                                             m_symbolsCollector.sourceFiles());
+                                                 m_symbolsCollector.sourceFiles());
 
-    m_symbolStorage.insertOrUpdateUsedMacros(m_symbolsCollector.usedMacros());
+        m_symbolStorage.insertOrUpdateUsedMacros(m_symbolsCollector.usedMacros());
 
-    m_symbolStorage.insertFileStatuses(m_symbolsCollector.fileStatuses());
+        m_symbolStorage.insertFileStatuses(m_symbolsCollector.fileStatuses());
 
-    m_symbolStorage.insertOrUpdateSourceDependencies(m_symbolsCollector.sourceDependencies());
+        m_symbolStorage.insertOrUpdateSourceDependencies(m_symbolsCollector.sourceDependencies());
-
-    transaction.commit();
 
+        transaction.commit();
+    }
 }
 
 void SymbolIndexer::pathsWithIdsChanged(const Utils::SmallStringVector &)
@@ -120,4 +121,15 @@ void SymbolIndexer::updateChangedPath(FilePathId filePathId)
     }
 }
 
+bool SymbolIndexer::compilerMacrosAreDifferent(const V2::ProjectPartContainer &projectPart) const
+{
+    const Utils::optional<ProjectPartArtefact> optionalArtefact = m_symbolStorage.fetchProjectPartArtefact(
+                projectPart.projectPartId());
+
+    if (optionalArtefact)
+        return projectPart.compilerMacros() != optionalArtefact.value().compilerMacros;
+
+    return true;
+}
+
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/symbolindexer.h

@@ -52,6 +52,8 @@ public:
     void pathsChanged(const FilePathIds &filePathIds) override;
     void updateChangedPath(FilePathId filePath);
 
+    bool compilerMacrosAreDifferent(const V2::ProjectPartContainer &projectPart) const;
+
 private:
     SymbolsCollectorInterface &m_symbolsCollector;
     SymbolStorageInterface &m_symbolStorage;

src/tools/clangrefactoringbackend/source/symbolstorage.h

@@ -91,6 +91,13 @@ public:
         return statement.template value<ProjectPartArtefact, 3>(sourceId.filePathId);
     }
 
+    Utils::optional<ProjectPartArtefact> fetchProjectPartArtefact(Utils::SmallStringView projectPartName) const override
+    {
+        ReadStatement &statement = m_statementFactory.getProjectPartCompilerArgumentsAndCompilerMacrosByProjectPartName;
+
+        return statement.template value<ProjectPartArtefact, 3>(projectPartName);
+    }
+
     void insertOrUpdateUsedMacros(const UsedMacros &usedMacros) override
     {
         WriteStatement &insertStatement = m_statementFactory.insertIntoNewUsedMacrosStatement;

src/tools/clangrefactoringbackend/source/symbolstorageinterface.h

@@ -60,6 +60,7 @@ public:
     virtual void insertFileStatuses(const FileStatuses &fileStatuses) = 0;
     virtual void insertOrUpdateSourceDependencies(const SourceDependencies &sourceDependencies) = 0;
     virtual Utils::optional<ProjectPartArtefact> fetchProjectPartArtefact(FilePathId sourceId) const = 0;
+    virtual Utils::optional<ProjectPartArtefact> fetchProjectPartArtefact(Utils::SmallStringView projectPartName) const = 0;
 };
 
 } // namespace ClangBackEnd

tests/unit/unittest/mocksqlitereadstatement.cpp

@@ -93,6 +93,13 @@ MockSqliteReadStatement::value<ClangBackEnd::ProjectPartArtefact, 3>(const int&
     return valueReturnProjectPartArtefact(sourceId);
 }
 
+template <>
+Utils::optional<ClangBackEnd::ProjectPartArtefact>
+MockSqliteReadStatement::value<ClangBackEnd::ProjectPartArtefact, 3>(const Utils::SmallStringView &projectPartName)
+{
+    return valueReturnProjectPartArtefact(projectPartName);
+}
+
 template <>
 Utils::optional<Utils::SmallString>
 MockSqliteReadStatement::value<Utils::SmallString>(const int &sourceId)

tests/unit/unittest/mocksqlitereadstatement.h

@@ -82,6 +82,9 @@ public:
     MOCK_METHOD1(valueReturnProjectPartArtefact,
                  Utils::optional<ClangBackEnd::ProjectPartArtefact>(int));
 
+    MOCK_METHOD1(valueReturnProjectPartArtefact,
+                 Utils::optional<ClangBackEnd::ProjectPartArtefact>(Utils::SmallStringView));
+
     template <typename ResultType,
               int ResultTypeCount = 1,
               typename... QueryType>
@@ -150,6 +153,10 @@ template <>
 Utils::optional<ClangBackEnd::ProjectPartArtefact>
 MockSqliteReadStatement::value<ClangBackEnd::ProjectPartArtefact, 3>(const int&);
 
+template <>
+Utils::optional<ClangBackEnd::ProjectPartArtefact>
+MockSqliteReadStatement::value<ClangBackEnd::ProjectPartArtefact, 3>(const Utils::SmallStringView&);
+
 template <>
 Utils::optional<Utils::SmallString>
 MockSqliteReadStatement::value<Utils::SmallString>(const int&);

tests/unit/unittest/mocksymbolstorage.h

@@ -55,4 +55,6 @@ public:
                 void (const ClangBackEnd::SourceDependencies &sourceDependencies));
    MOCK_CONST_METHOD1(fetchProjectPartArtefact,
                       Utils::optional<ClangBackEnd::ProjectPartArtefact> (ClangBackEnd::FilePathId sourceId));
+   MOCK_CONST_METHOD1(fetchProjectPartArtefact,
+                      Utils::optional<ClangBackEnd::ProjectPartArtefact> (Utils::SmallStringView projectPartName));
 };

tests/unit/unittest/storagesqlitestatementfactory-test.cpp

@@ -262,4 +262,10 @@ TEST_F(StorageSqliteStatementFactory, GetProjectPartCompilerArgumentsAndCompiler
                 Eq("SELECT compilerArguments, compilerMacros, projectPartId FROM projectParts WHERE projectPartId = (SELECT projectPartId FROM projectPartsSources WHERE sourceId = ?)"));
 }
 
+TEST_F(StorageSqliteStatementFactory, GetProjectPartCompilerArgumentsAndCompilerMacrosByProjectPartName)
+{
+    ASSERT_THAT(factory.getProjectPartCompilerArgumentsAndCompilerMacrosByProjectPartName.sqlStatement,
+                Eq("SELECT compilerArguments, compilerMacros, projectPartId FROM projectParts WHERE projectPartName = ?"));
+
+}
 }

tests/unit/unittest/symbolindexer-test.cpp

@@ -39,6 +39,7 @@ namespace {
 using Utils::PathString;
 using ClangBackEnd::CompilerMacro;
 using ClangBackEnd::FileStatuses;
+using ClangBackEnd::FilePathId;
 using ClangBackEnd::FilePathIds;
 using ClangBackEnd::FilePathView;
 using ClangBackEnd::V2::ProjectPartContainer;
@@ -70,7 +71,7 @@ protected:
         ON_CALL(mockCollector, usedMacros()).WillByDefault(ReturnRef(usedMacros));
         ON_CALL(mockCollector, fileStatuses()).WillByDefault(ReturnRef(fileStatus));
         ON_CALL(mockCollector, sourceDependencies()).WillByDefault(ReturnRef(sourceDependencies));
-        ON_CALL(mockStorage, fetchProjectPartArtefact(_)).WillByDefault(Return(artefact));
+        ON_CALL(mockStorage, fetchProjectPartArtefact(A<FilePathId>())).WillByDefault(Return(artefact));
     }
 
 protected:
@@ -218,11 +219,20 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsInsertOrUpdateSourceDependencies)
     indexer.updateProjectParts({projectPart1, projectPart2}, Utils::clone(unsaved));
 }
 
+TEST_F(SymbolIndexer, UpdateProjectPartsCallsFetchProjectPartArtefacts)
+{
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(TypedEq<Utils::SmallStringView>(projectPart1.projectPartId())));
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(TypedEq<Utils::SmallStringView>(projectPart2.projectPartId())));
+
+    indexer.updateProjectParts({projectPart1, projectPart2}, Utils::clone(unsaved));
+}
+
 TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrder)
 {
     InSequence s;
 
     EXPECT_CALL(mockCollector, clear());
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(TypedEq<Utils::SmallStringView>(projectPart1.projectPartId())));
     EXPECT_CALL(mockCollector, addFiles(_, _));
     EXPECT_CALL(mockCollector, addUnsavedFiles(unsaved));
     EXPECT_CALL(mockCollector, collectSymbols());
@@ -274,7 +284,8 @@ TEST_F(SymbolIndexer, UpdateChangedPathCallsInOrder)
 
 TEST_F(SymbolIndexer, HandleEmptyOptionalInUpdateChangedPath)
 {
-    ON_CALL(mockStorage, fetchProjectPartArtefact(_)).WillByDefault(Return(Utils::optional<ClangBackEnd::ProjectPartArtefact>()));
+    ON_CALL(mockStorage, fetchProjectPartArtefact(A<FilePathId>()))
+            .WillByDefault(Return(Utils::optional<ClangBackEnd::ProjectPartArtefact>()));
 
     EXPECT_CALL(mockCollector, clear());
     EXPECT_CALL(mockStorage, fetchProjectPartArtefact(sourceFileIds[0]));
@@ -291,4 +302,41 @@ TEST_F(SymbolIndexer, HandleEmptyOptionalInUpdateChangedPath)
     indexer.updateChangedPath(sourceFileIds[0]);
 }
 
+TEST_F(SymbolIndexer, CompilerMacrosAreNotDifferent)
+{
+    ON_CALL(mockStorage, fetchProjectPartArtefact(An<Utils::SmallStringView>())).WillByDefault(Return(artefact));
+
+    auto areDifferent = indexer.compilerMacrosAreDifferent(projectPart1);
+
+    ASSERT_FALSE(areDifferent);
+}
+
+TEST_F(SymbolIndexer, CompilerMacrosAreDifferent)
+{
+    ON_CALL(mockStorage, fetchProjectPartArtefact(An<Utils::SmallStringView>())).WillByDefault(Return(artefact));
+
+    auto areDifferent = indexer.compilerMacrosAreDifferent(projectPart2);
+
+    ASSERT_TRUE(areDifferent);
+}
+
+TEST_F(SymbolIndexer, DontReparseInUpdateProjectPartsIfDefinesAreTheSame)
+{
+    ON_CALL(mockStorage, fetchProjectPartArtefact(An<Utils::SmallStringView>())).WillByDefault(Return(artefact));
+
+    EXPECT_CALL(mockCollector, clear());
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(TypedEq<Utils::SmallStringView>(projectPart1.projectPartId())));
+    EXPECT_CALL(mockCollector, addFiles(_, _)).Times(0);
+    EXPECT_CALL(mockCollector, collectSymbols()).Times(0);
+    EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()).Times(0);
+    EXPECT_CALL(mockStorage, addSymbolsAndSourceLocations(_, _)).Times(0);
+    EXPECT_CALL(mockStorage, updateProjectPartSources(An<int>(), _)).Times(0);
+    EXPECT_CALL(mockStorage, insertOrUpdateUsedMacros(_)).Times(0);
+    EXPECT_CALL(mockStorage, insertFileStatuses(_)).Times(0);
+    EXPECT_CALL(mockStorage, insertOrUpdateSourceDependencies(_)).Times(0);
+    EXPECT_CALL(mockSqliteTransactionBackend, commit()).Times(0);
+
+    indexer.updateProjectPart(std::move(projectPart1), {});
+}
+
 }

tests/unit/unittest/symbolstorage-test.cpp

@@ -86,6 +86,7 @@ protected:
     MockSqliteWriteStatement &deleteOutdatedSourceDependenciesStatement = statementFactory.deleteOutdatedSourceDependenciesStatement;
     MockSqliteWriteStatement &deleteNewSourceDependenciesStatement = statementFactory.deleteNewSourceDependenciesStatement;
     MockSqliteReadStatement &getProjectPartCompilerArgumentsAndCompilerMacrosBySourceId = statementFactory.getProjectPartCompilerArgumentsAndCompilerMacrosBySourceId;
+    MockSqliteReadStatement &getProjectPartCompilerArgumentsAndCompilerMacrosByProjectPartName = statementFactory.getProjectPartCompilerArgumentsAndCompilerMacrosByProjectPartName;
     SymbolEntries symbolEntries{{1, {"functionUSR", "function"}},
                                 {2, {"function2USR", "function2"}}};
     SourceLocationEntries sourceLocations{{1, {1, 3}, {42, 23}, SymbolType::Declaration},
@@ -277,5 +278,23 @@ TEST_F(SymbolStorage, FetchProjectPartArtefactBySourceIdReturnArtefact)
 
     ASSERT_THAT(result, Eq(artefact));
 }
+
+TEST_F(SymbolStorage, FetchProjectPartArtefactByProjectNameCallsValueInStatement)
+{
+    EXPECT_CALL(getProjectPartCompilerArgumentsAndCompilerMacrosBySourceId, valueReturnProjectPartArtefact(1))
+            .WillRepeatedly(Return(artefact));
+
+    storage.fetchProjectPartArtefact({2, 1});
+}
+
+TEST_F(SymbolStorage, FetchProjectPartArtefactByProjectNameReturnArtefact)
+{
+    EXPECT_CALL(getProjectPartCompilerArgumentsAndCompilerMacrosBySourceId, valueReturnProjectPartArtefact(1))
+            .WillRepeatedly(Return(artefact));
+
+    auto result = storage.fetchProjectPartArtefact({2, 1});
+
+    ASSERT_THAT(result, Eq(artefact));
+}
 }