[Development] qsizetype

Alex Blasche alexander.blasche at qt.io
Mon Sep 5 14:46:10 CEST 2022


> -----Original Message-----
> From: Development <development-bounces at qt-project.org> On Behalf Of
> Marc Mutz

I am taking the hot potato and spin this futher.


> 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:

Absolutely this must be fixed. Those are plain bugs.
<...>

> 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.

Indeed. It was decided.

 
> 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.

I like the container.size() vs setFoo(int) use case as example to decide how far we want to spin this inconsistency further ahead throughout all of Qt . I totally agree with Marc's argument that it is not Qtish and causes pain to every user.  I guess we have three options:

1.) What I am wondering though is if we were to sink all the time into Qt to convert all API's, would Qt be blamed for all the continued porting effect throughout the years on this issue? After all we cannot do this for one single release in one go. It would probably find its way into each release over a couple of years. Does this help Qt in its goal to be easy to use? The compatibility layer does add its own layer of complexity.

2.) An alternative might be to make this change in one go for Qt 7. We would keep Qt 6.x on the status quo but start adding compatible replacement APi with an absolute change at 7.0 (ifdefs or typedefs come to mind). Users would only be burdened one time (though it being one BIG time effort). Such a change would be much simpler in Qt headers.

3.) On the other hand, we have had a lot of Qt 6 ports already and so far, it did not come up as a major issue in those ports. Could we simply accept this inconsistency? After all, would QSplitter ever need setWidget(qsizetype, QWidget*)? Under this scenario we might agree to convert all internal Qt uses of containers but leave the public API untouched. Of course some dedicated use case, where extra large files/chunks of data should be read, could get the treatment much earlier.

Personally, I cannot agree whether I like Option 2 or 3 more.
--
Alex




More information about the Development mailing list