Best practice for QTs parent/child memory model and clang-tidy

Norme

Hi,

I am interested in different ideas/opinions regarding how to make clang-tidy happy with regards to the "modern" way of writing, and how we usually write QT code.

Given the following standard code:

auto* widget = new QGraphicsWidget;
auto* layout = new QGraphicsLinearLayout(widget);
...

We will receive two identical clang-tidy errors, where for example one of them will say: Initializing non-owner 'QGraphicsLinearLayout *' with a newly created 'gsl::owner<>'.

Now, we can remove the warning for the QGraphicsWidget by just using a QPointer for the QObject-based class:

auto widget = QPointer(new QGraphicsWidget);
auto* layout = new QGraphicsLinearLayout(widget);
...

However this won't solve the warning for the QGraphicsLinearLayout. For that one we need to perhaps use a gsl::owner or similar. It's not possible to use a unique_ptr since the layout will then be deleted when it goes out of scope. I am just overall confused on how to please static code analyzers when it comes to the QT way of writing. We usually declare a lot of objects on the heap with new and then let the parent/child-model handle the deletion.

Are there any best practices regarding this? Yes, of course we can silence the warnings, but I would like to explore if there is anyone else that has stumbled across this scenario and if you found a good way of handling it. Do you wrap everything in gsl::owner? Do you use QPointer where it's possible? Any other ways?

Thanks for any input.

Best regards,
Norme

sierdzio

In theory you could do this:

auto widget = QPointer(new QGraphicsWidget);
widget->setLayout(new QGraphicsLinearLayout);
auto layout = widget->layout();

However, I think there is no need. The Qt way of handling objects - although outdated and by now unpopular - is valid, safe and correct. So I'd say ignoring clang-tidy is the way to go in this case.

Kent-Dorfman

a couple of things, but as always, just my opinion...

auto* is discouraged in favor of auto by itself, as it will correctly deduce when it is to produce a pointer type
when you choose to use a framework you should give priority to the recommended mechanisms of that framework, over other "stylistic preferences"

My preference is the old syntax because it is "comfortable and obvious".

Christian Ehrlicher

@sierdzio said in Best practice for QTs parent/child memory model and clang-tidy:

auto widget = QPointer(new QGraphicsWidget);

This just creates a QPointer which gets destroyed on leaving the scope for the sake of satisfying a tool. This is anti-programming in my pov.

SimonSchroeder

You should not change the way you write code based on a linter if the linter is wrong. However, widget should have a parent (maybe that happens when it is added to a layout).

If you are using a linter, you shouldn't just turn it off entirely. So, the option that you have is to turn specific linting options off on a per line basis (https://stackoverflow.com/questions/37950439/inline-way-to-disable-clang-tidy-checks): Use NOLINT(...) on specific lines or NOLINTBEGIN(...)/NOLINTEND(...) on the whole section of code creating widgets and layouts. Make sure to just disable that one specific warning.