From d20930e7a9ccc6dbeccc26e32aafa92a9a949b32 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 21 Mar 2023 14:43:20 +0100 Subject: 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 --- src/lib/corelib/language/evaluator.cpp | 19 +++++++++++++++++++ src/lib/corelib/language/scriptengine.cpp | 13 +++---------- 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) { -- cgit v1.2.1