<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 28, 2014 at 6:14 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></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 class=""><br>
> On 2014 Nov 24, at 09:01, David Blaikie <<a href="mailto:dblaikie@gmail.com">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">dexonsmith@apple.com</a>> wrote:<br>
> Author: dexonsmith<br>
> Date: Sun Nov 23 21:13:02 2014<br>
> New Revision: 222644<br>
><br>
> URL: <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 class=""><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>
<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">http://en.cppreference.com/w/cpp/memory/shared_ptr/pointer_cast</a> - 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 class=""><div class="h5"><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: <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: <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>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <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>