[PATCH] Improvements to llvm::Optional

Jordan Rose jordan_rose at apple.com
Fri Sep 26 11:50:47 PDT 2014


I'm not sure why I didn't look at a commit description that was a list of new features and didn't think to split that up. Thanks, doing that now...

On Sep 25, 2014, at 10:49 , David Blaikie <dblaikie at gmail.com> wrote:

> This might be a bit easier to review in Phab.
> 
> General ideas look plausible, though if it's not too much hassle, it might be easier to review each in isolation if they're essentially unrelated.
> 
> I'd also frontload anything that's already in std::experimental::optional - those should be less controversial and the prior-art argument should help simplify review there. (I won't worry too much about the design even if it's not optimal, if it's something we're hopefully going to migrate to later)
> 
> The non-std::experimental::optional features I'd like to give more scrutiny.
> 
> Some notes:
> 
> Your use of std::forward in the Optional(T&&) doesn't actually use perfect forwarding so I'm not sure if std::forward is the right tool there. I'd have to think about it more (or be told I'm wrong/it's right).
> 
> A direct variadic ctor for Optional<T> that builds a T seems a bit subtle/surprising/easily ambiguous - a utility make function is probably more appropriate (make_optional to go with make_unique, etc?).
> 
> 
> 
> On Wed, Sep 24, 2014 at 12:16 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> Hi, all. For some projects, we've been working with a modified version of llvm::Optional<T> for some time now, many of them inspired by std::experimental::optional in the "Library Fundamentals" C++ TS. I'd like to see how everyone feels about pushing these changes back into master:
> 
> - Move assignment operator.
> - getValueOr, which takes an argument convertible to T and returns the Optional's value, if it has one, or a T constructed from the argument if not
> - A typedef for T under the name 'value_type'.
> - For compilers with variadic templates, an "emplace" method for in-place initialization of non-moveable types.
> - Implicit conversion from any type convertible to T. We used to almost have this everywhere via the operator=(const T &) overload, but that didn't include construction or (for compilers with variadic templates) multi-argument initialization. This is not a feature of std::experimental::optional, but we've found it useful.
> 
> The additions require no changes to LLVM or Clang, except the OptionalTests suite. There is one downside here: assignment of a T&& now requires an extra move, because otherwise we'd have ambiguous overloads for assignment of a type convertible to T. Is anyone depending on that?
> 
> Jordan
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140926/6efcc362/attachment.html>


More information about the llvm-commits mailing list