[Boost][Outcome] Compile error?

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Boost][Outcome] Compile error?

Boost - Dev mailing list
When exceptions are disabled, I get:

In file included from ../../../../boost/outcome/result.hpp:34:
In file included from ../../../../boost/outcome/boost_result.hpp:36:
In file included from ../../../../boost/exception_ptr.hpp:9:
In file included from
../../../../boost/exception/detail/exception_ptr.hpp:17:
../../../../boost/exception/detail/clone_current_exception.hpp:22:6: error:
This header requires exception handling to be enabled.

Is there a way to configure Outcome so it does not use (boost)
exception_ptr?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
Emil Dotchevski wrote:

> When exceptions are disabled, I get:
>
> In file included from ../../../../boost/outcome/result.hpp:34:
> In file included from ../../../../boost/outcome/boost_result.hpp:36:
> In file included from ../../../../boost/exception_ptr.hpp:9:
> In file included from
> ../../../../boost/exception/detail/exception_ptr.hpp:17:
> ../../../../boost/exception/detail/clone_current_exception.hpp:22:6:
> error:
> This header requires exception handling to be enabled.

Are you reporting a bug to yourself?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On 04/07/2020 23:57, Peter Dimov via Boost wrote:

> Emil Dotchevski wrote:
>
>> When exceptions are disabled, I get:
>>
>> In file included from ../../../../boost/outcome/result.hpp:34:
>> In file included from ../../../../boost/outcome/boost_result.hpp:36:
>> In file included from ../../../../boost/exception_ptr.hpp:9:
>> In file included from
>> ../../../../boost/exception/detail/exception_ptr.hpp:17:
>> ../../../../boost/exception/detail/clone_current_exception.hpp:22:6:
>> error:
>> This header requires exception handling to be enabled.
>
> Are you reporting a bug to yourself?

Pretty much +1 from me as well.

Surely we test Boost with exceptions globally disabled in the regression
testing right?

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On Sat, Jul 4, 2020 at 5:58 PM Niall Douglas via Boost <
[hidden email]> wrote:

>
> On 04/07/2020 23:57, Peter Dimov via Boost wrote:
> > Emil Dotchevski wrote:
> >
> >> When exceptions are disabled, I get:
> >>
> >> In file included from ../../../../boost/outcome/result.hpp:34:
> >> In file included from ../../../../boost/outcome/boost_result.hpp:36:
> >> In file included from ../../../../boost/exception_ptr.hpp:9:
> >> In file included from
> >> ../../../../boost/exception/detail/exception_ptr.hpp:17:
> >> ../../../../boost/exception/detail/clone_current_exception.hpp:22:6:
> >> error:
> >> This header requires exception handling to be enabled.
> >
> > Are you reporting a bug to yourself?
>
> Pretty much +1 from me as well.
>
> Surely we test Boost with exceptions globally disabled in the regression
> testing right?

What Peter means is that boost::exception_ptr ought to support
BOOST_NO_EXCEPTIONS. However, it does not, it never has.

It is not trivial to support it because its implementation has been
complicated over the years; remember, it was added when there was no
standard support for exception_ptr. And I've learned that I can't remove
old features because there's code in the wild that uses them. I think this
is the complete list of legacy features:

- If there is standard support, it is able to wrap std::exception_ptr in a
boost::exception_ptr.
- Otherwise, If the exception is a boost::exception, works correctly.
- Otherwise, under old MSVC versions, it is able to talk to the ABI and
clone the exception object correctly anyway.
- Otherwise, it attempts a list of exception types as a fallback.

Not sure what's the best approach here.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
Emil Dotchevski wrote:

> - If there is standard support, it is able to wrap std::exception_ptr in a
> boost::exception_ptr.
> - Otherwise, If the exception is a boost::exception, works correctly.
> - Otherwise, under old MSVC versions, it is able to talk to the ABI and
> clone the exception object correctly anyway.
> - Otherwise, it attempts a list of exception types as a fallback.

boost::current_exception() just returns {} when exceptions are disabled, so
all this disappears. Only boost::copy_exception needs to be reimplemented to
not rely on throw+current_exception. I'm not sure why it needs to do so even
when exceptions are enabled though; the complete type is available at this
point.

Incidentally, the C++11 standard name for copy_exception is
make_exception_ptr, but we never fixed it.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 05/07/2020 03:29, Emil Dotchevski via Boost wrote:

>> Surely we test Boost with exceptions globally disabled in the regression
>> testing right?
>
> What Peter means is that boost::exception_ptr ought to support
> BOOST_NO_EXCEPTIONS. However, it does not, it never has.

I'll admit to surprise here. I always thought that one of the principle
reasons for Boost.Exception is exactly so that Boost can work well
enough with exceptions globally disabled.

I mean, I know I've seen bits of Boost in use on embedded platforms
where exceptions are globally disabled. It worked well enough. So you
surprise me that I was wrong here.

> It is not trivial to support it because its implementation has been
> complicated over the years; remember, it was added when there was no
> standard support for exception_ptr. And I've learned that I can't remove
> old features because there's code in the wild that uses them. I think this
> is the complete list of legacy features:
>
> - If there is standard support, it is able to wrap std::exception_ptr in a
> boost::exception_ptr.

So now I am confused. If std::exception_ptr works well enough with
exceptions globally disabled on all the major standard libraries, and
Boost.Exception is able to wrap that, what's the problem here?

Obviously, this is C++ 11 onwards only which Outcome requires in any
case, but I'm not seeing what the showstopper is here.


Coming back to your original question:

> Is there a way to configure Outcome so it does not use (boost)
> exception_ptr?

The outcome/boost_result.hpp is just a specialisation of
outcome::basic_result<> with Boost types, same as outcome/std_result.hpp
or outcome/experimental/status_result.hpp is.

You can absolutely configure your own outcome::basic_result<>
specialisation. If you simply want to remove the boost::exception_ptr
dependency, I'd suggest cloning outcome/boost_result.hpp into a new
file, and quite literally comment out the Boost.Exception stuff.
Obviously by doing this you'd lose support for result<T,
boost::exception_ptr>, but result<T, boost::system::error_code> should
work fine.

You're the first person to ever raise a need for a
outcome/boost_result.hpp which does not include Boost.Exception. If
there is at least one more person who would want that file split into
error code and exception_ptr editions, I am happy to oblige for after
this Boost release. Just open an issue at
https://github.com/ned14/outcome/issues.

Niall

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On Sun, Jul 5, 2020 at 4:35 AM Niall Douglas via Boost <
[hidden email]> wrote:
>
> On 05/07/2020 03:29, Emil Dotchevski via Boost wrote:
>
> >> Surely we test Boost with exceptions globally disabled in the
regression
> >> testing right?
> >
> > What Peter means is that boost::exception_ptr ought to support
> > BOOST_NO_EXCEPTIONS. However, it does not, it never has.
>
> I'll admit to surprise here. I always thought that one of the principle
> reasons for Boost.Exception is exactly so that Boost can work well
> enough with exceptions globally disabled.

"Well enough" generally means that libraries are supposed (not even
required) to always call boost::throw_exception to throw, which can be
user-defined to do-something-and-not-return under BOOST_NO_EXCEPTIONS.

On exception_ptr, my original thinking (more than 15 years ago) was this:
the parts of Boost Exception that could be used to call
boost::throw_exception should work under BOOST_NO_EXCEPTIONS, but since
exceptions can't be thrown, there's no reason why anyone would want to hold
a pointer to one.

Outcome is the first library to hit that restriction.

> You're the first person to ever raise a need for a
> outcome/boost_result.hpp which does not include Boost.Exception.

From my point of view, I was compiling an example that used outcome::result
and was surprised that it didn't work under BOOST_NO_EXCEPTIONS. Even if we
change boost::exception_ptr to work under BOOST_NO_EXCEPTONS, should
outcome::result be coupled with it?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On 05/07/2020 19:41, Emil Dotchevski via Boost wrote:

> On exception_ptr, my original thinking (more than 15 years ago) was this:
> the parts of Boost Exception that could be used to call
> boost::throw_exception should work under BOOST_NO_EXCEPTIONS, but since
> exceptions can't be thrown, there's no reason why anyone would want to hold
> a pointer to one.

What about make_exception_ptr()?

There is a long story behind that function and whether it ought to work
with exceptions globally disabled or not. Basically, many years ago, I
reported to those STL implementations which were still implementing it
as a throw...catch, instead of direct construction, benchmarks showing
the hideous performance of not using direct construction. The
maintainers of said STL implementations did not think it important
enough to fix, but to my best knowledge, they have all since fixed that.
This reduces the cost of std::make_exception_ptr() from 30k CPU cycles
to about 3k CPU cycles, big gain if you do a lot of those.

I'd pitch the same thing to you: thanks to make_exception_ptr(),
exception_ptr is quite useful with exceptions globally disabled.

>> You're the first person to ever raise a need for a
>> outcome/boost_result.hpp which does not include Boost.Exception.
>
> From my point of view, I was compiling an example that used outcome::result
> and was surprised that it didn't work under BOOST_NO_EXCEPTIONS. Even if we
> change boost::exception_ptr to work under BOOST_NO_EXCEPTONS, should
> outcome::result be coupled with it?

I propose two paths forward:

1. Boost.Exception gains an exception_ptr which works fine with
BOOST_NO_EXCEPTIONS.

2. Boost.Outcome disables use of boost::exception_ptr when
BOOST_NO_EXCEPTIONS is enabled.

I'm easy with either choice. But I think the former "nicer".

Also, for the record, about 70% of my users use result<> with error or
enum codes, perhaps 20% use exception_ptr, the remaining 10% use a
custom type. Those who use exception_ptr are quite vocal, however.


BTW if you are thinking of Boost.Exception improvements, that report
going around for why Boost libraries cannot be epoch-ised, it said the
only blocker for Outcome was its dependency on Exception. So, if
Exception could be epoch-ised, so could Outcome.

Me personally I give that whole proposal a very low priority, but while
we're talking etc etc

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On Mon, Jul 6, 2020 at 7:14 AM Niall Douglas via Boost <
[hidden email]> wrote:
>
> On 05/07/2020 19:41, Emil Dotchevski via Boost wrote:
>
> > On exception_ptr, my original thinking (more than 15 years ago) was
this:
> > the parts of Boost Exception that could be used to call
> > boost::throw_exception should work under BOOST_NO_EXCEPTIONS, but since
> > exceptions can't be thrown, there's no reason why anyone would want to
hold
> > a pointer to one.
>
> What about make_exception_ptr()?
>
> There is a long story behind that function and whether it ought to work
> with exceptions globally disabled or not. Basically, many years ago, I
> reported to those STL implementations which were still implementing it
> as a throw...catch

You see, rather than fixing STL they should have fixed the damn compiler to
not go full retard when it sees a throw. There is no reason why a throw
followed by a catch should be slow. Fix that and we wouldn't need
herbceptions or whatever. But I digress.

> I propose two paths forward:
>
> 1. Boost.Exception gains an exception_ptr which works fine with
> BOOST_NO_EXCEPTIONS.
>
> 2. Boost.Outcome disables use of boost::exception_ptr when
> BOOST_NO_EXCEPTIONS is enabled.
>
> I'm easy with either choice. But I think the former "nicer".

Not just nicer, it's the correct thing to do.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On 06/07/2020 22:24, Emil Dotchevski via Boost wrote:

>> I propose two paths forward:
>>
>> 1. Boost.Exception gains an exception_ptr which works fine with
>> BOOST_NO_EXCEPTIONS.
>>
>> 2. Boost.Outcome disables use of boost::exception_ptr when
>> BOOST_NO_EXCEPTIONS is enabled.
>>
>> I'm easy with either choice. But I think the former "nicer".
>
> Not just nicer, it's the correct thing to do.

We've done a half way house for the 1.74 release: Emil has added
boost::make_exception_ptr(). This has enabled Outcome to dispense with
including exception_ptr, if on Boost 1.74 or later, if C++ exceptions
are globally disabled. Thus, Outcome should now compile in Boost 1.74 if
C++ exceptions are globally disabled.

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
Niall Douglas wrote:

> We've done a half way house for the 1.74 release: Emil has added
> boost::make_exception_ptr().

Has he now?

This is a potentially breaking change which is not allowed at this point.
For reference, here's the release schedule:

06/24 closed for breaking changes
07/01 closed for major changes


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On 07/07/2020 15:37, Peter Dimov via Boost wrote:

> Niall Douglas wrote:
>
>> We've done a half way house for the 1.74 release: Emil has added
>> boost::make_exception_ptr().
>
> Has he now?
>
> This is a potentially breaking change which is not allowed at this
> point. For reference, here's the release schedule:
>
> 06/24 closed for breaking changes
> 07/01 closed for major changes

We discussed whether adding an alternative naming for copy_exception()
could be called a major change. You can see the commit at
https://github.com/boostorg/exception/commit/330008445c38aa793a07909d623addfc885464e6.

We concluded that it was so minor as to be unimportant, and for Outcome
the relevant commit
https://github.com/ned14/outcome/commit/eff410f5943e5acc16ab005e128096b27021902f
I'm classing as a minor bug fix.

So, in my opinion, we're all good.

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
Niall Douglas wrote:

> We discussed whether adding an alternative naming for copy_exception()
> could be called a major change.

It is both major (API change) and potentially breaking (code that already
defines its own make_exception_ptr may break).

"We discussed" carries no weight unless one of the participants in said
discussion was a release manager.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On 07/07/2020 16:05, Peter Dimov via Boost wrote:
> Niall Douglas wrote:
>
>> We discussed whether adding an alternative naming for copy_exception()
>> could be called a major change.
>
> It is both major (API change) and potentially breaking (code that
> already defines its own make_exception_ptr may break).

Do you mean injecting a make_exception_ptr() into the boost namespace?

> "We discussed" carries no weight unless one of the participants in said
> discussion was a release manager.

Our discussion was in good faith. Nothing was meant. If you want it
rolled back, that's also fine.

Niall

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
Am 07.07.20 um 17:19 schrieb Niall Douglas via Boost:
> On 07/07/2020 16:05, Peter Dimov via Boost wrote:
>> Niall Douglas wrote:
>>
>>> We discussed whether adding an alternative naming for copy_exception()
>>> could be called a major change.
>> It is both major (API change) and potentially breaking (code that
>> already defines its own make_exception_ptr may break).

The commit in question is
https://github.com/ned14/outcome/commit/eff410f5943e5acc16ab005e128096b27021902f

 From API change view it conditionally removes a function from the
detail namespace with no effect for the upcoming release.
In the case BOOST_NO_EXCEPTIONS is defined it replaces an include of
"boost/exception_ptr.hpp" with a forward declaration of that class.
 From what I understood "boost/exception_ptr.hpp" cannot be included
when BOOST_NO_EXCEPTIONS is defined or compilation will fail.

Hence I conclude that this is a pure bugfix (things compile that did not
before) with no API or breaking change. As bugfix commits are still
allowed according to the review schedule this is fine.

Or am I missing anything?



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On 07/07/2020 16:37, Alexander Grund via Boost wrote:

> Am 07.07.20 um 17:19 schrieb Niall Douglas via Boost:
>> On 07/07/2020 16:05, Peter Dimov via Boost wrote:
>>> Niall Douglas wrote:
>>>
>>>> We discussed whether adding an alternative naming for copy_exception()
>>>> could be called a major change.
>>> It is both major (API change) and potentially breaking (code that
>>> already defines its own make_exception_ptr may break).
>
> The commit in question is
> https://github.com/ned14/outcome/commit/eff410f5943e5acc16ab005e128096b27021902f
>
>
> From API change view it conditionally removes a function from the detail
> namespace with no effect for the upcoming release.
> In the case BOOST_NO_EXCEPTIONS is defined it replaces an include of
> "boost/exception_ptr.hpp" with a forward declaration of that class.
> From what I understood "boost/exception_ptr.hpp" cannot be included when
> BOOST_NO_EXCEPTIONS is defined or compilation will fail.
>
> Hence I conclude that this is a pure bugfix (things compile that did not
> before) with no API or breaking change. As bugfix commits are still
> allowed according to the review schedule this is fine.
>
> Or am I missing anything?

Yes, I think Peter's issue is with the commit to Boost.Exception, not to
Boost.Outcome.

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
On Tue, Jul 7, 2020 at 9:11 AM Niall Douglas via Boost <
[hidden email]> wrote:

> Yes, I think Peter's issue is with the commit to Boost.Exception, not to
> Boost.Outcome.
>

Maybe that was a bad idea.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 2020-07-07 18:05, Peter Dimov via Boost wrote:
> Niall Douglas wrote:
>
>> We discussed whether adding an alternative naming for copy_exception()
>> could be called a major change.
>
> It is both major (API change) and potentially breaking (code that
> already defines its own make_exception_ptr may break).

Not that I'm arguing with classification of this change as a major one,
but I wouldn't consider it a breaking one as I don't think adding any
symbols to a library namespace (except where explicitly allowed by
contract) is a valid use of the library. This is the same stance as the
standard C++ library is taking with regard to user's definitions in
namespace std.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 07/07/2020 21:10, Emil Dotchevski via Boost wrote:
> On Tue, Jul 7, 2020 at 9:11 AM Niall Douglas via Boost <
> [hidden email]> wrote:
>
>> Yes, I think Peter's issue is with the commit to Boost.Exception, not to
>> Boost.Outcome.
>>
>
> Maybe that was a bad idea.

Well, I guess we'd better go ask permission from a release manager so,
though I would still call that a "minor change". I had thought that by
now one would have interceded with an opinion. Emil do you want to do
the honours of emailing them?

Also, we're just before Beta right? So, an ideal time to see if
boost::make_exception_ptr() collides with user-injected
boost::make_exception_ptr() would be for the beta release right?

Though, to be honest, if there is collision, in my opinion that's a
situation where users will just auto-fix their code silently and we'll
never know. Users know not to be injecting new functions into not their
namespaces.

Niall

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost][Outcome] Compile error?

Boost - Dev mailing list
Niall Douglas wrote:
> Also, we're just before Beta right? So, an ideal time to see if...

Why the rush? It's 9 years late, why not wait a few more months?

> ... boost::make_exception_ptr() collides with user-injected
> boost::make_exception_ptr() would be for the beta release right?

Adding things to boost:: is uncommon for user code, but an unqualified
make_exception_ptr(x), where x has boost as an associated namespace, may
become ambiguous or change behavior.

Although defining boost::make_exception_ptr is not unheard of, either. See
f.ex.

https://github.com/boostorg/thread/blob/96cd717b331e62095aa58d6768dab3baa03fcdce/test/sync/futures/future/get_pass.cpp#L49-L53

which even does something different.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
12