[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