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

[River Riddle via Phabricator via llvm-commits]

rriddle accepted this revision.
rriddle added a comment.

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.



================
Comment at: llvm/include/llvm/Support/Casting.h:313
+template <typename OptionalDerived, typename Default>
+using SelfType = std::conditional_t<std::is_same<OptionalDerived, void>::value,
+                                    Default, OptionalDerived>;
----------------
Can we put this in detail? or a `cast_impl` namespace?


================
Comment at: llvm/include/llvm/Support/Casting.h:377
+struct ConstStrippingForwardingCast {
+  // Remove the pointer if it exists, then we can get rid of consts/volatiles.
+  using DecayedFrom = std::remove_cv_t<std::remove_pointer_t<From>>;
----------------
/// here please.


================
Comment at: llvm/include/llvm/Support/Casting.h:410
+/// For isa<> customization, provide:
+/// `static bool isPossible(const From &f)`
+/// For cast<> customization, provide:
----------------
Can you indent these? It's hard to pick them out when reading the comment. Maybe also add a newline between each `customization`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123901