[PATCH] D85072: [ADT] Add getAsOr to Optional

[Nathan James via Phabricator via llvm-commits]

njames93 added a comment.

In D85072#2191557 <https://reviews.llvm.org/D85072#2191557>, @dblaikie wrote:>

> Hmm, I'm not so sure - getValueOr, for instance, is in std::optional. (value_or)
>
> You're right that 'map', for instance, is not in std::optional, though. That and maybe the "create" function are the non-std::optional API?

And `getPointer`

> I think if this new function was a non-member, it'd probably tidier - wouldn't need an rvalue and non-rvalue overload, as it could use std::forward on both arguments.

non-member could work I guess, and could be adapted for other cases, std::optional or pointers spring to mind.



================
Comment at: llvm/include/llvm/ADT/Optional.h:295-297
+  // Delete this as it can lead to dangling reference issues if ``U`` references
+  // the value held in the Optional.
+  template <typename U> U getAsOr(U Default) && = delete;
----------------
dblaikie wrote:
> I have reservations about this - for the same reason that StringRef, for instance, doesn't block being built from an rvalue (because you might (& in StringRef's case, do quite often) create one from a temporary only for use within a single statement).
> 
> Matching the sort of overload/semantics of getValueOr seems appropriate here.
> 
> Is there a particular example you have in mind where this would be a problem?
Fair enough, I'll remove this. Though is it wise to use `static_cast<U>(getValue())` or `static_cast<U>(std::move(getValue()))`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85072