[Patch] Implementation of n3793 <experimental/optional>

[Luc Danton]

On 2013-10-31 14:22, Jonathan Wakely wrote:

Thank you for your feedback, parts of which I have omitted (but not 
ignored).

> [ snip ]
>>   - the ABI-related matters of bad_optional_access and
>>     __throw_bad_optional_access will need to be settled somehow (introduce
>>     src/experimental/optional.cc and
>>     src/experimental/compatibility_functexcept.cc? I can't really say), and
>> until
>>     then I have left the associated function definitions inline in the header
>> --
>>     they are marked by the only two XXX items of the file
> That's OK for now, but let's remember to revisit it some time!
> We usually don't bother marking definitions "inline" if they are
> implicitly inline due to being defined in the class body.
>
> [ snip patch lines ]
>
> I think we should also remove the "inline" specifier on the first
> declaration here, because when we move the definition to a .cc file it
> won't be inline anymore.

I wanted to put the emphasis on those inline definitions that shouldn't be,
but the // XXX items do that just as well and are as easily greppable. I 
agree
that it's a potential trap for the future, so no regrets.

> [ snip ]
>
> Could the SFINAE constraints on optional<T>::operator= and
> optional<T>::emplace be put on the return type, instead of as a
> parameter pack of the function?  Making it a parameter pack allows
> users to misuse the function by providing explicit template argument
> lists e.g. opt.emplace<int, 1, 2, 3, 4>(1) will compile, disabling the
> SFINAE constraint.  Arguably this falls into the category of "you
> can't protect against sufficiently motivated idiots" but putting the
> constraint on the return type makes it a non-issue.

No problem here. As an aside, I normally use something like

     enum struct enabled {};
     template<typename... Conditions>
     using EnableIf = typename std::enable_if<and_<Conditions...>::value,
enabled>::type;

which requires the user to really go out of their way to provide phony 
values.

>
> Shouldn't optional::swap() be using _M_construct(std::move(...)) when
> only one of the objects is engaged?

Yes. This is explicitly mentioned in the requirements for swap(), which 
I must have overlooked.
A test involving an optional move-only value would have caught that.

> I've attached a patch with these changes (except my suggestion for the
> constraints on operator= and emplace), and some re-indenting.  These
> are all trivial and so I think this is ready to commit. I can go ahead
> and do that for you if you like?

Looks good to me. I have attached your suggestion (applied over your own 
patch),
all tests pass.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: optional-sfinae-tweak.patch
Type: text/x-patch
Size: 1985 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20131031/31d17d66/attachment.bin>