From ab71cdafca87513a4e214d3af056d8990bc1eddb Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 11 Feb 2021 13:17:29 +0100 Subject: QML: Warn about variables being used before their declaration This collides with injected signal parameters. qmlcachegen cannot tell those cases apart. [ChangeLog][QML][Important Behavior Changes] QML warns about JavaScript variables being used before their declaration now. This is almost always a mistake. It is particularly dangerous in the presence of injected signal parameters because qmlcachegen cannot identify a name collision between an injected signal parameter and a variable being used before its declaration. It therefore miscompiles such code. You can turn off the deprecation warning using the "qt.qml.compiler" logging category. Pick-to: 6.1 Task-number: QTBUG-89943 Change-Id: I8a9424ca8c6edd562402fe5c560ba7e8344b5585 Reviewed-by: Fabian Kosmale --- src/qml/compiler/qv4codegen.cpp | 19 ++++++++++++++++++- src/qml/compiler/qv4compilercontext.cpp | 11 ++++++----- src/qml/compiler/qv4compilercontext_p.h | 6 +++--- src/qml/compiler/qv4compilerscanfunctions.cpp | 12 ++++++++---- 4 files changed, 35 insertions(+), 13 deletions(-) (limited to 'src/qml') diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index f642797761..38ac04ccbe 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,10 @@ static const bool disable_lookups = false; #undef CONST #endif -QT_USE_NAMESPACE +QT_BEGIN_NAMESPACE + +Q_LOGGING_CATEGORY(lcQmlCompiler, "qt.qml.compiler"); + using namespace QV4; using namespace QV4::Compiler; using namespace QQmlJS; @@ -2378,6 +2382,17 @@ Codegen::Reference Codegen::referenceForName(const QString &name, bool isLhs, co if (resolved.isArgOrEval && isLhs) // ### add correct source location throwSyntaxError(SourceLocation(), QStringLiteral("Variable name may not be eval or arguments in strict mode")); + + if (resolved.declarationLocation.isValid() && accessLocation.isValid() + && resolved.declarationLocation.begin() > accessLocation.end()) { + qCWarning(lcQmlCompiler).nospace().noquote() + << url().toString() << ":" << accessLocation.startLine + << ":" << accessLocation.startColumn << " Variable \"" << name + << "\" is used before its declaration at " + << resolved.declarationLocation.startLine << ":" + << resolved.declarationLocation.startColumn << "."; + } + Reference r; switch (resolved.type) { case Context::ResolvedName::Local: @@ -4458,3 +4473,5 @@ QT_WARNING_POP } Q_UNREACHABLE(); } + +QT_END_NAMESPACE diff --git a/src/qml/compiler/qv4compilercontext.cpp b/src/qml/compiler/qv4compilercontext.cpp index 08fb2ee993..94c7f0631e 100644 --- a/src/qml/compiler/qv4compilercontext.cpp +++ b/src/qml/compiler/qv4compilercontext.cpp @@ -80,14 +80,14 @@ bool Context::Member::requiresTDZCheck(const SourceLocation &accessLocation, boo if (accessAcrossContextBoundaries) return true; - if (!accessLocation.isValid() || !endOfInitializerLocation.isValid()) + if (!accessLocation.isValid() || !declarationLocation.isValid()) return true; - return accessLocation.begin() < endOfInitializerLocation.end(); + return accessLocation.begin() < declarationLocation.end(); } bool Context::addLocalVar(const QString &name, Context::MemberType type, VariableScope scope, FunctionExpression *function, - const QQmlJS::SourceLocation &endOfInitializer) + const QQmlJS::SourceLocation &declarationLocation) { // ### can this happen? if (name.isEmpty()) @@ -112,13 +112,13 @@ bool Context::addLocalVar(const QString &name, Context::MemberType type, Variabl // hoist var declarations to the function level if (contextType == ContextType::Block && (scope == VariableScope::Var && type != MemberType::FunctionDefinition)) - return parent->addLocalVar(name, type, scope, function, endOfInitializer); + return parent->addLocalVar(name, type, scope, function, declarationLocation); Member m; m.type = type; m.function = function; m.scope = scope; - m.endOfInitializerLocation = endOfInitializer; + m.declarationLocation = declarationLocation; members.insert(name, m); return true; } @@ -146,6 +146,7 @@ Context::ResolvedName Context::resolveName(const QString &name, const QQmlJS::So result.requiresTDZCheck = m.requiresTDZCheck(accessLocation, c != this); if (c->isStrict && (name == QLatin1String("arguments") || name == QLatin1String("eval"))) result.isArgOrEval = true; + result.declarationLocation = m.declarationLocation; return result; } const int argIdx = c->findArgument(name); diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h index 4a14009c35..da9d637c5f 100644 --- a/src/qml/compiler/qv4compilercontext_p.h +++ b/src/qml/compiler/qv4compilercontext_p.h @@ -183,7 +183,7 @@ struct Context { QQmlJS::AST::VariableScope scope = QQmlJS::AST::VariableScope::Var; mutable bool canEscape = false; QQmlJS::AST::FunctionExpression *function = nullptr; - QQmlJS::SourceLocation endOfInitializerLocation; + QQmlJS::SourceLocation declarationLocation; bool isLexicallyScoped() const { return this->scope != QQmlJS::AST::VariableScope::Var; } bool requiresTDZCheck(const QQmlJS::SourceLocation &accessLocation, bool accessAcrossContextBoundaries) const; @@ -331,7 +331,7 @@ struct Context { } bool addLocalVar(const QString &name, MemberType contextType, QQmlJS::AST::VariableScope scope, QQmlJS::AST::FunctionExpression *function = nullptr, - const QQmlJS::SourceLocation &endOfInitializer = QQmlJS::SourceLocation()); + const QQmlJS::SourceLocation &declarationLocation = QQmlJS::SourceLocation()); struct ResolvedName { enum Type { @@ -348,7 +348,7 @@ struct Context { bool requiresTDZCheck = false; int scope = -1; int index = -1; - QQmlJS::SourceLocation endOfDeclarationLocation; + QQmlJS::SourceLocation declarationLocation; bool isValid() const { return type != Unresolved; } }; ResolvedName resolveName(const QString &name, const QQmlJS::SourceLocation &accessLocation); diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp index 4c8f0d0658..c4f46ecf05 100644 --- a/src/qml/compiler/qv4compilerscanfunctions.cpp +++ b/src/qml/compiler/qv4compilerscanfunctions.cpp @@ -330,9 +330,13 @@ bool ScanFunctions::visit(PatternElement *ast) BoundNames names; ast->boundNames(&names); - QQmlJS::SourceLocation lastInitializerLocation = ast->lastSourceLocation(); - if (_context->lastBlockInitializerLocation.isValid()) - lastInitializerLocation = _context->lastBlockInitializerLocation; + QQmlJS::SourceLocation declarationLocation = ast->firstSourceLocation(); + if (_context->lastBlockInitializerLocation.isValid()) { + declarationLocation.length = _context->lastBlockInitializerLocation.end() + - declarationLocation.offset; + } else { + declarationLocation.length = ast->lastSourceLocation().end() - declarationLocation.offset; + } for (const auto &name : qAsConst(names)) { if (_context->isStrict && (name.id == QLatin1String("eval") || name.id == QLatin1String("arguments"))) @@ -345,7 +349,7 @@ bool ScanFunctions::visit(PatternElement *ast) return false; } if (!_context->addLocalVar(name.id, ast->initializer ? Context::VariableDefinition : Context::VariableDeclaration, ast->scope, - /*function*/nullptr, lastInitializerLocation)) { + /*function*/nullptr, declarationLocation)) { _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Identifier %1 has already been declared").arg(name.id)); return false; } -- cgit v1.2.3