<div dir="ltr">This might be a bit easier to review in Phab.<br><br>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.<br><br>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)<br><br>The non-std::experimental::optional features I'd like to give more scrutiny.<br><br>Some notes:<br><br>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).<br><br>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?).<br><br><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 24, 2014 at 12:16 PM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">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:<div><br></div><div>- Move assignment operator.</div><div>- 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<br>- A typedef for T under the name 'value_type'.<br>- For compilers with variadic templates, an "emplace" method for in-place initialization of non-moveable types.<br>- Implicit conversion from any type convertible to T. We used to <i>almost</i> 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 <i>not</i> a feature of std::experimental::optional, but we've found it useful.<br><br>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?</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Jordan</div><div><br></div><div></div></font></span></div><br><div style="word-wrap:break-word"><div></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>