[PATCH] D33125: Introduce isoneof<T0, T1, ...> as an extension of isa<T>

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Honestly, the more I think about this, the more I think that the complexity it involves doesn't pull its weight.

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` 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.

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.

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. And we have historically seen places where `isa` and `dyn_cast` did show up on the profile.

I feel like if we want to make chains of these things easier to code, that easier to code pattern should actually provide more value such as succinctly packaging up the single-dispatch approach. But currently, I don't think our system really exposes enough information to implement that more pattern-match-y approach, and it isn't clear that we really want to make such a big change as to introduce it.

Anyways, long story short, I'm no longer sure we should pursue this direction. Curious what others think though.


Repository:
  rL LLVM

https://reviews.llvm.org/D33125