Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix rewrite of multiline string literals.
This commits ensures that we don't rewrite `\'-terminated
multiline string literals. Also, it fixes the processing
of \r characters inside the string literals.

Change-Id: If3d7c1b83c7306b9ccb1be31412b6f8e76434c41
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@nokia.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>
  • Loading branch information
Roberto Raggi authored and Qt by Nokia committed Feb 23, 2012
1 parent 7e76d66 commit 212204b
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 80 deletions.
1 change: 1 addition & 0 deletions .gitattributes
@@ -1 +1,2 @@
.tag ident
*_crlf.* eol=crlf
123 changes: 53 additions & 70 deletions src/declarative/qml/qdeclarativerewrite.cpp
Expand Up @@ -51,6 +51,45 @@ DEFINE_BOOL_CONFIG_OPTION(rewriteDump, QML_REWRITE_DUMP);

namespace QDeclarativeRewrite {

static void rewriteStringLiteral(AST::StringLiteral *ast, const QString *code, int startPosition, TextWriter *writer)
{
const unsigned position = ast->firstSourceLocation().begin() - startPosition + 1;
const unsigned length = ast->literalToken.length - 2;
const QStringRef spell = code->midRef(position, length);
const int end = spell.size();
int index = 0;

while (index < end) {
const QChar ch = spell.at(index++);

if (index < end && ch == QLatin1Char('\\')) {
int pos = index;

// skip a possibly empty sequence of \r characters
while (pos < end && spell.at(pos) == QLatin1Char('\r'))
++pos;

if (pos < end && spell.at(pos) == QLatin1Char('\n')) {
// This is a `\' followed by a newline terminator.
// In this case there's nothing to replace. We keep the code
// as it is and we resume the searching.
index = pos + 1; // refresh the index
}
} else if (ch == QLatin1Char('\r') || ch == QLatin1Char('\n')) {
const QString sep = ch == QLatin1Char('\r') ? QLatin1String("\\r") : QLatin1String("\\n");
const int pos = index - 1;
QString s = sep;

while (index < end && spell.at(index) == ch) {
s += sep;
++index;
}

writer->replace(position + pos, index - pos, s);
}
}
}

bool SharedBindingTester::isSharable(const QString &code)
{
Engine engine;
Expand Down Expand Up @@ -204,40 +243,7 @@ bool RewriteBinding::visit(AST::ExpressionStatement *ast)

bool RewriteBinding::visit(AST::StringLiteral *ast)
{
/* When rewriting the code for bindings, we have to remove ILLEGAL JS tokens like newlines.
They're still in multi-line strings, because the QML parser allows them, but we have to
rewrite them to be JS compliant.
For performance reasons, we don't go for total correctness. \r is only replaced if a
\n was found (since most line endings are \n or \r\n) and QChar::LineSeparator is not
even considered. QTBUG-24064.
Note that rewriteSignalHandler has a function just like this one, for the same reason.
*/

unsigned startOfString = ast->firstSourceLocation().begin() + 1 - _position;
unsigned stringLength = ast->firstSourceLocation().length - 2;

int lastIndex = -1;
bool foundNewLine = false;
QStringRef subStr(_code, startOfString, stringLength);
while (true) {
lastIndex = subStr.indexOf(QLatin1Char('\n'), lastIndex + 1);
if (lastIndex == -1)
break;
foundNewLine = true;
_writer->replace(startOfString+lastIndex, 1, QLatin1String("\\n"));
}

if (foundNewLine) {
while (true) {
lastIndex = subStr.indexOf(QLatin1Char('\r'), lastIndex + 1);
if (lastIndex == -1)
break;
_writer->replace(startOfString+lastIndex, 1, QLatin1String("\\r"));
}
}

rewriteStringLiteral(ast, _code, _position, _writer);
return false;
}

Expand Down Expand Up @@ -361,49 +367,24 @@ void RewriteBinding::rewriteCaseStatements(AST::StatementList *statements, bool
}
}

RewriteSignalHandler::RewriteSignalHandler()
: _writer(0)
, _code(0)
, _position(0)
{
}

void RewriteSignalHandler::accept(AST::Node *node)
{
AST::Node::acceptChild(node, this);
}

bool RewriteSignalHandler::visit(AST::StringLiteral *ast)
{
unsigned startOfExpressionStatement = ast->firstSourceLocation().begin() - _position;
_strStarts << startOfExpressionStatement + 1;
_strLens << ast->firstSourceLocation().length - 2;

rewriteStringLiteral(ast, _code, _position, _writer);
return false;
}

void RewriteSignalHandler::rewriteMultilineStrings(QString &code)
{
QList<int> replaceR, replaceN;
for (int i=0; i < _strStarts.count(); i++) {
QStringRef curSubstr = QStringRef(&code, _strStarts[i], _strLens[i]);
int lastIndex = -1;
while (true) {
lastIndex = curSubstr.indexOf(QLatin1Char('\n'), lastIndex + 1);
if (lastIndex == -1)
break;
replaceN << _strStarts[i]+lastIndex;
}

if (replaceN.count()) {
while (true) {
lastIndex = curSubstr.indexOf(QLatin1Char('\r'), lastIndex + 1);
if (lastIndex == -1)
break;
replaceR << _strStarts[i]+lastIndex;
}
}
}
for (int ii = replaceN.count() - 1; ii >= 0; ii--)
code.replace(replaceN[ii], 1, QLatin1String("\\n"));
if (replaceR.count())
for (int ii = replaceR.count() - 1; ii >= 0; ii--)
code.replace(replaceR[ii], 1, QLatin1String("\\r"));
}

QString RewriteSignalHandler::operator()(QDeclarativeJS::AST::Node *node, const QString &code, const QString &name)
{
if (rewriteDump()) {
Expand All @@ -417,13 +398,15 @@ QString RewriteSignalHandler::operator()(QDeclarativeJS::AST::Node *node, const
if (!expression && !statement)
return code;

_strStarts.clear();
_strLens.clear();
TextWriter w;
_writer = &w;
_code = &code;

_position = expression ? expression->firstSourceLocation().begin() : statement->firstSourceLocation().begin();
accept(node);

QString rewritten = code;
rewriteMultilineStrings(rewritten);
w.write(&rewritten);

rewritten = QStringLiteral("(function ") + name + QStringLiteral("() { ") + rewritten + QStringLiteral(" })");

Expand Down
5 changes: 3 additions & 2 deletions src/declarative/qml/qdeclarativerewrite_p.h
Expand Up @@ -126,11 +126,12 @@ class RewriteBinding: protected AST::Visitor

class RewriteSignalHandler: protected AST::Visitor
{
QList<int> _strStarts;
QList<int> _strLens;
TextWriter *_writer;
const QString *_code;
int _position;

public:
RewriteSignalHandler();
QString operator()(QDeclarativeJS::AST::Node *node, const QString &code, const QString &name);

protected:
Expand Down
@@ -0,0 +1,13 @@
import QtQuick 2.0

Rectangle {
id: root

Component.onCompleted: {
var o = Qt.createQmlObject("import QtQuick 2.0; \
\
Item { \
property bool b: true; \
}", root, "Instance")
}
}
Expand Up @@ -1570,8 +1570,6 @@ void tst_qdeclarativeecmascript::compileInvalidBinding()
{
// QTBUG-23387: ensure that invalid bindings don't cause a crash.
QDeclarativeComponent component(&engine, testFileUrl("v8bindingException.qml"));
QString warning = component.url().toString() + ":16: SyntaxError: Unexpected token ILLEGAL";
QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData());
QObject *object = component.create();
QVERIFY(object != 0);
delete object;
Expand Down Expand Up @@ -5287,12 +5285,21 @@ void tst_qdeclarativeecmascript::qtbug_21864()

void tst_qdeclarativeecmascript::rewriteMultiLineStrings()
{
// QTBUG-23387
QDeclarativeComponent component(&engine, testFileUrl("rewriteMultiLineStrings.qml"));
QObject *o = component.create();
QVERIFY(o != 0);
QTRY_COMPARE(o->property("test").toBool(), true);
delete o;
{
// QTBUG-23387
QDeclarativeComponent component(&engine, testFileUrl("rewriteMultiLineStrings.qml"));
QObject *o = component.create();
QVERIFY(o != 0);
QTRY_COMPARE(o->property("test").toBool(), true);
delete o;
}

{
QDeclarativeComponent component(&engine, testFileUrl("rewriteMultiLineStrings_crlf.1.qml"));
QObject *o = component.create();
QVERIFY(o != 0);
delete o;
}
}

void tst_qdeclarativeecmascript::qobjectConnectionListExceptionHandling()
Expand Down

0 comments on commit 212204b

Please sign in to comment.