[C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Jakub Jelinek]

Hi!

info gcc on __label__ says:

Local label declarations must come at the beginning of the block,
before any ordinary declarations or statements.

which is something the C frontend enforces, but C++ was not.
Fixed thusly, ok for trunk?

2007-09-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/32121
	* parser.c (cp_parser_compound_statement): Handle label-declarations
	at the beginning of the compound statement.
	(cp_parser_block_declaration): Issue diagnostics about __label__
	not at the beginning of a block.

	* g++.dg/ext/label4.C: Adjust error regexp.
	* g++.dg/ext/label6.C: Adjust error regexp.
	* g++.dg/ext/label7.C: New test.
	* g++.dg/ext/label8.C: New test.
	* g++.dg/ext/label9.C: New test.

--- gcc/cp/parser.c.jj	2007-09-14 11:54:32.000000000 +0200
+++ gcc/cp/parser.c	2007-09-27 15:56:24.000000000 +0200
@@ -6821,6 +6821,11 @@ cp_parser_expression_statement (cp_parse
    compound-statement:
      { statement-seq [opt] }
 
+   GNU extension:
+
+   compound-statement:
+     { label-declaration [opt] statement-seq [opt] }
+
    Returns a tree representing the statement.  */
 
 static tree
@@ -6834,6 +6839,9 @@ cp_parser_compound_statement (cp_parser 
     return error_mark_node;
   /* Begin the compound-statement.  */
   compound_stmt = begin_compound_stmt (in_try ? BCS_TRY_BLOCK : 0);
+  /* If the next keyword is `__label__' we have a label declaration.  */
+  while (cp_lexer_next_token_is_keyword (parser->lexer, RID_LABEL))
+    cp_parser_label_declaration (parser);
   /* Parse an (optional) statement-seq.  */
   cp_parser_statement_seq_opt (parser, in_statement_expr);
   /* Finish the compound-statement.  */
@@ -7711,7 +7719,6 @@ cp_parser_declaration (cp_parser* parser
 
    block-declaration:
      __extension__ block-declaration
-     label-declaration
 
    C++0x Extension:
 
@@ -7772,12 +7779,16 @@ cp_parser_block_declaration (cp_parser *
 	cp_parser_using_declaration (parser,
 				     /*access_declaration_p=*/false);
     }
-  /* If the next keyword is `__label__' we have a label declaration.  */
+  /* If the next keyword is `__label__' we have a misplaced label
+     declaration.  */
   else if (token1->keyword == RID_LABEL)
     {
-      if (statement_p)
-	cp_parser_commit_to_tentative_parse (parser);
-      cp_parser_label_declaration (parser);
+      cp_lexer_consume_token (parser->lexer);
+      error ("%<__label__%> not at the beginning of a block");
+      cp_parser_skip_to_end_of_statement (parser);
+      /* If the next token is now a `;', consume it.  */
+      if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
+	cp_lexer_consume_token (parser->lexer);
     }
   /* If the next token is `static_assert' we have a static assertion.  */
   else if (token1->keyword == RID_STATIC_ASSERT)
--- gcc/testsuite/g++.dg/ext/label4.C.jj	2006-10-05 00:26:22.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/label4.C	2007-09-27 15:50:01.000000000 +0200
@@ -3,4 +3,4 @@
 
 // { dg-do compile }
 
-__label__ *l;  // { dg-error "before" }
+__label__ *l;  // { dg-error "not at the beginning of" }
--- gcc/testsuite/g++.dg/ext/label6.C.jj	2007-08-13 15:09:39.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/label6.C	2007-09-27 15:50:29.000000000 +0200
@@ -1,3 +1,3 @@
 // PR c++/32108
 
-__label__ L; // { dg-error "function scopes" }
+__label__ L; // { dg-error "not at the beginning" }
--- gcc/testsuite/g++.dg/ext/label7.C.jj	2007-09-27 15:50:50.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/label7.C	2007-09-27 15:52:04.000000000 +0200
@@ -0,0 +1,12 @@
+// PR c++/32121
+// { dg-do compile }
+
+int f (void)
+{
+  a:;
+  __label__ a;	// { dg-error "not at the beginning" }
+  int b;
+  __label__ c;	// { dg-error "not at the beginning" }
+  a:;		// { dg-error "duplicate label" }
+  c:;
+}
--- gcc/testsuite/g++.dg/ext/label8.C.jj	2007-09-27 15:59:17.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/label8.C	2007-09-27 15:58:06.000000000 +0200
@@ -0,0 +1,22 @@
+// PR c++/32121
+// { dg-do compile }
+
+int f (void)
+{
+  __label__ a, b;
+  __label__ c;
+  a:;
+  b:;
+  c:;
+  {
+    __label__ d;
+    d:;
+    if (0)
+      {
+	__label__ e;
+	__label__ f;
+	f:;
+	e:;
+      }
+  }
+}
--- gcc/testsuite/g++.dg/ext/label9.C.jj	2007-09-27 15:59:21.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/label9.C	2007-09-27 15:59:11.000000000 +0200
@@ -0,0 +1,10 @@
+// PR c++/32121
+// { dg-do compile }
+
+int f (void)
+{
+  while (1)
+    __label__ a;	// { dg-error "not at the beginning" }
+  for (;;)
+    __label__ b;	// { dg-error "not at the beginning" }
+}

	Jakub

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> --- gcc/cp/parser.c.jj	2007-09-14 11:54:32.000000000 +0200
> +++ gcc/cp/parser.c	2007-09-27 15:56:24.000000000 +0200
> @@ -6821,6 +6821,11 @@ cp_parser_expression_statement (cp_parse
>     compound-statement:
>       { statement-seq [opt] }
>  
> +   GNU extension:
> +
> +   compound-statement:
> +     { label-declaration [opt] statement-seq [opt] }

Minor nit: there can be several label declarations, but
label-declaration can only produce one, so this should actually say
label-declaration-seq.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Jakub Jelinek]

On Thu, Sep 27, 2007 at 04:31:03PM +0200, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > --- gcc/cp/parser.c.jj	2007-09-14 11:54:32.000000000 +0200
> > +++ gcc/cp/parser.c	2007-09-27 15:56:24.000000000 +0200
> > @@ -6821,6 +6821,11 @@ cp_parser_expression_statement (cp_parse
> >     compound-statement:
> >       { statement-seq [opt] }
> >  
> > +   GNU extension:
> > +
> > +   compound-statement:
> > +     { label-declaration [opt] statement-seq [opt] }
> 
> Minor nit: there can be several label declarations, but
> label-declaration can only produce one, so this should actually say
> label-declaration-seq.

So do I need to write:

/* Parse a compound-statement.

   compound-statement:
     { statement-seq [opt] }

   GNU extension:

   compound-statement:
     { label-declaration-seq [opt] statement-seq [opt] }

   label-declaration-seq:
     label-declaration
     label-declaration-seq label-declaration

   Returns a tree representing the statement.  */

or is it implicitly assumed in the comment grammar what *-seq
means?

	Jakub

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Tom Tromey]

>>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:

Jakub> info gcc on __label__ says:

Jakub> Local label declarations must come at the beginning of the block,
Jakub> before any ordinary declarations or statements.

Is there a reason we want this restriction?  To me it seems more
"C++-like" to allow label declarations anywhere variable declarations
are allowed.  I would tend to allow __label__ declarations to be mixed
with code in C99 mode as well.

Tom

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Jakub Jelinek]

On Fri, Sep 28, 2007 at 10:16:13AM -0600, Tom Tromey wrote:
> >>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:
> 
> Jakub> info gcc on __label__ says:
> 
> Jakub> Local label declarations must come at the beginning of the block,
> Jakub> before any ordinary declarations or statements.
> 
> Is there a reason we want this restriction?  To me it seems more
> "C++-like" to allow label declarations anywhere variable declarations
> are allowed.  I would tend to allow __label__ declarations to be mixed
> with code in C99 mode as well.

Until GCC 3.3 we allowed it only at the beginning of the block even for C++
and it is documented that way.

Anyway, even if it is supposed to be allowed anywhere, I think it should
be always declared before their label, or at least must not result in
allowing silently duplicate labels, as does:
a:;
i = 1;
__label__ a;
a:;
i = 2;
ATM.

	Jakub

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Tom Tromey]

>>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:

Jakub> Until GCC 3.3 we allowed it only at the beginning of the block
Jakub> even for C++ and it is documented that way.

Yeah.  If it changed back then, then perhaps people are using it.  I
wonder.

Anyway I'm just trying to look at it from a language design point of
view.  I couldn't think of a reason to restrict this, particularly for
C99/C++, where the parallel construct for variables works fine.

Jakub> Anyway, even if it is supposed to be allowed anywhere, I think it should
Jakub> be always declared before their label, or at least must not result in
Jakub> allowing silently duplicate labels, as does:

Yeah, I agree.

Tom

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Chris Lattner]

On Sep 28, 2007, at 12:02 PM, Tom Tromey wrote:

>>>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:
>
> Jakub> Until GCC 3.3 we allowed it only at the beginning of the block
> Jakub> even for C++ and it is documented that way.
>
> Yeah.  If it changed back then, then perhaps people are using it.  I
> wonder.
>
> Anyway I'm just trying to look at it from a language design point of
> view.  I couldn't think of a reason to restrict this, particularly for
> C99/C++, where the parallel construct for variables works fine.

It's hard to handle code like this both correctly and efficiently:

void foo() {
   l:
     {
        goto l;
        __label__ l;
        l:
     }
}

Re: [C++ PATCH] Only allow __label__ at the beginning of compound statements as documented (PR c++/32121)

[Mark Mitchell]

Jakub Jelinek wrote:

> So do I need to write:
> 
> /* Parse a compound-statement.
> 
>    compound-statement:
>      { statement-seq [opt] }
> 
>    GNU extension:
> 
>    compound-statement:
>      { label-declaration-seq [opt] statement-seq [opt] }
> 
>    label-declaration-seq:
>      label-declaration
>      label-declaration-seq label-declaration
> 
>    Returns a tree representing the statement.  */

Yes.

And the patch is OK with that change.  I'm sympathetic to Tom's idea
that the beginning-of-block restriction is artificial, and I'm concerned
about backwards-compatibility -- but as this feature exists primarily
for compatibility with GNU C and as I'm not at all confident we handle
the difficult middle-of-block cases correctly, I think this is OK.

However, if reports come in of this causing problems, we will need to
reconsider.

Thanks,

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