ClangPchManager: Don't update system PCH if no system include file changed

If a project or user file is touched the system PCH should not be
regenerated.

Change-Id: Id5989735bab4441fc1eebfb5d31e01ba9714428b
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

9 files changed, 184 insertions, 45 deletions

diff --git a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp
index dd3bd3e3d6..5a634f8fe1 100644
--- a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp
+++ b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp
@@ -221,7 +221,8 @@ struct Data // because we have a cycle dependency
                                                                     database};
     ClangBackEnd::PchTaskGenerator pchTaskGenerator{buildDependencyProvider,
                                                     pchTaskMerger,
-                                                    dependencyCreationProgressCounter};
+                                                    dependencyCreationProgressCounter,
+                                                    pchTaskQueue};
     PchManagerServer clangPchManagerServer{includeWatcher,
                                            pchTaskGenerator,
                                            projectParts,
diff --git a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp
index 9fde14c708..ec0d89d2c6 100644
--- a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp
+++ b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp
@@ -143,46 +143,67 @@ struct FilterResults
     ProjectPartIds userIds;
 };
 
-std::pair<ProjectPartIds, ProjectPartIds> pchProjectPartIds(const std::vector<IdPaths> &idPaths)
+ProjectPartIds removeIds(const ProjectPartIds &subtrahend, const ProjectPartIds &minuend)
 {
-    ProjectPartIds changedProjectPartIds;
-    changedProjectPartIds.reserve(idPaths.size());
+    ProjectPartIds difference;
+    difference.reserve(subtrahend.size());
 
-    ProjectPartIds changedPchProjectPartIds;
-    changedPchProjectPartIds.reserve(idPaths.size());
+    std::set_difference(subtrahend.begin(),
+                        subtrahend.end(),
+                        minuend.begin(),
+                        minuend.end(),
+                        std::back_inserter(difference));
+
+    return difference;
+}
+
+FilterResults pchProjectPartIds(const std::vector<IdPaths> &idPaths)
+{
+    ProjectPartIds changedUserProjectPartIds;
+    changedUserProjectPartIds.reserve(idPaths.size());
+
+    ProjectPartIds changedSystemPchProjectPartIds;
+    changedSystemPchProjectPartIds.reserve(idPaths.size());
+
+    ProjectPartIds changedProjectPchProjectPartIds;
+    changedProjectPchProjectPartIds.reserve(idPaths.size());
 
     for (const IdPaths &idPath : idPaths) {
         switch (idPath.id.sourceType) {
         case SourceType::TopSystemInclude:
         case SourceType::SystemInclude:
+            changedSystemPchProjectPartIds.push_back(idPath.id.id);
+            break;
         case SourceType::TopProjectInclude:
         case SourceType::ProjectInclude:
-            changedPchProjectPartIds.push_back(idPath.id.id);
+            changedProjectPchProjectPartIds.push_back(idPath.id.id);
             break;
         case SourceType::UserInclude:
         case SourceType::Source:
-            changedProjectPartIds.push_back(idPath.id.id);
+            changedUserProjectPartIds.push_back(idPath.id.id);
             break;
         }
     }
 
-    changedPchProjectPartIds.erase(std::unique(changedPchProjectPartIds.begin(),
-                                               changedPchProjectPartIds.end()),
-                                   changedPchProjectPartIds.end());
-    changedPchProjectPartIds.erase(std::unique(changedPchProjectPartIds.begin(),
-                                               changedPchProjectPartIds.end()),
-                                   changedPchProjectPartIds.end());
-
-    ProjectPartIds changedUserProjectPartIds;
-    changedProjectPartIds.reserve(changedProjectPartIds.size());
-
-    std::set_difference(changedProjectPartIds.begin(),
-                        changedProjectPartIds.end(),
-                        changedPchProjectPartIds.begin(),
-                        changedPchProjectPartIds.end(),
-                        std::back_inserter(changedUserProjectPartIds));
-
-    return {changedPchProjectPartIds, changedUserProjectPartIds};
+    changedSystemPchProjectPartIds.erase(std::unique(changedSystemPchProjectPartIds.begin(),
+                                                     changedSystemPchProjectPartIds.end()),
+                                         changedSystemPchProjectPartIds.end());
+    changedProjectPchProjectPartIds.erase(std::unique(changedProjectPchProjectPartIds.begin(),
+                                                      changedProjectPchProjectPartIds.end()),
+                                          changedProjectPchProjectPartIds.end());
+    changedUserProjectPartIds.erase(std::unique(changedUserProjectPartIds.begin(),
+                                                changedUserProjectPartIds.end()),
+                                    changedUserProjectPartIds.end());
+
+    changedProjectPchProjectPartIds = removeIds(changedProjectPchProjectPartIds,
+                                                changedSystemPchProjectPartIds);
+
+    changedUserProjectPartIds = removeIds(changedUserProjectPartIds, changedSystemPchProjectPartIds);
+    changedUserProjectPartIds = removeIds(changedUserProjectPartIds, changedProjectPchProjectPartIds);
+
+    return {std::move(changedSystemPchProjectPartIds),
+            std::move(changedProjectPchProjectPartIds),
+            std::move(changedUserProjectPartIds)};
 }
 } // namespace
 
@@ -190,14 +211,11 @@ void PchManagerServer::pathsWithIdsChanged(const std::vector<IdPaths> &idPaths)
 {
     auto changedProjectPartIds = pchProjectPartIds(idPaths);
 
-    ArgumentsEntries entries = m_toolChainsArgumentsCache.arguments(changedProjectPartIds.first);
+    addCompleteProjectParts(changedProjectPartIds.systemIds);
 
-    for (ArgumentsEntry &entry : entries) {
-        m_pchTaskGenerator.addProjectParts(m_projectPartsManager.projects(entry.ids),
-                                           std::move(entry.arguments));
-    }
+    addNonSystemProjectParts(changedProjectPartIds.projectIds);
 
-    client()->precompiledHeadersUpdated(std::move(changedProjectPartIds.second));
+    client()->precompiledHeadersUpdated(std::move(changedProjectPartIds.userIds));
 }
 
 void PchManagerServer::pathsChanged(const FilePathIds &filePathIds)
@@ -215,4 +233,24 @@ void PchManagerServer::setDependencyCreationProgress(int progress, int total)
     client()->progress({ProgressType::DependencyCreation, progress, total});
 }
 
+void PchManagerServer::addCompleteProjectParts(const ProjectPartIds &projectPartIds)
+{
+    ArgumentsEntries entries = m_toolChainsArgumentsCache.arguments(projectPartIds);
+
+    for (ArgumentsEntry &entry : entries) {
+        m_pchTaskGenerator.addProjectParts(m_projectPartsManager.projects(entry.ids),
+                                           std::move(entry.arguments));
+    }
+}
+
+void PchManagerServer::addNonSystemProjectParts(const ProjectPartIds &projectPartIds)
+{
+    ArgumentsEntries entries = m_toolChainsArgumentsCache.arguments(projectPartIds);
+
+    for (ArgumentsEntry &entry : entries) {
+        m_pchTaskGenerator.addNonSystemProjectParts(m_projectPartsManager.projects(entry.ids),
+                                                    std::move(entry.arguments));
+    }
+}
+
 } // namespace ClangBackEnd
diff --git a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h
index 57b500edd9..c8e5be4de5 100644
--- a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h
+++ b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h
@@ -66,6 +66,10 @@ public:
     void setDependencyCreationProgress(int progress, int total);
 
 private:
+    void addCompleteProjectParts(const ProjectPartIds &projectPartIds);
+    void addNonSystemProjectParts(const ProjectPartIds &projectPartIds);
+
+private:
     ClangPathWatcherInterface &m_fileSystemWatcher;
     PchTaskGeneratorInterface &m_pchTaskGenerator;
     ProjectPartsManagerInterface &m_projectPartsManager;
diff --git a/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp b/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp
index 5067d9e265..7fe75d4779 100644
--- a/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp
+++ b/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp
@@ -26,6 +26,7 @@
 #include "pchtaskgenerator.h"
 
 #include "builddependenciesproviderinterface.h"
+#include "pchtaskqueueinterface.h"
 #include "pchtasksmergerinterface.h"
 #include "usedmacrofilter.h"
 
@@ -88,4 +89,38 @@ void PchTaskGenerator::removeProjectParts(const ProjectPartIds &projectsPartIds)
     m_pchTasksMergerInterface.removePchTasks(projectsPartIds);
 }
 
+void PchTaskGenerator::addNonSystemProjectParts(ProjectPartContainers &&projectParts,
+                                                Utils::SmallStringVector &&)
+{
+    PchTasks pchTasks;
+    pchTasks.reserve(projectParts.size());
+
+    m_progressCounter.addTotal(static_cast<int>(projectParts.size()));
+
+    for (auto &projectPart : projectParts) {
+        BuildDependency buildDependency = m_buildDependenciesProvider.create(projectPart);
+        UsedMacroFilter filter{buildDependency.sources,
+                               buildDependency.usedMacros,
+                               projectPart.compilerMacros};
+
+        pchTasks.emplace_back(projectPart.projectPartId,
+                              std::move(filter.topProjectIncludes),
+                              std::move(filter.systemIncludes),
+                              std::move(filter.projectIncludes),
+                              std::move(filter.userIncludes),
+                              std::move(filter.sources),
+                              std::move(filter.projectCompilerMacros),
+                              std::move(filter.projectUsedMacros),
+                              projectPart.toolChainArguments,
+                              projectPart.systemIncludeSearchPaths,
+                              projectPart.projectIncludeSearchPaths,
+                              projectPart.language,
+                              projectPart.languageVersion,
+                              projectPart.languageExtension);
+        m_progressCounter.addProgress(1);
+    }
+
+    m_pchTaskQueue.addProjectPchTasks(std::move(pchTasks));
+}
+
 } // namespace ClangBackEnd
diff --git a/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.h b/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.h
index dc7d201cde..be3d91c892 100644
--- a/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.h
+++ b/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.h
@@ -35,6 +35,7 @@ namespace ClangBackEnd {
 class PchTasksMergerInterface;
 
 class BuildDependenciesProviderInterface;
+class PchTaskQueueInterface;
 class ProgressCounter;
 
 class PchTaskGenerator : public PchTaskGeneratorInterface
@@ -42,21 +43,26 @@ class PchTaskGenerator : public PchTaskGeneratorInterface
 public:
     PchTaskGenerator(BuildDependenciesProviderInterface &buildDependenciesProvider,
                      PchTasksMergerInterface &pchTasksMergerInterface,
-                     ProgressCounter &progressCounter)
+                     ProgressCounter &progressCounter,
+                     PchTaskQueueInterface &pchTaskQueue)
         : m_buildDependenciesProvider(buildDependenciesProvider)
         , m_pchTasksMergerInterface(pchTasksMergerInterface)
         , m_progressCounter(progressCounter)
+        , m_pchTaskQueue(pchTaskQueue)
 
     {}
 
     void addProjectParts(ProjectPartContainers &&projectParts,
                          Utils::SmallStringVector &&toolChainArguments) override;
     void removeProjectParts(const ProjectPartIds &projectsPartIds) override;
+    void addNonSystemProjectParts(ProjectPartContainers &&projectParts,
+                                  Utils::SmallStringVector &&toolChainArguments) override;
 
 private:
     BuildDependenciesProviderInterface &m_buildDependenciesProvider;
     PchTasksMergerInterface &m_pchTasksMergerInterface;
     ProgressCounter &m_progressCounter;
+    PchTaskQueueInterface &m_pchTaskQueue;
 };
 
 } // namespace ClangBackEnd
diff --git a/src/tools/clangpchmanagerbackend/source/pchtaskgeneratorinterface.h b/src/tools/clangpchmanagerbackend/source/pchtaskgeneratorinterface.h
index abb25d2ef1..302789ae9a 100644
--- a/src/tools/clangpchmanagerbackend/source/pchtaskgeneratorinterface.h
+++ b/src/tools/clangpchmanagerbackend/source/pchtaskgeneratorinterface.h
@@ -35,6 +35,9 @@ public:
     virtual void addProjectParts(ProjectPartContainers &&projectParts,
                                  Utils::SmallStringVector &&toolChainArguments)
         = 0;
+    virtual void addNonSystemProjectParts(ProjectPartContainers &&projectParts,
+                                          Utils::SmallStringVector &&toolChainArguments)
+        = 0;
     virtual void removeProjectParts(const ProjectPartIds &projectsPartIds) = 0;
 
 protected:
diff --git a/tests/unit/unittest/mockpchtaskgenerator.h b/tests/unit/unittest/mockpchtaskgenerator.h
index d2573f0c35..492e320a33 100644
--- a/tests/unit/unittest/mockpchtaskgenerator.h
+++ b/tests/unit/unittest/mockpchtaskgenerator.h
@@ -35,6 +35,9 @@ public:
     MOCK_METHOD2(addProjectParts,
                  void(const ClangBackEnd::ProjectPartContainers &projectParts,
                       const Utils::SmallStringVector &toolChainArguments));
+    MOCK_METHOD2(addNonSystemProjectParts,
+                 void(const ClangBackEnd::ProjectPartContainers &projectParts,
+                      const Utils::SmallStringVector &toolChainArguments));
     MOCK_METHOD1(removeProjectParts, void(const ClangBackEnd::ProjectPartIds &projectsPartIds));
 
     void addProjectParts(ClangBackEnd::ProjectPartContainers &&projectParts,
@@ -42,4 +45,10 @@ public:
     {
         addProjectParts(projectParts, toolChainArguments);
     }
+
+    void addNonSystemProjectParts(ClangBackEnd::ProjectPartContainers &&projectParts,
+                                  Utils::SmallStringVector &&toolChainArguments) override
+    {
+        addNonSystemProjectParts(projectParts, toolChainArguments);
+    }
 };
diff --git a/tests/unit/unittest/pchmanagerserver-test.cpp b/tests/unit/unittest/pchmanagerserver-test.cpp
index 9ece91234f..4657051fc5 100644
--- a/tests/unit/unittest/pchmanagerserver-test.cpp
+++ b/tests/unit/unittest/pchmanagerserver-test.cpp
@@ -227,26 +227,36 @@ TEST_F(PchManagerServer, SetPathWatcherNotifier)
 
 TEST_F(PchManagerServer, UpdateProjectPartQueueByPathIds)
 {
+    InSequence s;
     server.updateProjectParts(ClangBackEnd::UpdateProjectPartsMessage{
         {projectPart1, projectPart2, projectPart3, projectPart4, projectPart5, projectPart6},
         {"toolChainArgument"}});
 
     EXPECT_CALL(mockProjectPartsManager,
-                projects(ElementsAre(projectPart1.projectPartId,
-                                     projectPart2.projectPartId,
-                                     projectPart4.projectPartId,
-                                     projectPart5.projectPartId)))
-        .WillOnce(Return(std::vector<ClangBackEnd::ProjectPartContainer>{
-            {projectPart1, projectPart2, projectPart4, projectPart5}}));
+                projects(ElementsAre(projectPart1.projectPartId, projectPart2.projectPartId)))
+        .WillOnce(
+            Return(std::vector<ClangBackEnd::ProjectPartContainer>{{projectPart1, projectPart2}}));
     EXPECT_CALL(mockPchTaskGenerator,
-                addProjectParts(ElementsAre(projectPart1, projectPart2, projectPart4, projectPart5),
+                addProjectParts(ElementsAre(projectPart1, projectPart2),
                                 ElementsAre("toolChainArgument")));
-
-    server.pathsWithIdsChanged({{{projectPartId1, ClangBackEnd::SourceType::TopProjectInclude}, {}},
-                                {{projectPartId2, ClangBackEnd::SourceType::TopSystemInclude}, {}},
+    EXPECT_CALL(mockProjectPartsManager,
+                projects(ElementsAre(projectPart4.projectPartId, projectPart5.projectPartId)))
+        .WillOnce(
+            Return(std::vector<ClangBackEnd::ProjectPartContainer>{{projectPart4, projectPart5}}));
+    EXPECT_CALL(mockPchTaskGenerator,
+                addNonSystemProjectParts(ElementsAre(projectPart4, projectPart5),
+                                         ElementsAre("toolChainArgument")));
+
+    server.pathsWithIdsChanged({{{projectPartId1, ClangBackEnd::SourceType::TopSystemInclude}, {}},
+                                {{projectPartId1, ClangBackEnd::SourceType::ProjectInclude}, {}},
+                                {{projectPartId1, ClangBackEnd::SourceType::UserInclude}, {}},
+                                {{projectPartId2, ClangBackEnd::SourceType::SystemInclude}, {}},
+                                {{projectPartId2, ClangBackEnd::SourceType::ProjectInclude}, {}},
                                 {{projectPartId3, ClangBackEnd::SourceType::UserInclude}, {}},
-                                {{projectPartId4, ClangBackEnd::SourceType::ProjectInclude}, {}},
-                                {{projectPartId5, ClangBackEnd::SourceType::SystemInclude}, {}},
+                                {{projectPartId4, ClangBackEnd::SourceType::TopProjectInclude}, {}},
+                                {{projectPartId4, ClangBackEnd::SourceType::Source}, {}},
+                                {{projectPartId4, ClangBackEnd::SourceType::UserInclude}, {}},
+                                {{projectPartId5, ClangBackEnd::SourceType::ProjectInclude}, {}},
                                 {{projectPartId6, ClangBackEnd::SourceType::Source}, {}}});
 }
 
diff --git a/tests/unit/unittest/pchtaskgenerator-test.cpp b/tests/unit/unittest/pchtaskgenerator-test.cpp
index b2293d6ec0..9dd2e09273 100644
--- a/tests/unit/unittest/pchtaskgenerator-test.cpp
+++ b/tests/unit/unittest/pchtaskgenerator-test.cpp
@@ -26,6 +26,7 @@
 #include "googletest.h"
 
 #include "mockbuilddependenciesprovider.h"
+#include "mockpchtaskqueue.h"
 #include "mockpchtasksmerger.h"
 
 #include <pchtaskgenerator.h>
@@ -52,11 +53,13 @@ class PchTaskGenerator : public testing::Test
 protected:
     NiceMock<MockBuildDependenciesProvider> mockBuildDependenciesProvider;
     NiceMock<MockPchTasksMerger> mockPchTaskMerger;
+    NiceMock<MockPchTaskQueue> mockPchTaskQueue;
     NiceMock<MockFunction<void(int, int)>> mockProgressCounterCallback;
     ClangBackEnd::ProgressCounter progressCounter{mockProgressCounterCallback.AsStdFunction()};
     ClangBackEnd::PchTaskGenerator generator{mockBuildDependenciesProvider,
                                              mockPchTaskMerger,
-                                             progressCounter};
+                                             progressCounter,
+                                             mockPchTaskQueue};
     ClangBackEnd::ProjectPartContainer projectPart1{
         1,
         {"--yi"},
@@ -145,6 +148,36 @@ TEST_F(PchTaskGenerator, AddProjectParts)
     generator.addProjectParts({projectPart1}, {"ToolChainArgument"});
 }
 
+TEST_F(PchTaskGenerator, AddNonSystemProjectParts)
+{
+    ON_CALL(mockBuildDependenciesProvider, create(_)).WillByDefault(Return(buildDependency));
+
+    EXPECT_CALL(
+        mockPchTaskQueue,
+        addProjectPchTasks(ElementsAre(AllOf(
+            Field(&PchTask::projectPartIds, ElementsAre(ProjectPartId{1})),
+            Field(&PchTask::includes, ElementsAre(3)),
+            Field(&PchTask::watchedSystemIncludes, ElementsAre(4, 5)),
+            Field(&PchTask::watchedProjectIncludes, ElementsAre(1, 3)),
+            Field(&PchTask::watchedUserIncludes, ElementsAre(2)),
+            Field(&PchTask::watchedUserSources, ElementsAre(6)),
+            Field(&PchTask::compilerMacros,
+                  ElementsAre(CompilerMacro{"YI", "1", 1}, CompilerMacro{"SAN", "3", 3})),
+            Field(&PchTask::systemIncludeSearchPaths,
+                  ElementsAre(IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System},
+                              IncludeSearchPath{"/builtin/path", 3, IncludeSearchPathType::BuiltIn},
+                              IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System})),
+            Field(&PchTask::projectIncludeSearchPaths,
+                  ElementsAre(IncludeSearchPath{"/to/path1", 1, IncludeSearchPathType::User},
+                              IncludeSearchPath{"/to/path2", 2, IncludeSearchPathType::User})),
+            Field(&PchTask::toolChainArguments, ElementsAre("--yi")),
+            Field(&PchTask::language, Eq(Utils::Language::Cxx)),
+            Field(&PchTask::languageVersion, Eq(Utils::LanguageVersion::CXX11)),
+            Field(&PchTask::languageExtension, Eq(Utils::LanguageExtension::All))))));
+
+    generator.addNonSystemProjectParts({projectPart1}, {"ToolChainArgument"});
+}
+
 TEST_F(PchTaskGenerator, ProgressCounter)
 {
     ON_CALL(mockBuildDependenciesProvider, create(_)).WillByDefault(Return(buildDependency));