
<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>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...</div><br><div><div>On Sep 25, 2014, at 10:49 , David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><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 class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: 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<span class="Apple-converted-space"> </span><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<span class="Apple-converted-space"> </span><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></blockquote></div></div></div></div></blockquote></div><br></body></html>