diff options
author | Joerg Bornemann <joerg.bornemann@qt.io> | 2017-11-08 11:30:22 +0100 |
---|---|---|
committer | Joerg Bornemann <joerg.bornemann@qt.io> | 2017-11-08 15:35:17 +0000 |
commit | 8f8ea38dfd9c76092917fe63e7a132b00c456739 (patch) | |
tree | 2f8a3bf86e4add60da465728d7f1ba201b144809 | |
parent | ad524b0f1cda91c26e6f697831e24d6e78b336a5 (diff) |
Fix evaluation of Properties.condition
Use the same code path for the evaluation of Properties.condition and
the actual values.
Task-number: QBS-1240
Change-Id: I1cfb2e13349d73bf347bf90ee5aef4b0835630f6
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
4 files changed, 124 insertions, 111 deletions
diff --git a/src/lib/corelib/language/evaluatorscriptclass.cpp b/src/lib/corelib/language/evaluatorscriptclass.cpp index d0c508037..fbc7b0193 100644 --- a/src/lib/corelib/language/evaluatorscriptclass.cpp +++ b/src/lib/corelib/language/evaluatorscriptclass.cpp @@ -101,6 +101,25 @@ public: } private: + friend class AutoScopePopper; + + class AutoScopePopper + { + public: + AutoScopePopper(SVConverter *converter) + : m_converter(converter) + { + } + + ~AutoScopePopper() + { + m_converter->popScopes(); + } + + private: + SVConverter *m_converter; + }; + void setupConvenienceProperty(const QString &conveniencePropertyName, QScriptValue *extraScope, const QScriptValue &scriptValue) { @@ -186,100 +205,54 @@ private: void handle(JSSourceValue *value) override { - const Item *conditionScopeItem = 0; - QScriptValue conditionScope; - QScriptValue conditionFileScope; Item *outerItem = data->item->outerItem(); for (const JSSourceValue::Alternative &alternative : value->alternatives()) { - const Evaluator::FileContextScopes fileCtxScopes - = data->evaluator->fileContextScopes(value->file()); - if (!conditionScopeItem) { - // We have to differentiate between module instances and normal items here. - // - // The module instance case: - // Product { - // property bool something: true - // Properties { - // condition: something - // cpp.defines: ["ABC"] - // } - // } - // - // data->item points to cpp and the condition's scope chain must contain cpp's - // scope, which is the item where cpp is instantiated. The scope chain must not - // contain data->item itself. - // - // The normal item case: - // Product { - // property bool something: true - // property string value: "ABC" - // Properties { - // condition: something - // value: "DEF" - // } - // } - // - // data->item points to the product and the condition's scope chain must contain - // the product item. - conditionScopeItem = data->item->type() == ItemType::ModuleInstance - ? data->item->scope() : data->item; - conditionScope = data->evaluator->scriptValue(conditionScopeItem); - QBS_ASSERT(conditionScope.isObject(), return); - conditionFileScope = fileCtxScopes.fileScope; - } - scriptContext->pushScope(conditionFileScope); - pushItemScopes(conditionScopeItem); - if (alternative.value->definingItem()) - pushItemScopes(alternative.value->definingItem()); - scriptContext->pushScope(conditionScope); - const QScriptValue &theImportScope = fileCtxScopes.importScope; - if (theImportScope.isError()) { - scriptContext->popScope(); - scriptContext->popScope(); - popScopes(); - *result = theImportScope; - return; + if (alternative.value->sourceUsesOuter() && !outerItem) { + // Clone value but without alternatives. + JSSourceValuePtr outerValue = + std::static_pointer_cast<JSSourceValue>(value->clone()); + outerValue->setNext(ValuePtr()); + outerValue->clearCreatedByPropertiesBlock(); + outerValue->clearAlternatives(); + outerItem = Item::create(data->item->pool(), ItemType::Outer); + outerItem->setProperty(propertyName->toString(), outerValue); } - scriptContext->pushScope(theImportScope); - const QScriptValue cr = engine->evaluate(alternative.condition); - const QScriptValue overrides = engine->evaluate(alternative.overrideListProperties); - scriptContext->popScope(); - scriptContext->popScope(); - scriptContext->popScope(); - popScopes(); - if (engine->hasErrorOrException(cr)) { - *result = engine->lastErrorValue(cr); + JSSourceValueEvaluationResult sver = evaluateJSSourceValue(alternative.value.get(), + outerItem, &alternative, + value); + if (!sver.tryNextAlternative) { + *result = sver.scriptValue; return; } - if (cr.toBool()) { - // condition is true, let's use the value of this alternative - if (alternative.value->sourceUsesOuter() && !outerItem) { - // Clone value but without alternatives. - JSSourceValuePtr outerValue = - std::static_pointer_cast<JSSourceValue>(value->clone()); - outerValue->setNext(ValuePtr()); - outerValue->clearCreatedByPropertiesBlock(); - outerValue->clearAlternatives(); - outerItem = Item::create(data->item->pool(), ItemType::Outer); - outerItem->setProperty(propertyName->toString(), outerValue); - } - if (overrides.toBool()) - value->setIsExclusiveListValue(); - value = alternative.value.get(); - break; - } } + *result = evaluateJSSourceValue(value, outerItem).scriptValue; + } + struct JSSourceValueEvaluationResult + { + QScriptValue scriptValue; + bool tryNextAlternative = true; + }; + + JSSourceValueEvaluationResult evaluateJSSourceValue(const JSSourceValue *value, Item *outerItem, + const JSSourceValue::Alternative *alternative = nullptr, + JSSourceValue *elseCaseValue = nullptr) + { + JSSourceValueEvaluationResult result; + QBS_ASSERT(!alternative || value == alternative->value.get(), return result); + AutoScopePopper autoScopePopper(this); auto maybeExtraScope = createExtraScope(value, outerItem); if (!maybeExtraScope.second) { - *result = maybeExtraScope.first; - return; + result.scriptValue = maybeExtraScope.first; + result.tryNextAlternative = false; + return result; } const Evaluator::FileContextScopes fileCtxScopes = data->evaluator->fileContextScopes(value->file()); if (fileCtxScopes.importScope.isError()) { - *result = fileCtxScopes.importScope; - return; + result.scriptValue = fileCtxScopes.importScope; + result.tryNextAlternative = false; + return result; } pushScope(fileCtxScopes.fileScope); pushItemScopes(data->item); @@ -291,9 +264,33 @@ private: pushItemScopes(value->definingItem()); pushScope(maybeExtraScope.first); pushScope(fileCtxScopes.importScope); - *result = engine->evaluate(value->sourceCodeForEvaluation(), value->file()->filePath(), - value->line()); - popScopes(); + if (alternative) { + QScriptValue sv = engine->evaluate(alternative->condition); + if (engine->hasErrorOrException(sv)) { + result.scriptValue = sv; + result.tryNextAlternative = false; + return result; + } + if (sv.toBool()) { + // The condition is true. Continue evaluating the value. + result.tryNextAlternative = false; + } else { + // The condition is false. Try the next alternative or the else value. + result.tryNextAlternative = true; + return result; + } + sv = engine->evaluate(alternative->overrideListProperties); + if (engine->hasErrorOrException(sv)) { + result.scriptValue = sv; + result.tryNextAlternative = false; + return result; + } + if (sv.toBool()) + elseCaseValue->setIsExclusiveListValue(); + } + result.scriptValue = engine->evaluate(value->sourceCodeForEvaluation(), + value->file()->filePath(), value->line()); + return result; } void handle(ItemValue *value) override diff --git a/tests/auto/language/testdata/modules/module-with-properties-item/module-with-properties-item.qbs b/tests/auto/language/testdata/modules/module-with-properties-item/module-with-properties-item.qbs new file mode 100644 index 000000000..3c2ab4ab3 --- /dev/null +++ b/tests/auto/language/testdata/modules/module-with-properties-item/module-with-properties-item.qbs @@ -0,0 +1,8 @@ +Module { + property bool boolProperty: true + property string stringProperty: "set in Module item" + Properties { + condition: boolProperty + stringProperty: "overridden in Properties item" + } +} diff --git a/tests/auto/language/testdata/propertiesblocks.qbs b/tests/auto/language/testdata/propertiesblocks.qbs index 8566166d9..c67612b28 100644 --- a/tests/auto/language/testdata/propertiesblocks.qbs +++ b/tests/auto/language/testdata/propertiesblocks.qbs @@ -206,4 +206,8 @@ Project { dummy.defines: ["a string"] } } + Product { + name: "use-module-with-properties-item" + Depends { name: "module-with-properties-item" } + } } diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp index 462a3e2af..694c5d0c4 100644 --- a/tests/auto/language/tst_language.cpp +++ b/tests/auto/language/tst_language.cpp @@ -2089,107 +2089,111 @@ void TestLanguage::productDirectories() void TestLanguage::propertiesBlocks_data() { QTest::addColumn<QString>("propertyName"); - QTest::addColumn<QStringList>("expectedValues"); + QTest::addColumn<QVariant>("expectedValue"); QTest::addColumn<QString>("expectedStringValue"); - QTest::newRow("init") << QString() << QStringList() << QString(); + QTest::newRow("init") << QString() << QVariant() << QString(); QTest::newRow("property_overwrite") << QString("dummy.defines") - << QStringList("OVERWRITTEN") + << QVariant(QStringList("OVERWRITTEN")) << QString(); QTest::newRow("property_set_indirect") << QString("dummy.cFlags") - << QStringList("VAL") + << QVariant(QStringList("VAL")) << QString(); QTest::newRow("property_overwrite_no_outer") << QString("dummy.defines") - << QStringList("OVERWRITTEN") + << QVariant(QStringList("OVERWRITTEN")) << QString(); QTest::newRow("property_append_to_outer") << QString("dummy.defines") - << (QStringList() << QString("ONE") << QString("TWO")) + << QVariant(QStringList() << QString("ONE") << QString("TWO")) << QString(); QTest::newRow("property_append_to_indirect_outer") << QString("dummy.defines") - << (QStringList() << QString("ONE") << QString("TWO")) + << QVariant(QStringList() << QString("ONE") << QString("TWO")) << QString(); QTest::newRow("property_append_to_indirect_merged_outer") << QString("dummy.rpaths") - << (QStringList() << QString("ONE") << QString("TWO") << QString("$ORIGIN")) + << QVariant(QStringList() << QString("ONE") << QString("TWO") << QString("$ORIGIN")) << QString(); QTest::newRow("multiple_exclusive_properties") << QString("dummy.defines") - << QStringList("OVERWRITTEN") + << QVariant(QStringList("OVERWRITTEN")) << QString(); QTest::newRow("multiple_exclusive_properties_no_outer") << QString("dummy.defines") - << QStringList("OVERWRITTEN") + << QVariant(QStringList("OVERWRITTEN")) << QString(); QTest::newRow("multiple_exclusive_properties_append_to_outer") << QString("dummy.defines") - << (QStringList() << QString("ONE") << QString("TWO")) + << QVariant(QStringList() << QString("ONE") << QString("TWO")) << QString(); QTest::newRow("condition_refers_to_product_property") << QString("dummy.defines") - << QStringList("OVERWRITTEN") + << QVariant(QStringList("OVERWRITTEN")) << QString("OVERWRITTEN"); QTest::newRow("condition_refers_to_project_property") << QString("dummy.defines") - << QStringList("OVERWRITTEN") + << QVariant(QStringList("OVERWRITTEN")) << QString("OVERWRITTEN"); QTest::newRow("ambiguous_properties") << QString("dummy.defines") - << (QStringList() << QString("ONE") << QString("TWO")) + << QVariant(QStringList() << QString("ONE") << QString("TWO")) << QString(); QTest::newRow("inheritance_overwrite_in_subitem") << QString("dummy.defines") - << (QStringList() << QString("OVERWRITTEN_IN_SUBITEM")) + << QVariant(QStringList() << QString("OVERWRITTEN_IN_SUBITEM")) << QString(); QTest::newRow("inheritance_retain_base1") << QString("dummy.defines") - << (QStringList() << QString("BASE") << QString("SUB")) + << QVariant(QStringList() << QString("BASE") << QString("SUB")) << QString(); QTest::newRow("inheritance_retain_base2") << QString("dummy.defines") - << (QStringList() << QString("BASE") << QString("SUB")) + << QVariant(QStringList() << QString("BASE") << QString("SUB")) << QString(); QTest::newRow("inheritance_retain_base3") << QString("dummy.defines") - << (QStringList() << QString("BASE") << QString("SUB")) + << QVariant(QStringList() << QString("BASE") << QString("SUB")) << QString(); QTest::newRow("inheritance_retain_base4") << QString("dummy.defines") - << (QStringList() << QString("BASE")) + << QVariant(QStringList() << QString("BASE")) << QString(); QTest::newRow("inheritance_condition_in_subitem1") << QString("dummy.defines") - << (QStringList() << QString("SOMETHING") << QString("SUB")) + << QVariant(QStringList() << QString("SOMETHING") << QString("SUB")) << QString(); QTest::newRow("inheritance_condition_in_subitem2") << QString("dummy.defines") - << (QStringList() << QString("SOMETHING")) + << QVariant(QStringList() << QString("SOMETHING")) << QString(); QTest::newRow("condition_references_id") << QString("dummy.defines") - << (QStringList() << QString("OVERWRITTEN")) + << QVariant(QStringList() << QString("OVERWRITTEN")) << QString(); QTest::newRow("using_derived_Properties_item") << "dummy.defines" - << (QStringList() << "string from MyProperties") << QString(); + << QVariant(QStringList() << "string from MyProperties") << QString(); QTest::newRow("conditional-depends") << QString("dummy.defines") - << QStringList() + << QVariant() << QString(); - QTest::newRow("cleanup") << QString() << QStringList() << QString(); + QTest::newRow("use-module-with-properties-item") + << QString("module-with-properties-item.stringProperty") + << QVariant(QString("overridden in Properties item")) + << QString(); + QTest::newRow("cleanup") << QString() << QVariant() << QString(); } void TestLanguage::propertiesBlocks() { HANDLE_INIT_CLEANUP_DATATAGS("propertiesblocks.qbs"); QFETCH(QString, propertyName); - QFETCH(QStringList, expectedValues); + QFETCH(QVariant, expectedValue); QFETCH(QString, expectedStringValue); QVERIFY(!!project); QHash<QString, ResolvedProductPtr> products = productsFromProject(project); @@ -2198,7 +2202,7 @@ void TestLanguage::propertiesBlocks() QVERIFY(!!product); QCOMPARE(product->name, productName); QVariant v = productPropertyValue(product, propertyName); - QCOMPARE(v.toStringList(), expectedValues); + QCOMPARE(v, expectedValue); if (!expectedStringValue.isEmpty()) { v = productPropertyValue(product, "someString"); QCOMPARE(v.toString(), expectedStringValue); |