Re: [boost] [Stacktrace] review

Subject: Re: [boost] [Stacktrace] review
From: Barrett Adair (barrettellisadair_at_[hidden])
Date: 2016-12-22 19:26:39

On Wed, Dec 14, 2016 at 2:43 AM, Niall Douglas
<s_sourceforge_at_[hidden]> wrote:
> The formal review of the Stacktrace library by Antony Polukhin starts
> today 14th Dec and will conclude before Christmas. I appreciate we
> are likely a bit tired out from the many library reviews recently and
> of course it's Christmas, but given the lack of a portable way to
> work with stack backtraces, which you inevitably need to do
> eventually in any non-toy production application, Stacktrace needs
> your review!
>
> For your review you may wish to consider the following questions:
>
> What is your evaluation of the design?

I like it.

> What is your evaluation of the implementation?

I've never implemented a stacktrace reader, so I don't have much to add.
The code did not raise any red flags for me, though.

> What is your evaluation of the documentation?

Documentation is solid.

> What is your evaluation of the potential usefulness of the library?

Very useful. I will likely use this at work.

> Did you try to use the library?  With what compiler?  Did you have any problems?

Yes, GCC 5.3, no problems. This stacktrace made me smile:

0# boost::stacktrace::detail::backend::backend(void**, unsigned long)
 1# s::~s()
 2# void __gnu_cxx::new_allocator<s>::destroy<s>(s*)
 3# void std::allocator_traits<std::allocator<s>
>::destroy<s>(std::allocator<s>&, s*)
 4# std::_Sp_counted_ptr_inplace<s, std::allocator<s>,
(__gnu_cxx::_Lock_policy)2>::_M_dispose()
 5# std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release()
 6# std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
 7# std::__shared_ptr<s, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr()
 8# std::shared_ptr<s>::~shared_ptr()
 9# hello()::{lambda()#1}::~hello()
10# std::_Function_base::_Base_manager<hello()::{lambda()#1}>::_M_destroy(std::_Any_data&,
std::integral_constant<bool, false>)
11# std::_Function_base::_Base_manager<hello()::{lambda()#1}>::_M_manager(std::_Any_data&,
std::_Function_base::_Base_manager<hello()::{lambda()#1}> const&,
std::_Manager_operation)
12# std::_Function_base::~_Function_base()
13# std::function<void ()>::~function()
14# void std::_Destroy<std::function<void ()> >(std::function<void ()>*)
15# void std::_Destroy_aux<false>::__destroy<std::function<void
()>*>(std::function<void ()>*, std::function<void ()>*)
16# void std::_Destroy<std::function<void ()>*>(std::function<void
()>*, std::function<void ()>*)
17# void std::_Destroy<std::function<void ()>*, std::function<void ()>
>(std::function<void ()>*, std::function<void ()>*,
std::allocator<std::function<void ()> >&)
18# std::vector<std::function<void ()>,
std::allocator<std::function<void ()> > >::~vector()
19# hello()
20# main
21# __libc_start_main
22# _start

> How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

About 45 minutes total. I skimmed the code, the documentation, and the
test cases. Then,
I made a little test program.

> Are you knowledgeable about the problem domain?

No.

> Do you think the library should be accepted as a Boost library?

Yes, unequivocally. The concerns about async safety are interesting,
but ultimately
this library is useful to me (and likely many others) regardless of
signal support.

Barrett