[boost] [review] [Local] Review Result - ACCEPTED

Subject: [boost] [review] [Local] Review Result - ACCEPTED
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2011-12-12 03:15:19

Local is ACCEPTED into Boost.

After following the lively Local review discussion several weeks ago, and
reviewing the discussion a second (and sometimes third) time, I have come
to the above conclusion. There was quite a bit of passion on both the sides
of "aisle", and thus, obviously, no decision I make would be well-received
by all.

Let me start by summarizing the main arguments against including Local in
Boost:

(a) Local functions are not very useful, at least in the presence of
existing alternatives (e.g., namespace functions and Boost.Phoenix).
(b) Local is really a portability library for C++03 presenting an imperfect
emulation of features readily available in C++11.
(c) Local's interface is primarily macro-based, making code ugly and
difficult to read.

In my opinion as the review manager, a sufficient number of individuals in
the discussion found the library "useful" to address (a) (sometimes with
additional positive adverbs); indeed, at least a couple individuals have
shared positive experiences with real-world use. Namespace functions
require one to move code to somewhere other than where one may prefer to
have it, and often requires a significant amount of boilerplate when
binding local variables. Aside from any perceived issues with
Boost.Phoenix's syntax and compiler error messages, it has been noted that
binding member functions within Boost.Phoenix can get ugly. As far as (b)
is concerned, the community seems pretty far from reaching a consensus on
whether a library described by (b) belongs in Boost. There are certainly
libraries currently in Boost that could be pegged to satisfy (b) as well,
though they all have other mitigating features they make their situation
different from Local in some way. As far as (c) is concerned, several have
acknowledged that a macro-based interface is necessary to implement local
functions in C++03, and it seems to have been generally agreed that, given
this limitation, Lorenzo has done an admireable job making the interface as
easy-to-use as possible. Some find it ugly, others find it reasonable.

In addition to the counterarguments of (a-c) above, the following facts
weighed into my final decision:
* Lorenzo has been engaging and in constant communication with the
developer's list during the development of Local. This gives me confidence
that he will continue to actively maintain (and improve?) Local.
* The documentation is unanimously agreed to be of Boost quality.
* The transition of some organizations from C++03 to C++11 may take several
more years, and Boost has a history of supporting "ancient" compilers (for
better or worse). The point being, a library that eases the transition from
C++03 to C++11 has some merit based on current precedent.
* There had been previous work on local functions by Alexander Nasanov and
Steven Watanabe shared on the developer's list, suggesting a desire for
this functionality for some time.
* Local is approximately an extension of Boost.ScopeExit; indeed, it
basically fulfills the request to Alexander Nasanov from the review result
[2] of Boost.ScopeExit to create such an extension.

Lastly, my own opinion of "what Boost is" factored in here. I view Boost as
*partly* a collection of general purpose libraries that can be used in wide
variety of applications (and thus Boost frequently acts as a staging ground
for standard adoption). Based on review feedback, I believe Local satisfies
this criterion; and based on the mailing list discussion, I believe this
view of Boost is not entirely inconsistent with others on the mailing list.

Ultimately, it wasn't so much a # of "yes" votes versus # of "no" votes as
it was the above general considerations. Regardless, I think independent of
how the votes were counted, the "yes" votes outnumbered the "no" votes.
This required some discretion on my part as not everyone who expressed an
opinion submitted a formal review, and some participants were only arguing
in favor of some specific point supporting either acceptance or rejection
of Local.

"Yes" reviews (7)
--------
Krzysztof Czainski
Andrzej Krzemienski
Pierre Morcello
Nat Lindon
John Bytheway
Edward Diener
Gregory Crosswhite

"No" reviews (3)
--------
Vicente J. Botet
Thomas Heller
Hartmut Kaiser

Paul A. Bristow and Alexander Nasanov (the author of Boost.ScopeExit) both
submitted reviews but did not express an opinion (as far as I could tell)
on whether Local should be included in Boost, though if I had to peg
Paul's, it would be to reject Local. From what I gathered, Joel de Guzman,
Joel Falcou, Dean Michael Berris, and Lucanus J. Simonson were opposed to
including Local in Boost (the aforementioned did not submit a formal
review, though a formal review might be unnecessary if your vote is "no").
On the other hand, Brian Wood, Philippe Vaucher, Mathias Gaunard, Robert
Ramey, Nathan Ridge, Brent Spillner, Thomas Klimpel, Christopher Jefferson,
Daniel James, Rafael Fourquet, Matthias Schabel, and Robert Stewart
participated in the discussion and argued in favor of some point that
supported accepting Local in Boost. I want to be clear here that certainly
not everyone in the aforementioned list even implied that they supported
acceptance of Local (I would guess that only roughly half implied as much),
but they indirectly helped its case by addressing arguments against
inclusion. Overall, that indicates to me that more individuals support
acceptance of Local into Boost than rejection.

[Apologies for any name misspellings and absent accents.]

Regarding Boost.ScopeExit: 4 reviews were in favor of deprecating
Boost.ScopeExit; 3 reviews felt there was nothing wrong with both
Boost.ScopeExit and Local coexisting; and 3 reviews mentioned the
possibility of improving Boost.ScopeExit with the features provided by
Local's exits. As such I'm inclined to let Alexander (who opposed any kind
of merging of Local and Boost.ScopeExit) work with Lorenzo on improving
Boost.ScopeExit as he sees fit, and hopefully both libraries can address
this apparent duplication of functionality within their respective
documentation to avoid user confusion.

Regarding local::function::overload: Based on the review comments, it makes
the most sense to add this to Boost.Functional.

Regarding BOOST_IDENTITY_TYPE: This should be added to Boost.Utility. On
the other hand, there doesn't appear to be a compelling use case for
BOOST_IDENTITY_VALUE.

The following are some suggested *possible* improvements that reviewers
brought up. This list is by no means exhaustive. Further, I personally
don't think all of these suggestions are necessarily "good", but I think
it's fair for Lorenzo (and the community) to consider them nonetheless.
Parenthetic comments are my own opinions.

* Some aren't convinced of the utility of LOCAL_BLOCK. (It's use cases
appear fairly narrow so it might be best to simplify the library and remove
this capability.)
* Use "this_" (as opposed to "_this") as an alias for "this" within
function bodies, and possibly also within bind declarations.
* Present Boost.PP sequence interface as a workaround for the variadic
interface? (I don't have a problem with supporting both interfaces at the
top level.)
* "Local" and "Locale" look too much alike, suggesting a name change to
"Scope", "Scoped", "LocalFunction", "Closure" may be a good idea?
* Allow use without dependence on Boost.TypeOf.
* Rename prefix and postfix macros from XXX_PARAMS/XXX_NAME to XXX/XXX_END?
(I don't mind the current macros.)
* Explicitly separate bound variables from function parameters. (I think
this suggestion has merit but I don't know what the interface could look
like.)
* Remove support for default parameters to simplify interface and
documentation? (It doesn't seem like default function parameters would be
very useful.)
* Use "capture" instead of "bind" for the bind/capture keyword? (I like and
prefer "bind".)

Finally, here the links to the submitted formal reviews, for reference. Of
particular note are Krzysztof's and John's reviews for their comments on
the documentation. Some "yes" votes were conditional, but AFAIK Lorenzo has
already agreed to address the relevant conditions.

Krzysztof Czainski
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225543

Vicente J. Botet
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225604

Andrzej Krzemienski
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225656

Paul A. Bristow
[...cannot find link to review...]

Pierre Morcello
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225694

Nat Lindon
http://permalink.gmane.org/gmane.comp.lib.boost.user/71508

Thomas Heller
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225736

Hartmut Kaiser
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225745

John Bytheway
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225746

Alexander Nasanov
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225783

Edward Diener
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225795

Gregory Crosswhite
http://permalink.gmane.org/gmane.comp.lib.boost.devel/225807

Finally, really big thanks to everyone for participating in the review and
ancillary discussions. I attempted to be as transparent as possible in
outlining the rationale for my decision above, but if you have any further
questions, do not hesitate to ask.

- Jeff, Review Manager for Local

[1] http://thread.gmane.org/gmane.comp.lib.boost.devel/168612
[2] http://lists.boost.org/boost-announce/2008/05/0190.php