From 609bc2a38916931514241f5fbba9b4ad3c6ea0fd Mon Sep 17 00:00:00 2001
From: Nikolai Kosjar <nikolai.kosjar@theqtcompany.com>
Date: Mon, 23 Nov 2015 13:31:46 +0100
Subject: [PATCH] Clang: Avoid needless reparse after first parse

Change-Id: Ie97818f29d1df61380cd1c22ef2598091050b10d
Reviewed-by: Marco Bubke <marco.bubke@theqtcompany.com>
---
 .../ipcsource/translationunit.cpp             |  4 +-
 .../ipcsource/translationunits.cpp            |  4 +-
 tests/unit/unittest/clangipcservertest.cpp    | 41 +++++++++++++++----
 tests/unit/unittest/translationunittest.cpp   | 33 +++++++++++++++
 4 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/src/tools/clangbackend/ipcsource/translationunit.cpp b/src/tools/clangbackend/ipcsource/translationunit.cpp
index 8769f7caef3..052761d3639 100644
--- a/src/tools/clangbackend/ipcsource/translationunit.cpp
+++ b/src/tools/clangbackend/ipcsource/translationunit.cpp
@@ -83,8 +83,8 @@ public:
     CXTranslationUnit translationUnit = nullptr;
     CXIndex index = nullptr;
     uint documentRevision = 0;
-    bool needsToBeReparsed  = false;
-    bool hasNewDiagnostics = false;
+    bool needsToBeReparsed = false;
+    bool hasNewDiagnostics = true;
 };
 
 TranslationUnitData::TranslationUnitData(const Utf8String &filePath,
diff --git a/src/tools/clangbackend/ipcsource/translationunits.cpp b/src/tools/clangbackend/ipcsource/translationunits.cpp
index 8fd3b14b7c6..a14f15a55f9 100644
--- a/src/tools/clangbackend/ipcsource/translationunits.cpp
+++ b/src/tools/clangbackend/ipcsource/translationunits.cpp
@@ -62,10 +62,8 @@ void TranslationUnits::create(const QVector<FileContainer> &fileContainers)
 {
     checkIfTranslationUnitsDoesNotExists(fileContainers);
 
-    for (const FileContainer &fileContainer : fileContainers) {
+    for (const FileContainer &fileContainer : fileContainers)
         createTranslationUnit(fileContainer);
-        updateTranslationUnitsWithChangedDependency(fileContainer.filePath());
-    }
 }
 
 void TranslationUnits::update(const QVector<FileContainer> &fileContainers)
diff --git a/tests/unit/unittest/clangipcservertest.cpp b/tests/unit/unittest/clangipcservertest.cpp
index 175e19e5213..be19eab6eed 100644
--- a/tests/unit/unittest/clangipcservertest.cpp
+++ b/tests/unit/unittest/clangipcservertest.cpp
@@ -77,11 +77,18 @@ using ClangBackEnd::TranslationUnitDoesNotExistMessage;
 using ClangBackEnd::ProjectPartsDoNotExistMessage;
 using ClangBackEnd::UpdateTranslationUnitsForEditorMessage;
 
-MATCHER_P3(HasDirtyTranslationUnit, filePath, projectPartId, documentRevision,
+MATCHER_P5(HasDirtyTranslationUnit,
+           filePath,
+           projectPartId,
+           documentRevision,
+           isNeedingReparse,
+           hasNewDiagnostics,
            std::string(negation ? "isn't" : "is")
            + " translation unit with file path "+ PrintToString(filePath)
            + " and project " + PrintToString(projectPartId)
            + " and document revision " + PrintToString(documentRevision)
+           + " and isNeedingReparse = " + PrintToString(isNeedingReparse)
+           + " and hasNewDiagnostics = " + PrintToString(hasNewDiagnostics)
            )
 {
     auto &&translationUnits = arg.translationUnitsForTestOnly();
@@ -89,16 +96,24 @@ MATCHER_P3(HasDirtyTranslationUnit, filePath, projectPartId, documentRevision,
         auto translationUnit = translationUnits.translationUnit(filePath, projectPartId);
 
         if (translationUnit.documentRevision() == documentRevision) {
-            if (translationUnit.hasNewDiagnostics()) {
-                if (translationUnit.isNeedingReparse())
-                    return true;
 
+            if (translationUnit.hasNewDiagnostics() && !hasNewDiagnostics) {
+                *result_listener << "hasNewDiagnostics is true";
+                return false;
+            } else if (!translationUnit.hasNewDiagnostics() && hasNewDiagnostics) {
+                *result_listener << "hasNewDiagnostics is false";
+                return false;
+            }
+
+            if (translationUnit.isNeedingReparse() && !isNeedingReparse) {
+                *result_listener << "isNeedingReparse is true";
+                return false;
+            } else if (!translationUnit.isNeedingReparse() && isNeedingReparse) {
                 *result_listener << "isNeedingReparse is false";
                 return false;
             }
 
-            *result_listener << "hasNewDiagnostics is false";
-            return false;
+            return true;
         }
 
         *result_listener << "revision number is " << PrintToString(translationUnit.documentRevision());
@@ -406,8 +421,18 @@ TEST_F(ClangIpcServer, TicketNumberIsForwarded)
     clangServer.completeCode(completeCodeMessage);
 }
 
-TEST_F(ClangIpcServer, TranslationUnitIsDirtyAfterCreation)
+TEST_F(ClangIpcServer, TranslationUnitAfterCreationNeedsNoReparseAndHasNewDiagnostics)
 {
-    ASSERT_THAT(clangServer, HasDirtyTranslationUnit(functionTestFilePath, projectPartId, 0));
+    ASSERT_THAT(clangServer, HasDirtyTranslationUnit(functionTestFilePath, projectPartId, 0U, false, true));
 }
+
+TEST_F(ClangIpcServer, TranslationUnitAfterUpdateNeedsReparseAndHasNewDiagnostics)
+{
+    const auto fileContainer = FileContainer(functionTestFilePath, projectPartId,unsavedContent(unsavedTestFilePath), true, 1);
+
+    clangServer.updateTranslationUnitsForEditor({{fileContainer}});
+
+    ASSERT_THAT(clangServer, HasDirtyTranslationUnit(functionTestFilePath, projectPartId, 1U, true, true));
+}
+
 }
diff --git a/tests/unit/unittest/translationunittest.cpp b/tests/unit/unittest/translationunittest.cpp
index 452aa171aec..4bc2d110611 100644
--- a/tests/unit/unittest/translationunittest.cpp
+++ b/tests/unit/unittest/translationunittest.cpp
@@ -194,6 +194,38 @@ TEST_F(TranslationUnit, DependedFilePaths)
                       Contains(Utf8StringLiteral(TESTDATA_DIR"/translationunits.h"))));
 }
 
+TEST_F(TranslationUnit, NeedsNoReparseAfterCreation)
+{
+    translationUnit.cxTranslationUnit();
+
+    ASSERT_FALSE(translationUnit.isNeedingReparse());
+}
+
+TEST_F(TranslationUnit, HasNewDiagnosticsAfterCreation)
+{
+    translationUnit.cxTranslationUnit();
+
+    ASSERT_TRUE(translationUnit.hasNewDiagnostics());
+}
+
+TEST_F(TranslationUnit, NeedsReparseAfterChangeOfMainFile)
+{
+    translationUnit.cxTranslationUnit();
+
+    translationUnit.setDirtyIfDependencyIsMet(translationUnitFilePath);
+
+    ASSERT_TRUE(translationUnit.isNeedingReparse());
+}
+
+TEST_F(TranslationUnit, HasNewDiagnosticsAfterChangeOfMainFile)
+{
+    translationUnit.cxTranslationUnit();
+
+    translationUnit.setDirtyIfDependencyIsMet(translationUnitFilePath);
+
+    ASSERT_TRUE(translationUnit.hasNewDiagnostics());
+}
+
 TEST_F(TranslationUnit, NoNeedForReparsingForIndependendFile)
 {
     translationUnit.cxTranslationUnit();
@@ -234,6 +266,7 @@ TEST_F(TranslationUnit, NeedsNoReparsingAfterReparsing)
 TEST_F(TranslationUnit, HasNoNewDiagnosticsForIndependendFile)
 {
     translationUnit.cxTranslationUnit();
+    translationUnit.diagnostics(); // Reset hasNewDiagnostics
 
     translationUnit.setDirtyIfDependencyIsMet(Utf8StringLiteral(TESTDATA_DIR"/otherfiles.h"));