From c21d3a0094b0692f2f888b04e258229234200e3c Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 23 Oct 2009 14:45:13 +0200 Subject: Refactor QScriptProgram and related API Get rid of QScriptEngine::compile(); you pass arguments to the QScriptProgram constructor instead. evaluate() will lazily compile the program the first time it is evaluated, then use the cached, compiled form on subsequent calls. --- src/script/api/qscriptengine.cpp | 118 ++++++++------------- src/script/api/qscriptengine.h | 1 - src/script/api/qscriptengine_p.h | 5 +- src/script/api/qscriptprogram.cpp | 61 +++++++---- src/script/api/qscriptprogram.h | 7 +- src/script/api/qscriptprogram_p.h | 21 ++-- tests/auto/qscriptengine/tst_qscriptengine.cpp | 45 ++++++-- .../qscriptengineagent/tst_qscriptengineagent.cpp | 87 +++++++++++++++ 8 files changed, 231 insertions(+), 114 deletions(-) diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp index 25f815f..9bc4d6b 100644 --- a/src/script/api/qscriptengine.cpp +++ b/src/script/api/qscriptengine.cpp @@ -1188,9 +1188,36 @@ void QScriptEnginePrivate::agentDeleted(QScriptEngineAgent *agent) } } -QScriptValue QScriptEnginePrivate::evaluateHelper(JSC::ExecState *exec, JSC::Debugger* debugger, - intptr_t sourceId, JSC::EvalExecutable *executable) +JSC::JSValue QScriptEnginePrivate::evaluateHelper(JSC::ExecState *exec, intptr_t sourceId, + JSC::EvalExecutable *executable, + bool &compile) { + JSC::JSLock lock(false); // ### hmmm + QBoolBlocker inEvalBlocker(inEval, true); + q_func()->currentContext()->activationObject(); //force the creation of a context for native function; + + JSC::Debugger* debugger = originalGlobalObject()->debugger(); + if (debugger) + debugger->evaluateStart(sourceId); + + exec->clearException(); + JSC::DynamicGlobalObjectScope dynamicGlobalObjectScope(exec, exec->scopeChain()->globalObject()); + + if (compile) { + JSC::JSObject* error = executable->compile(exec, exec->scopeChain()); + if (error) { + compile = false; + exec->setException(error); + + if (debugger) { + debugger->exceptionThrow(JSC::DebuggerCallFrame(exec, error), sourceId, false); + debugger->evaluateStop(error, sourceId); + } + + return error; + } + } + JSC::JSValue thisValue = thisForContext(exec); JSC::JSObject* thisObject = (!thisValue || thisValue.isUndefinedOrNull()) ? exec->dynamicGlobalObject() : thisValue.toObject(exec); @@ -1208,7 +1235,7 @@ QScriptValue QScriptEnginePrivate::evaluateHelper(JSC::ExecState *exec, JSC::Deb if (debugger) debugger->evaluateStop(scriptValueToJSCValue(abortResult), sourceId); - return abortResult; + return scriptValueToJSCValue(abortResult); } if (exceptionValue) { @@ -1217,14 +1244,14 @@ QScriptValue QScriptEnginePrivate::evaluateHelper(JSC::ExecState *exec, JSC::Deb if (debugger) debugger->evaluateStop(exceptionValue, sourceId); - return scriptValueFromJSCValue(exceptionValue); + return exceptionValue; } if (debugger) debugger->evaluateStop(result, sourceId); Q_ASSERT(!exec->hadException()); - return scriptValueFromJSCValue(result); + return result; } #ifndef QT_NO_QOBJECT @@ -2187,66 +2214,15 @@ QScriptSyntaxCheckResult QScriptEnginePrivate::checkSyntax(const QString &progra QScriptValue QScriptEngine::evaluate(const QString &program, const QString &fileName, int lineNumber) { Q_D(QScriptEngine); - - JSC::JSLock lock(false); // ### hmmm - QBoolBlocker inEval(d->inEval, true); - currentContext()->activationObject(); //force the creation of a context for native function; - - JSC::UString jscProgram = program; - JSC::UString jscFileName = fileName; - JSC::ExecState* exec = d->currentFrame; WTF::PassRefPtr provider - = QScript::UStringSourceProviderWithFeedback::create(jscProgram, jscFileName, lineNumber, d); + = QScript::UStringSourceProviderWithFeedback::create(program, fileName, lineNumber, d); intptr_t sourceId = provider->asID(); JSC::SourceCode source(provider, lineNumber); //after construction of SourceCode provider variable will be null. - JSC::Debugger* debugger = d->originalGlobalObject()->debugger(); - if (debugger) - debugger->evaluateStart(sourceId); - - exec->clearException(); - JSC::DynamicGlobalObjectScope dynamicGlobalObjectScope(exec, exec->scopeChain()->globalObject()); - - JSC::EvalExecutable executable(exec, source); - JSC::JSObject* error = executable.compile(exec, exec->scopeChain()); - if (error) { - exec->setException(error); - - if (debugger) { - debugger->exceptionThrow(JSC::DebuggerCallFrame(exec, error), sourceId, false); - debugger->evaluateStop(error, sourceId); - } - - return d->scriptValueFromJSCValue(error); - } - - return d->evaluateHelper(exec, debugger, sourceId, &executable); -} - -/*! - \internal - \since 4.6 -*/ -QScriptProgram QScriptEngine::compile(const QString &program, const QString &fileName, int lineNumber) -{ - Q_D(QScriptEngine); - JSC::UString jscProgram = program; - JSC::UString jscFileName = fileName; - WTF::PassRefPtr provider - = QScript::UStringSourceProviderWithFeedback::create(jscProgram, jscFileName, lineNumber, d); - JSC::SourceCode source(provider, lineNumber); //after construction of SourceCode provider variable will be null. - JSC::ExecState* exec = d->currentFrame; - exec->clearException(); - JSC::DynamicGlobalObjectScope dynamicGlobalObjectScope(exec, exec->scopeChain()->globalObject()); - - JSC::EvalExecutable *executable = new JSC::EvalExecutable(exec, source); - JSC::JSObject* error = executable->compile(exec, exec->scopeChain()); - if (error != 0) { - delete executable; - return QScriptProgram(); - } - return QScriptProgramPrivate::create(d, executable, provider->asID()); + JSC::EvalExecutable executable(exec, source); + bool compile = true; + return d->scriptValueFromJSCValue(d->evaluateHelper(exec, sourceId, &executable, compile)); } /*! @@ -2257,23 +2233,17 @@ QScriptValue QScriptEngine::evaluate(const QScriptProgram &program) { Q_D(QScriptEngine); QScriptProgramPrivate *program_d = QScriptProgramPrivate::get(program); - if (!program_d || !program_d->engine || !program_d->executable) + if (!program_d) return QScriptValue(); - JSC::JSLock lock(false); - QBoolBlocker inEval(d->inEval, true); - currentContext()->activationObject(); //force the creation of a context for native function; - - intptr_t sourceId = program_d->sourceId; - JSC::Debugger* debugger = d->originalGlobalObject()->debugger(); - if (debugger) - debugger->evaluateStart(sourceId); - JSC::ExecState* exec = d->currentFrame; - exec->clearException(); - JSC::DynamicGlobalObjectScope dynamicGlobalObjectScope(exec, exec->scopeChain()->globalObject()); - - return d->evaluateHelper(exec, debugger, sourceId, program_d->executable); + JSC::EvalExecutable *executable = program_d->executable(exec, d); + bool compile = !program_d->isCompiled; + JSC::JSValue result = d->evaluateHelper(exec, program_d->sourceId, + executable, compile); + if (compile) + program_d->isCompiled = true; + return d->scriptValueFromJSCValue(result); } /*! diff --git a/src/script/api/qscriptengine.h b/src/script/api/qscriptengine.h index c58bb7c..3f438da 100644 --- a/src/script/api/qscriptengine.h +++ b/src/script/api/qscriptengine.h @@ -167,7 +167,6 @@ public: QScriptValue evaluate(const QString &program, const QString &fileName = QString(), int lineNumber = 1); - QScriptProgram compile(const QString &program, const QString &fileName = QString(), int lineNumber = 1); QScriptValue evaluate(const QScriptProgram &program); bool isEvaluating() const; diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h index c7e9f2a..22de29c 100644 --- a/src/script/api/qscriptengine_p.h +++ b/src/script/api/qscriptengine_p.h @@ -197,8 +197,9 @@ public: const QByteArray &targetType, void **result); - QScriptValue evaluateHelper(JSC::ExecState *exec, JSC::Debugger* debugger, - intptr_t sourceId, JSC::EvalExecutable *executable); + JSC::JSValue evaluateHelper(JSC::ExecState *exec, intptr_t sourceId, + JSC::EvalExecutable *executable, + bool &compile); QScript::QObjectData *qobjectData(QObject *object); void disposeQObject(QObject *object); diff --git a/src/script/api/qscriptprogram.cpp b/src/script/api/qscriptprogram.cpp index 9f88363..3ea4fa7 100644 --- a/src/script/api/qscriptprogram.cpp +++ b/src/script/api/qscriptprogram.cpp @@ -58,17 +58,18 @@ QT_BEGIN_NAMESPACE */ -QScriptProgramPrivate::QScriptProgramPrivate(QScriptEnginePrivate *e, - JSC::EvalExecutable *x, - intptr_t id) - : engine(e), executable(x), sourceId(id) +QScriptProgramPrivate::QScriptProgramPrivate(const QString &src, + const QString &fn, + int ln) + : sourceCode(src), fileName(fn), lineNumber(ln), + engine(0), _executable(0), sourceId(-1), isCompiled(false) { ref = 0; } QScriptProgramPrivate::~QScriptProgramPrivate() { - delete executable; + delete _executable; } QScriptProgramPrivate *QScriptProgramPrivate::get(const QScriptProgram &q) @@ -76,18 +77,26 @@ QScriptProgramPrivate *QScriptProgramPrivate::get(const QScriptProgram &q) return const_cast(q.d_func()); } -QScriptProgram QScriptProgramPrivate::create(QScriptEnginePrivate *engine, - JSC::EvalExecutable *executable, - intptr_t sourceId) +JSC::EvalExecutable *QScriptProgramPrivate::executable(JSC::ExecState *exec, + QScriptEnginePrivate *eng) { - QScriptProgramPrivate *d = new QScriptProgramPrivate(engine, executable, sourceId); - QScriptProgram result; - result.d_ptr = d; - return result; + if (_executable) { + if (eng == engine) + return _executable; + delete _executable; + } + WTF::PassRefPtr provider + = QScript::UStringSourceProviderWithFeedback::create(sourceCode, fileName, lineNumber, eng); + sourceId = provider->asID(); + JSC::SourceCode source(provider, lineNumber); //after construction of SourceCode provider variable will be null. + _executable = new JSC::EvalExecutable(exec, source); + engine = eng; + isCompiled = false; + return _executable; } /*! - Constructs an invalid QScriptProgram. + Constructs a null QScriptProgram. */ QScriptProgram::QScriptProgram() : d_ptr(0) @@ -95,6 +104,17 @@ QScriptProgram::QScriptProgram() } /*! + Constructs a new QScriptProgram with the given \a sourceCode, \a + fileName and \a lineNumber. +*/ +QScriptProgram::QScriptProgram(const QString &sourceCode, + const QString fileName, + int lineNumber) + : d_ptr(new QScriptProgramPrivate(sourceCode, fileName, lineNumber)) +{ +} + +/*! Constructs a new QScriptProgram that is a copy of \a other. */ QScriptProgram::QScriptProgram(const QScriptProgram &other) @@ -125,13 +145,13 @@ QScriptProgram &QScriptProgram::operator=(const QScriptProgram &other) } /*! - Returns true if this QScriptProgram is valid; otherwise + Returns true if this QScriptProgram is null; otherwise returns false. */ -bool QScriptProgram::isValid() const +bool QScriptProgram::isNull() const { Q_D(const QScriptProgram); - return (d && d->engine); + return (d == 0); } /*! @@ -142,7 +162,7 @@ QString QScriptProgram::sourceCode() const Q_D(const QScriptProgram); if (!d) return QString(); - return d->executable->source().toString(); + return d->sourceCode; } /*! @@ -153,7 +173,7 @@ QString QScriptProgram::fileName() const Q_D(const QScriptProgram); if (!d) return QString(); - return d->executable->sourceURL(); + return d->fileName; } /*! @@ -164,7 +184,7 @@ int QScriptProgram::lineNumber() const Q_D(const QScriptProgram); if (!d) return -1; - return d->executable->lineNo(); + return d->lineNumber; } /*! @@ -173,6 +193,9 @@ int QScriptProgram::lineNumber() const */ bool QScriptProgram::operator==(const QScriptProgram &other) const { + Q_D(const QScriptProgram); + if (d == other.d_func()) + return true; return (sourceCode() == other.sourceCode()) && (fileName() == other.fileName()) && (lineNumber() == other.lineNumber()); diff --git a/src/script/api/qscriptprogram.h b/src/script/api/qscriptprogram.h index b6782b8..6ab56dc 100644 --- a/src/script/api/qscriptprogram.h +++ b/src/script/api/qscriptprogram.h @@ -44,6 +44,8 @@ #include +#include + QT_BEGIN_HEADER QT_BEGIN_NAMESPACE @@ -55,12 +57,15 @@ class Q_SCRIPT_EXPORT QScriptProgram { public: QScriptProgram(); + QScriptProgram(const QString &sourceCode, + const QString fileName = QString(), + int lineNumber = 1); QScriptProgram(const QScriptProgram &other); ~QScriptProgram(); QScriptProgram &operator=(const QScriptProgram &other); - bool isValid() const; + bool isNull() const; QString sourceCode() const; QString fileName() const; diff --git a/src/script/api/qscriptprogram_p.h b/src/script/api/qscriptprogram_p.h index fe06b38..861ef32 100644 --- a/src/script/api/qscriptprogram_p.h +++ b/src/script/api/qscriptprogram_p.h @@ -58,6 +58,7 @@ namespace JSC { class EvalExecutable; + class ExecState; } QT_BEGIN_NAMESPACE @@ -67,20 +68,26 @@ class QScriptEnginePrivate; class QScriptProgramPrivate { public: - QScriptProgramPrivate(QScriptEnginePrivate*, - JSC::EvalExecutable*, - intptr_t); + QScriptProgramPrivate(const QString &sourceCode, + const QString &fileName, + int lineNumber); ~QScriptProgramPrivate(); static QScriptProgramPrivate *get(const QScriptProgram &q); - static QScriptProgram create(QScriptEnginePrivate*, - JSC::EvalExecutable*, - intptr_t); + + JSC::EvalExecutable *executable(JSC::ExecState *exec, + QScriptEnginePrivate *engine); QBasicAtomicInt ref; + + QString sourceCode; + QString fileName; + int lineNumber; + QScriptEnginePrivate *engine; - JSC::EvalExecutable *executable; + JSC::EvalExecutable *_executable; intptr_t sourceId; + bool isCompiled; }; QT_END_NAMESPACE diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp index c00bde6..062cbfa 100644 --- a/tests/auto/qscriptengine/tst_qscriptengine.cpp +++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp @@ -152,7 +152,7 @@ private slots: void installTranslatorFunctions(); void functionScopes(); void nativeFunctionScopes(); - void compileAndEvaluate(); + void evaluateProgram(); void qRegExpInport_data(); void qRegExpInport(); @@ -4291,7 +4291,7 @@ void tst_QScriptEngine::nativeFunctionScopes() } } -void tst_QScriptEngine::compileAndEvaluate() +void tst_QScriptEngine::evaluateProgram() { QScriptEngine eng; @@ -4299,21 +4299,23 @@ void tst_QScriptEngine::compileAndEvaluate() QString code("1 + 2"); QString fileName("hello.js"); int lineNumber(123); - QScriptProgram program = eng.compile(code, fileName, lineNumber); - QVERIFY(program.isValid()); + QScriptProgram program(code, fileName, lineNumber); + QVERIFY(!program.isNull()); QCOMPARE(program.sourceCode(), code); QCOMPARE(program.fileName(), fileName); QCOMPARE(program.lineNumber(), lineNumber); QScriptValue expected = eng.evaluate(code); - QScriptValue ret = eng.evaluate(program); - QVERIFY(ret.equals(expected)); + for (int x = 0; x < 10; ++x) { + QScriptValue ret = eng.evaluate(program); + QVERIFY(ret.equals(expected)); + } } // Program that accesses variable in the scope { - QScriptProgram program = eng.compile("a"); - QVERIFY(program.isValid()); + QScriptProgram program("a"); + QVERIFY(!program.isNull()); { QScriptValue ret = eng.evaluate(program); QVERIFY(ret.isError()); @@ -4350,8 +4352,8 @@ void tst_QScriptEngine::compileAndEvaluate() // Program that creates closure { - QScriptProgram program = eng.compile("(function() { var count = 0; return function() { return count++; }; })"); - QVERIFY(program.isValid()); + QScriptProgram program("(function() { var count = 0; return function() { return count++; }; })"); + QVERIFY(!program.isNull()); QScriptValue createCounter = eng.evaluate(program); QVERIFY(createCounter.isFunction()); QScriptValue counter = createCounter.call(); @@ -4368,6 +4370,29 @@ void tst_QScriptEngine::compileAndEvaluate() QVERIFY(ret.isNumber()); } } + + // Same program run in different engines + { + QString code("1 + 2"); + QScriptProgram program(code); + QVERIFY(!program.isNull()); + double expected = eng.evaluate(program).toNumber(); + for (int x = 0; x < 2; ++x) { + QScriptEngine eng2; + for (int y = 0; y < 2; ++y) { + double ret = eng2.evaluate(program).toNumber(); + QCOMPARE(ret, expected); + } + } + } + + // No program + { + QScriptProgram program; + QVERIFY(program.isNull()); + QScriptValue ret = eng.evaluate(program); + QVERIFY(!ret.isValid()); + } } static QRegExp minimal(QRegExp r) { r.setMinimal(true); return r; } diff --git a/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp b/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp index 283e489..bdb0414 100644 --- a/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp +++ b/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp @@ -44,6 +44,7 @@ #include #include +#include #include //TESTED_CLASS= @@ -109,6 +110,9 @@ private slots: void extension_invoctaion(); void extension(); void isEvaluatingInExtension(); + void evaluateProgram(); + void evaluateProgram_SyntaxError(); + void evaluateNullProgram(); private: double m_testProperty; @@ -2182,5 +2186,88 @@ void tst_QScriptEngineAgent::isEvaluatingInExtension() QVERIFY(spy->wasEvaluating); } +void tst_QScriptEngineAgent::evaluateProgram() +{ + QScriptEngine eng; + QScriptProgram program("1 + 2", "foo.js", 123); + ScriptEngineSpy *spy = new ScriptEngineSpy(&eng); + qint64 scriptId = -1; + for (int x = 0; x < 10; ++x) { + spy->clear(); + (void)eng.evaluate(program); + QCOMPARE(spy->count(), (x == 0) ? 4 : 3); + + if (x == 0) { + // script is only loaded on first execution + QCOMPARE(spy->at(0).type, ScriptEngineEvent::ScriptLoad); + scriptId = spy->at(0).scriptId; + QVERIFY(scriptId != -1); + QCOMPARE(spy->at(0).script, program.sourceCode()); + QCOMPARE(spy->at(0).fileName, program.fileName()); + QCOMPARE(spy->at(0).lineNumber, program.lineNumber()); + spy->removeFirst(); + } + + QCOMPARE(spy->at(0).type, ScriptEngineEvent::FunctionEntry); // evaluate() + QCOMPARE(spy->at(0).scriptId, scriptId); + + QCOMPARE(spy->at(1).type, ScriptEngineEvent::PositionChange); + QCOMPARE(spy->at(1).scriptId, scriptId); + QCOMPARE(spy->at(1).lineNumber, program.lineNumber()); + + QCOMPARE(spy->at(2).type, ScriptEngineEvent::FunctionExit); // evaluate() + QCOMPARE(spy->at(2).scriptId, scriptId); + QVERIFY(spy->at(2).value.isNumber()); + QCOMPARE(spy->at(2).value.toNumber(), qsreal(3)); + } +} + +void tst_QScriptEngineAgent::evaluateProgram_SyntaxError() +{ + QScriptEngine eng; + QScriptProgram program("this is not valid syntax", "foo.js", 123); + ScriptEngineSpy *spy = new ScriptEngineSpy(&eng); + qint64 scriptId = -1; + for (int x = 0; x < 10; ++x) { + spy->clear(); + (void)eng.evaluate(program); + QCOMPARE(spy->count(), (x == 0) ? 8 : 7); + + if (x == 0) { + // script is only loaded on first execution + QCOMPARE(spy->at(0).type, ScriptEngineEvent::ScriptLoad); + scriptId = spy->at(0).scriptId; + QVERIFY(scriptId != -1); + QCOMPARE(spy->at(0).script, program.sourceCode()); + QCOMPARE(spy->at(0).fileName, program.fileName()); + QCOMPARE(spy->at(0).lineNumber, program.lineNumber()); + spy->removeFirst(); + } + + QCOMPARE(spy->at(0).type, ScriptEngineEvent::FunctionEntry); // evaluate() + QCOMPARE(spy->at(0).scriptId, scriptId); + + QCOMPARE(spy->at(1).type, ScriptEngineEvent::ContextPush); // SyntaxError constructor + QCOMPARE(spy->at(2).type, ScriptEngineEvent::FunctionEntry); // SyntaxError constructor + QCOMPARE(spy->at(3).type, ScriptEngineEvent::FunctionExit); // SyntaxError constructor + QCOMPARE(spy->at(4).type, ScriptEngineEvent::ContextPop); // SyntaxError constructor + + QCOMPARE(spy->at(5).type, ScriptEngineEvent::ExceptionThrow); + QVERIFY(spy->at(5).value.isError()); + QCOMPARE(spy->at(5).value.toString(), QString::fromLatin1("SyntaxError: Parse error")); + + QCOMPARE(spy->at(6).type, ScriptEngineEvent::FunctionExit); // evaluate() + QCOMPARE(spy->at(6).scriptId, scriptId); + } +} + +void tst_QScriptEngineAgent::evaluateNullProgram() +{ + QScriptEngine eng; + ScriptEngineSpy *spy = new ScriptEngineSpy(&eng); + (void)eng.evaluate(QScriptProgram()); + QCOMPARE(spy->count(), 0); +} + QTEST_MAIN(tst_QScriptEngineAgent) #include "tst_qscriptengineagent.moc" -- cgit v0.12