<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 3, 2014 at 2:13 PM, Pete Cooper <span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On Dec 3, 2014, at 2:00 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Nov 28, 2014 at 6:14 PM, Duncan P. N. Exon Smith<span> </span><span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span><br>> On 2014 Nov 24, at 09:01, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>><br>><br>><br>> On Sun, Nov 23, 2014 at 7:13 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>> Author: dexonsmith<br>> Date: Sun Nov 23 21:13:02 2014<br>> New Revision: 222644<br>><br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=222644&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222644&view=rev</a><br>> Log:<br>> Support: Add *cast_or_null<> for pointer wrappers<br>><br>> I'm assuming this is intended to work with unique_ptr? (is it worth testing with unique_ptr itself, then? (I'm honestly not sure))<br><br></span>Well, yes, but not only `unique_ptr<>`.  My motivating use case is a<br>yet-to-be-committed pointer-wrapper for `Metadata` operands, similar<br>to `Use` for `Value`.<br><br>I think testing a dumb pointer-wrapper directly matches better how<br>`simpify_type` is implemented.  And it's a bit more flexible if at<br>some point we want to support a type that supports `simplify_type`<br>but doesn't have an `operator bool()`.<br></blockquote><div><br>Hmm, OK. (we could use "== nullptr" as the boolean test, rather than operator bool, not sure either's better, so long as it can cope with explicit operator bool, which I see you tested)<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span><br>><br>> In some cases, we actually want ownership transfer:<br>><br>>   unique_ptr<Base> b = ...;<br>>   unique_ptr<Derived> d = dyn_cast<Derived>(b);<br>>   assert(b ^ d);<br>><br>> But I guess it's probably not the right thing to use the existing names/templates for this task? (if we mostly /don't/ want to transfer ownership, which I guess is probably the case(?)) Maybe we'll eventually add some new names for this (*cast*_transfer(?))<br></span></blockquote></div></div></div></div></blockquote></span>cast*_move perhaps?  Given that i’d write std::move to transfer ownership from one unique_ptr to another, would it be reasonable to overload that name here?</div></div></blockquote><div><br>Quite possibly.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><span class="HOEnZb"><font color="#888888"><div><br></div></font></span><div><span class="HOEnZb"><font color="#888888">Pete</font></span><div><div class="h5"><br><blockquote type="cite"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span><br></span>I agree we would need different names, but I'm not sure it fits in<br>the same library -- these are meant to be lightweight RTTI<br>replacements.  Ownership transfer seems perpendicular to RTTI.<br></blockquote><div><br>Except it unfortunately isn't - see <a href="http://en.cppreference.com/w/cpp/memory/shared_ptr/pointer_cast" target="_blank">http://en.cppreference.com/w/cpp/memory/shared_ptr/pointer_cast</a><span> </span>- mostly because those shared_ptr is more finicky to transfer ownership while changing types, etc. So there aren't equivalents for unique_ptr - but like make_shared, there's some convenience in also having make_unique (& thus in having unique_casts).<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Nevertheless, I wouldn't be against having convenient wrappers for<br>casting+transfer if we have enough use cases for it...<br></blockquote><div><br>*nod* some day, perhaps<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><br>><br>><br>> Fill in omission of `cast_or_null<>` and `dyn_cast_or_null<>` for types<br>> that wrap pointers (e.g., smart pointers).<br>><br>> Type traits need to be slightly stricter than for `cast<>` and<br>> `dyn_cast<>` to resolve ambiguities with simple types.<br>><br>> There didn't seem to be any unit tests for pointer wrappers, so I tested<br>> `isa<>`, `cast<>`, and `dyn_cast<>` while I was in there.<br>><br>> This only supports pointer wrappers with a conversion to `bool` to check<br>> for null.  If in the future it's useful to support wrappers without such<br>> a conversion, it should be a straightforward incremental step to use the<br>> `simplify_type` machinery for the null check.  In that case, the unit<br>> tests should be updated to remove the `operator bool()` from the<br>> `pointer_wrappers::PTy`.<br>><br>> Modified:<br>>     llvm/trunk/include/llvm/Support/Casting.h<br>>     llvm/trunk/unittests/Support/Casting.cpp<br>><br>> Modified: llvm/trunk/include/llvm/Support/Casting.h<br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Casting.h?rev=222644&r1=222643&r2=222644&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Casting.h?rev=222644&r1=222643&r2=222644&view=diff</a><br>> ==============================================================================<br>> --- llvm/trunk/include/llvm/Support/Casting.h (original)<br>> +++ llvm/trunk/include/llvm/Support/Casting.h Sun Nov 23 21:13:02 2014<br>> @@ -243,6 +243,26 @@ inline typename cast_retty<X, Y *>::ret_<br>>  // accepted.<br>>  //<br>>  template <class X, class Y><br>> +LLVM_ATTRIBUTE_UNUSED_RESULT inline typename std::enable_if<<br>> +    !is_simple_type<Y>::value, typename cast_retty<X, const Y>::ret_type>::type<br>> +cast_or_null(const Y &Val) {<br>> +  if (!Val)<br>> +    return nullptr;<br>> +  assert(isa<X>(Val) && "cast_or_null<Ty>() argument of incompatible type!");<br>> +  return cast<X>(Val);<br>> +}<br>> +<br>> +template <class X, class Y><br>> +LLVM_ATTRIBUTE_UNUSED_RESULT inline typename std::enable_if<<br>> +    !is_simple_type<Y>::value, typename cast_retty<X, Y>::ret_type>::type<br>> +cast_or_null(Y &Val) {<br>> +  if (!Val)<br>> +    return nullptr;<br>> +  assert(isa<X>(Val) && "cast_or_null<Ty>() argument of incompatible type!");<br>> +  return cast<X>(Val);<br>> +}<br>> +<br>> +template <class X, class Y><br>>  LLVM_ATTRIBUTE_UNUSED_RESULT inline typename cast_retty<X, Y *>::ret_type<br>>  cast_or_null(Y *Val) {<br>>    if (!Val) return nullptr;<br>> @@ -282,6 +302,20 @@ dyn_cast(Y *Val) {<br>>  // value is accepted.<br>>  //<br>>  template <class X, class Y><br>> +LLVM_ATTRIBUTE_UNUSED_RESULT inline typename std::enable_if<<br>> +    !is_simple_type<Y>::value, typename cast_retty<X, const Y>::ret_type>::type<br>> +dyn_cast_or_null(const Y &Val) {<br>> +  return (Val && isa<X>(Val)) ? cast<X>(Val) : nullptr;<br>> +}<br>> +<br>> +template <class X, class Y><br>> +LLVM_ATTRIBUTE_UNUSED_RESULT inline typename std::enable_if<<br>> +    !is_simple_type<Y>::value, typename cast_retty<X, Y>::ret_type>::type<br>> +dyn_cast_or_null(Y &Val) {<br>> +  return (Val && isa<X>(Val)) ? cast<X>(Val) : nullptr;<br>> +}<br>> +<br>> +template <class X, class Y><br>>  LLVM_ATTRIBUTE_UNUSED_RESULT inline typename cast_retty<X, Y *>::ret_type<br>>  dyn_cast_or_null(Y *Val) {<br>>    return (Val && isa<X>(Val)) ? cast<X>(Val) : nullptr;<br>><br>> Modified: llvm/trunk/unittests/Support/Casting.cpp<br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Casting.cpp?rev=222644&r1=222643&r2=222644&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Casting.cpp?rev=222644&r1=222643&r2=222644&view=diff</a><br>> ==============================================================================<br>> --- llvm/trunk/unittests/Support/Casting.cpp (original)<br>> +++ llvm/trunk/unittests/Support/Casting.cpp Sun Nov 23 21:13:02 2014<br>> @@ -232,3 +232,99 @@ namespace TemporaryCast {<br>>  struct pod {};<br>>  IllegalCast *testIllegalCast() { return cast<foo>(pod()); }<br>>  }<br>> +<br>> +namespace {<br>> +namespace pointer_wrappers {<br>> +<br>> +struct Base {<br>> +  bool IsDerived;<br>> +  Base(bool IsDerived = false) : IsDerived(IsDerived) {}<br>> +};<br>> +<br>> +struct Derived : Base {<br>> +  Derived() : Base(true) {}<br>> +  static bool classof(const Base *B) { return B->IsDerived; }<br>> +};<br>> +<br>> +class PTy {<br>> +  Base *B;<br>> +public:<br>> +  PTy(Base *B) : B(B) {}<br>> +  LLVM_EXPLICIT operator bool() const { return get(); }<br>> +  Base *get() const { return B; }<br>> +};<br>> +<br>> +} // end namespace pointer_wrappers<br>> +} // end namespace<br>> +<br>> +namespace llvm {<br>> +<br>> +template <> struct simplify_type<pointer_wrappers::PTy> {<br>> +  typedef pointer_wrappers::Base *SimpleType;<br>> +  static SimpleType getSimplifiedValue(pointer_wrappers::PTy &P) {<br>> +    return P.get();<br>> +  }<br>> +};<br>> +template <> struct simplify_type<const pointer_wrappers::PTy> {<br>> +  typedef pointer_wrappers::Base *SimpleType;<br>> +  static SimpleType getSimplifiedValue(const pointer_wrappers::PTy &P) {<br>> +    return P.get();<br>> +  }<br>> +};<br>> +<br>> +} // end namespace llvm<br>> +<br>> +namespace {<br>> +namespace pointer_wrappers {<br>> +<br>> +// Some objects.<br>> +pointer_wrappers::Base B;<br>> +pointer_wrappers::Derived D;<br>> +<br>> +// Mutable "smart" pointers.<br>> +pointer_wrappers::PTy MN(nullptr);<br>> +pointer_wrappers::PTy MB(&B);<br>> +pointer_wrappers::PTy MD(&D);<br>> +<br>> +// Const "smart" pointers.<br>> +const pointer_wrappers::PTy CN(nullptr);<br>> +const pointer_wrappers::PTy CB(&B);<br>> +const pointer_wrappers::PTy CD(&D);<br>> +<br>> +TEST(CastingTest, smart_isa) {<br>> +  EXPECT_TRUE(!isa<pointer_wrappers::Derived>(MB));<br>> +  EXPECT_TRUE(!isa<pointer_wrappers::Derived>(CB));<br>> +  EXPECT_TRUE(isa<pointer_wrappers::Derived>(MD));<br>> +  EXPECT_TRUE(isa<pointer_wrappers::Derived>(CD));<br>> +}<br>> +<br>> +TEST(CastingTest, smart_cast) {<br>> +  EXPECT_TRUE(cast<pointer_wrappers::Derived>(MD) == &D);<br>> +  EXPECT_TRUE(cast<pointer_wrappers::Derived>(CD) == &D);<br>> +}<br>> +<br>> +TEST(CastingTest, smart_cast_or_null) {<br>> +  EXPECT_TRUE(cast_or_null<pointer_wrappers::Derived>(MN) == nullptr);<br>> +  EXPECT_TRUE(cast_or_null<pointer_wrappers::Derived>(CN) == nullptr);<br>> +  EXPECT_TRUE(cast_or_null<pointer_wrappers::Derived>(MD) == &D);<br>> +  EXPECT_TRUE(cast_or_null<pointer_wrappers::Derived>(CD) == &D);<br>> +}<br>> +<br>> +TEST(CastingTest, smart_dyn_cast) {<br>> +  EXPECT_TRUE(dyn_cast<pointer_wrappers::Derived>(MB) == nullptr);<br>> +  EXPECT_TRUE(dyn_cast<pointer_wrappers::Derived>(CB) == nullptr);<br>> +  EXPECT_TRUE(dyn_cast<pointer_wrappers::Derived>(MD) == &D);<br>> +  EXPECT_TRUE(dyn_cast<pointer_wrappers::Derived>(CD) == &D);<br>> +}<br>> +<br>> +TEST(CastingTest, smart_dyn_cast_or_null) {<br>> +  EXPECT_TRUE(dyn_cast_or_null<pointer_wrappers::Derived>(MN) == nullptr);<br>> +  EXPECT_TRUE(dyn_cast_or_null<pointer_wrappers::Derived>(CN) == nullptr);<br>> +  EXPECT_TRUE(dyn_cast_or_null<pointer_wrappers::Derived>(MB) == nullptr);<br>> +  EXPECT_TRUE(dyn_cast_or_null<pointer_wrappers::Derived>(CB) == nullptr);<br>> +  EXPECT_TRUE(dyn_cast_or_null<pointer_wrappers::Derived>(MD) == &D);<br>> +  EXPECT_TRUE(dyn_cast_or_null<pointer_wrappers::Derived>(CD) == &D);<br>> +}<br>> +<br>> +} // end namespace pointer_wrappers<br>> +} // end namespace<br>><br>><br>> _______________________________________________<br>> llvm-commits mailing list<br>><span> </span><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>><span> </span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>><br><br></div></div></blockquote></div><br></div></div><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">_______________________________________________</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">llvm-commits mailing list</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important"><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a></span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span></div></blockquote></div></div></div><br></div></blockquote></div><br></div></div>