Re: [boost] [Stacktrace] review

Subject: Re: [boost] [Stacktrace] review
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2016-12-15 19:00:42

On Thu, Dec 15, 2016 at 3:11 PM, Andrey Semashev <andrey.semashev_at_[hidden]>
wrote:

> On Thu, Dec 15, 2016 at 11:21 PM, Emil Dotchevski
> <emildotchevski_at_[hidden]> wrote:
> > On Thu, Dec 15, 2016 at 11:26 AM, Robert Ramey <ramey_at_[hidden]> wrote:
> >
> > The boost::exception type (which, again, does not depend on anything,
> > boost/exception/exception.hpp does not include any headers) currently
> holds
> > an int and a char const * for BOOST_THROW_EXCEPTION to store __LINE__ and
> > __FILE__. The cost of adding the ability of boost::exception to carry a
> > stack trace would be one more pointer (note: for this purpose it's
> possible
> > to use Boost Exception's ability to transport arbitrary data in any
> > exception, but that would make boost::throw_exception dependent on Boost
> > Exception, which I've worked very hard to avoid so far.) That pointer can
> > remain unused if the user chooses to turn off the automatic capture of
> the
> > stack trace.
>
> The problem is not the pointer per se. The problem is that your
> proposal implies a dependency on the code that fills that pointer, the
> Stacktrace library.

I am not proposing a dependency of throw_exception on Starktrace, but
removing a minimal amount of code from Stacktrace (e.g. something that
packs everything into a memory buffer) and putting it into throw_exception.
I do think that logically, this code should be left in a separate header,
but it has to be written without any Boost dependencies. If that is not
possible, I agree that the integration is a bad idea.

> Robert expands on the dependency injection and I'm
> worried by performance degradation caused by this change.
>

I admit I worry about coupling more than I do about the performance. I am
speculating, but I suspect that in my own uses emitting exceptions has to
be something like 1000x slower than it is to maybe show in my profiling.
Perhaps I'm wrong.

I must say I'm not really buying the "automatic stacktrace from
> exception" advantage. Really, the source file and line, combined with
> sufficient logging, were quite enough for me for years; if there were
> cases I wished to have a backtrace, they were rare enough I don't
> remember one of them.
>
> So please, just define a separate macro with this new feature and
> leave BOOST_THROW_EXCEPTION as is.
>

First, let's again clarify: there is no reason to change the
BOOST_THROW_EXCEPTION macro. The stack trace can be captured by the
boost::throw_exception function template.

The problem is that users who want take advantage of the stack trace (I
understand you are not one of them) have no control over the library level
code that uses boost::throw_exception.

That said, if you agree that ultimately it is the user who should be able
to decide whether or not "good" Boost libraries embed stack trace in
exceptions they emit, the argument wouldn't be whether or not capturing the
stack trace is integrated into boost::throw_exception, but whether or not
it is enabled by default. Either way, the boost::exception class (defined
in boost/exception/exception.hpp) should provide a hook for that
integration in order to avoid coupling between boost::throw_exception and
Stacktrace.

> The config macro to disable the stacktrace doesn't satisfy me. First,
> because it is a way to opt out whereas I believe such a feature should
> be an opt in as it is expensive.

I think that how expensive it is needs to be investigated and discussed.
Either way it is a compromise.

> What I'm thinking is that there could be a hook that would allow a
> user-defined function to be called on the exception that is about to
> be thrown. By default no hook is installed, so throwing an exception
> works exactly as BOOST_THROW_EXCEPTION currently works.
> Boost.Stacktrace could provide its hook, which the user could install
> to enable automatic stacktraces. The important part is that this is
> user's choice and that it decouples throwing the exception from
> Boost.Stacktrace.
>

Do you mean a run-time hook or a compile-time hook?

I think this should be compile-time hook in order to avoid the perils of
dynamic initialization in complex multi-thread programs. But we already
have that compile-time hook, it's called boost::throw_exception.

Emil