Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replaced QVector with QList #7397

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

replaced QVector with QList #7397

wants to merge 1 commit into from

Conversation

firewave
Copy link
Collaborator

it is just an alias

it is just an alias
@chrchr-github
Copy link
Collaborator

chrchr-github commented Mar 23, 2025

Looks like this is true for Qt6+: https://doc.qt.io/qt-6/qvector.html
We should probably adapt qt.cpp to reflect that.

Actually, it sounds like QList has become QVector (elements are contiguous), so QVector seems to be the proper name.
https://doc.qt.io/qt-6/qlist.html

@firewave
Copy link
Collaborator Author

https://doc.qt.io/qt-6/qlist.html#details

QVector<T> used to be a different class in Qt 5, but is now a simple alias to QList.

Note: QList in Qt 5 did not always have a C-compatible array layout and we often recommended to use QVector instead for more predictable performance. This is not the case in Qt 6 anymore, where both classes now share an implementation and can be used interchangeably.

So to be pedantic this would need to wait until Qt 5 support has been removed. And I do not think that it would have any effect on our code.

@chrchr-github
Copy link
Collaborator

I just think it's odd that Qt has changed the behavior of one of its favorite containers so that its now a misnomer...

@danmar
Copy link
Owner

danmar commented Apr 3, 2025

I think the motivation is strange. It's not a problem that the underlying type is QList.

A better motivation would be: "it's more logical to use QList because.." and then motivate why we don't want contiguous memory... and well if we get contiguous memory anyway that is how it is but at least the intent is clear..

I think the typename itself signals intent..

@danmar
Copy link
Owner

danmar commented Apr 3, 2025

I just think it's odd that Qt has changed the behavior of one of its favorite containers so that its now a misnomer...

Yes. Just a wild guess maybe they saw that the "wrong" container was used too much by developers.. or maybe they were lazy and only wanted to maintain 1 implementation :-)

@firewave
Copy link
Collaborator Author

firewave commented Apr 3, 2025

There is a whole official blog post about it: https://www.qt.io/blog/qlist-changes-in-qt-6

@danmar
Copy link
Owner

danmar commented Apr 5, 2025

That does not say that QVector should be avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants