SourceLocation: make begin() and end() qsizetype

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 <olivier.decanniere@qt.io>

src/qml/common/qqmljssourcelocation_p.h

@@ -66,8 +66,8 @@ public:
 
     bool isValid() const { return *this != SourceLocation(); }
 
-    quint32 begin() const { return offset; }
+    qsizetype begin() const { return qsizetype(offset); }
-    quint32 end() const { return offset + length; }
+    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

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();
 }
 
 /*!

src/qmldom/qqmldomcomments.cpp

@@ -282,8 +282,8 @@ public:
 class ElementRef
 {
 public:
-    ElementRef(AST::Node *node, quint32 size) : element(node), size(size) { }
+    ElementRef(AST::Node *node, qsizetype size) : element(node), size(size) { }
-    ElementRef(const Path &path, FileLocationRegion region, quint32 size)
+    ElementRef(const Path &path, FileLocationRegion region, qsizetype size)
         : element(RegionRef{ path, region }), size(size)
     {
     }
@@ -294,7 +294,7 @@ public:
     ElementRef() = default;
 
     std::variant<AST::Node *, RegionRef> 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();
+            qsizetype start = n->firstSourceLocation().begin();
-            quint32 end = n->lastSourceLocation().end();
+            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<quint32, ElementRef> starts;
+    QMap<qsizetype, ElementRef> starts;
-    QMap<quint32, ElementRef> ends;
+    QMap<qsizetype, ElementRef> 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();
+            qsizetype startI = it.value().begin();
-            quint32 endI = it.value().end();
+            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),
+        return Comment{ m_code.mid(preSpacesIndex, postSpacesIndex - preSpacesIndex),
-                        m_commentLocation,
+                        m_commentLocation, static_cast<int>(preNewlineCount), m_commentType };
-                        static_cast<int>(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();
+        qsizetype iPre = m_commentLocation.begin();
-        int preNewline = 0;
+        qsizetype preNewline = 0;
-        int postNewline = 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

src/qmldom/qqmldomelements.cpp

@@ -1629,20 +1629,17 @@ bool ScriptExpression::iterateDirectSubpaths(const DomItem &self, DirectVisitor
 class FirstNodeVisitor : public VisitAll
 {
 public:
-    quint32 minStart = 0;
+    qsizetype minStart = 0;
-    quint32 maxEnd = std::numeric_limits<quint32>::max();
+    qsizetype maxEnd = std::numeric_limits<qint32>::max(); // see also Lexer::checkFileLength().
     AST::Node *firstNodeInRange = nullptr;
 
-    FirstNodeVisitor(quint32 minStart = 0, quint32 maxEnd = std::numeric_limits<quint32>::max())
+    FirstNodeVisitor(qsizetype minStart, qsizetype maxEnd) : minStart(minStart), maxEnd(maxEnd) { }
-        : minStart(minStart), maxEnd(maxEnd)
-    {
-    }
 
     bool preVisit(AST::Node *n) override
     {
         if (!VisitAll::uiKinds().contains(n->kind)) {
-            quint32 start = n->firstSourceLocation().begin();
+            qsizetype start = n->firstSourceLocation().begin();
-            quint32 end = n->lastSourceLocation().end();
+            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<qint32>::max())
 {
     FirstNodeVisitor visitor(minStart, maxEnd);
     AST::Node::accept(n, &visitor);

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 &currentItem,
@@ -657,7 +657,7 @@ bool QQmlLSCompletion::cursorAfterColon(const DomItem &currentItem,
     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);

src/qmlls/qqmllsutils.cpp

@@ -290,7 +290,7 @@ handlePropertyDefinitionAndBindingOverlap(const QList<ItemLocation> &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<ItemLocation> filterItemsFromTextLocation(const QList<ItemLocation>
     filteredItems.append(*smallest);
 
     const QQmlJS::SourceLocation smallestLoc = smallest->fileLocation->info().fullRegion;
-    const quint32 smallestBegin = smallestLoc.begin();
+    const qsizetype smallestBegin = smallestLoc.begin();
-    const quint32 smallestEnd = smallestLoc.end();
+    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 qsizetype itBegin = itLoc.begin();
-        const quint32 itEnd = itLoc.end();
+        const qsizetype itEnd = itLoc.end();
         if (itBegin == smallestEnd || smallestBegin == itEnd) {
             filteredItems.append(*it);
         }
@@ -364,11 +364,7 @@ QList<ItemLocation> 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() <= targetPos && targetPos < l.end() + (c == Normal ? 1 : 0);
-            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);
-        }
     };
     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,
+Location Location::from(const QString &fileName, const QString &code, qsizetype startLine,
-                        quint32 startCharacter, quint32 length)
+                        qsizetype startCharacter, qsizetype length)
 {
-    const quint32 offset = QQmlLSUtils::textOffsetFrom(code, startLine - 1, startCharacter - 1);
+    const auto offset = QQmlLSUtils::textOffsetFrom(code, startLine - 1, startCharacter - 1);
-    return from(fileName, QQmlJS::SourceLocation{ offset, length, startLine, startCharacter },
+    return from(fileName,
+                QQmlJS::SourceLocation::fromQSizeType(offset, length, startLine, startCharacter),
                 code);
 }
 
-Edit Edit::from(const QString &fileName, const QString &code, quint32 startLine,
+Edit Edit::from(const QString &fileName, const QString &code, qsizetype startLine,
-                quint32 startCharacter, quint32 length, const QString &newName)
+                qsizetype startCharacter, qsizetype length, const QString &newName)
 {
     Edit rename;
     rename.location = Location::from(fileName, code, startLine, startCharacter, length);

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,
+    static Location from(const QString &fileName, const QString &code, qsizetype startLine,
-                         quint32 startCharacter, quint32 length);
+                         qsizetype startCharacter, qsizetype length);
     static Location from(const QString &fileName, const QQmlJS::SourceLocation &sourceLocation,
                          const QString &code);
     static std::optional<Location> 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,
+    static Edit from(const QString &fileName, const QString &code, qsizetype startLine,
-                     quint32 startCharacter, quint32 length, const QString &newName);
+                     qsizetype startCharacter, qsizetype length, const QString &newName);
 
     friend bool operator<(const Edit &a, const Edit &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 qsizetype parentBegin = parent->firstSourceLocation().begin();
-            const quint32 parentEnd = parent->lastSourceLocation().end();
+            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<QQmlJS::AST::StringLiteral *>(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<QQmlJS::AST::StringLiteral *>(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<QQmlJS::AST::StringLiteral *>(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<QQmlJS::AST::StringLiteral *>(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<QQmlJS::AST::StringLiteral *>(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<QQmlJS::AST::TemplateLiteral *>(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()));
 }