search

mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.25k stars 1.24k forks source link

Improve use of parented_ptr #13411

Closed daschuer closed 3 days ago

daschuer commented 4 days ago

This PR fixed some cases where a patented_ptr is constructed without a parent. This creates a pointer that is temporary without any owner.

I have moved the assertion to the constructor to avoid this situation in future.

The risk is that we with debug assertion enabled, Mixxx will crash in case I have missed a case. So a double check during review is required.

This is part of the pending discussion about the future of parented_ptr in: https://github.com/mixxxdj/mixxx/pull/13395

Swiftb0y commented 4 days ago

I have moved the assertion to the constructor to avoid this situation in future.

Wdyt about adding some compile time checks to make_parented? This code will fail if make_parented is called without any QObjects (if concepts don't work in all the compilers, you can also replace it with constexpr bool and static_assert). I know its a bit liberal in the sense that it can't know whether the passed object is actually the parent, but it can know that the code is definitely wrong if none of the parameters is a QObject.

template<typename... Args>
concept AnyQObject = (... || std::is_base_of_v<QObject, Args>);

template<typename T, typename... Args> 
parented_ptr<T> make_parented(Args...) requires (AnyQObject<Args...>)
Swiftb0y commented 4 days ago

whoops. sorry its 

template<typename... Args>
constexpr bool AnyQObject = (... || std::is_base_of_v<QObject, std::remove_pointer_t<Args>>);
Swiftb0y commented 3 days ago

ah nice, the static check actually caught something

daschuer commented 3 days ago

All green finally. Thank you for your perfect idea with the compile time check.