[PATCH] D33125: Introduce isoneof<T0, T1, ...> as an extension of isa<T>
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 1 09:34:39 PDT 2017
mehdi_amini added a comment.
In https://reviews.llvm.org/D33125#769930, @chandlerc wrote:
> Honestly, the more I think about this, the more I think that the complexity it involves doesn't pull its weight.
I'm fairly neutral on this.
> First, we either have to deal with the semantic confusion of `isa<T0, T1, T2, ...>` or the readability burden of inventing and using a somewhat terrible name like `isa_anyof`
For a non-native speaker, I still don't understand why the correct form isn't `is_anyof` ( "is any of")? You may explain this to me tonight :)
> or the technical complexity of magical tag templates that are *unqualified* and jammed in here for `isa<anyof<...>>`. Note that we have `llvm::any_of` already and it is very different! =[ None of these are insurmountable issues, but they add cost to this abstraction.
I thought exactly about these after the last update!
> Second, the benefit is relatively small. Until the list of types is loooooong, `isa<T0> || isa<T1> || isa<T2>` really doesn't seem that bad. It's a bit more characters to type, but it neatly solves any and all of the semantic / syntactic ambiguity with zero complex implementation. Kinda nice.
Well I can't refrain but remembering all the people that didn't want to use the STL algorithm and write raw for loop using this same argument.
What's missing is that "any_of" express clearly the intent and is a lot easier to parse for a human. As Serge mentioned in his last example, DRY can be annoying to attain without such construct.
> But even worse, when these lists *do* get long, using `isa`, even with the new tool, is usually *the wrong approach*. People should instead be writing a proper switch over whatever data field is available rather than a series of tests. Once that series is long, I worry will not be reliable.
Reliable on which aspect? Optimization?
> And we have historically seen places where `isa` and `dyn_cast` did show up on the profile.
I see the positive aspect of it: more opportunities to improve compiler optimizations ;)
Driving syntactic changes because of optimizer failure makes me sad...
Repository:
rL LLVM
https://reviews.llvm.org/D33125
More information about the llvm-commits
mailing list