[PATCH] D15910: Make isa, cast, dyn_cast, etc. work with std::unique_ptr and std::shared_ptr.

[Justin Lebar via llvm-commits]

> I worry about accidentally escaping (and then capturing) the raw pointer via a type cast.

Right.  I considered that maybe it would be better if we made it so
that dyn_cast / cast don't work on rvalue references, but then
perfectly legal code like

  unique_ptr<Foo> GetFoo();
  cast<Bar>(GetFoo())->Frob();

wouldn't work.

My main motivation for this patch was that IntrusiveRefCntPtr already
implements simplify_type, and has exactly the problem as shared_ptr,
as far as I can tell.  And since a shared_ptr may have just one owner,
I don't think it is any more or less safe in this respect than
unique_ptr.  Of course two wrongs don't make a right, but consistency
is good.

But maybe we're saying that the right thing to do would be to remove
the simplify_type specialization for IntrusiveRefCntPtr.  We could
make it work only for isa, but that may be an excessive amount of
magic for not a lot of instances.

On Wed, Jan 13, 2016 at 10:47 PM, Justin Bogner <mail at justinbogner.com> wrote:
> Justin Lebar via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> jlebar created this revision.
>> jlebar added a reviewer: echristo.
>> jlebar added a subscriber: llvm-commits.
>>
>> This lets us use these functions without calling 'get()' everywhere.
>
> Making this work for isa<> is really nice, but I'm kind of uncomfortable
> witht the subtlety it adds to cast/dyn_cast/dyn_cast_or_null wrt
> unique_ptr. I worry about accidentally escaping (and then capturing) the
> raw pointer via a type cast.
>
>> http://reviews.llvm.org/D15910
>>
>> Files:
>>   include/llvm/Support/Casting.h
>>   unittests/Support/Casting.cpp
>>
>> Index: unittests/Support/Casting.cpp
>> ===================================================================
>> --- unittests/Support/Casting.cpp
>> +++ unittests/Support/Casting.cpp
>> @@ -165,6 +165,26 @@
>>    EXPECT_NE(F5, null_foo);
>>  }
>>
>> +// We should be able to treat unique_ptr<T> just like T* for the purposes of
>> +// isa, dyn_cast, cast, etc.
>> +TEST(CastingTest, unique_ptr) {
>> +  std::unique_ptr<foo> a(new foo());
>> +  EXPECT_TRUE(isa<foo>(a));
>> +
>> +  std::unique_ptr<bar> b(new bar());
>> +  EXPECT_NE(dyn_cast_or_null<foo>(b), null_foo);
>> +}
>> +
>> +// We should be able to treat shared_ptr<T> just like T* for the purposes of
>> +// isa, dyn_cast, cast, etc.
>> +TEST(CastingTest, shared_ptr) {
>> +  auto a = std::make_shared<foo>();
>> +  EXPECT_TRUE(isa<foo>(a));
>> +
>> +  auto b = std::make_shared<bar>();
>> +  EXPECT_NE(dyn_cast_or_null<foo>(b), null_foo);
>> +}
>> +
>>  // These lines are errors...
>>  //foo *F20 = cast<foo>(B2);  // Yields const foo*
>>  //foo &F21 = cast<foo>(B3);  // Yields const foo&
>> Index: include/llvm/Support/Casting.h
>> ===================================================================
>> --- include/llvm/Support/Casting.h
>> +++ include/llvm/Support/Casting.h
>> @@ -18,6 +18,7 @@
>>  #include "llvm/Support/Compiler.h"
>>  #include "llvm/Support/type_traits.h"
>>  #include <cassert>
>> +#include <memory>
>>
>>  namespace llvm {
>>
>> @@ -47,6 +48,30 @@
>>    }
>>  };
>>
>> +template<class T> struct simplify_type<std::unique_ptr<T>> {
>> +  typedef T* SimpleType;
>> +  static SimpleType getSimplifiedValue(std::unique_ptr<T> &Val) { return Val.get(); }
>> +};
>> +
>> +template<class T> struct simplify_type<const std::unique_ptr<T>> {
>> +  typedef /*const*/ T* SimpleType;
>> +  static SimpleType getSimplifiedValue(const std::unique_ptr<T> &Val) {
>> +    return Val.get();
>> +  }
>> +};
>> +
>> +template<class T> struct simplify_type<std::shared_ptr<T>> {
>> +  typedef T* SimpleType;
>> +  static SimpleType getSimplifiedValue(std::shared_ptr<T> &Val) { return Val.get(); }
>> +};
>> +
>> +template<class T> struct simplify_type<const std::shared_ptr<T>> {
>> +  typedef /*const*/ T* SimpleType;
>> +  static SimpleType getSimplifiedValue(const std::shared_ptr<T> &Val) {
>> +    return Val.get();
>> +  }
>> +};
>> +
>>  // The core of the implementation of isa<X> is here; To and From should be
>>  // the names of classes.  This template can be specialized to customize the
>>  // implementation of isa<> without rewriting it from scratch.
>>