[boost] [stacktrace] Review

Subject: [boost] [stacktrace] Review
From: Bjorn Reese (breese_at_[hidden])
Date: 2017-03-25 12:54:21

I vote for the unconditional acceptance of Boost.Stacktrace.

The design is sound. The separation of stacktrace and frame is just
right, and safe_dump_to is an elegant solution to handle async-safety
(much better than the fork-exec alternative.)

The implementation (I did not look at the Windows parts) looks solid.
There are details that looks platform-specific -- e.g. "/proc/self/exe"
in detail/addr2line_impls.hpp is not Posix -- but I did not notice
anything that cannot be ported if needed.

The documentation is well-structured. I appreciate that the reference
documentation contains both complexity and async-handler-safety
comments.

This library is very useful to developers.

I spent a couple of hours going through the documentation and source
for the review. Prior to that I have followed the development of
this library, and used an earlier version in a commercial product.

I have in-depth knowledge of the Posix side (having once implemented
an async-safe stacktrace.)

Wishlist:

Make throw_with_trace of the API. Put it in a separate header so
there is no dependency on Boost.Exception unless this header is
included. In order to be consistent with Boost.Exception I suggest
that the throwing function is called stacktrace::throw_exception().

Add an example with basic_stacktrace that uses a custom allocator.

Add a simpler example that uses std::terminate(). The current one
is rather overwhelming. Notice that I am not asking for the current
example to be removed.

Minor comments:

Add a note to Getting Started that the stored backtrace.dump only is
valid if the program has not be re-compiled between runs, as this is
what developers do all the time.

I have no opinion about windows.h.