Enable evaluation cache for module validation

After the modules have been merged and all Probes have run, the cache
can be enabled, preventing unnecessary re-evaluation of properties
accessed in the validation script.

Output of qbs_benchmarker on Linux using qbs itself as the test project:
========== Performance data for Resolving ==========
    Old instruction count: 1817936584
    New instruction count: 1696757221
    Relative change: -7 %
    Old peak memory usage: 2714982 Bytes
    New peak memory usage: 2743848 Bytes
    Relative change: +1 %

This does not technically fix the bug mentioned below, but alleviates
the pain caused by it.

Task-number: QBS-1033
Change-Id: Ib0041067ff82a2f4b1818e6f76259d544e5184bd
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

4 files changed, 47 insertions, 28 deletions

diff --git a/src/lib/corelib/language/evaluator.h b/src/lib/corelib/language/evaluator.h
index 3701935d3..2c12abb34 100644
--- a/src/lib/corelib/language/evaluator.h
+++ b/src/lib/corelib/language/evaluator.h
@@ -95,6 +95,20 @@ private:
     mutable QHash<FileContextConstPtr, QScriptValue> m_fileScopeMap;
 };
 
+class EvalCacheEnabler
+{
+public:
+    EvalCacheEnabler(Evaluator *evaluator) : m_evaluator(evaluator)
+    {
+        m_evaluator->setCachingEnabled(true);
+    }
+
+    ~EvalCacheEnabler() { m_evaluator->setCachingEnabled(false); }
+
+private:
+    Evaluator * const m_evaluator;
+};
+
 } // namespace Internal
 } // namespace qbs
 
diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp
index 52a9ee299..33fa51e23 100644
--- a/src/lib/corelib/language/moduleloader.cpp
+++ b/src/lib/corelib/language/moduleloader.cpp
@@ -753,24 +753,25 @@ void ModuleLoader::handleProduct(ModuleLoader::ProductContext *productContext)
                                                module.versionRange.maximum.toString()));
                 }
             }
-            m_evaluator->boolValue(module.item, QLatin1String("validate"));
         } catch (const ErrorInfo &error) {
-            if (module.required) {
-                try {
-                    handleProductError(error, productContext);
-                } catch (const ErrorInfo &) {
-                    // Error will be thrown for enabled products only
-                    module.item->setDelayedError(error);
-                }
-            } else {
-                createNonPresentModule(module.name.toString(), QLatin1String("failed validation"),
-                                       module.item);
-            }
+            handleModuleSetupError(productContext, module, error);
         }
     }
 
     resolveProbes(productContext, item);
 
+    // Module validation must happen in an extra pass, after all Probes have been resolved.
+    EvalCacheEnabler cacheEnabler(m_evaluator);
+    foreach (const Item::Module &module, sortedModules) {
+        if (!module.item->isPresentModule() || module.item->delayedError().hasError())
+            continue;
+        try {
+            m_evaluator->boolValue(module.item, QLatin1String("validate"));
+        } catch (const ErrorInfo &error) {
+            handleModuleSetupError(productContext, module, error);
+        }
+    }
+
     if (!checkItemCondition(item)) {
         Item * const productModule = m_productModuleCache.value(productContext->name);
         if (productModule && productModule->isPresentModule())
@@ -790,6 +791,22 @@ void ModuleLoader::handleProduct(ModuleLoader::ProductContext *productContext)
     productContext->project->result->productInfos.insert(item, productContext->info);
 }
 
+void ModuleLoader::handleModuleSetupError(ModuleLoader::ProductContext *productContext,
+                                          const Item::Module &module, const ErrorInfo &error)
+{
+    if (module.required) {
+        try {
+            handleProductError(error, productContext);
+        } catch (const ErrorInfo &) {
+            // Error will be thrown for enabled products only
+            module.item->setDelayedError(error);
+        }
+    } else {
+        createNonPresentModule(module.name.toString(), QLatin1String("failed validation"),
+                               module.item);
+    }
+}
+
 void ModuleLoader::initProductProperties(const ProductContext &product)
 {
     QString buildDir = ResolvedProduct::deriveBuildDirectoryName(product.name, product.profileName);
diff --git a/src/lib/corelib/language/moduleloader.h b/src/lib/corelib/language/moduleloader.h
index 4719748bb..04f0bca3e 100644
--- a/src/lib/corelib/language/moduleloader.h
+++ b/src/lib/corelib/language/moduleloader.h
@@ -205,6 +205,8 @@ private:
     void prepareProduct(ProjectContext *projectContext, Item *productItem);
     void setupProductDependencies(ProductContext *productContext);
     void handleProduct(ProductContext *productContext);
+    void handleModuleSetupError(ProductContext *productContext, const Item::Module &module,
+                                const ErrorInfo &error);
     void initProductProperties(const ProductContext &product);
     void handleSubProject(ProjectContext *projectContext, Item *projectItem,
             const QSet<QString> &referencedFilePaths);
diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index b8dd5c649..f6e390bff 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -558,20 +558,6 @@ static void gatherAssignedProperties(ItemValue *iv, const QualifiedId &prefix,
     }
 }
 
-class CachingEnabler
-{
-public:
-    CachingEnabler(Evaluator *evaluator) : m_evaluator(evaluator)
-    {
-        m_evaluator->setCachingEnabled(true);
-    }
-
-    ~CachingEnabler() { m_evaluator->setCachingEnabled(false); }
-
-private:
-    Evaluator * const m_evaluator;
-};
-
 QVariantMap ProjectResolver::resolveAdditionalModuleProperties(const Item *group,
                                                                const QVariantMap &currentValues)
 {
@@ -599,7 +585,7 @@ QVariantMap ProjectResolver::resolveAdditionalModuleProperties(const Item *group
                 = QualifiedId(fullPropName.mid(0, fullPropName.count() - 1)).toString();
         propsPerModule[moduleName] << fullPropName.last();
     }
-    CachingEnabler cachingEnabler(m_evaluator);
+    EvalCacheEnabler cachingEnabler(m_evaluator);
     for (auto it = group->modules().cbegin(); it != group->modules().cend(); ++it) {
         const QString &fullModName = it->name.toString();
         const QStringList propsForModule = propsPerModule.take(fullModName);
@@ -1232,7 +1218,7 @@ QVariantMap ProjectResolver::evaluateProperties(const Item *item, const Item *pr
 
 QVariantMap ProjectResolver::createProductConfig()
 {
-    CachingEnabler cachingEnabler(m_evaluator);
+    EvalCacheEnabler cachingEnabler(m_evaluator);
     QVariantMap cfg = evaluateModuleValues(m_productContext->item);
     cfg = evaluateProperties(m_productContext->item, m_productContext->item, cfg);
     return cfg;