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

Nathan James via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 3 02:43:29 PDT 2020


njames93 added a comment.

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

> I forget: Have we already decided that Optional won't be replaced by std::optional? If not, then would be good to not add API surface area that doesn't match std::optional (instead preferring non-member functions, for instance).
> If that's already been decided in some previous reviews, etc, this sounds OK I guess.

If that's the case then I'd agree with that.
 But I kind of gathered it wouldn't be the case considering how much the `llvm::Optional` API already differs from `std::optional`.

> Probably doesn't need the enable_if, I'd have thought (could just let the implementation fail as usual - there aren't competing member functions, etc)

Fair enough point there.

> the parameter should be U&& to use with std::forward, avoid unnecessary copies, etc.

I'm inclined to disagree with that point, this is mean't to be used with types that are cheap to construct and usually passed by value.
Probably don't need the call to `std::forward` though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85072



More information about the llvm-commits mailing list