Re: [boost] smartptr maintained

Subject: Re: [boost] smartptr maintained
From: Karolin Varner (karo_at_[hidden])
Date: 2016-07-15 16:13:07

Morning!

On 05/18/2016 05:51 PM, Peter Dimov wrote:
> Notes on https://github.com/boostorg/smart_ptr/pull/23:
>
> - you're using constexpr without checking whether it's supported; should be BOOST_CONSTEXPR.
> - since you're adding new functionality, the existing test (pointer_cast_test.cpp) should be left alone, and new tests should be added instead.
> - the test uses lambdas without checking whether they're supported.
> - the unique_ptr casts do not seem to properly account for T or U being X cv []. So for example in
>
> std::unique_ptr<Y const[]> p1( new Y[1] );
> std::unique_ptr<Y> p2 = const_pointer_cast<Y>( std::move(p1) );
> std::unique_ptr<Y[]> p3 = const_pointer_cast<Y[]>( std::move(p1) );
>
> the second line compiles and the third doesn't, when the opposite should be the case.

All of those should be fixed now

> - there should be tests that make sure that the legal unique_ptr conversions compile and that the illegal ones do not.
> - there should be tests that check that the existing static_assert guard actually prevents illegal derived to base conversions from compiling.

I must admit I am not entirely sure about the precise semantics here; would you mind actually spelling out which testcases you would like to see?
The current tests I added are based on the test file for the old pointer casts. I found some other tests that invoke pointer casts (list below), but I am not sure these implement the tests you asked for above.
Should we add similar tests for the other smart pointers implemented by boost?

> - ditto for static_assert.
> - it seems a bit odd to disallow static_cast'ing from derived to base with base lacking a virtual destructor and yet allow base to derived.
> I find the static_assert check is a bit odd here, actually, given that unique_ptr<derived> is convertible to unique_ptr<base>. We're in a way being more Catholic than the pope here.

I added that check because of reviews; removed it again!

Best,
Karolin

--
test/shared_from_raw_test.cpp
test/shared_from_this_test.cpp
test/esft_void_test.cpp
test/pointer_cast_test.cpp
test/sp_array_cast_test.cpp
test/cpp11_pointer_cast_test.cpp
test/shared_from_raw_test4.cpp
test/shared_ptr_test.cpp
test/shared_ptr_basic_test.cpp