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

Wed May 11 09:57:47 PDT 2022

bzcheeseman added a comment.

In D123901#3506680 <https://reviews.llvm.org/D123901#3506680>, @nhaehnle wrote:

> In D123901#3455711 <https://reviews.llvm.org/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!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123901