[LLVM][Casting.h] Update dyn_cast machinery to provide more control over how the casting is performed.

[bzcheeseman]

Summary

This patch expands the expressive capability of the casting utilities in LLVM by introducing several levels of configurability. By creating modular CastInfo classes we can enable projects like MLIR that need more fine-grained control over how a cast is actually performed to retain that control, while making it easy to express the easy cases (like a checked pointer to pointer cast).

The current implementation of Casting.h doesn't make it clear where the entry points for customizing the cast behavior are, so part of the motivation for this patch is adding that documentation. Another part of the motivation is to support using LLVM RTTI with a wider set of use cases, such as nullable value to value casts, or pointer to value casts (as in MLIR).

[lebedev.ri]

It may be helpful to also show the patch with the intended use-case.

[nikic]

Not a big fan of this overload. dyn_cast is such a core operation, I'd rather not muddy things by making it return Optional<> -- but only sometimes. I expect this is going to cause a lot of confusion when people work on an Instruction &, but really want to be working on Instruction *.

[lattner]

Not a big fan of this overload. dyn_cast is such a core operation, I'd rather not muddy things by making it return Optional<> -- but only sometimes.

I agree with you that we need to tread lightly here, but dyn_cast being such a core operation means that we need to continue to invest in it. There are a bunch of cases that it doesn't handle well, which means LLVM projects using this (e.g. MLIR) end up resorting to similar-but-different things, like x.dyn_cast<foo>() which have similar but different behavior on null. It would be great for the project to pull this together.

These three functions provide forwarding for non-optional types passed into
dyn_cast_or_none.

I don't understand the addition of the "or_none" helper. I get what it "does": it allows none propagation for Optional arguments (but it also works with nullptr propagation from pointers as well). In practice, this seems strictly stronger than the existing dyn_cast_or_null function - can they be merged into a single null propagating thing? Maybe we should rename it to "dyn_cast_if_present" or some other name that is more generic?

Channeling the comments above, we need to be careful with such a core API like this, it would be good to send a "head's up" note to the llvm forum when the design point settles.

-Chris

[bzcheeseman]

It may be helpful to also show the patch with the intended use-case.

The intended use case is to be used for projects like MLIR that have t.dyn_cast<IntegerType>() so they can instead have dyn_cast<IntegerType>(t) for consistency, and to reduce the RTTI burden.

I expect this is going to cause a lot of confusion when people work on an Instruction &, but really want to be working on Instruction *.

I'm maybe missing something here, but my understanding is that if you have Value *v; auto instruction = dyn_cast<Instruction>(v); you're going to end up with an Instruction *. Am I reading the enable_if/conditional rules incorrectly?

can they be merged into a single null propagating thing?

I like this idea - I originally had dyn_cast_or_null with an overload for Optional but it looked goofy. I like dyn_cast_if_present with maybe some forwarding dyn_cast_or_null to avoid breaking everyone.

dyn_cast is such a core operation <>
...
we need to be careful with such a core API like this, it would be good to send a "head's up" note to the llvm forum when the design point settles

Agreed 100%. Everyone uses dyn_cast and that's why I wanted to put up the patch, so we can talk about it!

Going forward I see 2 options (would love to hear about it if someone has other ideas):

1. Keep the overload, maybe swap the dyn_cast_or_none for dyn_cast_if_present
2. Have a new function optional_dyn_cast/optional_dyn_cast_if_present (or something, not enamored with that name) so that folks can opt-in to the new behavior with optionals

I would personally prefer option 1 here because it makes the most sense to me right now.

[lattner]

I'd prefer we have one thing. Re:

I like this idea - I originally had dyn_cast_or_null with an overload for Optional but it looked goofy. I like dyn_cast_if_present with maybe some forwarding dyn_cast_or_null to avoid breaking everyone.

it is super awesome to stage things in and give people a migration window, but if it is the right thing to rename this, then we should drop the old name, not carry it around forever.

[rriddle]

I want to +1 a general need for something better than what we currently have. The current casting infra kind of assumes a very specific use case, which means that anything outside of that needs to duplicate a bunch of logic and/or handle two different ways of casting. An easy example of this is TypeSwitch(https://github.com/llvm/llvm-project/blob/bdabe505f417a9a6636913e8ea253ff9dc42f32e/llvm/include/llvm/ADT/TypeSwitch.h#L77), which has to SFINAE which cast it's using. TypeSwitch definitely isn't alone here, this kind of logic permeates throughout the codebase and makes some things more annoying than they need to be. In MLIR we use "value-typed" classes
for many things, and that effectively breaks the current casting infras assumption that everything goes through pointer/reference.

All of that being said, I don't think that Optional helps for the MLIR case (or other cases in the codebase that are like it). In most of the cases we have today, the result of the dyn_cast is already "nullable" so we wouldn't want to reroute through Optional. IMO I think one of the best paths forward would be to completely rework the abstractions
of how types opt-in to support casting. The current combination of isa_impl + cast_retty + cast_convert_val is quite clunky, and it feels like an evolution of that would both be cleaner than what we have today, and more extensible for "exotic" use cases. Having suffered from this a lot whenever trying to expand beyond the traditional LLVM
use case, I would love to see movement in this area.

[yurai007]

llvm/unittests/Support/Casting.cpp

214

I guess braces are redundant here.

220

Missing test cases for pointer overloads?

[bzcheeseman]

Working on updating this in accordance with comments (so folks know I'm not ignoring anyone). Thanks all for the reviews, hope to have an updated patch soon.

[lattner]

Overall, this looks really great to me. Please get River's review, then (when any feedback is incorporated) ping an LLVM forum for visibility. This isn't normally needed but casting.h is very pervasive, so extra discussion is helpful.

llvm/include/llvm/Support/Casting.h

18

Is there any reasonable way to avoid STLExtras.h? It is a heavy weight header that pulls in tons of stuff, and Casting.h is used roughly everywhere.

Optional is nearly but not quite as bad, and is more core to what you're doing here, so ok.

[bzcheeseman]

llvm/include/llvm/Support/Casting.h

18

I was using is_detected in an earlier iteration, but it's now been removed, so I think I can remove this too. Thanks for catching it!

[bzcheeseman]

Sorry for all the traffic on this last night - my machine is a bit slow so I was partly using CI to help run all the llvm-project tests. Should be ready for review at this point, all passing :)

[lattner]

This looks good to me, the community discussion seems quiet. Feel free to land when the CI tests pass, thanks!

[rriddle]

Ran through and this LGTM as an iteration point. It would be nice to add improvements to the user facing documentation on how to add RTTI to a class hierarchy as well: https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html, but I'm fine with that being a follow up.

llvm/include/llvm/Support/Casting.h

342

Can we put this in detail? or a cast_impl namespace?

406

/// here please.

439

Can you indent these? It's hard to pick them out when reading the comment. Maybe also add a newline between each customization.

[bzcheeseman]

llvm/include/llvm/Support/Casting.h

342

Yes nice catch, thanks!

[nhaehnle]

In D123901#3455711, @lattner wrote:

I'd prefer we have one thing. Re:

I like this idea - I originally had dyn_cast_or_null with an overload for Optional but it looked goofy. I like dyn_cast_if_present with maybe some forwarding dyn_cast_or_null to avoid breaking everyone.

it is super awesome to stage things in and give people a migration window, but if it is the right thing to rename this, then we should drop the old name, not carry it around forever.

FWIW, having a migration window makes a massive difference for downstream projects. Especially with the currently ongoing discussions about just how big the monorepo should be, this is a consideration to take seriously.

In many cases a migration window helps even if it is "only a microsecond long", i.e. if you have two patches, one to add the new name (and make the old name a plain forward), and a separate follow-up to remove the old name, that can help migrations even if the two commits are added back-to-back.

Which the current version does, so thank you very much for that :)

[bzcheeseman]

In D123901#3506680, @nhaehnle wrote:

In D123901#3455711, @lattner wrote:

I'd prefer we have one thing. Re:

I like this idea - I originally had dyn_cast_or_null with an overload for Optional but it looked goofy. I like dyn_cast_if_present with maybe some forwarding dyn_cast_or_null to avoid breaking everyone.

it is super awesome to stage things in and give people a migration window, but if it is the right thing to rename this, then we should drop the old name, not carry it around forever.

FWIW, having a migration window makes a massive difference for downstream projects. Especially with the currently ongoing discussions about just how big the monorepo should be, this is a consideration to take seriously.

In many cases a migration window helps even if it is "only a microsecond long", i.e. if you have two patches, one to add the new name (and make the old name a plain forward), and a separate follow-up to remove the old name, that can help migrations even if the two commits are added back-to-back.

Which the current version does, so thank you very much for that :)

Oh definitely, removing *_or_null will be an ENORMOUS patch so we can do it piecemeal and stage it in NFC patches :) I don't like breaking folks when we don't have to!

[uabelho]

Did you notice this:

FAILED: unittests/Support/CMakeFiles/SupportTests.dir/Casting.cpp.o 
/home/buildbots/clang.11.0.0/bin/clang++ --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iunittests/Support -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/Support -Iinclude -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG    -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT unittests/Support/CMakeFiles/SupportTests.dir/Casting.cpp.o -MF unittests/Support/CMakeFiles/SupportTests.dir/Casting.cpp.o.d -o unittests/Support/CMakeFiles/SupportTests.dir/Casting.cpp.o -c /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/Support/Casting.cpp
In file included from /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/Support/Casting.cpp:9:
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include/llvm/Support/Casting.h:216:12: error: returning reference to local temporary object [-Werror,-Wreturn-stack-address]
    return Res2;
           ^~~~
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include/llvm/Support/Casting.h:453:72: note: in instantiation of member function 'llvm::cast_convert_val<llvm::foo, const llvm::bar, const llvm::bar>::doit' requested here
                            typename simplify_type<From>::SimpleType>::doit(f);
                                                                       ^
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include/llvm/Support/Casting.h:564:36: note: in instantiation of member function 'llvm::CastInfo<llvm::foo, const llvm::bar, void>::doCast' requested here
  return CastInfo<To, const From>::doCast(Val);
                                   ^
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include/llvm/Support/Casting.h:214:47: note: binding reference variable 'Res2' here
    typename cast_retty<To, FromTy>::ret_type Res2 =
                                              ^
1 error generated.

from
https://lab.llvm.org/buildbot/#/builders/57/builds/17829/steps/6/logs/stdio

I see the same thing compiling locally with clang 8.

[qiongsiwu1]

In D123901#3508156, @uabelho wrote:

from
https://lab.llvm.org/buildbot/#/builders/57/builds/17829/steps/6/logs/stdio

I see the same thing compiling locally with clang 8.

We are seeing the same error in our internal upstream ppc RHEL buildbot as well. @bzcheeseman could you take a look? If you need access to a PPC machine we should be able to arrange that. FYI @nemanjai

Thanks so much!

[bzcheeseman]

I'm just now seeing this (just woke up) so I'll take a look and hopefully resolve it shortly! Thanks for bringing this to my attention.

[bzcheeseman]

https://reviews.llvm.org/D125482 doesn't exhibit the problem in the unit test, but now I have to wait for all the other tests to complete so hopefully nothing else breaks!

[uabelho]

In D123901#3509525, @bzcheeseman wrote:

https://reviews.llvm.org/D125482 doesn't exhibit the problem in the unit test, but now I have to wait for all the other tests to complete so hopefully nothing else breaks!

The warning went away with your fix. Thanks!

[sylvestre.ledru]

@bzcheeseman FYI, this is breaking the build with gcc 6 (for Debian stretch)
if you are aware of a simple fix, it would be appreciated
See https://github.com/llvm/llvm-project/issues/55626

(I am not sure I want to backport gcc 7 or 8 to Debian stretch ;)

thanks

[bzcheeseman]

In D123901#3530352, @sylvestre.ledru wrote:

@bzcheeseman FYI, this is breaking the build with gcc 6 (for Debian stretch)
if you are aware of a simple fix, it would be appreciated
See https://github.com/llvm/llvm-project/issues/55626

(I am not sure I want to backport gcc 7 or 8 to Debian stretch ;)

thanks

Moving discussion to the GH issue so it's a little clearer what's up :)

[reames]

Is there any chance that this change removed the assert on cast<X> that the thing being pointed to isa<X>? I'm looking at a local case where I have cast<FixedVectorType(T) succeeding on something which clearly *isn't* a fixed vector, and isa<FixedVectorType>(T) returning false on the same pointer. Its possible that I'm looking at some weird bit of undefined behavior, but since I can't find where the corresponding assert is in the new version of the code, I wanted to ask.

[bzcheeseman]

In D123901#3561964, @reames wrote:

Is there any chance that this change removed the assert on cast<X> that the thing being pointed to isa<X>? I'm looking at a local case where I have cast<FixedVectorType(T) succeeding on something which clearly *isn't* a fixed vector, and isa<FixedVectorType>(T) returning false on the same pointer. Its possible that I'm looking at some weird bit of undefined behavior, but since I can't find where the corresponding assert is in the new version of the code, I wanted to ask.

That looks like what happened, yep. I meant to add it to the default implementation of doCast (so that you don't have to pay the price of isa) but apparently forgot to do so. I'll try and get to this when I get the chance - and clean up all the assert usage at once. If you need it urgently, asserting isa<T> inside cast is fine for now and I can come back and clean things up later.

[reames]

In D123901#3562333, @bzcheeseman wrote:

In D123901#3561964, @reames wrote:

Is there any chance that this change removed the assert on cast<X> that the thing being pointed to isa<X>? I'm looking at a local case where I have cast<FixedVectorType(T) succeeding on something which clearly *isn't* a fixed vector, and isa<FixedVectorType>(T) returning false on the same pointer. Its possible that I'm looking at some weird bit of undefined behavior, but since I can't find where the corresponding assert is in the new version of the code, I wanted to ask.

That looks like what happened, yep. I meant to add it to the default implementation of doCast (so that you don't have to pay the price of isa) but apparently forgot to do so. I'll try and get to this when I get the chance - and clean up all the assert usage at once. If you need it urgently, asserting isa<T> inside cast is fine for now and I can come back and clean things up later.

See https://discourse.llvm.org/t/cast-x-is-broken-implications-and-proposal-to-address/63033

If you could help with the cleanup here, it would be appreciated.

[whisperity]

I got here from D131319, but I am not sure why this change is only touching comments but not the hand-written documentation. Is this CastInfo an internal detail that should not be publicly documented to the everyday documentation reader? Are the old-style "classof" method and enums still applicable?

[0x59616e]

In D123901#3709642, @whisperity wrote:

I got here from D131319, but I am not sure why this change is only touching comments but not the hand-written documentation. Is this CastInfo an internal detail that should not be publicly documented to the everyday documentation reader? Are the old-style "classof" method and enums still applicable?

The old style classof is still valid.

Here is the document about the new CastInfo :

https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#advanced-use-cases

llvm/include/llvm/Support/Casting.h

 //===- llvm/Support/Casting.h - Allow flexible, checked, casts --*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the isa<X>(), cast<X>(), dyn_cast<X>(), cast_or_null<X>(),
-// and dyn_cast_or_null<X>() templates.
+// dyn_cast_or_null<X>(), and dyn_cast_or_none<X>() templates.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_CASTING_H
 #define LLVM_SUPPORT_CASTING_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/type_traits.h"
 #include <cassert>
 #include <memory>
 #include <type_traits>
 
 namespace llvm {
 
 //===----------------------------------------------------------------------===//

 template <class X, class Y>
 inline typename cast_retty<X, std::unique_ptr<Y>>::ret_type
 cast_or_null(std::unique_ptr<Y> &&Val) {
   if (!Val)
     return nullptr;
   return cast<X>(std::move(Val));
 }
 
+//===----------------------------------------------------------------------===//
+//                          dyn_cast<x> Support Templates
+//===----------------------------------------------------------------------===//
+
+template <class X, class Y> struct dyn_cast_retty {
+private:
+  using CastReturnType = typename cast_retty<X, Y>::ret_type;
+  static constexpr bool ConstructibleFromNullptr =
+      std::is_constructible<CastReturnType, std::nullptr_t>::value ||
+      std::is_pointer<CastReturnType>::value;
+
+public:
+  using ret_type = typename std::conditional<
+      ConstructibleFromNullptr, CastReturnType,
+      Optional<std::remove_reference_t<CastReturnType>>>::type;
+
+  static ret_type castFailed() { return ret_type{}; }
+};
+
 // dyn_cast<X> - Return the argument parameter cast to the specified type.  This
 // casting operator returns null if the argument is of the wrong type, so it can
 // be used to test for a type as well as cast if successful.  This should be
 // used in the context of an if statement like this:
 //
 //  if (const Instruction *I = dyn_cast<Instruction>(myVal)) { ... }
 //
+// Or, if the target type is not constructible from nullptr then it could be
+// used like this:
+//
+//  if (Optional<Instruction> I = dyn_cast<Instruction>(myVal)) { ... }
+//
 
 template <class X, class Y>
 LLVM_NODISCARD inline std::enable_if_t<
-    !is_simple_type<Y>::value, typename cast_retty<X, const Y>::ret_type>
+    !is_simple_type<Y>::value, typename dyn_cast_retty<X, const Y>::ret_type>
 dyn_cast(const Y &Val) {
-  return isa<X>(Val) ? cast<X>(Val) : nullptr;
+  return isa<X>(Val) ? cast<X>(Val) : dyn_cast_retty<X, const Y>::castFailed();
 }
 
 template <class X, class Y>
-LLVM_NODISCARD inline typename cast_retty<X, Y>::ret_type dyn_cast(Y &Val) {
-  return isa<X>(Val) ? cast<X>(Val) : nullptr;
+LLVM_NODISCARD inline typename dyn_cast_retty<X, Y>::ret_type dyn_cast(Y &Val) {
+  return isa<X>(Val) ? cast<X>(Val) : dyn_cast_retty<X, Y>::castFailed();
 }
 
 template <class X, class Y>
-LLVM_NODISCARD inline typename cast_retty<X, Y *>::ret_type dyn_cast(Y *Val) {
-  return isa<X>(Val) ? cast<X>(Val) : nullptr;
+LLVM_NODISCARD inline typename dyn_cast_retty<X, Y *>::ret_type
+dyn_cast(Y *Val) {
+  return isa<X>(Val) ? cast<X>(Val) : dyn_cast_retty<X, Y *>::castFailed();
 }
 
 // dyn_cast_or_null<X> - Functionally identical to dyn_cast, except that a null
 // value is accepted.
 //
 template <class X, class Y>
 LLVM_NODISCARD inline std::enable_if_t<
     !is_simple_type<Y>::value, typename cast_retty<X, const Y>::ret_type>

 }
 
 template <class X, class Y>
 LLVM_NODISCARD inline typename cast_retty<X, Y *>::ret_type
 dyn_cast_or_null(Y *Val) {
   return (Val && isa<X>(Val)) ? cast<X>(Val) : nullptr;
 }
 
+// dyn_cast_or_none<X> - Functionally identical to dyn_cast, except that an
+// empty optional value is accepted.
+//
+
+//===----------------------------------------------------------------------===//
+//                          dyn_cast_or_none support
+//===----------------------------------------------------------------------===//
+
+// These three functions provide forwarding for non-optional types passed into
+// dyn_cast_or_none.
+//
+
+template <class X, class Y>
+LLVM_NODISCARD inline auto dyn_cast_or_none(const Y &Val)
+    -> decltype(dyn_cast<X>(Val)) {
+  return dyn_cast<X>(Val);
+}
+
+template <class X, class Y>
+LLVM_NODISCARD inline auto dyn_cast_or_none(Y &Val)
+    -> decltype(dyn_cast<X>(Val)) {
+  return dyn_cast<X>(Val);
+}
+
+template <class X, class Y>
+LLVM_NODISCARD inline auto dyn_cast_or_none(Y *Val)
+    -> decltype(dyn_cast_or_null<X>(Val)) {
+  return dyn_cast_or_null<X>(Val);
+}
+
+template <class X, class Y>
+LLVM_NODISCARD inline std::enable_if_t<
+    !is_simple_type<Y>::value, typename dyn_cast_retty<X, const Y>::ret_type>
+dyn_cast_or_none(Optional<const Y> &Val) {
+  return (Val.hasValue() && isa<X>(*Val)) ? cast<X>(*Val)
+                                          : dyn_cast_retty<X, Y>::castFailed();
+}
+
+template <class X, class Y>
+LLVM_NODISCARD inline typename dyn_cast_retty<X, Y>::ret_type
+dyn_cast_or_none(Optional<Y> &Val) {
+  return (Val.hasValue() && isa<X>(*Val)) ? cast<X>(*Val)
+                                          : dyn_cast_retty<X, Y>::castFailed();
+}
+
 // unique_dyn_cast<X> - Given a unique_ptr<Y>, try to return a unique_ptr<X>,
 // taking ownership of the input pointer iff isa<X>(Val) is true.  If the
 // cast is successful, From refers to nullptr on exit and the casted value
 // is returned.  If the cast is unsuccessful, the function returns nullptr
 // and From is unchanged.
 template <class X, class Y>
 LLVM_NODISCARD inline auto unique_dyn_cast(std::unique_ptr<Y> &Val)
     -> decltype(cast<X>(Val)) {

llvm/unittests/Support/Casting.cpp

 #include <cstdlib>
 
 namespace llvm {
 // Used to test illegal cast. If a cast doesn't match any of the "real" ones,
 // it will match this one.
 struct IllegalCast;
 template <typename T> IllegalCast *cast(...) { return nullptr; }
 
-// set up two example classes
+// set up three example classes
 // with conversion facility
 //
 struct bar {
   bar() {}
   struct foo *baz();
   struct foo *caz();
   struct foo *daz();
   struct foo *naz();
+  Optional<struct foo> optional();
 private:
   bar(const bar &);
 };
 struct foo {
   void ext() const;
   /*  static bool classof(const bar *X) {
     cerr << "Classof: " << X << "\n";
     return true;

 foo *bar::daz() {
     return dyn_cast<foo>(this);
 }
 
 foo *bar::naz() {
     return dyn_cast_or_null<foo>(this);
 }
 
+Optional<foo> bar::optional() {
+  return dyn_cast<foo>(*this);
+}
+
 
 bar *fub();
 
 template <> struct simplify_type<foo> {
   typedef int SimpleType;
   static SimpleType getSimplifiedValue(foo &Val) { return 0; }
 };
 
TEST(CastingTest, dyn_cast_or_null) {
   const foo *F3 = dyn_cast_or_null<foo>(B4);
   EXPECT_NE(F3, null_foo);
   foo *F4 = dyn_cast_or_null<foo>(fub());
   EXPECT_EQ(F4, null_foo);
   foo *F5 = B1.naz();
   EXPECT_NE(F5, null_foo);
 }
 
+TEST(CastingTest, dyn_cast_optional) {
+  Optional<foo> F1 = dyn_cast<foo>(B);
+  EXPECT_TRUE(F1.hasValue());
+  Optional<foo> F2 = dyn_cast<foo>(B1);
+  EXPECT_TRUE(F2.hasValue());
+  Optional<const foo> F3 = dyn_cast<foo>(B3);
+  EXPECT_TRUE(F3.hasValue());
+
+  Optional<foo> F4 = B1.optional();
+  EXPECT_TRUE(F4.hasValue());
+}
+
+TEST(CastingTest, dyn_cast_or_none) {
+  Optional<bar> empty{};
+  Optional<foo> F1 = dyn_cast_or_none<foo>(empty);
+  EXPECT_FALSE(F1.hasValue());
+
+  Optional<foo> F2 = dyn_cast_or_none<foo>(B1);
+  EXPECT_TRUE(F2.hasValue());
+}
+
 std::unique_ptr<derived> newd() { return std::make_unique<derived>(); }
 std::unique_ptr<base> newb() { return std::make_unique<derived>(); }
 
 TEST(CastingTest, unique_dyn_cast) {
   derived *OrigD = nullptr;
   auto D = std::make_unique<derived>();
   OrigD = D.get();