On 2/14/22 08:49, Vinnie Falco via Boost wrote:

On Sun, Feb 13, 2022 at 1:44 PM Peter Dimov via Boost wrote:

...

Recommendation one: add source_location default arguments to your throwing functions so that the information logged at the catch site points to the call that threw.

I am less excited about changing the signatures of all public functions. I thought that BOOST_THROW_EXCEPTION attached a stack trace to the exception?

It doesn't. Collecting the stacktrace is expensive and rarely needed, so it would be unreasonable to do that by default. But you can attach it using Boost.Exception if you want.

On Mon, Feb 14, 2022 at 7:46 AM Peter Dimov via Boost wrote:

Vinnie Falco wrote:

...

On Sun, Feb 13, 2022 at 1:44 PM Peter Dimov via Boost wrote:

...

Recommendation one: add source_location default arguments to your throwing functions so that the information logged at the catch site points to the call that threw.

I am less excited about changing the signatures of all public

functions. I

...

thought that BOOST_THROW_EXCEPTION attached a stack trace to the exception?

No, it doesn't attach a stack trace.

No, it attached the throw location in a different way. The plan is to expose the new source_location type through the same get_error_info interface that is used to access the legacy boost::throw_function, boost::throw_file and boost::throw_line. Also, we should continue to provide this information the "old" way, so that old code can read the source_location.

On Mon, Feb 14, 2022 at 3:30 PM Gavin Lambert via Boost < boost@lists.boost.org> wrote:

On 14/02/2022 18:49, Vinnie Falco wrote:

...

I am less excited about changing the signatures of all public functions. I thought that BOOST_THROW_EXCEPTION attached a stack trace to the exception? Wouldn't that provide the same or more information as BOOST_CURRENT_LOCATION at the user's call site?

Personally, I find passing around source-locations to be completely useless (and subject to binary bloat and undesirable disclosure, as Niall pointed out). Stack traces all the way.

That's the reason I think that we should keep the ability of all variants of the throw machinery in Boost to attach arbitrary information to exceptions: we don't know what user finds which info useful and which info unacceptable. The boost::exception type itself is designed to add negligible cost, and if that is not the case, we solve that problem rather than removing it (currently, the cost is due to support for boost::exception_ptr, and while I am not convinced that it causes any practical problems, in the spirit of C++ it should be optimized if C++11 is available).

On 2/15/22 03:50, Peter Dimov via Boost wrote:

Gavin Lambert wrote:

...

Not sure about gcc/clang/Linux, but with msvc/Windows it's possible to recover a stack trace from any thrown exception regardless of type, without any fanfare at the throw site (albeit only by relying on some fugly compiler internals).

Not sure that's true for Linux/macOS. Windows exception handling is unique in that the stack is not touched (except for running the destructors) during unwinding so the frames are still intact.

But the destructors can clobber the stack, can't they?

...

Although also I thought one of the main selling points of Boost.StackTrace is that it's nearly free to capture a trace at time of exception; it's only formatting into human-readable that's potentially expensive, but that only happens at the other end on demand.

Personally, I find passing around source-locations to be completely useless (and subject to binary bloat and undesirable disclosure, as Niall pointed out). Stack traces all the way.

Now that Stacktrace is in the C++23 standard, there's an interesting proposal to make stacktrace captures automatic on throw:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2370r2.html

although I'm not sure it will fly, even for C++26.

Note the "Recommend to turn off by default" in Design decisions. Capturing the stacktrace isn't cheap, even without symbol resolution, so doing this everywhere by default is unacceptable, IMHO. BTW, the original discussion was here: https://lists.isocpp.org/std-proposals/2021/04/2539.php

But there's more to it. If you follow a straightforward style in which you immediately throw on error, capturing stack traces on throw works well. But if your library is written to use error codes internally, so that it can expose a dual API to the user, that is, to be usable with either exceptions or error codes, things are not so rosy. That's because failures are propagated upwards through several layers using error codes, and only at the last layer is that turned into an exception. So you get a stack trace from that point upwards, which is not as helpful.

This is one of the reasons why I keep the dual error reporting down to the lowest level in Boost.Filesystem. But I admit, writing such code is a pain, to say the least.

On Mon, Feb 14, 2022 at 4:50 PM Peter Dimov via Boost wrote:

It's nice to have stack traces, but we can't realistically make ThrowException depend on Stacktrace, for several reasons. First and least, dependency cycles. Second, Stacktrace is not header-only, so all header-only libraries that throw exceptions will acquire a compiled dependency, which is a massive pain.

And third, even for compiled libraries, the way Stacktrace uses separate library per backend makes it hard to express the dependency in b2 or CMake, because you have no idea what target your library needs to link to.

All in all, that's currently impractical (for Boost libraries.)

It is not practical to make all code that deals with Boost exceptions depend on Stacktrace, but is it practical to provide an interface for the user to attach a stack trace, e.g. a callback that takes a boost::exception by reference, and we pass the exception object to it before we throw it?

But there's more to it. If you follow a straightforward style in which you immediately throw on error, capturing stack traces on throw works well. But if your library is written to use error codes internally, so that it can expose a dual API to the user, that is, to be usable with either exceptions or error codes, things are not so rosy. That's because failures are propagated upwards through several layers using error codes, and only at the last layer is that turned into an exception. So you get a stack trace from that point upwards, which is not as helpful.

Boost LEAF fixes this. :) In principle it is not correct to focus on any particular error info, because what error info is needed is project-specific and/or user-specific. It also doesn't make sense in terms of coupling, not to mention (as you pointed out) it could lead to circular dependencies. Ideally, we provide the capability for obtaining different kinds of error info, e.g. a stack trace, and the capability to transport it when errors are propagated, and leave it to the user to stitch things up together.

It's nice to have stack traces, but we can't realistically make ThrowException depend on Stacktrace, for several reasons. First and least, dependency cycles. Second, Stacktrace is not header-only, so all header-only libraries that throw exceptions will acquire a compiled dependency, which is a massive pain.

And third, even for compiled libraries, the way Stacktrace uses separate library per backend makes it hard to express the dependency in b2 or CMake, because you have no idea what target your library needs to link to. Perhaps StackTrace could be split into a capture-only component that has

On 15/02/2022 13:50, Peter Dimov wrote: the properties you desire and a formatting component for the rest? Although I suspect platform differences for stack capture would likely make having it be header-only too impractical. Still, I usually regard header-only as a misfeature anyway. What's probably a more practical solution might be to have potential throw sites declare a context object on their stack. Any Boost-aware throw that occurs while these objects are on the stack would capture whatever additional information is requested by the context objects that are still in scope at the time. This might be a stack trace, a source location, a plain string, or any other context info (e.g. method parameters) that seems useful. (This fixes the dependency problem because it could be StackTrace, or a third library dependent on both, that provides the stack-tracing-capture context class. And one instance of this could be declared at the top of each thread to cause stack tracing on any Boost-aware exception thrown on the thread.) One possible downside is that this is unlikely to be zero-cost in the successful path, unless you can avoid making copies of the data unless an exception is actually thrown, which might limit what can be captured (e.g. capturing calculated rvalues may be harder, though possibly a lambda may help hide that).

But there's more to it. If you follow a straightforward style in which you immediately throw on error, capturing stack traces on throw works well. But if your library is written to use error codes internally, so that it can expose a dual API to the user, that is, to be usable with either exceptions or error codes, things are not so rosy. That's because failures are propagated upwards through several layers using error codes, and only at the last layer is that turned into an exception. So you get a stack trace from that point upwards, which is not as helpful. Sure, but there still has to be some mechanism to pass that additional information along with the error code. (Such as Boost.LEAF.)

On 15/02/2022 14:53, Peter Dimov wrote:

...

What's probably a more practical solution might be to have potential throw sites declare a context object on their stack. Any Boost-aware throw that occurs while these objects are on the stack would capture whatever additional information is requested by the context objects that are still in scope at the time. This might be a stack trace, a source location, a plain string, or any other context info (e.g. method parameters) that seems useful.

I think I like Emil's user-provided handler better, because with it the user would control whether and what is collected.

I did consider mentioning that, but what I don't like about that option is that it leaves things a bit too vague. (And in terms of ThrowException itself, it's fundamentally equivalent to the NO_EXCEPTIONS path, by making it call a user-provided method to do the actual work. Which then seems to defeat the point of having it be a library at all.) It puts the burden entirely on the end-user to select the mechanism for error info propagation, and library authors would have a harder time interfacing with it.

Gavin Lambert wrote:

On 15/02/2022 14:53, Peter Dimov wrote:

...

I think I like Emil's user-provided handler better, because with it the user would control whether and what is collected.

I did consider mentioning that, but what I don't like about that option is that it leaves things a bit too vague.

(And in terms of ThrowException itself, it's fundamentally equivalent to the NO_EXCEPTIONS path, by making it call a user-provided method to do the actual work. Which then seems to defeat the point of having it be a library at all.)

It puts the burden entirely on the end-user to select the mechanism for error info propagation, and library authors would have a harder time interfacing with it.

I think there might be a misunderstanding here. What I envisage is this void (*throw_handler)( std::exception const& ex, boost::exception& bx, boost::source_location const* loc ) = default_throw_handler; void set_throw_handler( auto* p ) { throw_handler = p; } void default_throw_handler( std::exception const& ex, boost::exception& bx, boost::source_location const* loc ) { if( loc ) bx.set_location( *loc ); } template<class E> BOOST_NORETURN void throw_exception( E const & e ) { wrapexcept<E> wx( e ); throw_handler( e, e, 0 ); throw wx; } template<class E> BOOST_NORETURN void throw_exception( E const & e, boost::source_location const & loc ) { wrapexcept<E> wx( e ); throw_handler( e, e, &loc ); throw wx; } That is, boost::throw_exception still adds a boost::exception base, and that's the mechanism for error info propagation; the user just gets to put things into it before it's thrown - such as a stack trace.

Emil Dotchevski wrote:

Thank you for spelling it out. Maybe take the boost::exception object as a pointer though? That way this (standard for Boost in the future) handler can be invoked with exception objects that do not derive from boost::exception (even though boost::throw_exception guarantees that they do).

Well, the point of the handler is to put things into the boost::exception base, so if there isn't one... :-)

On 15/02/2022 17:25, Emil Dotchevski wrote:

I know it's why you did it by reference, but this handler can be useful for other libraries also, as well as for e.g. logging, etc. It's a chance to do something before a Boost library throws.

Why would such a library call the callback without just using throw_exception itself? And throw_exception guarantees that the boost::exception exists.

On 2/15/22 09:56, Emil Dotchevski via Boost wrote:

On Mon, Feb 14, 2022 at 8:32 PM Gavin Lambert via Boost < boost@lists.boost.org> wrote:

...

On 15/02/2022 17:25, Emil Dotchevski wrote:

...

I know it's why you did it by reference, but this handler can be useful

for

...

...

other libraries also, as well as for e.g. logging, etc. It's a chance to do something before a Boost library throws.

Why would such a library call the callback without just using throw_exception itself? And throw_exception guarantees that the boost::exception exists.

To avoid the dependency on boost::throw_exception.

The callback pointer has to live somewhere, as does the default callback. My understanding is it's Boost.ThrowException, so the dependency will be there anyway.

Gavin Lambert wrote:

But that's also a bit different from what I was envisaging -- why is it that you've modified the callback signature to carry the source_location across? Wouldn't this be better:

void (*throw_handler)( std::exception const& ex, boost::exception& bx ) = default_throw_handler;

void set_throw_handler( auto* p ) { throw_handler = p; }

void default_throw_handler( std::exception const& ex, boost::exception& bx ) { }

template<class E> BOOST_NORETURN void throw_exception( E const & e ) { wrapexcept<E> wx( e ); throw_handler( e, wx ); throw wx; }

template<class E> BOOST_NORETURN void throw_exception( E const & e, boost::source_location const & loc ) { wrapexcept<E> wx( e ); wx.set_location( loc ); throw_handler( e, wx ); throw wx; }

Yeah, I made a mistake with calling the handler with `e`, it should have been `wx` in both places. Yes, that works too. My idea was to allow the user to _not_ put the source location into `bx` but we might as well let him clear it if needed (and obtain it from there for logging etc.)

On 15/02/2022 15:11, Emil Dotchevski wrote:

In terms of the latter, Gavin quite literally describes Boost LEAF. It is indeed possible for "Boost-aware" throws to automatically detect what info is actually needed for error handling, and only spend cycles capturing that. This is exactly how leaf::on_error works.

Not quite. LEAF works explicitly through TLS rather than the local stack, which makes it less compatible with coroutines, AFAIK. Granted that the mechanism I proposed would probably not be implementable without a TLS root to register with or something; while the context objects themselves would naturally be coroutine-compatible, a way to actually *find* which ones are still in scope in a particular context is harder to manage without making those sorts of assumptions. Unfortunately C++ lacks execution-context-ambient state that would work in either scenario.

On 15/02/2022 15:46, Emil Dotchevski wrote:

...

Not quite. LEAF works explicitly through TLS rather than the local stack, which makes it less compatible with coroutines, AFAIK.

Are exceptions compatible with coroutines? Exception handling does use TLS.

Exceptions propagate back through the coroutine to its consumer via entirely compiler-generated code (or equivalent library code for pre-C++20 coroutines), somewhat like (but not identical to) futures.

This can work to exactly the same extent it can be implemented in LEAF. To make it compatible with coroutines, the leaf::context object (which contains the error info objects) just needs to be deactivate()d in one thread, then activate()d in the new thread (which points TLS pointers to its contents).

Where would you put such calls? How would you know which ones to call?

Niall Douglas wrote:

On 13/02/2022 21:43, Peter Dimov via Boost wrote:

...

Recommendation two: add source locations to your error_codes if using Boost.System.

That seems unwise advice to give unqualified. Every time you instance a source location you add the path of your source file to your binary, which bloats your binary.

That's correct. In addition to bloating the binary, it also embeds potentially sensitive information into it (file and function names.) The macro BOOST_DISABLE_CURRENT_LOCATION, when defined, makes BOOST_CURRENT_LOCATION resolve to a default-constructed boost::source_location, which stores nothing except empty strings. See https://www.boost.org/doc/libs/develop/libs/assert/doc/html/assert.html#boos... This mirrors the behavior of the already existing BOOST_DISABLE_CURRENT_FUNCTION, which has a similar purpose.

Niall Douglas wrote:

I also think you need a separate macro control for boost::system::error_code's use of source location, because it won't be unusual to want global source location function to be working but error_code's use of it to be globally disabled.

boost::system::error_code doesn't create any source locations by itself. If you give it a pointer to one, it stores it. If not, not.