[C++ Patch] PR 32108

[C++ Patch] PR 32108

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

Hi,

the issue here seems indeed rather simple. I'm only slightly nervous
about the semantics of 'at_function_scope_p' exactly matching the
documentation, which talks about "GCC allows you to declare local labels
in any nested block scope". Otherwise, with the minimal patch the
testcase is rejected consistently with the C front-end and issuing a
consistent error message.

Tested x86_64-linux. Ok mainline and 4_2-branch?

Paolo.

///////////////

[-- Attachment #2: CL_32108 --]
[-- Type: text/plain, Size: 267 bytes --]

cp/
2007-07-12  Paolo Carlini  <pcarlini@suse.de>

	PR c++/32108
	* parser.c (cp_parser_block_declaration): Reject the
	__label__ extension outside function-scope.

testsuite/
2007-07-12  Paolo Carlini  <pcarlini@suse.de>

	PR c++/32108
	* g++.dg/ext/label6.C: New.


[-- Attachment #3: patch_32108 --]
[-- Type: text/plain, Size: 1083 bytes --]

Index: testsuite/g++.dg/ext/label6.C
===================================================================
*** testsuite/g++.dg/ext/label6.C	(revision 0)
--- testsuite/g++.dg/ext/label6.C	(revision 0)
***************
*** 0 ****
--- 1,3 ----
+ // PR c++/32108
+ 
+ __label__ L; // { dg-error "before" }
Index: cp/parser.c
===================================================================
*** cp/parser.c	(revision 126546)
--- cp/parser.c	(working copy)
*************** cp_parser_block_declaration (cp_parser *
*** 7747,7753 ****
  				     /*access_declaration_p=*/false);
      }
    /* If the next keyword is `__label__' we have a label declaration.  */
!   else if (token1->keyword == RID_LABEL)
      {
        if (statement_p)
  	cp_parser_commit_to_tentative_parse (parser);
--- 7747,7754 ----
  				     /*access_declaration_p=*/false);
      }
    /* If the next keyword is `__label__' we have a label declaration.  */
!   else if (token1->keyword == RID_LABEL
! 	   && at_function_scope_p ())
      {
        if (statement_p)
  	cp_parser_commit_to_tentative_parse (parser);

Re: [C++ Patch] PR 32108

[Mark Mitchell]

Paolo Carlini wrote:

> the issue here seems indeed rather simple. I'm only slightly nervous
> about the semantics of 'at_function_scope_p' exactly matching the
> documentation, which talks about "GCC allows you to declare local labels
> in any nested block scope". Otherwise, with the minimal patch the
> testcase is rejected consistently with the C front-end and issuing a
> consistent error message.

What does it say, exactly?  I think it would be better to call
cp_parser_label_declaration (just as we do now), but have
finish_label_decl issue an error message.  Then, you can say "__label__
declarations are only allowed in function scopes".

The tricky case here will be my least-favorite GNU extension: statement
expressions.  Consider:

  void f(int i = ({ __label__ f; ... }))

This should probably be allowed.  But, I bet that in_function_scope_p
will return false when parsing the statement expression.  You will have
to try it and see...

Thanks,

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

Re: [C++ Patch] PR 32108

[Paolo Carlini]

Hi Mark, and sorry about the delay, still a bit jet-lagged,

>Paolo Carlini wrote:
>
>>the issue here seems indeed rather simple. I'm only slightly nervous
>>about the semantics of 'at_function_scope_p' exactly matching the
>>documentation, which talks about "GCC allows you to declare local labels
>>in any nested block scope". Otherwise, with the minimal patch the
>>testcase is rejected consistently with the C front-end and issuing a
>>consistent error message.
>>    
>>
>What does it say, exactly?
>
It says: "error: expected unqualified-id before '__label__'". For 
comparison, the C front-end says "error: expected identifier or '(' 
before '__label__'".

>  I think it would be better to call
>cp_parser_label_declaration (just as we do now), but have
>finish_label_decl issue an error message.  Then, you can say "__label__
>declarations are only allowed in function scopes".
>  
>
Yes, I see what you mean, at some point I was figuring out a specific 
error message. Then, frankly, noticed that the C front-end was rejecting 
it with a similar error and decided to post the simple patch.

>The tricky case here will be my least-favorite GNU extension: statement
>expressions.  Consider:
>
>  void f(int i = ({ __label__ f; ... }))
>
>This should probably be allowed.  But, I bet that in_function_scope_p
>will return false when parsing the statement expression.  You will have
>to try it and see...
>  
>
What happens (I'm not expert of such extensions, admittedly), is that 
the contruct is already rejected without my patchlet: "error: 
statement-expressions are allowed only inside functions"...

Please let me know if you want me to try something more...

Thanks,
Paolo.

Re: [C++ Patch] PR 32108

[Mark Mitchell]

Paolo Carlini wrote:

>>  I think it would be better to call
>> cp_parser_label_declaration (just as we do now), but have
>> finish_label_decl issue an error message.  Then, you can say "__label__
>> declarations are only allowed in function scopes".
>>  
> Yes, I see what you mean, at some point I was figuring out a specific
> error message. Then, frankly, noticed that the C front-end was rejecting
> it with a similar error and decided to post the simple patch.

I still think we should do better, using the approach I suggested above.
 If that turns out to be hard, we can fall back to your simpler patch.

>>  void f(int i = ({ __label__ f; ... }))
>>
> What happens (I'm not expert of such extensions, admittedly), is that
> the contruct is already rejected without my patchlet: "error:
> statement-expressions are allowed only inside functions"...

OK, good.

Thanks,

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

Re: [C++ Patch] PR 32108

[Paolo Carlini]

Mark Mitchell wrote:

>>Yes, I see what you mean, at some point I was figuring out a specific
>>error message. Then, frankly, noticed that the C front-end was rejecting
>>it with a similar error and decided to post the simple patch.
>>    
>>
>I still think we should do better, using the approach I suggested above.
>If that turns out to be hard, we can fall back to your simpler patch.
>  
>
Sure, no problem.

Thanks,
Paolo.