On Wed, Dec 14, 2016 at 3:43 AM, Niall Douglas
- What is your evaluation of the design?
I'm generally pleased with the design. There's a documentation section "Exceptions with stacktrace" that discusses techniques for incorporating stacktrace data into your own exceptions. I see a real possibility for integration with Boost.Exception, perhaps even a mechanism to make BOOST_THROW_EXCEPTION embed the stacktrace at the throw point. This is not an objection to accepting the library, but rather a feature request for when we do. I do have a question about use of a template parameter to constrain the number of entries, rather than a runtime parameter. Presumably that's necessary to permit certain backend implementations to be async-signal-safe? (Whatever the rationale, a note in the documentation explaining it would be nice.) My objection to that is that apparently, with the present design, if I want to write a function that accepts any specialization of basic_stacktrace, that must itself be a template function. If I'm correct about the reason for using a template parameter, would it be reasonable to split the class into a non-template base class supporting all the methods, with a template subclass that provides the storage? The base class could store a (pointer, length) passed by the subclass constructor. Without having looked at the implementation, I expect that you wouldn't even need virtual-function dispatch: the only indirection would be through the data buffer pointer. That would even permit a subclass that provides dynamic storage, for async-signal-unsafe use. Then a processing function could accept reference-to-base-class and handle any stacktrace instance.
- What is your evaluation of the implementation?
Sorry, I haven't looked at the implementation.
- What is your evaluation of the documentation?
Generally good. It could stand a cleanup pass; there are a few typos here and there. I do like that each method explicitly states both complexity and async signal safety. The description of the nullary frame() constructor says that source_line() will return both empty string and 0.
- What is your evaluation of the potential usefulness of the library?
VERY! I want this.
- Did you try to use the library? With what compiler? Did you have any problems?
No, I haven't tried to use it. (A deadline is coming up fast.)
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read through the documentation.
- Are you knowledgeable about the problem domain?
I have occasionally used platform-specific tactics to try to achieve the same thing, on a couple different platforms. Each time it's been painful enough to make me wish that this problem were solved, once and for all.
And finally, every review should attempt to answer this question:
- Do you think the library should be accepted as a Boost library?
Yes I do. I would like to see the non-template base class, but that's not a showstopper: I would rather have this library without than not have it at all. As I mentioned, I would like to see integration with Boost.Exception. That just seems like a natural next step. I see further opportunities for integration with Boost.Context, Boost.Fiber...