[webkit-reviews] review requested: [Bug 36123] [Qt] QtScript functionality should be extended by syntax checking. : [Attachment 51016] Fix v2

[bugzilla-daemon at webkit.org]

Jędrzej Nowacki <jedrzej.nowacki at nokia.com> has asked  for review:
Bug 36123: [Qt] QtScript functionality should be extended by syntax checking.
https://bugs.webkit.org/show_bug.cgi?id=36123

Attachment 51016: Fix v2
https://bugs.webkit.org/attachment.cgi?id=51016&action=review

------- Additional Comments from Jędrzej Nowacki <jedrzej.nowacki at nokia.com>
Default constructor of the QScriptSyntaxCheckResult was removed as it was
useless.

(In reply to comment #4)
> I would put a comment here saying that we construct a temporary engine
because
> this is a static function (for compatibility with the corresponding API
that's
> part of Qt) and the JSC C API requires a JSContextRef in order to perform
> syntax checking. That's a restriction we should try to get rid of. Could also

> mention, then, that the QScriptSyntaxCheckResult object takes ownership of
the
> engine (=context). It needs to keep the engine around in order to do the lazy

> querying of the message and line number properties on the JS error object...
> This is not so obvious.
Done.

> I would just put the contents of this file in qscriptsyntaxcheckresult.cpp,
no
> need for a separate _p.cpp.
Done.

> The rest looks good to me.
Cool.