From b14891e3d29e7c19717aaa77bb63aad94e358f4d Mon Sep 17 00:00:00 2001 From: Sami Shalayel Date: Mon, 16 Sep 2024 11:53:17 +0200 Subject: [PATCH] SourceLocation: make begin() and end() qsizetype MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change begin() and end() to return a qsizetype, as now we only process QML files where quint32 can safely be casted to qsizetype. This allows to change all users of begin() and end() to use qsizetype, and to silence all MSVC compile warnings about comparison of ints with different signedness. Fixes: QTBUG-127833 Change-Id: I251435aa598386effe0259549dbe94d17b0d806b Reviewed-by: Olivier De Cannière --- src/qml/common/qqmljssourcelocation_p.h | 11 ++--- src/qmlcompiler/qqmlsasourcelocation.cpp | 4 +- src/qmldom/qqmldomcomments.cpp | 46 ++++++++++---------- src/qmldom/qqmldomelements.cpp | 15 +++---- src/qmlls/qqmllscompletion.cpp | 14 +++--- src/qmlls/qqmllsutils.cpp | 29 ++++++------ src/qmlls/qqmllsutils_p.h | 8 ++-- tests/auto/qml/qqmlparser/tst_qqmlparser.cpp | 20 ++++----- 8 files changed, 70 insertions(+), 77 deletions(-) diff --git a/src/qml/common/qqmljssourcelocation_p.h b/src/qml/common/qqmljssourcelocation_p.h index 68e9471e35..bea953046b 100644 --- a/src/qml/common/qqmljssourcelocation_p.h +++ b/src/qml/common/qqmljssourcelocation_p.h @@ -66,8 +66,8 @@ public: bool isValid() const { return *this != SourceLocation(); } - quint32 begin() const { return offset; } - quint32 end() const { return offset + length; } + qsizetype begin() const { return qsizetype(offset); } + qsizetype end() const { return qsizetype(offset) + length; } // Returns a zero length location at the start of the current one. SourceLocation startZeroLengthLocation() const @@ -80,11 +80,12 @@ public: quint32 i = offset; quint32 endLine = startLine; quint32 endColumn = startColumn; - while (i < end()) { + const quint32 end = offset + length; + while (i < end) { QChar c = text.at(i); switch (c.unicode()) { case '\n': - if (i + 1 < end() && text.at(i + 1) == QLatin1Char('\r')) + if (i + 1 < end && text.at(i + 1) == QLatin1Char('\r')) ++i; Q_FALLTHROUGH(); case '\r': @@ -96,7 +97,7 @@ public: } ++i; } - return SourceLocation(offset + length, 0, endLine, endColumn); + return SourceLocation(end, 0, endLine, endColumn); } // attributes diff --git a/src/qmlcompiler/qqmlsasourcelocation.cpp b/src/qmlcompiler/qqmlsasourcelocation.cpp index 63b61cce5b..bc22d1b93a 100644 --- a/src/qmlcompiler/qqmlsasourcelocation.cpp +++ b/src/qmlcompiler/qqmlsasourcelocation.cpp @@ -38,7 +38,7 @@ bool QQmlSA::SourceLocation::isValid() const */ quint32 QQmlSA::SourceLocation::begin() const { - return QQmlSA::SourceLocationPrivate::sourceLocation(*this).begin(); + return offset(); } /*! @@ -46,7 +46,7 @@ quint32 QQmlSA::SourceLocation::begin() const */ quint32 QQmlSA::SourceLocation::end() const { - return QQmlSA::SourceLocationPrivate::sourceLocation(*this).end(); + return offset() + length(); } /*! diff --git a/src/qmldom/qqmldomcomments.cpp b/src/qmldom/qqmldomcomments.cpp index 8e94758b57..68b17719cb 100644 --- a/src/qmldom/qqmldomcomments.cpp +++ b/src/qmldom/qqmldomcomments.cpp @@ -282,8 +282,8 @@ public: class ElementRef { public: - ElementRef(AST::Node *node, quint32 size) : element(node), size(size) { } - ElementRef(const Path &path, FileLocationRegion region, quint32 size) + ElementRef(AST::Node *node, qsizetype size) : element(node), size(size) { } + ElementRef(const Path &path, FileLocationRegion region, qsizetype size) : element(RegionRef{ path, region }), size(size) { } @@ -294,7 +294,7 @@ public: ElementRef() = default; std::variant element; - quint32 size = 0; + qsizetype size = 0; }; /*! @@ -350,8 +350,8 @@ public: bool preVisit(Node *n) override { if (!kindsToSkip().contains(n->kind)) { - quint32 start = n->firstSourceLocation().begin(); - quint32 end = n->lastSourceLocation().end(); + qsizetype start = n->firstSourceLocation().begin(); + qsizetype end = n->lastSourceLocation().end(); if (!starts.contains(start)) starts.insert(start, { n, end - start }); if (!ends.contains(end)) @@ -360,8 +360,8 @@ public: return true; } - QMap starts; - QMap ends; + QMap starts; + QMap ends; }; void AstRangesVisitor::addNodeRanges(AST::Node *rootNode) @@ -381,12 +381,12 @@ void AstRangesVisitor::addItemRanges( if (comments) { auto regs = itemLocations->info().regions; for (auto it = regs.cbegin(), end = regs.cend(); it != end; ++it) { - quint32 startI = it.value().begin(); - quint32 endI = it.value().end(); + qsizetype startI = it.value().begin(); + qsizetype endI = it.value().end(); if (!shouldSkipRegion(item, it.key())) { if (!starts.contains(startI)) - starts.insert(startI, { currentP, it.key(), quint32(endI - startI) }); + starts.insert(startI, { currentP, it.key(), endI - startI }); if (!ends.contains(endI)) ends.insert(endI, { currentP, it.key(), endI - startI }); } @@ -452,7 +452,7 @@ bool AstRangesVisitor::shouldSkipRegion(const DomItem &item, FileLocationRegion class CommentLinker { public: - CommentLinker(QStringView code, ElementRef &commentedElement, const AstRangesVisitor &ranges, quint32 &lastPostCommentPostEnd, + CommentLinker(QStringView code, ElementRef &commentedElement, const AstRangesVisitor &ranges, qsizetype &lastPostCommentPostEnd, const SourceLocation &commentLocation) : m_code{ code }, m_commentedElement{ commentedElement }, @@ -481,18 +481,16 @@ public: [[nodiscard]] Comment createComment() const { const auto [preSpacesIndex, postSpacesIndex, preNewlineCount] = m_spaces; - return Comment{ m_code.mid(preSpacesIndex, quint32(postSpacesIndex) - preSpacesIndex), - m_commentLocation, - static_cast(preNewlineCount), - m_commentType}; + return Comment{ m_code.mid(preSpacesIndex, postSpacesIndex - preSpacesIndex), + m_commentLocation, static_cast(preNewlineCount), m_commentType }; } private: struct SpaceTrace { - quint32 iPre; + qsizetype iPre; qsizetype iPost; - int preNewline; + qsizetype preNewline; }; /*! \internal @@ -505,9 +503,9 @@ private: */ [[nodiscard]] SpaceTrace findSpacesAroundComment() const { - quint32 iPre = m_commentLocation.begin(); - int preNewline = 0; - int postNewline = 0; + qsizetype iPre = m_commentLocation.begin(); + qsizetype preNewline = 0; + qsizetype postNewline = 0; QStringView commentStartStr; while (iPre > 0) { QChar c = m_code.at(iPre - 1); @@ -522,7 +520,7 @@ private: } else if (c == QLatin1Char('\n') || c == QLatin1Char('\r')) { preNewline = 1; // possibly add an empty line if it was there (but never more than one) - int i = iPre - 1; + qsizetype i = iPre - 1; if (c == QLatin1Char('\n') && i > 0 && m_code.at(i - 1) == QLatin1Char('\r')) --i; while (i > 0 && m_code.at(--i).isSpace()) { @@ -589,7 +587,7 @@ private: // expression (think a + //comment\n b ==> a // comment\n + b), in this // case attaching as preComment of iStart (b in the example) should be // preferred as it is safe - quint32 i = m_spaces.iPre; + qsizetype i = m_spaces.iPre; while (i != 0 && m_code.at(--i).isSpace()) ; if (i <= preEnd.key() || i < m_lastPostCommentPostEnd @@ -648,7 +646,7 @@ private: private: QStringView m_code; ElementRef &m_commentedElement; - quint32 &m_lastPostCommentPostEnd; + qsizetype &m_lastPostCommentPostEnd; Comment::CommentType m_commentType = Comment::Pre; const AstRangesVisitor &m_ranges; const SourceLocation &m_commentLocation; @@ -714,7 +712,7 @@ void CommentCollector::collectComments( ranges.addItemRanges(m_rootItem.item(), m_fileLocations, Path()); ranges.addNodeRanges(rootNode); QStringView code = engine->code(); - quint32 lastPostCommentPostEnd = 0; + qsizetype lastPostCommentPostEnd = 0; for (const SourceLocation &commentLocation : engine->comments()) { // collect whitespace before and after cLoc -> iPre..iPost contains whitespace, // do not add newline before, but add the one after diff --git a/src/qmldom/qqmldomelements.cpp b/src/qmldom/qqmldomelements.cpp index 1b937429d3..e99a554161 100644 --- a/src/qmldom/qqmldomelements.cpp +++ b/src/qmldom/qqmldomelements.cpp @@ -1629,20 +1629,17 @@ bool ScriptExpression::iterateDirectSubpaths(const DomItem &self, DirectVisitor class FirstNodeVisitor : public VisitAll { public: - quint32 minStart = 0; - quint32 maxEnd = std::numeric_limits::max(); + qsizetype minStart = 0; + qsizetype maxEnd = std::numeric_limits::max(); // see also Lexer::checkFileLength(). AST::Node *firstNodeInRange = nullptr; - FirstNodeVisitor(quint32 minStart = 0, quint32 maxEnd = std::numeric_limits::max()) - : minStart(minStart), maxEnd(maxEnd) - { - } + FirstNodeVisitor(qsizetype minStart, qsizetype maxEnd) : minStart(minStart), maxEnd(maxEnd) { } bool preVisit(AST::Node *n) override { if (!VisitAll::uiKinds().contains(n->kind)) { - quint32 start = n->firstSourceLocation().begin(); - quint32 end = n->lastSourceLocation().end(); + qsizetype start = n->firstSourceLocation().begin(); + qsizetype end = n->lastSourceLocation().end(); if (!firstNodeInRange && minStart <= start && end <= maxEnd && start < end) firstNodeInRange = n; } @@ -1650,7 +1647,7 @@ public: } }; -AST::Node *firstNodeInRange(AST::Node *n, quint32 minStart = 0, quint32 maxEnd = ~quint32(0)) +AST::Node *firstNodeInRange(AST::Node *n, qsizetype minStart = 0, qsizetype maxEnd = std::numeric_limits::max()) { FirstNodeVisitor visitor(minStart, maxEnd); AST::Node::accept(n, &visitor); diff --git a/src/qmlls/qqmllscompletion.cpp b/src/qmlls/qqmllscompletion.cpp index 9afd6317d5..22783e6bf7 100644 --- a/src/qmlls/qqmllscompletion.cpp +++ b/src/qmlls/qqmllscompletion.cpp @@ -645,7 +645,7 @@ bool QQmlLSCompletion::cursorInFrontOfItem(const DomItem &parentForContext, const QQmlLSCompletionPosition &positionInfo) { auto fileLocations = FileLocations::treeOf(parentForContext)->info().fullRegion; - return positionInfo.offset() <= fileLocations.offset; + return positionInfo.offset() <= fileLocations.begin(); } bool QQmlLSCompletion::cursorAfterColon(const DomItem ¤tItem, @@ -657,7 +657,7 @@ bool QQmlLSCompletion::cursorAfterColon(const DomItem ¤tItem, if (region == location.regions.constEnd()) return false; - if (region.value().isValid() && region.value().offset < positionInfo.offset()) { + if (region.value().isValid() && region.value().begin() < positionInfo.offset()) { return true; } return false; @@ -815,7 +815,7 @@ void QQmlLSCompletion::insidePropertyDefinitionCompletion( const QQmlJS::SourceLocation propertyKeyword = info.regions[PropertyKeywordRegion]; // do completions for the keywords - if (positionInfo.offset() < propertyKeyword.offset + propertyKeyword.length) { + if (positionInfo.offset() < propertyKeyword.end()) { const QQmlJS::SourceLocation readonlyKeyword = info.regions[ReadonlyKeywordRegion]; const QQmlJS::SourceLocation defaultKeyword = info.regions[DefaultKeywordRegion]; const QQmlJS::SourceLocation requiredKeyword = info.regions[RequiredKeywordRegion]; @@ -825,21 +825,21 @@ void QQmlLSCompletion::insidePropertyDefinitionCompletion( bool completeDefault = true; // if there is already a readonly keyword before the cursor: do not auto complete it again - if (readonlyKeyword.isValid() && readonlyKeyword.offset < positionInfo.offset()) { + if (readonlyKeyword.isValid() && readonlyKeyword.begin() < positionInfo.offset()) { completeReadonly = false; // also, required keywords do not like readonly keywords completeRequired = false; } // same for required - if (requiredKeyword.isValid() && requiredKeyword.offset < positionInfo.offset()) { + if (requiredKeyword.isValid() && requiredKeyword.begin() < positionInfo.offset()) { completeRequired = false; // also, required keywords do not like readonly keywords completeReadonly = false; } // same for default - if (defaultKeyword.isValid() && defaultKeyword.offset < positionInfo.offset()) { + if (defaultKeyword.isValid() && defaultKeyword.begin() < positionInfo.offset()) { completeDefault = false; } auto addCompletionKeyword = [&result](QUtf8StringView view, bool complete) { @@ -860,7 +860,7 @@ void QQmlLSCompletion::insidePropertyDefinitionCompletion( const QQmlJS::SourceLocation propertyIdentifier = info.regions[IdentifierRegion]; if (propertyKeyword.end() <= positionInfo.offset() - && positionInfo.offset() < propertyIdentifier.offset) { + && positionInfo.offset() < propertyIdentifier.begin()) { suggestReachableTypes(currentItem, LocalSymbolsType::ObjectType | LocalSymbolsType::ValueType, CompletionItemKind::Class, result); diff --git a/src/qmlls/qqmllsutils.cpp b/src/qmlls/qqmllsutils.cpp index 617262faaa..e665682978 100644 --- a/src/qmlls/qqmllsutils.cpp +++ b/src/qmlls/qqmllsutils.cpp @@ -290,7 +290,7 @@ handlePropertyDefinitionAndBindingOverlap(const QList &items, qsiz // sanity check: is it the definition of the current binding? check if they both have their // ':' at the same location if (propertyDefinitionColon.isValid() && propertyDefinitionColon == smallestColon - && offsetInFile < smallestColon.offset) { + && offsetInFile < smallestColon.begin()) { return smallestPropertyDefinition; } } @@ -316,16 +316,16 @@ static QList filterItemsFromTextLocation(const QList filteredItems.append(*smallest); const QQmlJS::SourceLocation smallestLoc = smallest->fileLocation->info().fullRegion; - const quint32 smallestBegin = smallestLoc.begin(); - const quint32 smallestEnd = smallestLoc.end(); + const qsizetype smallestBegin = smallestLoc.begin(); + const qsizetype smallestEnd = smallestLoc.end(); for (auto it = items.begin(); it != items.end(); it++) { if (it == smallest) continue; const QQmlJS::SourceLocation itLoc = it->fileLocation->info().fullRegion; - const quint32 itBegin = itLoc.begin(); - const quint32 itEnd = itLoc.end(); + const qsizetype itBegin = itLoc.begin(); + const qsizetype itEnd = itLoc.end(); if (itBegin == smallestEnd || smallestBegin == itEnd) { filteredItems.append(*it); } @@ -364,11 +364,7 @@ QList itemsFromTextLocation(const DomItem &file, int line, int cha enum ComparisonOption { Normal, ExcludePositionAfterLast }; auto containsTarget = [targetPos](QQmlJS::SourceLocation l, ComparisonOption c) { - if constexpr (sizeof(qsizetype) <= sizeof(quint32)) { - return l.begin() <= quint32(targetPos) && quint32(targetPos) < l.end() + (c == Normal ? 1 : 0) ; - } else { - return l.begin() <= targetPos && targetPos < l.end() + (c == Normal ? 1 : 0); - } + return l.begin() <= targetPos && targetPos < l.end() + (c == Normal ? 1 : 0); }; if (containsTarget(t->info().fullRegion, Normal)) { ItemLocation loc; @@ -2372,16 +2368,17 @@ Location Location::from(const QString &fileName, const QQmlJS::SourceLocation &s return Location{ fileName, sourceLocation, textRowAndColumnFrom(code, sourceLocation.end()) }; } -Location Location::from(const QString &fileName, const QString &code, quint32 startLine, - quint32 startCharacter, quint32 length) +Location Location::from(const QString &fileName, const QString &code, qsizetype startLine, + qsizetype startCharacter, qsizetype length) { - const quint32 offset = QQmlLSUtils::textOffsetFrom(code, startLine - 1, startCharacter - 1); - return from(fileName, QQmlJS::SourceLocation{ offset, length, startLine, startCharacter }, + const auto offset = QQmlLSUtils::textOffsetFrom(code, startLine - 1, startCharacter - 1); + return from(fileName, + QQmlJS::SourceLocation::fromQSizeType(offset, length, startLine, startCharacter), code); } -Edit Edit::from(const QString &fileName, const QString &code, quint32 startLine, - quint32 startCharacter, quint32 length, const QString &newName) +Edit Edit::from(const QString &fileName, const QString &code, qsizetype startLine, + qsizetype startCharacter, qsizetype length, const QString &newName) { Edit rename; rename.location = Location::from(fileName, code, startLine, startCharacter, length); diff --git a/src/qmlls/qqmllsutils_p.h b/src/qmlls/qqmllsutils_p.h index 0105df2c98..be61bd92b4 100644 --- a/src/qmlls/qqmllsutils_p.h +++ b/src/qmlls/qqmllsutils_p.h @@ -87,8 +87,8 @@ public: QQmlJS::SourceLocation sourceLocation() const { return m_sourceLocation; } TextPosition end() const { return m_end; } - static Location from(const QString &fileName, const QString &code, quint32 startLine, - quint32 startCharacter, quint32 length); + static Location from(const QString &fileName, const QString &code, qsizetype startLine, + qsizetype startCharacter, qsizetype length); static Location from(const QString &fileName, const QQmlJS::SourceLocation &sourceLocation, const QString &code); static std::optional tryFrom(const QString &fileName, @@ -141,8 +141,8 @@ struct Edit Location location; QString replacement; - static Edit from(const QString &fileName, const QString &code, quint32 startLine, - quint32 startCharacter, quint32 length, const QString &newName); + static Edit from(const QString &fileName, const QString &code, qsizetype startLine, + qsizetype startCharacter, qsizetype length, const QString &newName); friend bool operator<(const Edit &a, const Edit &b) { diff --git a/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp b/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp index 62f67e548f..a96a7605ab 100644 --- a/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp +++ b/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp @@ -77,8 +77,8 @@ public: { if (! nodeStack.isEmpty()) { AST::Node *parent = nodeStack.last(); - const quint32 parentBegin = parent->firstSourceLocation().begin(); - const quint32 parentEnd = parent->lastSourceLocation().end(); + const qsizetype parentBegin = parent->firstSourceLocation().begin(); + const qsizetype parentEnd = parent->lastSourceLocation().end(); if (node->firstSourceLocation().begin() < parentBegin) qDebug() << "first source loc failed: node:" << node->kind << "at" << node->firstSourceLocation().startLine << "/" << node->firstSourceLocation().startColumn @@ -195,7 +195,7 @@ public: ++startLine; } QCOMPARE(expected, found); - SourceLocation combined(first.offset, last.end() - first.begin(), + SourceLocation combined(first.offset, quint32(last.end() - first.begin()), first.startLine, first.startColumn); SourceLocation cStart = combined.startZeroLengthLocation(); SourceLocation cEnd = combined.endZeroLengthLocation(m_codeStr); @@ -338,7 +338,7 @@ void tst_qqmlparser::stringLiteral() auto *literal = QQmlJS::AST::cast(expression); QVERIFY(literal); QCOMPARE(literal->value, u"hello string"); - QCOMPARE(literal->firstSourceLocation().begin(), 0u); + QCOMPARE(literal->firstSourceLocation().begin(), 0); QCOMPARE(literal->lastSourceLocation().end(), quint32(code.size())); // test for correct handling escape sequences inside strings @@ -357,7 +357,7 @@ void tst_qqmlparser::stringLiteral() literal = QQmlJS::AST::cast(binaryExpression->left); QVERIFY(literal); QCOMPARE(literal->value, u"hello\n\tstring"); - QCOMPARE(literal->firstSourceLocation().begin(), 0u); + QCOMPARE(literal->firstSourceLocation().begin(), 0); QCOMPARE(literal->firstSourceLocation().startLine, 1u); QCOMPARE(literal->lastSourceLocation().end(), quint32(leftCode.size())); @@ -365,7 +365,7 @@ void tst_qqmlparser::stringLiteral() literal = QQmlJS::AST::cast(binaryExpression->right); QVERIFY(literal); QCOMPARE(literal->value, u"\nbye"); - quint32 offset = quint32(leftCode.size() + plusCode.size()); + qsizetype offset = leftCode.size() + plusCode.size(); QCOMPARE(literal->firstSourceLocation().begin(), offset); QCOMPARE(literal->firstSourceLocation().startLine, 1u); QCOMPARE(literal->lastSourceLocation().end(), quint32(code.size())); @@ -383,14 +383,14 @@ void tst_qqmlparser::stringLiteral() literal = QQmlJS::AST::cast(binaryExpression->left); QVERIFY(literal); QCOMPARE(literal->value, u"\nhello\nbye"); - QCOMPARE(literal->firstSourceLocation().begin(), 0u); + QCOMPARE(literal->firstSourceLocation().begin(), 0); QCOMPARE(literal->firstSourceLocation().startLine, 1u); QCOMPARE(literal->lastSourceLocation().end(), leftCode.size()); literal = QQmlJS::AST::cast(binaryExpression->right); QVERIFY(literal); QCOMPARE(literal->value, u"\nbye"); - offset = quint32(leftCode.size() + plusCode.size()); + offset = leftCode.size() + plusCode.size(); QCOMPARE(literal->firstSourceLocation().begin(), offset); QCOMPARE(literal->lastSourceLocation().startLine, 3u); QCOMPARE(literal->lastSourceLocation().end(), code.size()); @@ -495,7 +495,7 @@ void tst_qqmlparser::templateLiteral() auto *templateLiteral = QQmlJS::AST::cast(expression); QVERIFY(templateLiteral); - QCOMPARE(templateLiteral->firstSourceLocation().begin(), 0u); + QCOMPARE(templateLiteral->firstSourceLocation().begin(), 0); auto *e = templateLiteral->expression; QVERIFY(e); } @@ -538,7 +538,7 @@ void tst_qqmlparser::numericSeparator() { QVERIFY(literal); QCOMPARE(literal->value, expected_value); - QCOMPARE(literal->firstSourceLocation().begin(), 0u); + QCOMPARE(literal->firstSourceLocation().begin(), 0); QCOMPARE(literal->lastSourceLocation().end(), quint32(code.size())); }