[Development] qsizetype

[Marc Mutz]

Hi,

You may have seen the qsizetype patches we've been pushing over the last 
months. This is to give my perspective on the issue and why I think it's 
needed:

First off, while I made QStringView qsizetype'd, because it was designed 
to take input from STL containers[1], I was not involved in the decision 
to port even just the Qt containers to qsizetype in 6.0.

[1] (it originally used size_t, which is what the STL containers use as 
size_type but it was requested to use a signed type during review)

I do observe, though, that, starting with the premiss of a Qt container 
of size > 2 Gi, the current code base has a pretty high bug density in 
this area:

- https://codereview.qt-project.org/c/qt/qtbase/+/422246
- https://codereview.qt-project.org/c/qt/qtbase/+/422989
- https://codereview.qt-project.org/c/qt/qtbase/+/421912
- https://codereview.qt-project.org/c/qt/qtbase/+/422999
- https://codereview.qt-project.org/c/qt/qtbase/+/403614
- https://codereview.qt-project.org/c/qt/qtbase/+/358114

The goal of the exercise was initially twofold: 1. to fix bugs, assuming 
the Qt containers hold more than 2 Gi of elements and 2. go back to a CI 
build where narrowing conversions cause compile errors. I believe we had 
that in Qt 5, at least on MSVC, and it was apparently disabled for 6.0.

The majority of changes are targeting the second goal. Bugs that fall in 
the first category, like the ones listed above. are user-visible, 
usually get a commit and Jira ticket of their own.

I don't really want to start a discussion on whether (owning) Qt 
containers _should_  support more than 2 Gi elements in the first place. 
>From my pov, that decision has been made in the run-up to 6.0, and 
everyone who knows me knows that I don't much care about the owning Qt 
containers and their shortcomings.

I just want to raise awareness of the issue, which, really, is just an 
incomplete port of the Qt containers to qsizetype in 6.0.

Over the last months, however, I've realized that "just the Qt 
containers" introduces a nasty API inconsistency in Qt: qsizetype in Qt 
containers and int elsewhere. This inconsistency is un-Qt-ish and 
creates problems for our users.

First up, it's unclear what a Qt container is: is QRegion a container? 
What does numRects() return when you setRects with a QList with > 2Gi 
elements? Where do we draw the line between Qt container and Qt 
non-container? See, in particular, QDataStream's inability to serialize 
containers > 2 Gi (and, in the case of QString, even > 1 Gi).

Second, the truncation that users face on c.size() (qsizetype) -> 
setFoo() (int) is using modulo arithmetic, not saturation arithmetic, so 
the result is very unpredictable: INT_MAX + 1 elements turn into 0 
elements, not INT_MAX, etc, creating a lot of opportunities for False 
Positives or Negatives, even with modestly oversized containers (INT_MAX 
+ ε). The central Qt API guideline is to make APIs easy to use and hard 
to misuse. Both parts are violated here, especially since, while 
compilers regularly warn about signed/unsigned mismatches, they seldomly 
warn about narrowing (proof: Qt compiles). To fix the inconsistency 
means to port also the "non-container" APIs to qsizetype, and 
handle/detect/defend against overflow _centrally_, so we don't inflict 
the issue on every user of the APIs.

This should be non-issue for function parameters, because widening isn't 
an error. The value is less pronounced for return values, because 
widening those may cause narrowing in user code, but the hope is that 
once we've progressed somewhat, we can enlist compiler support by 
globally enabling warnings such as -Wshorten-64-to-32, even though, as 
the porting guide included in the 
https://bugreports.qt.io/browse/QTBUG-102461 epic describes, this will 
not catch manual casts, which is why, sadly, one needs to look at every 
int/uint manually.

Comments (and contributions) welcome.

Thanks,
Marc