On Wed, Dec 14, 2016 at 2:43 AM, Niall Douglas <s_sourceforge@nedprod.com> 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