Am 30.11.23 um 18:49 schrieb Andrey Semashev via Boost:
3. #ifdef BOOST_HAS_PRAGMA_ONCE : why bother? Why not? It is extra code without any benefit, isn't it?
6. include/boost/scope/detail/is_not_like.hpp: no human readable explanation of what this type-trait is supposed to do. It tests whether the left-hand type is not an instantiation of the right-hand template. I think the point was that this explanation should be in the file. I don't want to include <functional> just for std::reference_wrapper. I don't want to include <algorithm> just for std::swap. I don't want to include <utility> just for std::move. I don't want to include <functional> just for that trivial function object wrapper. I collected all such statements from your replies and would like to object: Reimplementing std-library components to avoid an include looks very wrong to me.
Not only is it likely premature optimization: User code including your headers likely included those files already making the include "free". Especially as many std-headers transitively include <utility> or even <algorithm>. And 2nd: With the collection above there are now 2 points obvious that <functional> is not included "just for x" and then "just for y". So now you have 2 reasons already to include it. And finally this is about readability: Everyone knows what `std::move(that)` does, but it is not that obvious for `static_cast< unique_resource_data&& >(that)` Similar for swapping "m_allocated" being expanded to 3 lines instead of `std::swap` obfuscating what it does. And ref_wrapper begs the question if and how it is different to std::reference_wrapper TLDR: I'd strongly suggest to use the std-library where possible to a) not reinvent the wheel and b) improve readability by using existing idioms/names. And a possible bug I've just seen: template< typename R, typename D,...> explicit unique_resource_data(R&& res, D&& del)... : unique_resource_data(static_cast< R&& >(res), static_cast< D&& >(del), traits_type::is_allocated(static_cast< R&& >(res))) { } The `static_cast<R&&>` look wrong. Should that be a std::move or std::forward? It is std::forward, but I had to check to be sure. (see above where a similar cast was a std::move) To me it looks like it`is_allocated` should not have a cast, should it? What if it is implemented to accept an instance, you castit to an rvalue, i.e. move it into that call and then use the moved-from value to construct `unique_resource_data` Similar for the deleter where using std::forward makes the intent clear. So please use the existing vocabulary types/functions/idioms. Alex