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

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

In D146061#4194551 <https://reviews.llvm.org/D146061#4194551>, @kazu wrote:

> In D146061#4194466 <https://reviews.llvm.org/D146061#4194466>, @kuhar wrote:
>
>> Then I propose we frame it like this: `llvm::is_contained` is the (llvm-only) canonical/generic way to check if an element is in the range.
>
> How about something like the (llvm-only) canonical/generic way to check if an element is in the range if a template needs to work with containers that may or may not have the `.contains` method (like an array and a set).
>
> I'm saying this because I personally find `Foo.contains(X)` a little more readable than `llvm::is_contained(Foo, X)`, and I would like our code base to look friendly to people new to the LLVM project (with the use of function names present in C++ standards) wherever reasonable to do so.  (Sure, `std::set::contains` is very new, but I expect people to become familiar with this over time.)

Sure, I agree with that. If I have map with a known type like `DenseMap` I'd rather call `.contains(x)` directly too. But this is not possible with C++17 containers `std::`, so I'd argue that until then `is_contained(container, x)` is more readable than something like `container.find(x) != container.end()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146061