[PATCH] D143976: [ADT] Add `at` method (assertive lookup) to DenseMap and StringMap

Chris Lattner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 15 21:27:02 PST 2023


lattner accepted this revision.
lattner added a comment.
This revision is now accepted and ready to land.

Awesome, thank you for driving this!



================
Comment at: llvm/include/llvm/ADT/DenseMap.h:206
+  /// entry exists.
+  ValueT at(const_arg_type_t<KeyT> Val) const {
+    auto Iter = this->find(std::move(Val));
----------------
StrongerXi wrote:
> foad wrote:
> > Just a question: would it be better to return `const ValueT &` like `std::map::at` would? Do we also want a non-const version of this?
> Good question, I'm following the signature of existing `lookup` methods, I _guess_ the reason is more flexible API?
+1, no need to copy non-trivial values.


================
Comment at: llvm/include/llvm/ADT/StringMap.h:242
+  /// entry exists.
+  ValueTy at(StringRef Val) const {
+    auto Iter = this->find(std::move(Val));
----------------
Similarly should return const&


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143976



More information about the llvm-commits mailing list