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

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

> Could you also add this to StringMap?

Maybe IntervalMap and MapVector too?

> I agree with you that `at` seems like the right name for this, given the STL precedent. I wasn't aware of that existing for maps.

Agreed.

> Renaming lookup -> `lookupOrDefaultConstruct()` or `getValueIfPresent()` or something like that would make sense as well, but seems like a separate issue.

I only suggested that to free up the name "lookup". I don't see any pressing need to change it if we are going with "at" for the new function.



================
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));
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143976