[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