Merge changes mostly related to compiler warnings

[mrbean-bremen]

• replace NULL/0 with nullptr
• remove unused variables
• add override where needed
• use C++ struct syntax
• fix some more warnings from newer compilers

[he-hesce]

I think it would be better to use Q_NULLPTR as we are using Qt anyway so it is available because nullptr is C++11 and PythonQt still works with pre-C++11 compilers like on RHEL 6. Q_NULLPTR is there as macro to support both pre and post C++11. On C++11 and later it is probably defined as nullptr.

I would also be careful that fixing some of the warnings do not break compatibility with pre-C++11 compilers. I know this can happen from experience. Then it is better to keep the warning and not exclude those who are forced to use for example gcc 4.4 on RHEL 6 still.

[mrbean-bremen]

Thanks - good point about compatibility! I actually wasn't aware that this still supports pre-C++11 compilers. Looking at the supported platforms in Qt5 I see that they support gcc5 as a minimum, which does have C++11 support to my knowledge, so I'm not sure how much sense it makes to support older compilers in PythonQt.

On a related note - it would really help to have some CI builds with different compilers to catch these kinds of problems. I may have a look at this some time later. Also, switching to CMake probably also makes sense at some time, as Qt has done.

For some background: we have a private fork of PythonQt that has these (and other) changes (it also uses CMake for building), and our goal is to eventually make this obsolete and just use the published version, therefore I want to merge back as much as possible (and what makes sense).

[he-hesce]

RHEL/CentOS 6 (still heavily used in scientific and production environments) ship with Qt 5 (5.6 for a while and later 5.9) and gcc 4.4 (and Python 2.7). The majority of scientific Python code in the world is still on Python 2.6 and 2.7 because the Python developers decided to break compatibility with Python 3.

I never see a reason to break compatibility with pre-C++11 compilers just for the sake of it, specially when there are fully working and supported compatibility stuff like Q_NULLPTR and Q_FOREACH/foreach. Specially if the compatible code already exist and work, I see no reason to change it to be incompatible just for the reason to chase the latest standard or latest cool paradigms (cf., Python 2 versus 3).

The only real nice thing in C++11 would be lambda functions which saves a lot of coding
compared to using QSignalHandler but people who can require C++11 can then choose to use those in their code. But it is nice if your supporting libraries such as PythonQt doesn't require C++11 unless it is an absolute necessity.

In the end, there is more "old" and working code running in scientific, banking, or other "legacy" production environments than at your latest start-up/SV web platforms. Not every organization has an unlimited flow of venture capital money to keep changing coding paradigm and rewrite all (working) code every two months just because it is cool to use the latest (or to keep the thousands of employed developers busy).

Personally, I still maintain 40 years old C78 (K&R) standard code where the biggest improvement would be to upgrade it to C89 (one can only dream about C99) but that would take multiple dozens of developer years to do (plus several hundreds of developer years to test and prove with having unit tests that the C89 code behaves like the old well-tested in production C78 code). Luckily, the latest gcc still compiles it and will hopefully continue to do so for the 10-30 years or more the code still has to continue run.

[mrbean-bremen]

Well, I see your point, though I can't say that I agree with all of what you wrote here. There have been reasons for Python 3 as well as for newer C++ versions besides the hype of better and newer, and there are other relevant new things in C++ besides lambdas (though I agree about them). I have written K&R C code myself in my time, and I wouldn't want to (and probably wouldn't be able to) write it in the same way today. Anyway, I don't think it will get us anywhere discussing the merits of new features in programming languages...

More to the point: I see no problem in using the compatibilty stuff if it makes sense. I also understand that there is a lot of old software out there that still needs to run, but my question is if they need new versions of PythonQt. As I understand it, PythonQt will mostly evolve to accomodate newer Qt and Python versions, not to add more features for the existing ones. Also, there is always the possibility to make patch releases for an older version, if really needed.

What I would like to see is a clearly defined minimum compiler version for a specific version of PythonQt, and a CI build that tests this version with several compilers. I really don't think that we will need builds for pre-C++11 compilers for a Qt6 version, but I may be wrong of course. I'll probably wait and see if somebody else chimes in...

[he-hesce]

It is true one can always freeze on an older version of PythonQt and backport fixes from newer versions if needed. I just thought I would voice my opinion as I always run into code changing just because its is cool to use the latest features when the old code worked just fine and then I end up maintaining patch sets to change the code back for compatibility reasons. Cheers!

A classic case is changing ES6 code to ES5 code so it works in QtWebKit. And in the end, most ES6 stuff is just syntactic sugar and doing it the ES5 way works just fine. This is especially annoying when 99% of the code (the older/existing code) is ES5 compatible but then someone adds new code which is ES6 or changes old code to be ES6, I guess, just out of ignorance and not understanding people may still be only able to run ES5 code (like QtWebKit). Thank goodness for open source code so access to source code makes this possible at all!

[mrbean-bremen]

Yes, supporting older code bases is certainly a common problem. I shudder if I imagine the plethora of old Fortran 66 and Cobol code still out there...
I think for small projects like this one the best option is to use release branches to be able to backport fixes if needed, but otherwise just go ahead. I have done this with Python projects with a Python 2 - supporting release branch, and while I did backport bugfixes a couple of times when asked to, that was far less effort than having to support Python 2 all along in the current code.
I just made a new GitHub release for the current state, and if needed we can also make a branch from the release tag. As I wrote, I think it makes sense to support at most the same compilers as Qt does, e.g. gcc5 for Qt 5.15 and gcc 9 for Qt 6.2.

[he-hesce]

I still maintain Fortran 77 code :-) Nobody is going to rewrite it when it works. And it continues to link with and be used from C99 and C++ code.

I already have to use the official revert-refcount-fixes branch as otherwise my use of PythonQt crashes (see issue #33). So a branch compatible with pre-C++11 is okay.

As for Python 2/3, it's not that hard to write code which works on both 2.7 and 3.6/8/9. Just takes some effort and testing. Though when done better than implementing new features in two copies of the code.

It should be noted that RHEL/CentOS 7.9 ships with Qt 5.9 and gcc 4.8 and it is not EOL yet (1.5 years left for security updates and then probably another 5 years of Extended Life Cycle Support after that). (Though gcc 4.8 can compile C++11 code.) So not everyone is using Qt 5.15. Qt 6 will likely be something production environments can look at in 10-15 years from now. 5.x series will be used for a long time. If one spent a decade porting a 30 year old GUI app from Motif/Athena/Xt to Qt 5, it will need to run for another 20-30 years at least before looking at porting to a new GUI kit. Similarly, after transitioning from a bespoke embedded extension language or Scheme to Python (via PythonQt) as embedded extension language, one will not do such an embedded language transition again for another couple of decades either.

You may find https://www.nature.com/articles/d41586-021-01431-y a fun read.

Cheers and Happy New Year!

[mrbean-bremen]

I still maintain Fortran 77 code :-) Nobody is going to rewrite it when it works. And it continues to link with and be used from C99 and C++ code.

Well, lucky you :) I didn't have much to do with Fortran - though when I did (also Fortran 77), I can't say that I liked it (compared to K&R C I was mainly using at the time).

As for Python 2/3, it's not that hard to write code which works on both 2.7 and 3.6/8/9. Just takes some effort and testing. Though when done better than implementing new features in two copies of the code.

Yes, I did this also, but it really depends on the usage. If there is almost no use case for Python 2 left, and some Python3 features are actually needed for full functionality, it just isn't worth it to maintain both versions. But I know that are still many packages out there that only support Python 2, though with the Python 2 sunsetting effort their number has been largely reduced.

so not everyone is using Qt 5.15. Qt 6 will likely be something production environments can look at in 10-15 years from now. 5.x series will be used for a long time.

Well, that's a bit overly pessimistic IMHO. I know that this is certainly true for some environments, but I also know that production environments have switched to Qt 6, or switching right now. But I get your point, especially for embedded and high-risk environments.

You may find https://www.nature.com/articles/d41586-021-01431-y a fun read.

Thanks, always one for a fun read :)

Happy New Year to you too!

[he-hesce]

If you just moved to RHEL/Rocky 9 you will be using Qt 5.15 for another 10-15 years supported and probably more than that unsupported. Perhaps RHEL/Rocky 10 will ship with Qt 6 in 3-4 years from now.

As for Python 2, RHEL/CentOS 7 still ships with and supports Python 2.7 for another 1.5-6.5 years from now. Hence, if you're on RHEL/CentOS 7 that is what you have to use. They threw in Python 3.6 in the final feature update to 7 but without any modules and not default, so half-hearted desperate move before 7 moved into security-only support mode.

[mrbean-bremen]

As for Python 2, RHEL/CentOS 7 still ships with and supports Python 2.7 for another 1.5-6.5 years from now. Hence, if you're on RHEL/CentOS 7 that is what you have to use. They threw in Python 3.6 in the final feature update to 7 but without any modules and not default, so half-hearted desperate move before 7 moved into security-only support mode.

Yes, I know, but that just means that they will use a version of the package that still supports that version, and not the newest one. CentOS is basically dead (thanks to Red Hat), e.g. while it still may be used for a long time, it is not expected to need any new features. They won't get any more security fixes for Python 2 anyway, so they should not expect such fixes for any other Python 2 packages (though if really needed, these can still be done).

[he-hesce]

All true what you say. Though the fun for us developers begins when you have to support the latest version of your own software (containing the feature the users asked for) on RHEL 6, 7, 8 & 9 as your users can be on any of these platforms and unable to move beyond what they are on. That is when you appreciate your dependencies such as PythonQt still working on all of these with the same code so you don't need to keep different copies of PythonQt in your source tree for linking into application depending on gcc/Python version. Of course, I know this is a lot to ask for an open-source project to support when these ancient versions of gcc/Python are officially not supported anymore. So all I am asking for in this pull request is to use Q_NULLPTR instead of nullptr as an example because that will give you the same benefit on gcc 4.8+ as using nullptr directly but still continue to work for those without nullptr.

[mrbean-bremen]

Well, that would be a fair point, but while trying to setup building a CI build for PythonQt (I will make a separate PR for this) I noticed that the current version already does not compile with pre-C++11 compilers.
The gcc 5.5 build failed because of that (I didn't add the -std=c++11 flag that is needed here to compile C++11 code, so it compiled in c++98 mode).
It fails on some move constructors and move assignment operators that are in the code since 2018, so I really don't see how you can use the current version with a pre-gcc4.8 compiler. Maybe I'm missing something here...

[he-hesce]

There are indeed pre-existing code which has to be re-written back to C++98 (or actually just use the Qt syntactic sugar macros instead of C++11 syntax) each time one wants to use a new release. I would have stayed at r502 if it wasn't for that I needed Python 3.8+ support. And then when moving up to that commit I have to rebase to remove a bunch of commits to avoid segfaults (#33). So it ain't just downloading the HEAD of master branch and get going :-)

I also ran into a header file having been moved in Python 3.10 on Ubuntu 22.04 so I will submit an issue for that (see #81).

[florianlink]

I think we should revert the leak fixes that have led to the segfaults. It seems like one if the changes was too aggressive, and since we did not spot which one, I would vote for reverting.

…

On Thu 29. Dec 2022 at 17:50, HE ***@***.***> wrote: There are indeed pre-existing code which has to be re-written back to C++98 each time one wants to use a new release. I would have stayed at r502 if it wasn't for that I needed Python 3.8+ support. And then when moving up to that commit I have to rebase to remove a bunch of commits to avoid segfaults (#33 <#33>). So it ain't just downloading the HEAD of mater branch and get going :-) I also ran into a header file having been moved in Python 3.10 on Ubuntu 22.04 so I will submit an issue for that. — Reply to this email directly, view it on GitHub <#79 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKHPHB3OYIFGN6LZBEVWXLWPW6OHANCNFSM6AAAAAATHYUFOU> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[mrbean-bremen]

@he-hesce - is it possible to reproduce this segfault, maybe in a docker container that can be run in the CI? This would make it easier to pin it down to the problematic change.

Using the Qt macros instead of the C++11 constructs should not be a problem, but this still leaves at least the move constructors and assignment operators - I didn't check if there are more problems with compiling in C++-98 mode. Though maybe that isn't actually much and can actually be wrapped...

[he-hesce]

The problem reproducing is in creating a minimal test case for this to reproduced. Right now there is a whole application with many layers where this is happening which also makes it difficult to create a test case as it requires the whole call chain and signaling and so on. We discussed this in #33. Another user also had the same issue. Basically it comes down to some race condition between calls and too aggressive garbage collection where it seems objects are freed while they are still in use.

[mrbean-bremen]

Ok, I had to ask :) In this case it is probably really the best to just revert the changes as in that branch, as proposed by @florianlink. Though I would at least wait for the opinion of @usiems, who created the revert-refcount-fixes branch.

[he-hesce]

Here are two changes from C++11 to Qt syntactic sugar macros to libsrc/PythonQt/src/PythonQtMethodInfo.cpp:

-  for (const ParameterInfo& info : _parameters) {
+  Q_FOREACH (const ParameterInfo& info, _parameters) {

and libsrc/PythonQt/src/PythonQtSignalReceiver.cpp:

-  for(const PythonQtSignalTarget& t : _targets) {
+  Q_FOREACH (const PythonQtSignalTarget& t, _targets) {

Q_FOREACH used in many other places before this but then new code was added using C++11-style for (C++11 range-based for loops) instead.

[mrbean-bremen]

Q_FOREACH used in many other places before this but then new code was added using C++11-style for (C++11 range-based for loops) instead.

This is a case which I don't really like - Q_FOREACH is convenient, but implemented as an ugly macro that is problematic to debug. Well, you can't have everything...

[he-hesce]

I think Q_FOREACH is Qt 4 and Qt 5 got foreach. Not sure if they are different or the same implementation behind the scenes.
https://doc.qt.io/qt-6/foreach-keyword.html

Though this is not a hill I am going to die on. Just voicing my opinion that compatible code is preferable if possible.

[mrbean-bremen]

Yeah, as far as I remember foreach is the same as Q_FOREACH, just better readable. As they write on the the site you point to:

Note: The foreach keyword was introduced before the C++11 range-based loops existed. New code should prefer C++11 range-based loops.

[mrbean-bremen]

I tried compiling with -std=c++98 and noticed that this is not possible with Qt 5.15 - it does use C++11 itself (somewhat predictable given the supported compiler versions). There are many things that cannot easily be hidden just using the provided macros.

As further changes in PythonQt will be mainly for Qt 5.15 and Qt 6 support, I don't think it is worth the effort to try to make the master branch C++98-compatible. We probably should make sure that it is C++11-compatible by using the respective compiler flag for Qt 5 builds, though.

[he-hesce]

Ok. C++11 is fine :-) RHEL/CentOS 7 has gcc 4.8 which supports C++11. However, RHEL/CentOS 7 still ships with Qt 5.9.7 so not breaking compatibility with pre-Qt-5.15 would be nice.

Luckily RHEL/Rocky 8 and RHEL/Rocky 9 both ship with Qt 5.15.3.

Indeed, changing Qt 4 macros to Qt 5 macros (such as Q_FOREACH to foreach) or C++11 would be okay as I don't think Qt 4 compatibility is required anymore by anyone (hopefully).

[he-hesce]

I am still using generated_cpp_56 as it is the last one including QtWebKit. QtWebKit is shipped with RHEL 6/7/8/9 and Ubuntu 18.04/20.04/22.04 LTS and still used by many, though not part of official Qt anymore since they dropped it after Qt 5.6. It is maintained/developed separately from the Qt Project now. I once managed to re-generate the files for QtWebKit for Qt 5.9 but could never replicate it for Qt 5.12 and 5.15. So still using good old stable generated_cpp_56 with Qt 5.9 and 5.15 which works fine as long I don't need to use any API added after Qt 5.6 or in QtWebKit versions after 5.6 (which is actually never so far).

[mrbean-bremen]

RHEL/CentOS 7 has gcc 4.8 which supports C++11. However, RHEL/CentOS 7 still ships with Qt 5.9.7 so not breaking compatibility with pre-Qt-5.15 would be nice.

A docker image with CentOS 7 that builds pythonqt could ensure this, I will try to set this up.

[mrbean-bremen]

This is ready for merge - it basically merges all relevant changes that we had in our local fork (except the CMAKE-specific files). I split the changes into several commits, and I added the define PYTHON2 for a Python3-specifix feature (last commit) made by Florian. If compiling for Python 2, this has currently to be set manually as done in the CentOS build (I failed to correctly define it automatically in the prf).