[boost] [review] BOOST_SCOPE_EXIT review results

From: John Phillips (phillips_at_[hidden])
Date: 2008-05-07 15:17:44

   Here are the review results for the Scope Exit library, submitted by 
Alexander Nasonov.

   I would be remiss if I didn't begin these results with an apology and 
a thank you. The review wizards are sincerely sorry that the result of 
this review took so long to be presented, and very grateful that 
Alexander has been so patient and according to a post on the forum has 
even been working to improve the library in the interim when time allowed.

   The following people contributed to the review.

Kim Barrett, Steven Wantanabe, Oleg Abrosimov, Andrey Semashev, Peder 
Holt, Johan Nilsson, Maxim Yanchenko, Pavel Vozenilek, Aaron 
LaFramboise, Ben Kuppers, Goran Mitrovic, Tom Brinkman, Martin Wille, 
Joseph Gauterin, Matt Gruenke, Dave Abrahams, Christian Holmquist, 
Mathais Gaunard, Michael Marcin, Daniel Wallin, Zach Laine, Sid Sacek, 
Wang Yun, and Ilya Sokolov

   After carefully considering the review comments, I'm pleased to 
announce that the Scope Exit library is accepted into Boost.

   As is almost always the case, there are some notes for improvement 
for this library, and in this case there is even a suggestion for a 
substantial future restructuring.

   Let's start with the big one first. As it is currently implemented, 
SCOPE_EXIT suggests a method for creating a general closure mechanism as 
a library. This is a useful enough idea that many of the reviewers 
strongly encouraged Alexander to do it, and provided ideas for 
implementation details. Alexander should continue to work on this, and 
when it is ready for review, submit it to Boost. When he gets it 
accepted into boost, he should consider reworking SCOPE_EXIT to build it 
on top of the closures and a more traditional scope guard. If this is 
practical, he should create the new implementation and possibly submit 
it for a mini-review, since it will be such a big change from the 
current form.

   On to the other issues.

   There were a number of comments on the documentation. Some of the 
most detailed were in the review by Pavel. Much of the requested 
information is currently present in the documentation, but not in a 
format that new readers find intuitive. Along with reorganization, some 
more examples, some more comparison information, and some actual 
performance information would be a good idea.

   Kim would like to see variadic macros supported in the 
implementation. This is a good idea if a large fraction of currently 
available compilers support them, but otherwise a low priority.

   Currently the library requires the user to include type_of headers, 
even though there is no place in the user code where a type_of appears. 
This will be confusing for many new users. If possible, look for a way 
to make the library more self-contained in this regard.

   Other reviewers requested ways to use the library without any type_of 
dependence. This would expand the audience for the library, but may not 
be practical. Consider it and act appropriately.

   Aaron requested some syntactic sugar for things like if(condition) 
tests. This may not be a good choice, since it greatly expands the 
interface while providing only minor simplifications to calls. Consider 
it and choose.

   The code needs to check for msvc compilers, since the __LINE__ 
statement has to be replaced for them.

   BOOST_SCOPE_EXIT_FASTER_IMPL should be removed. It isn't always 
faster, and putting experimental features in a release makes no one 
confident. If it becomes stable, cross-platform and consistently faster, 
then it can come back.

   BOOST_SCOPE_EXIT_TRY and BOOST_SCOPE_EXIT_CATCH_ALL are not very useful

   There is a compile warning because boost_scope_exit_params_struct_nn 
does not have an assignment operator. Define one and it will go away.

   Look for ways to make the syntax clearer. Goran pointed out that he 
doubts code reviews at his job or many others would allow the current 
syntax. So far, the best suggestion seems to be BOOST_SCOPE_EXIT((cref 
c) (ref r) (val v)). This also addresses desires to be able to pass by 
other than references.

   There was a request for the ability to nest scope_exits and build 
more complicated structures. I don't see how this could be done in an 
exception safe manner, so it is probably not a good idea.

    There was an extended discussion on the use of macros in the public 
interfaces of Boost libraries. It is certainly true that indiscriminate 
use of macros is a bad idea, but there is no indication that that is the 
case for this library. As is always the case for any Boost library, the 
real question should be "What is the best way to provide the best 
functionallity?" In this case, no suggestions other than macros have 
come forth, so macros are the best available choice.

   My thanks again to everyone who participated, to Alexander for 
creating and submitting the library and for his patience,  and to Jody 
for organizing the review period.

                        John Phillips
                        Review Wizard