[llvm] [Support][Casting] Add predicates for `isa*` functions (PR #83753)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 5 09:48:15 PST 2024


dwblaikie wrote:

> > It'd probably be an issue for some callers, but we could make the actual `isa`, etc into functors so we don't need to introduce new names? Something like this: https://stackoverflow.com/questions/62928396/what-is-a-niebloid (not in this case to thwart overloading, but to make it easy to pass as functors)
> 
> I'm worried about the side effects, given that this is probably one of the most commonly used function across the whole project and downstream projects:

While it's used a lot, I'd hope it's used enough in LLVM that we'd get a pretty good idea of breakage from LLVM, but yeah, probably some novel things in downstream users.

> * Any code that explicitly specified the `From` template argument would **silently** break (the interpretation would change to another possible cast type)

Hmm, not sure I follow this - I'd expect the functor to have one template parameter, so anyone explicitly specifying 2 would fail to compile at least... 

> * This disables ADL

Yeah, that's a hard one to argue with... I'm sure there's a bunch of code out there doing unqualified `isa`/`cast` etc that only works due to ADL. Which in some ways might not be a bad thing (big discussions about ADL costs/complexities/maintenance issues except where needed/intended for things like swap) - but probably a separate discussion not worth binding to this change/direction.

> As an alternative solution, I considered redefining isa* as function objects, but I decided against doing that because it would create asymmetry with other cast functions and could break code that depends on them being actual functions.

Oh, sorry I didn't read this the first time through, btw - thanks for considering/noting it.

Open to discussion here - but I'm inclined to think the ability specify the second template parameter to isa is a bug/incidental feature of isa being a function template, not a feature & one we could choose not to expose in these predicates (by only providing one template parameter, rather than the parameter pack on the outer functor class)? Like if someone wants to use a specific argument type - they should convert the argument expression themselves, rather than by specifying a second template argument and then relying on impliict conversion of the function parameter?


https://github.com/llvm/llvm-project/pull/83753


More information about the llvm-commits mailing list