[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