[PATCH] Improvements to llvm::Optional

David Blaikie dblaikie at gmail.com
Thu Sep 25 10:49:55 PDT 2014


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/20140925/742a1305/attachment.html>


More information about the llvm-commits mailing list