ScriptEngine: Remove extra evaluation scopes

These were only kept for temporary bug compatibility with the QtScript-
based implementation.
Apart from being more correct, this also yields a small, but consistent
speed-up of about 2%.

Change-Id: I857fed3bd64eb68ce5e1ded80647e46593f410b4
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

2 files changed, 22 insertions, 10 deletions

diff --git a/src/lib/corelib/language/evaluator.cpp b/src/lib/corelib/language/evaluator.cpp
index fe3ee9db2..3d4c98a79 100644
--- a/src/lib/corelib/language/evaluator.cpp
+++ b/src/lib/corelib/language/evaluator.cpp
@@ -712,6 +712,25 @@ private:
             ScopedJsValue sv(engine->context(), engine->evaluate(JsValueOwner::Caller,
                     alternative->condition.value, {}, 1, scopeChain));
             if (JsException ex = engine->checkAndClearException(alternative->condition.location)) {
+
+                // This handles cases like the following:
+                //   Depends { name: "cpp" }
+                //   Properties {
+                //     condition: qbs.targetOS.contains("darwin")
+                //     bundle.isBundle: false
+                //   }
+                // On non-Darwin platforms, bundle never gets instantiated, and thus bundle.isBundle
+                // has no scope, so the qbs item in the condition is not found.
+                // TODO: Ideally, we would never evaluate values in placeholder items, but
+                //       there are currently several contexts where we do that, e.g. Export
+                //       and Group items. Perhaps change that, or try to collect all such
+                //       exceptions and don't try to evaluate other cases.
+                if (itemOfProperty->type() == ItemType::ModuleInstancePlaceholder) {
+                    result.scriptValue = JS_UNDEFINED;
+                    result.tryNextAlternative = false;
+                    return result;
+                }
+
                 result.scriptValue = engine->throwError(ex.toErrorInfo().toString());
                 //result.scriptValue = JS_Throw(engine->context(), ex.takeValue());
                 //result.scriptValue = ex.takeValue();
diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp
index bd7394b16..c5d4656cb 100644
--- a/src/lib/corelib/language/scriptengine.cpp
+++ b/src/lib/corelib/language/scriptengine.cpp
@@ -135,17 +135,10 @@ LookupResult ScriptEngine::doExtraScopeLookup(JSContext *ctx, JSAtom prop)
 
     ScriptEngine * const engine = engineForContext(ctx);
     engine->m_lastLookupWasSuccess = false;
-    JSValueList scopes;
-
-    // FIXME: This is probably wrong, and kept only for "bug compatibility"
-    // The correct code should be the one commented out below. Fix for 2.1.
-    for (const auto &l : qAsConst(engine->m_scopeChains)) {
-        for (const auto &s : l)
-            scopes.push_back(s);
-    }
-    //    if (!engine->m_scopeChains.isEmpty())
-    //        scopes = engine->m_scopeChains.last();
 
+    JSValueList scopes;
+    if (!engine->m_scopeChains.isEmpty())
+        scopes = engine->m_scopeChains.last();
     if (JS_IsObject(engine->m_globalObject))
         scopes.insert(scopes.begin(), engine->m_globalObject);
     for (auto it = scopes.rbegin(); it != scopes.rend(); ++it) {