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

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 11 09:53:48 PDT 2022


nhaehnle added a comment.

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 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123901



More information about the llvm-commits mailing list