[PATCH] D146061: [ADT] Make llvm::is_contained call member `contains` when available

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D146061#4193581 <https://reviews.llvm.org/D146061#4193581>, @dblaikie wrote:

> In D146061#4193557 <https://reviews.llvm.org/D146061#4193557>, @nikic wrote:
>
>>> I also considered detecting member contains and triggering a
>>> static_assert instead, but decided against it because it's just as easy
>>> to do the right thing and call .contains.
>>
>> I think I would prefer this option, so we keep the ability to distinguish O(1) contains from O(n) is_contained at a glance.
>
> Yeah, that's fair - I'm OK with that too/see the value there. Only issue is in generic code where you might not know the type of a container.

I'd certainly discourage uses of is_contained where the type is known to have a contains member function - maybe the generic code cases are so rare they're not worth leaving this generic functionality in where it's more likely to reduce readability in the non-generic cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146061