[Corrected subject to target optional.] On August 24, 2014 7:55:11 AM EDT, Dean Michael Berris wrote:

I realize that this message may have not passed the spam filters.

Can someone comment on the changes in Boost.Optional in 1.56.0 breaking existing code? On Mon Aug 18 2014 at 8:10:52 PM Dean Michael Berris wrote:

...

Suppose I have the following type:

struct container { boost::optional optional() const { return value_; } private: boost::optional value_; };

struct value_wrapper { value_wrapper(const container& c) : c_(c) {} operator uint16_t () { return 1; } operator boost::optional() { return c_.optional(); } private: const container& c_; };

And that I have the following function:

value_wrapper value(const container& c) { return value_wrapper(c); }

The following used to work in MSVC 2010:

container c; // leave c.value_ alone. boost::optional some_value = value(c); assert(!some_value);

Now with Boost 1.56, this breaks because the constructor for some_value from a temporary optional is now marked explicit.

What is the work-around for the previous behavior to be regained? All the following changes cause the assertion to fail:

boost::optional some_value(value(c)); assert(!some_value); // some_value's constructor will use the uint16_t conversion operator, therefore some_value == 1

boost::optional some_value = static_cast >(value(c)); assert(!some_value); // same problem as above, the temporary created will favor the uint16_t conversion operator

Help?

PS. For better context, see https://github.com/cpp-netlib/cpp-netlib/blob/0.11-devel/ boost/network/protocol/http/algorithms/linearize.hpp#L145

___ Rob (Sent from my portable computation engine)

On Tue Aug 26 2014 at 9:38:37 AM Eric Niebler wrote:

On 08/24/2014 11:20 AM, Rob Stewart wrote:

...

[Corrected subject to target optional.]

On August 24, 2014 7:55:11 AM EDT, Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

I realize that this message may have not passed the spam filters.

Can someone comment on the changes in Boost.Optional in 1.56.0 breaking existing code?

<snip>

Hi Dean,

I don't know the specifics, but if you file a bug, at least it won't get lost.

Filed: https://svn.boost.org/trac/boost/ticket/10399 Thanks Eric.

Le 01/09/14 17:48, Dean Michael Berris a écrit :

On Tue Aug 26 2014 at 3:59:12 PM Dean Michael Berris wrote:

...

On Tue Aug 26 2014 at 9:38:37 AM Eric Niebler wrote:

...

On 08/24/2014 11:20 AM, Rob Stewart wrote:

...

[Corrected subject to target optional.]

On August 24, 2014 7:55:11 AM EDT, Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

I realize that this message may have not passed the spam filters.

Can someone comment on the changes in Boost.Optional in 1.56.0 breaking existing code? <snip>

Hi Dean,

I don't know the specifics, but if you file a bug, at least it won't get lost.

Filed: https://svn.boost.org/trac/boost/ticket/10399

Thanks Eric.

It's now been almost a week and there's been no update on the issue. I think this is serious enough that I'm positive it couldn't just be me that's experiencing this regression.

Can someone with access to MSVC 2010 confirm whether this indeed is an issue, and whether it's actually a regression?

If I wanted to get a fix in for this, whom do I send the pull request to (community maintenance team) so it gets fixed for 1.57?

Hi, it could be that the maintainers are on vacation. Best, Vicente

On Tue Sep 02 2014 at 5:48:09 AM Edward Diener wrote:

On 9/1/2014 11:48 AM, Dean Michael Berris wrote:

...

On Tue Aug 26 2014 at 3:59:12 PM Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

On Tue Aug 26 2014 at 9:38:37 AM Eric Niebler wrote:

...

On 08/24/2014 11:20 AM, Rob Stewart wrote:

...

[Corrected subject to target optional.]

On August 24, 2014 7:55:11 AM EDT, Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

I realize that this message may have not passed the spam filters.

Can someone comment on the changes in Boost.Optional in 1.56.0 breaking existing code?

<snip>

Hi Dean,

I don't know the specifics, but if you file a bug, at least it won't get lost.

Filed: https://svn.boost.org/trac/boost/ticket/10399

Thanks Eric.

It's now been almost a week and there's been no update on the issue. I think this is serious enough that I'm positive it couldn't just be me that's experiencing this regression.

Can someone with access to MSVC 2010 confirm whether this indeed is an issue, and whether it's actually a regression?

If I wanted to get a fix in for this, whom do I send the pull request to (community maintenance team) so it gets fixed for 1.57?

Just make a pull request against Boost Optional.

Which fork? Just direct to the boostorg repo?

Whoever is a Boost Optional maintainer will get the pull request. It is not up to you to have to determine, when making a pull request, who should get the request.

It actually is, because I need to determine which maintainer is active, which one has access to MSVC 2010, be iterating on the pull request with that maintainer's fork, and then have that maintainer in their leisure merge it into the boostorg repo. If the process is a free-for-all on the boostorg repo, then I'm afraid I'm going to have to say that may not scale well especially if there's more than one maintainer of the library. So I ask again -- to which fork, what are the expected turn-around times (who do I @mention in github) to get their attention?

On Tue Sep 02 2014 at 12:16:52 PM Edward Diener wrote:

On 9/1/2014 10:04 PM, Dean Michael Berris wrote:

...

On Tue Sep 02 2014 at 5:48:09 AM Edward Diener wrote:

...

On 9/1/2014 11:48 AM, Dean Michael Berris wrote:

...

On Tue Aug 26 2014 at 3:59:12 PM Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

On Tue Aug 26 2014 at 9:38:37 AM Eric Niebler wrote:

...

On 08/24/2014 11:20 AM, Rob Stewart wrote: > [Corrected subject to target optional.] > > On August 24, 2014 7:55:11 AM EDT, Dean Michael Berris < mikhailberis@gmail.com> wrote: >> I realize that this message may have not passed the spam filters. >> >> Can someone comment on the changes in Boost.Optional in 1.56.0 breaking >> existing code?

<snip>

Hi Dean,

I don't know the specifics, but if you file a bug, at least it won't get lost.

Filed: https://svn.boost.org/trac/boost/ticket/10399

Thanks Eric.

It's now been almost a week and there's been no update on the issue. I think this is serious enough that I'm positive it couldn't just be me that's experiencing this regression.

Can someone with access to MSVC 2010 confirm whether this indeed is an issue, and whether it's actually a regression?

If I wanted to get a fix in for this, whom do I send the pull request to (community maintenance team) so it gets fixed for 1.57?

Just make a pull request against Boost Optional.

Which fork? Just direct to the boostorg repo?

...

Whoever is a Boost Optional maintainer will get the pull request. It is not up to you to have to determine, when making a pull request, who should get the request.

It actually is, because I need to determine which maintainer is active, which one has access to MSVC 2010, be iterating on the pull request with that maintainer's fork, and then have that maintainer in their leisure merge it into the boostorg repo. If the process is a free-for-all on the boostorg repo, then I'm afraid I'm going to have to say that may not scale well especially if there's more than one maintainer of the library.

So I ask again -- to which fork, what are the expected turn-around times (who do I @mention in github) to get their attention?

Look at https://svn.boost.org/trac/boost/wiki/StartModPatchAndPullReq and follow the directions under "Pull Requests" and "Forking a Single Repo". Having access to MSVC 2010 should not be a big deal for a maintainer. If you find after your pull request that nobody is responding to it within a week or two after you make it, then I would say to post back here and try to find the right person to respond. But until then let the normal Pull Request process play itself out.

Interesting. Thanks for that link. I'm sorry I have to say this, but the instructions there look like more trouble than the normal GitHub pull request process. :( I'm now having second thoughts on submitting a patch and just wait for a maintainer to respond. /me wishes Boost didn't have to break up the libraries into different repo's.

2014-09-01 17:48 GMT+02:00 Dean Michael Berris :

On Tue Aug 26 2014 at 3:59:12 PM Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

On Tue Aug 26 2014 at 9:38:37 AM Eric Niebler wrote:

...

On 08/24/2014 11:20 AM, Rob Stewart wrote:

...

[Corrected subject to target optional.]

On August 24, 2014 7:55:11 AM EDT, Dean Michael Berris < mikhailberis@gmail.com> wrote:

...

I realize that this message may have not passed the spam filters.

Can someone comment on the changes in Boost.Optional in 1.56.0 breaking existing code?

<snip>

Hi Dean,

I don't know the specifics, but if you file a bug, at least it won't get lost.

Filed: https://svn.boost.org/trac/boost/ticket/10399

Thanks Eric.

It's now been almost a week and there's been no update on the issue. I think this is serious enough that I'm positive it couldn't just be me that's experiencing this regression.

Can someone with access to MSVC 2010 confirm whether this indeed is an issue, and whether it's actually a regression?

If I wanted to get a fix in for this, whom do I send the pull request to (community maintenance team) so it gets fixed for 1.57?

Hi Dean, I am a maintainer of Boost.Optional. Was on vacation so couldn't hear you. I just verified that your code example works with 1.55 on VS2010, so it is a regression. I will not be able to have a look at it for the next couple of days. I would like to determine why it even works on 1.55. The converting constructor from arbitrary U is also explicit in 1.55, so it works because of some sequence of conversions that I cannot figure out right now. If you wish to submit a patch you can do it via GitHub at https://github.com/boostorg/optional. I can see and act upon the pull requests there. Regards, &rzej

2014-09-02 11:23 GMT+02:00 Dean Michael Berris :

...

I just verified that your code example works with 1.55 on VS2010, so it is a regression. I will not be able to have a look at it for the next couple of days. I would like to determine why it even works on 1.55. The converting constructor from arbitrary U is also explicit in 1.55, so it works because of some sequence of conversions that I cannot figure out right now.

Right. My hunch is that 1.56 now has suppression for the cases where references would have been deduced (as discussed in

http://www.boost.org/doc/libs/1_56_0/libs/optional/doc/html/boost_optional/d... ). Maybe the suppression applies even for the case when the conversion operator may have been useful for copy initialization.

However, it works fine with GCC. It looks like VC++ compiler has a bug in overload resolution in copy initialization. It should never consider explicit constructors.

2014-09-02 15:39 GMT+02:00 Dean Michael Berris :

On Tue Sep 02 2014 at 9:39:24 PM Andrzej Krzemienski wrote:

...

2014-09-02 11:23 GMT+02:00 Dean Michael Berris :

...

Right. My hunch is that 1.56 now has suppression for the cases where references would have been deduced (as discussed in

http://www.boost.org/doc/libs/1_56_0/libs/optional/doc/html/ boost_optional/dependencies_and_portability/optional_ reference_binding.html ). Maybe the suppression applies even for the case when the conversion operator may have been useful for copy initialization.

However, it works fine with GCC. It looks like VC++ compiler has a bug in overload resolution in copy initialization. It should never consider explicit constructors.

Agreed that it works with GCC. I tested with Clang on OS X too and it doesn't display this problem.

I think the specialization of the behavior for VC++ 2010 that came in 1.56.0 may be disabling the copy constructor inadvertently because the RHS resolves to a `const boost::optional<T>&` -- it may just be the case that the suppression to address the bug is too aggressive, and is causing a once-valid non-explicit constructor to be considered explicit? I can't be sure until I get to play around with it, or look deeply at the code to try and understand it.

I won't be surprised if this is a compiler bug though. I would still like to know though whether there's a work-around, or whether I'm going to have to say in my upcoming release of cpp-netlib that we can't support Boost 1.56.0 with MSVC 2010. That announcement would be unfortunate. :(

Boost 1.56 has already been released, so we cannot fix your problem by changing Boost sources. What you could try in your library is to detect if you are compiling in VC++ and in that case not to cast to optional directly, but to an auxiliary wrapper type: #include struct container { boost::optional<int> optional() const { return value_; } private: boost::optional<int> value_; }; struct OptionalInt { boost::optional<int> _o; explicit OptionalInt(boost::optional<int> o) :_o(o) {} operator boost::optional<int>() { return _o; } }; struct value_wrapper { value_wrapper(const container& c) : c_(c) {} operator int () { return 1; } operator OptionalInt() { return OptionalInt(c_.optional()); } private: const container& c_; }; value_wrapper value(const container& c) { return value_wrapper(c); } int main() { container c; // leave c.value_ alone. boost::optional<int> some_value = value(c); assert(!some_value); } This is not a correct C++, but it happens to work on VC++.

2014-09-02 15:57 GMT+02:00 Andrzej Krzemienski :

2014-09-02 13:39 GMT+02:00 Andrzej Krzemienski :

...

2014-09-02 11:23 GMT+02:00 Dean Michael Berris :

...

...

I just verified that your code example works with 1.55 on VS2010, so it is a regression. I will not be able to have a look at it for the next couple of days. I would like to determine why it even works on 1.55. The converting constructor from arbitrary U is also explicit in 1.55, so it works because of some sequence of conversions that I cannot figure out right now.

Right. My hunch is that 1.56 now has suppression for the cases where references would have been deduced (as discussed in

http://www.boost.org/doc/libs/1_56_0/libs/optional/doc/html/boost_optional/d... ). Maybe the suppression applies even for the case when the conversion operator may have been useful for copy initialization.

However, it works fine with GCC. It looks like VC++ compiler has a bug in overload resolution in copy initialization. It should never consider explicit constructors.

Ok, I managed to isolate the problem. The following code does not compile on VC++2010, although it is correct:

struct A { A(int &&) {} A(A &&) {}

template <typename T> explicit A(T&&) {} };

struct B { operator A() { return A(1); } operator int() { return 0; } };

int main() { A t = B(); }

The unambiguous conversion path is to use B::operator A(), but VC++ somehow finds it ambiguous.

And the above incorrect behaviour is the consequence of a more simple bug in VC++. It allows two user defined conversions in copy initialization: struct A { A(int &&) {} }; struct B { operator int() { return 0; } }; int main() { A t = B(); } This works in VC++, although the code is incorrect.

2014-09-02 16:11 GMT+02:00 Dean Michael Berris :

On Wed Sep 03 2014 at 12:00:56 AM Andrzej Krzemienski wrote:

...

And the above incorrect behaviour is the consequence of a more simple bug in VC++. It allows two user defined conversions in copy initialization:

struct A { A(int &&) {}

};

struct B { operator int() { return 0; } };

int main() { A t = B(); }

This works in VC++, although the code is incorrect.

So it does sound like this is one of those r-value reference implementation bugs in MSVC that I've heard before but couldn't cite. Could it just be that Boost.Optional shouldn't turn on rvalue reference support with MSVC 2010? Will defining just the copy constructor "fix" it? More importantly is there something users of Boost 1.56.0 can do to side-step this particular issue in Boost.Optional?

No, it has nothing to do with rvalue refs -- only with illegal double conversion. struct A { A(int) {} }; struct B { operator int() { return 0; } }; int main() { A t = B(); } This is also an illegal C++ but VC++ allows it. See my other reply for a workaround.

2014-09-02 15:57 GMT+02:00 Andrzej Krzemienski :

2014-09-02 13:39 GMT+02:00 Andrzej Krzemienski :

...

2014-09-02 11:23 GMT+02:00 Dean Michael Berris :

...

...

I just verified that your code example works with 1.55 on VS2010, so it is a regression. I will not be able to have a look at it for the next couple of days. I would like to determine why it even works on 1.55. The converting constructor from arbitrary U is also explicit in 1.55, so it works because of some sequence of conversions that I cannot figure out right now.

Right. My hunch is that 1.56 now has suppression for the cases where references would have been deduced (as discussed in

http://www.boost.org/doc/libs/1_56_0/libs/optional/doc/html/boost_optional/d... ). Maybe the suppression applies even for the case when the conversion operator may have been useful for copy initialization.

However, it works fine with GCC. It looks like VC++ compiler has a bug in overload resolution in copy initialization. It should never consider explicit constructors.

Ok, I managed to isolate the problem. The following code does not compile on VC++2010, although it is correct:

struct A { A(int &&) {} A(A &&) {}

template <typename T> explicit A(T&&) {} };

struct B { operator A() { return A(1); } operator int() { return 0; } };

int main() { A t = B(); }

The unambiguous conversion path is to use B::operator A(), but VC++ somehow finds it ambiguous.

I went to see how Boost.Variant addresses this problem. Interestingly, it doesn't. #include struct Wrapper { operator int () { return 9; } operator boost::variant () { return 7; } }; int main() { boost::variant v = Wrapper(); assert(boost::get<int>(v) == 7); } This program assertion-fails on MSVC. Regards, &rzej

2014-09-09 7:45 GMT+02:00 Dean Michael Berris :

On Tue Sep 09 2014 at 2:20:24 AM Andrzej Krzemienski wrote:

...

I went to see how Boost.Variant addresses this problem. Interestingly, it doesn't.

#include

struct Wrapper { operator int () { return 9; } operator boost::variant () { return 7; } };

int main() { boost::variant v = Wrapper(); assert(boost::get<int>(v) == 7); }

This program assertion-fails on MSVC.

Okay, that's interesting.

What's the process here, do we file a bug to Microsoft? Send in a defect report to the committee? Do we know the clauses in the standard that's supposed to make this "do the right thing"?

I filed this bug report: https://connect.microsoft.com/VisualStudio/feedbackdetail/view/967397/incorr... I suppose MSVC 10.0 is no longer supported, but the same bug is present in all the newer versions of the compiler as well. The standard is clear about how the initialization should behave here, so the bug belongs to MSVC. It so obvious that I do not think that any reference to the standard is necessary. If the developers need it, I will provide it. Regards, &rzej

2014-09-09 16:05 GMT+02:00 Pete bartlett :

I'm not sure we should hold our breath waiting for fully general fix here. Not so long ago I ported a (admittedly huge) code base from MSC++ to standard C++ and there were many many instances of this issue - we called them "doubly-implicit conversions" - to be fixed. Ie cases where MS allows conversion chains but standard does not. Fixing these caused more pain than fixing the Windows API calls! I seriously doubt MS would break all the similar code that must be out there.

-maybe- something could be done in the limited case where the existence of a doubly-implicit conversion causes a compile time error due to ambiguity (as in the case posted to connect)

Yes, it is the limited case we need. Our problem here is the interaction between (1) double conversions and (2) overload ordering in the face of rvalue references. This correct problem does not compile on MSVC: struct A { A(int) {} A(A &&) {} }; struct B { operator A() { return A(1); } operator int() { return 0; } }; int main() { A t = B(); } Because the conversion path is ambiguous. I do not even mind that MSVC considers the double conversion, but I am pretty sure it should be ranked as worse than the direct binding from A to A&&. If I change the rvalue reference to lvalue reference: struct A { A(int) {} A(A const&) {} }; struct B { operator A() { return A(1); } operator int() { return 0; } }; int main() { A t = B(); } This program compiles fine. So it must be conversion path ranking. Regards, &rzej