This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Hi Mark,

Thanks for reviewing my patch. A revised and improved version is
attached; some comments are inline.

On 11/12/06, Mark Mitchell <mark@codesourcery.com> wrote:
Douglas Gregor wrote:
Let's adopt the convention that any future such keywords begin with
__gnu -- so that would be __gnu_static_assert.

Okay.


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.

I've removed the uglified version from this patch.


Some questions on the patch:

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

Oops, fixed.


* 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

I've removed this mess.


+ 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).

Okay, thank you.


+  /*  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.

Yes, I'm sure. static_asserts are declarations, and the keyword comes first in the declaration. There's no way we could have an ambiguity once we hit `static_assert'.

+  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.

__static_assert is gone, so is this warning.


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.

I've made it a tcc_exceptional. Things are much cleaner when we stop trying to cram static assertions into _DECLs.

Tested i386-apple-darwin8.8.1; no new regressions. New tests pass, as
do tests for a suitably-modified Boost. Combined ChangeLog follows.

Okay for mainline?

 Cheers,
 Doug

2006-11-19 Douglas Gregor <doug.gregor@gmail.com>

       * cp-tree.def (STATIC_ASSERT): New.
	* cp-objcp-common.c (cp_tree_size): Handle STATIC_ASSERT.
	* error.c (dump_decl): Handle STATIC_ASSERT.
	* cp-tree.h (STATIC_ASSERT_CONDITION): New.
       (STATIC_ASSERT_MESSAGE): New.
	(STATIC_ASSERT_SOURCE_LOCATION): New.
	(struct tree_static_assert): New.
	(enum cp_tree_node_structure_enum): Add TS_CP_STATIC_ASSERT.
	(union lang_tree_node): Add static_assertion.
       (finish_static_assert): Declare.
	* cxx-pretty-print.c (pp_cxx_statement): Handle STATIC_ASSERT.
	(pp_cxx_declaration): Handle STATIC_ASSERT.
	* pt.c (instantiate_class_template): Handle
	STATIC_ASSERT members.
       (tsubst_expr): Handle STATIC_ASSERT statements.
	* semantics.c (finish_static_assert): New.
       * lex.c (D_CPP0X): New.
       (reswords): Add static_assert keyword.
       (init_reswords): If not flag_cpp0x, mask out C++0x keywords.
       * parser.c (cp_parser_block_declaration): Parse static
	assertions.
       (cp_parser_static_assert): New.
       (cp_parser_member_declaration): Parse static assertions.

2006-11-19 Douglas Gregor <doug.gregor@gmail.com>

       * g++.dg/cpp0x/static_assert1.C: New.
       * g++.dg/cpp0x/static_assert2.C: New.

2006-11-19 Douglas Gregor <doug.gregor@gmail.com>

* c-common.h (enum rid): Add RID_STATIC_ASSERT.

Attachment: static_assert.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]