Re: [C++ PATCH] C++0x static assertions

[Mark Mitchell <mark at codesourcery dot com>]

Douglas Gregor wrote:
> Hello,
> 
> Picking off some low-hanging fruit for the C++0x mode...
> 
> The attached patch introduces static assertions into the C++ front end,
> as specified in the latest C++0x working draft (N2009). The feature is
> under the control of -std=(c|gnu)++0x. However, the uglified
> "__static_assert" form can always be used, resulting in a warning.

Let's adopt the convention that any future such keywords begin with
__gnu -- so that would be __gnu_static_assert.

It's true that __ keeps things out of the user namespace, but we may as
well further protect ourselves and users from other compiler's magic
keywords.  Especially as  introduction this feature will inevitably
result in it showing up in the public headers for various packages, etc...

However, as the presumed users of this functionality (the library
maintainers) don't seem to mind that they can't use it in the current
C++ mode, then we don't need the uglified name at all.

Some questions on the patch:

* Are all your lines < 72 characters?  Some look like they might be a
bit long.

* In:

 add_decl_expr (tree decl)
 {
   tree r = build_stmt (DECL_EXPR, decl);
-  if (DECL_INITIAL (decl)
-      || (DECL_SIZE (decl) && TREE_SIDE_EFFECTS (DECL_SIZE (decl))))
+  if (TREE_CODE (decl) != STATIC_ASSERT_DECL
+      && (DECL_INITIAL (decl)
+          || (DECL_SIZE (decl) && TREE_SIDE_EFFECTS (DECL_SIZE (decl)))))

how can we have a

+   static assertion in the source code. When MEMBER_P, this static

Two spaces after periods.

+   assertion is a member of a class. */

Even before the close of a comment.

+finish_static_assert (
tree cond, tree msg, location_t location,  bool member_p)

Extra space before member_p.

+  if (cond == boolean_true_node)

I think this is a little dicey.  You're relying on conversion of a
constant to a boolean being boolean_true_node.  That might not always be
so; consider something like:

  typedef bool Bool;
  ... Bool(true) ...

Whether we presently do so or not, we might at some point want that
expression to have type "Bool" (rather than "bool"), and then it
wouldn't be boolean_true_node.  So, I think you should just check
TREE_CODE (cond) == INTGER_CST && !integer_zerop (cond).

+  /*  We know we are in a static assertion; commit to any tentative
+      parse. */
+  if (cp_parser_parsing_tentatively (parser))
+    cp_parser_commit_to_tentative_parse (parser);

Are you sure?  This commits the parser to the entire stack of parsing up
until this point, not just the innermost static_assert.  This might be
fine; I'm just not sure.

+  if (!flag_cpp0x)
+    /* Complain about the use of static_assert. */
+    warning (0, "`static_assert' is a C++0x extension");

You certainly need a warning category for these, as this will not be the
only C++0x extension. :-)  Of course, if you don't have the
__static_assert variant, then this code should just go.

+  /* Complete the static assertion, which may mean either */

Fragment.

+/* The condition attached to a static assertion. */
+#define STATIC_ASSERT_COND(NODE) TREE_TYPE (STATIC_ASSERT_DECL_CHECK
(NODE))
+
+/* The error message associated with a static assertion. This must be
+   a string literal. */
+#define STATIC_ASSERT_MSG(NODE) DECL_INITIAL (STATIC_ASSERT_DECL_CHECK
(NODE))

Ugh.  We're trying to make things more type-safe, and this makes them
less so.  The condition is an expression, so it would sorta kinda make
sense to call that the DECL_INITIAL.  There's no obvious place to put
the message.

I think it's a mistake to use a _DECL node for this at all.  Although
these things are syntactically declarations, they're semantic no-ops,
and they don't have many of the normal declaration properties.  For
example, they don't have names -- which is perhaps *the* fundamental
property of a declaration.  So, I would create a tcc_exceptional node
for these.  (See BASELINK for an example.)  That will save some memory.
 You'll still waste the TYPE field and a bunch of the flags, but that's
a global problem with trees at the moment.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713