Re: [boost] Review Request: static asserts

From: John Maddock (John_Maddock_at_[hidden])
Date: 2000-11-05 07:41:33

>I realize the review hasn't been scheduled yet, but comments before the
>official review period are OK too, right?

Yes, thanks for the quick response.

>1. It should be made clear that static assert is not allowed at global
>namespace scope.  (Actually, even better would be to modify the code so
that
>static assertions worked at global namespace scope, given the existing
>caveat about being used in include files when the compiler has a problem
>with identical typdefs.  AFAICT, all that would be needed is to remove the
>leading underscore from _boost_static_assert_typedef_.)

Done.

>2. Two different terms, "static assert" and "compile time assert", are
used.
>They appear to be synonymous; however, no explanation is given describing
>the circumstances that cause one or another to be preferred.  It may be
>simpler to settle on a single term.  (This applies to the implementation
>code, too.)

Accepted, however how do people want the error message to read, retain
"COMPILE_TIME_ASSERTION_FAILURE" or change to "STATIC_ASSERTION_FAILURE"
for consistency?

>3. Example 2's static assert has a superfluous pair of parentheses. 
Example
>3's second static assert has a superfluous pair of parentheses around
>UnsignedInt.  In both cases, the reader is left to wonder whether there is
>some bizarre reason that is a parentheses are needed that he just can't
>think of.

They're not superfluous: in example two, they are required to stop the
comma in the middle of the expression being treated as a macro argument
separator. In example 3, they are there as casts: these probably shouldn't
be required, but I've been trying to work around some VC6 problems (not
very successfully in this case).

>4. There are mysterious non-breaking spaces and a period at the end of the
>HTML file.

Got it.

>1. A nit concerning, #ifdef BOOST_USE_ENUM_STATIC_ASSERT and #ifdef
>BOOST_MSVC: Generally in boost, ifndef is used, so that the primary
>implementation is given first, with alternatives for special situations
>following.

Yep, got it.

>2. Without looking through the mailing archives (assuming the answer is
>there), I have no clue why the chained BOOST_DO_JOIN to BOOST_DO_JOIN2
>construct is used.  A comment there would be very nice.

Added comment:
//
// The following piece of macro magic joins the two 
// arguments together, even when one of the arguments is
// itself a macro (see 16.3.1 in C++ standard).  The key
// is that macro expantion of macro arguments occurs in
// BOOST_DO_JOIN but not BOOST_JOIN or BOOST_DO_JOIN2.
#define BOOST_DO_JOIN( X, Y ) BOOST_DO_JOIN2(X,Y)
#define BOOST_DO_JOIN2(X, Y) X##Y
#define BOOST_JOIN( X, Y ) BOOST_DO_JOIN( X, Y )

>3. BOOST_USE_ENUM_PRECONDITION referred to in the comments should be
>BOOST_USE_ENUM_STATIC_ASSERT.

Yep.

>4. The comment fragment "// It's is not particularly" has two verbs.

Oops.

>Overall, I'm very pleased with how well the static assert solution works.
>For me, it will make relying on the compiler/platform specifics necessary
to
>deal with COM much more comforting. :-)

Thanks for the prompt feedback!

I've replaced the zip file in the vault with a fixed version (primarily to
prevent people from reporting the same errors all over again).

- John.