[PATCH] D123901: [LLVM][Casting.h] Allow dyn_cast to return Optional for types that are not constructble from nullptr.

[River Riddle via Phabricator via llvm-commits]

rriddle added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123901/new/

https://reviews.llvm.org/D123901