[PATCH] D145895: [ADT] Implement {DenseMap,MapVector,StringMap}::contains

Kazu Hirata via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 09:41:29 PDT 2023


kazu added a comment.

In D145895#4189370 <https://reviews.llvm.org/D145895#4189370>, @kuhar wrote:

> LGTM.
>
> Separately, it would be cool if we could make `llvm::is_contained` use the `contains` member when available, instead of doing a linear scan. It seems like a bit of a foot gun to me to have 2 functions with almost identical names but vastly different performance characteristics.

Thanks for the reivew!  I might just migrate those to `$set.contains()` or `$map.contains()`.  In theory, it's possible to write a template that calls `llvm::is_contained` so that it can work with either an array or a set, but I don't know how common that is.

With some template tricks, we could implement a check for the `contains()` method.  If so, we could do `static_assert(!sizeof(T*), "Use contains() instead.")` or something, at least in our working source tree, maybe not in production.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145895



More information about the llvm-commits mailing list