<div dir="ltr">Though in this particar case optional support was just a side effect (and the main purpose was to get rid of 5 specializations for isa_imp_cl with almost identical code), I hope it's not harmful at least. <div><br></div><div>Upcasts in that context are also might make sence. I could imagine something like<div><div>auto * const base = cast_or_null<Base>(derived); // alternatively [&derived](){if (derived) return derived.getPointer(); return nullptr;}</div></div><div>Though again it was not my intension to write code like this, but probably it's not that bad code after all.</div></div><div><br></div><div>Another side effect of the patch is shared_ptr support. For isa only by now.</div><div><br></div><div>Probably it makes sense for me to proceed with casts implementation to see if I could really make them more generic and clean because isa is almost useless without casts (so you could consider the current patch as refactoring only). Then we could return back to the question if unified behavior for cast usefull and if there are any corner cases when it would break programmers' expectations. If you think it might be usefull, that's enough for me at the moment. I posted the unfinished work to check if it sensible at all or am I just wasting time.</div><div><br></div><div>--</div><div>Kind regards, Dmitry</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 24, 2018 at 10:27 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">In retrospect is it useful for Optional to be supported in isa? Since its dynamic type is the same as its static type?</div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Tue, Jan 23, 2018 at 10:50 PM Dmitry Borisenkov via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dmitry added inline comments.<br>
<br>
<br>
================<br>
Comment at: unittests/Support/Casting.cpp:<wbr>288-289<br>
+  EXPECT_TRUE(isa<Base>(DShared)<wbr>);<br>
+  llvm::Optional<Derived> DOpt;<br>
+  EXPECT_TRUE(isa<Base>(DOpt));<br>
+}<br>
----------------<br>
dblaikie wrote:<br>
> Does this change then mean that one could cast<T> from Optional<Derived> to Optional<Base>? Because that'd be problematic, owing to Optional being a value-type. (it'd slice the object)<br>
This change doesn't modify casts behavior - just isa.<br>
As for the plan, cast should return T*, so<br>
```<br>
Optional<Derived> DOpt;<br>
Base *b = cast<Base>(DOpt); //Just cast pointer to underlying data to Base*, does not handle potential ownership issues.<br>
```<br>
should work, but it's ok.<br>
Move cast would be indeed problematic for value-types. So I would probably restrict it.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D42027" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D42027</a><br>
<br>
<br>
<br>
</blockquote></div>
</div></div></blockquote></div><br></div></div>