From 0ae74e4c267c7b15a405240ec4dc038374d95bd2 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Thu, 1 Oct 2009 12:14:12 +0200 Subject: Fix column number provided to QScriptEngineAgent Introduced a helper function in our custom source provider, columnNumberFromOffset(), that maps an absolute offset in the source input to a relative column number. Reviewed-by: Jedrzej Nowacki --- src/script/api/qscriptengine.cpp | 67 +------------------- src/script/api/qscriptengine_p.h | 73 +++++++++++++++++++++- src/script/api/qscriptengineagent.cpp | 6 ++ tests/auto/qscriptengine/tst_qscriptengine.cpp | 1 - .../qscriptengineagent/tst_qscriptengineagent.cpp | 9 ++- 5 files changed, 88 insertions(+), 68 deletions(-) diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp index 9604fff..059b102 100644 --- a/src/script/api/qscriptengine.cpp +++ b/src/script/api/qscriptengine.cpp @@ -382,65 +382,6 @@ private: bool m_shouldAbortEvaluation; }; -/*Helper class. Main purpose is to give debugger feedback about unloading and loading scripts. - It keeps pointer to JSGlobalObject assuming that it is always the same - there is no way to update - this data. Class is internal and used as an implementation detail in and only in QScriptEngine::evaluate.*/ -class UStringSourceProviderWithFeedback: public JSC::UStringSourceProvider -{ -public: - - static PassRefPtr create(const JSC::UString& source, const JSC::UString& url, int lineNumber, QScriptEnginePrivate* engine) - { - return adoptRef(new UStringSourceProviderWithFeedback(source, url, lineNumber, engine)); - } - - /* Destruction means that there is no more copies of script so create scriptUnload event - and unregister script in QScriptEnginePrivate::loadedScripts */ - virtual ~UStringSourceProviderWithFeedback() - { - if (m_ptr) { - if (JSC::Debugger* debugger = this->debugger()) - debugger->scriptUnload(asID()); - m_ptr->loadedScripts.remove(this); - } - } - - /* set internal QScriptEnginePrivate pointer to null and create unloadScript event, should be called - only if QScriptEnginePrivate is about to be destroyed.*/ - void disconnectFromEngine() - { - if (JSC::Debugger* debugger = this->debugger()) - debugger->scriptUnload(asID()); - m_ptr = 0; - } - -protected: - UStringSourceProviderWithFeedback(const JSC::UString& source, const JSC::UString& url, int lineNumber, QScriptEnginePrivate* engine) - : UStringSourceProvider(source, url), - m_ptr(engine) - { - if (JSC::Debugger* debugger = this->debugger()) - debugger->scriptLoad(asID(), source, url, lineNumber); - if (m_ptr) - m_ptr->loadedScripts.insert(this); - } - - JSC::Debugger* debugger() - { - //if m_ptr is null it mean that QScriptEnginePrivate was destroyed and scriptUnload was called - //else m_ptr is stable and we can use it as normal pointer without hesitation - if(!m_ptr) - return 0; //we are in ~QScriptEnginePrivate - else - return m_ptr->originalGlobalObject()->debugger(); //QScriptEnginePrivate is still alive - } - - //trace global object and debugger instance - QScriptEnginePrivate* m_ptr; -}; - - - static int toDigit(char c) { if ((c >= '0') && (c <= '9')) @@ -900,11 +841,9 @@ QScriptEnginePrivate::QScriptEnginePrivate() QScriptEnginePrivate::~QScriptEnginePrivate() { //disconnect all loadedScripts and generate all jsc::debugger::scriptUnload events - QSet::const_iterator i = loadedScripts.constBegin(); - while(i!=loadedScripts.constEnd()) { - (*i)->disconnectFromEngine(); - i++; - } + QHash::const_iterator it; + for (it = loadedScripts.constBegin(); it != loadedScripts.constEnd(); ++it) + it.value()->disconnectFromEngine(); while (!ownedAgents.isEmpty()) delete ownedAgents.takeFirst(); diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h index b8b805e..5f31054 100644 --- a/src/script/api/qscriptengine_p.h +++ b/src/script/api/qscriptengine_p.h @@ -60,7 +60,10 @@ #include "qscriptvalue_p.h" #include "qscriptstring_p.h" +#include "Debugger.h" +#include "Lexer.h" #include "RefPtr.h" +#include "SourceProvider.h" #include "Structure.h" #include "JSGlobalObject.h" #include "JSValue.h" @@ -259,7 +262,7 @@ public: QSet importedExtensions; QSet extensionsBeingImported; - QSet loadedScripts; + QHash loadedScripts; #ifndef QT_NO_QOBJECT QHash m_qobjectData; @@ -273,6 +276,74 @@ public: namespace QScript { +/*Helper class. Main purpose is to give debugger feedback about unloading and loading scripts. + It keeps pointer to JSGlobalObject assuming that it is always the same - there is no way to update + this data. Class is internal and used as an implementation detail in and only in QScriptEngine::evaluate.*/ +class UStringSourceProviderWithFeedback: public JSC::UStringSourceProvider +{ +public: + static PassRefPtr create( + const JSC::UString& source, const JSC::UString& url, + int lineNumber, QScriptEnginePrivate* engine) + { + return adoptRef(new UStringSourceProviderWithFeedback(source, url, lineNumber, engine)); + } + + /* Destruction means that there is no more copies of script so create scriptUnload event + and unregister script in QScriptEnginePrivate::loadedScripts */ + virtual ~UStringSourceProviderWithFeedback() + { + if (m_ptr) { + if (JSC::Debugger* debugger = this->debugger()) + debugger->scriptUnload(asID()); + m_ptr->loadedScripts.remove(asID()); + } + } + + /* set internal QScriptEnginePrivate pointer to null and create unloadScript event, should be called + only if QScriptEnginePrivate is about to be destroyed.*/ + void disconnectFromEngine() + { + if (JSC::Debugger* debugger = this->debugger()) + debugger->scriptUnload(asID()); + m_ptr = 0; + } + + int columnNumberFromOffset(int offset) const + { + for (const UChar *c = m_source.data() + offset; c >= m_source.data(); --c) { + if (JSC::Lexer::isLineTerminator(*c)) + return offset - static_cast(c - data()); + } + return offset + 1; + } + +protected: + UStringSourceProviderWithFeedback(const JSC::UString& source, const JSC::UString& url, + int lineNumber, QScriptEnginePrivate* engine) + : UStringSourceProvider(source, url), + m_ptr(engine) + { + if (JSC::Debugger* debugger = this->debugger()) + debugger->scriptLoad(asID(), source, url, lineNumber); + if (m_ptr) + m_ptr->loadedScripts.insert(asID(), this); + } + + JSC::Debugger* debugger() + { + //if m_ptr is null it mean that QScriptEnginePrivate was destroyed and scriptUnload was called + //else m_ptr is stable and we can use it as normal pointer without hesitation + if(!m_ptr) + return 0; //we are in ~QScriptEnginePrivate + else + return m_ptr->originalGlobalObject()->debugger(); //QScriptEnginePrivate is still alive + } + + //trace global object and debugger instance + QScriptEnginePrivate* m_ptr; +}; + class SaveFrameHelper { public: diff --git a/src/script/api/qscriptengineagent.cpp b/src/script/api/qscriptengineagent.cpp index 84ae380..bc2eea2 100644 --- a/src/script/api/qscriptengineagent.cpp +++ b/src/script/api/qscriptengineagent.cpp @@ -169,6 +169,9 @@ void QScriptEngineAgentPrivate::exceptionCatch(const JSC::DebuggerCallFrame& fra void QScriptEngineAgentPrivate::atStatement(const JSC::DebuggerCallFrame& frame, intptr_t sourceID, int lineno, int column) { + QScript::UStringSourceProviderWithFeedback *source = engine->loadedScripts.value(sourceID); + Q_ASSERT(source != 0); + column = source->columnNumberFromOffset(column); JSC::CallFrame *oldFrame = engine->currentFrame; int oldAgentLineNumber = engine->agentLineNumber; engine->currentFrame = frame.callFrame(); @@ -195,6 +198,9 @@ void QScriptEngineAgentPrivate::didReachBreakpoint(const JSC::DebuggerCallFrame& intptr_t sourceID, int lineno, int column) { if (q_ptr->supportsExtension(QScriptEngineAgent::DebuggerInvocationRequest)) { + QScript::UStringSourceProviderWithFeedback *source = engine->loadedScripts.value(sourceID); + Q_ASSERT(source != 0); + column = source->columnNumberFromOffset(column); JSC::CallFrame *oldFrame = engine->currentFrame; int oldAgentLineNumber = engine->agentLineNumber; engine->currentFrame = frame.callFrame(); diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp index 183aa3f..f2c7157 100644 --- a/tests/auto/qscriptengine/tst_qscriptengine.cpp +++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp @@ -1632,7 +1632,6 @@ void tst_QScriptEngine::errorMessage_QT679() engine.globalObject().setProperty("foo", 15); QScriptValue error = engine.evaluate("'hello world';\nfoo.bar.blah"); QVERIFY(error.isError()); - QEXPECT_FAIL("", "Task QT-679: the error message always contains the first line of the script, even if the error was on a different line", Continue); QCOMPARE(error.toString(), QString::fromLatin1("TypeError: Result of expression 'foo.bar' [undefined] is not an object.")); } diff --git a/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp b/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp index 82bca8f..283e489 100644 --- a/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp +++ b/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp @@ -1193,10 +1193,15 @@ void tst_QScriptEngineAgent::positionChange_1() QCOMPARE(spy->at(0).columnNumber, 1); } - { + QStringList lineTerminators; + lineTerminators << "\n" << "\r" << "\n\r" << "\r\n"; + for (int i = 0; i < lineTerminators.size(); ++i) { spy->clear(); int lineNumber = 456; - eng.evaluate("1 + 2; 3 + 4;\n5 + 6", "foo.qs", lineNumber); + QString code = "1 + 2; 3 + 4;"; + code.append(lineTerminators.at(i)); + code.append("5 + 6"); + eng.evaluate(code, "foo.qs", lineNumber); QCOMPARE(spy->count(), 3); // 1 + 2 -- cgit v0.12