[boost] Boost.Local Review (Nov 10, 2011 to Nov 19, 2011) by Krzysztof Czaiński

Subject: [boost] Boost.Local Review (Nov 10, 2011 to Nov 19, 2011) by Krzysztof Czaiński
From: Krzysztof Czainski (1czajnik_at_[hidden])
Date: 2011-11-12 12:59:04

Hello,

About me: I write C++ apps and RTS, mostly for embedded devices. I often
use complicated C++ structures, and of course boost. Boost.Local caught my
interest, so I decided to review it.

I intend to try to use this library on MinGW-4.5, and on a Texas
Instruments/DSP compiler, however I can't do that this week, so I won't
make it until the end of this review.

I studied the docs in depth. I state my comments below. Each of my comments
is numbered in the style (0), so they can be referenced.

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Tutorial.html
:

Variable Binding:
[quote] programmers might want to bind variables of complex types by
(constant) reference instead than by value [/qoute]
(1) s/instead than/instead of/

Binding the Object this:
[quote] It is also possible to bind the object this when it is in scope
(e.g., from an enclosing non-static member function). This is done by
using this as the name of the variable to bind in the local function
declaration and by using the special symbol this_ (instead of this) to
access the object within the local function body. [/qoute]
(2) Why not use this_ in both places?

Local Blocks:
(3) How is a Local Block better then just an ordinary C++ block: {}? So
far, I have an idea of what such a Local Block is for, but a reference to a
rationale at this point in the tutorial would be nice.

Local Exits:
[qoute]The execution of the local exit body code is guaranteed only if the
program does not terminate because of an uncaught exception. [16] [/qoute]
(4) To me this note requires more explanation. Consider three examples:
(4a) [code]
int main() {
  BOOST_LOCAL_EXIT( (void) ) { // Local exit.
    cout << "hello" << endl;
  } BOOST_LOCAL_EXIT_END
  throw 0;
} // Local exit executed here.
[/code]
(4b) [code]
int main() {
  try {
    BOOST_LOCAL_EXIT( (void) ) { // Local exit.
      cout << "hello" << endl;
    } BOOST_LOCAL_EXIT_END
    throw 0;
  } catch ( ... ) {}
} // Local exit executed here.
[/code]
(4c) [code]
int main() {
  try {
    throw 0;
    BOOST_LOCAL_EXIT( (void) ) { // Local exit.
      cout << "hello" << endl;
    } BOOST_LOCAL_EXIT_END
  } catch ( ... ) {}
} // Local exit executed here.
[/code]
I which of the abouve examples is printing "hello" guaranteed?

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Advanced_Topics.html
:

Assigning Local Functions:
(5) In the example, variables named "l" (lowercase L) are used, which is
easy to confuse with "1" (one), and therefore the example is hard to read.
I suggest renaming that variable.

Deducing Bound Types (concepts, etc):
[qoute] it will also be a reference is the variable is bound by reference
[/qoute]
(6) typo: is -> if

Inlining Local Functions
[qoute]On C++03 compliant compilers, inlined local functions always have a
run-time comparable to their equivalent implementation that uses local
functors (see the Alternatives section). However, inlined local functions
have the limitation that they cannot be assigned to other functors
(like boost::function) and they cannot be passed as template
parameters. [21] On C++11 compilers, inline has no effect because this
library will automatically generate code that usesC++11 specific features
to inline the local function calls whenever possible even if the local
function is not declared inlined (unless
the BOOST_LOCAL_CONFIG_COMPLIANT configuration macro is defined).
Furthermore, non C++11local functions can always be passes as template
parameters even when they are declared inlined.[/quote]
(7) I read this paragraph four time, and still don't get it. Please
rephrase is somehow ;-)

(8) After reading the Introduction, Getting started, Tutorial and Advanced
topics I still don't have an overview of the overhead, memory allocations,
and exceptions that Local constructs can cause. Please add this
information. Right now, I can deduce this information from the
Implementation section. However, could things like lack of memory
allocations or not emitting exceptions be promised in the public interface?

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_LOCAL_BLOCK.html
:

Description:
[qoute]As usual, exceptions specifications can be optionally programmed
before the body code block (but the specifications will only apply the body
code and not to the library code automatically generated by the macro
expansion, see Advanced Topics). [/quote]
(9)What are: the overhead, possible memory allocations, and exceptions,
that Local constructs can cause?

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_LOCAL_CONFIG_COMPLIANT.html
:

Description:
[quote]If programmers leave this configuration macro undefined, its default
value is to be left not defined.[/qoute]
(10) What does it mean "its default value is to be left not defined"? Does
it mean, that whether it's defined or not depends on the library, which
tries to guess if the compiler supports variadic macros?
[quote]If this macro is defined, variadic macros and empty macro parameters
are not used by this library. Using variadic macros and empty macro
parameters allows this library to provide the variadic macro and empty
macro syntaxes which some programmers might find more readable than the
sequencing macro syntax (see the Tutorial section,
BOOST_LOCAL_FUNCTION_PARAMS, BOOST_LOCAL_BLOCK, and BOOST_LOCAL_EXIT). If
this configuration macro is defined then only the sequencing macro syntax
is allowed (regardless of whether the compiler supports variadic and empty
macros or not).[/quote]
(11)This paragraph is a bit unclear: first it says what happens if this
macro is defined, then it says why variadic macros are useful, and lastly
is says again what happens when this macro is defined. I suggest to make
two paragraphs: first: state clearly what this macro does - disables the
use of variadic macros, and second: comment on why variadic macros are
useful.

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_LOCAL_EXIT.html
:

Description:
[qoute]The execution of the local exit body code is guaranteed only if the
program does not terminate because of an uncaught exception. As usual,
exceptions specifications can be optionally programmed before the body code
block (but the specifications will only apply the body code and not to the
library code automatically generated by the macro expansion, see Advanced
Topics).[/qoute]
(12) Notes (4) and (9) apply here as well.

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_LOCAL_FUNCTION_PARAMS.html
:

Description:
[qoute]As usual, exceptions specifications can be optionally programmed
before the body code block (but the specifications will only apply the body
code and not to the library code automatically generated by the macro
expansion, see Advanced Topics).[/quote]
(13) Note (9) applies here as well.

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_LOCAL_TYPEOF.html
:
Description:
(14) What about the typename keyword inside templates? I think i remember
from the previous sections, that typename is not required, but that
information is missing here.

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDENTITY_VALUE.html
Description:
[qoute]For example, if the expression type is know (and contains no
commas)[/quote]
(15) typo: know -> known

This concludes my study of the docs, and I won't be able to try to use this
library before the end of this review, so I state my conclusions now.

2011/11/10 Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung_at_[hidden]>

> Please state clearly whether you think this library should be accepted as a
> Boost library.
>

Yes.

Other questions you may want to consider:
>
> - What is your evaluation of the design?
>

I like it.

> - What is your evaluation of the documentation?
>

I enjoyed reading it, and I think it gave me a pretty good idea about the
library.

> - What is your evaluation of the implementation?
>

>From what I read in the Implementation section of the docs, looks
reasonable.

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

I can imagine it would improve readability of some of my code.

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

No. I intend to, but not until the end of this review.

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

In-depth study of the docs.

> - Are you knowledgeable about the problem domain?
>

Hmm... I'll just say, I miss something like this for most of my 3-year
experience of C++ programming.

Please also consider the following issues and proposals specific to
> Boost.Local.  Your opinion is welcome on any or all of these.
>
> - Boost.Local's local exits provide the same functionality (and then some)
> as Boost.ScopeExit.  Does this duplication of functionality need to be
> dealt with, and if so, how?

I haven't used ScopeExit, so I don't have an opinion on this.

- Lorenzo is proposing to add boost::local::function::overload to
> Boost.Function as boost::function::overload.  See
>
> https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost/local/function/overload.html

I don't remember missing such a utility in my experience, but it seems
reasonable to add it to boost function. However, I don't understand, how
can it be added as boost::function::overload, when boost::function is a
class template?

> - Lorenzo is proposing to add BOOST_IDENTITY_TYPE to boost/utility.  See
>
> https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDENTITY_TYPE.html
> - Likewise, Lorenzo is proposing to add BOOST_IDENTITY_VALUE to
> boost/utility.  See
>
> https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDENTITY_VALUE.html
>

I don't remember missing any of the above in my experience. One of "more
readable alternatives" described in the docs have sufficed.

Thanks in advance to all who participate in the review discussion; I'm
> looking forward to it!
>
> - Jeffrey Hellrung (Review Manager)
> - Gregory Crosswhite (Review Assistant)

This was my first review, and I hope it's useful ;-)

Regards,
Kris