diff mbox

[c++] Fix pr22138

Message ID 4C10A1C9.1040306@oracle.com
State New
Headers show

Commit Message

Shujing Zhao June 10, 2010, 8:26 a.m. UTC
On 06/10/2010 03:44 PM, Gabriel Dos Reis wrote:
> On Thu, Jun 10, 2010 at 1:40 AM, Shujing Zhao <pearly.zhao@oracle.com> wrote:
>> Hi,
>>
>> This patch is to fix pr22138. It would error "local template declarations is
>> not allowed" instead of "expected primary-expression before 'template'".
> 
> that isn't grammatically correct.  What about
> 
>    a template declaration cannot appear at block scope
Ok. Fix the message.
> 
> Also why don't you use the function at_function_scope_p() to test
> whether you are at local scope?
The parser itself know whether or not it is in the body of a function. The 
in_funtion_body is also more efficient that calling at_function_scope_p. I think 
using parser->in_function_body is better than at_function_scope_p.
Is it ok?

Thanks
Pearly

Comments

Paolo Carlini June 10, 2010, 9:34 a.m. UTC | #1
On 06/10/2010 10:26 AM, Shujing Zhao wrote:
>>
>> Also why don't you use the function at_function_scope_p() to test
>> whether you are at local scope?
> The parser itself know whether or not it is in the body of a function.
> The in_funtion_body is also more efficient that calling
> at_function_scope_p. I think using parser->in_function_body is better
> than at_function_scope_p.
As a matter of fact, in the entire parser.c there are no uses of
at_function_scope_p, and ~20 of in_function_body. Was curious... ;)

Paolo.
Manuel López-Ibáñez June 10, 2010, 9:41 a.m. UTC | #2
On 10 June 2010 11:34, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 06/10/2010 10:26 AM, Shujing Zhao wrote:
>>>
>>> Also why don't you use the function at_function_scope_p() to test
>>> whether you are at local scope?
>> The parser itself know whether or not it is in the body of a function.
>> The in_funtion_body is also more efficient that calling
>> at_function_scope_p. I think using parser->in_function_body is better
>> than at_function_scope_p.
> As a matter of fact, in the entire parser.c there are no uses of
> at_function_scope_p, and ~20 of in_function_body. Was curious... ;)

It would be nice to add a comment to at_function_scope_p() or prevent
somehow to use it in the parser, so no one uses inadvertently. Perhaps
moving it to semantics.h. You know, modularization. Moreover, that
function should probably be inline. Although perhaps this does not
matter with LTO available.

Manuel.
Gabriel Dos Reis June 10, 2010, 10:01 a.m. UTC | #3
On Thu, Jun 10, 2010 at 4:41 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 10 June 2010 11:34, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> On 06/10/2010 10:26 AM, Shujing Zhao wrote:
>>>>
>>>> Also why don't you use the function at_function_scope_p() to test
>>>> whether you are at local scope?
>>> The parser itself know whether or not it is in the body of a function.
>>> The in_funtion_body is also more efficient that calling
>>> at_function_scope_p. I think using parser->in_function_body is better
>>> than at_function_scope_p.
>> As a matter of fact, in the entire parser.c there are no uses of
>> at_function_scope_p, and ~20 of in_function_body. Was curious... ;)
>
> It would be nice to add a comment to at_function_scope_p() or prevent
> somehow to use it in the parser, so no one uses inadvertently. Perhaps
> moving it to semantics.h. You know, modularization. Moreover, that
> function should probably be inline. Although perhaps this does not
> matter with LTO available.
>
 agreed.
Jason Merrill June 30, 2010, 10:05 p.m. UTC | #4
I would check that the next token after 'template' is '<', so we don't 
get this error message for ill-formed code like

   template f<int>();

which isn't a template declaration at all, but rather someone trying to 
get the compiler to treat 'f' as a template name even though there's no 
f template in scope.

I agree that the at_*_scope_p issue isn't relevant to this patch.

Jason
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 160431)
+++ cp/parser.c	(working copy)
@@ -3754,6 +3754,14 @@  cp_parser_primary_expression (cp_parser 
 	case RID_AT_SELECTOR:
 	  return cp_parser_objc_expression (parser);
 
+	case RID_TEMPLATE:
+	  if (parser->in_function_body)
+	    {
+	      error_at (token->location,
+			"a template declaration cannot appear at block scope");
+	      cp_parser_skip_to_end_of_block_or_statement (parser);
+	      return error_mark_node;
+	    }
 	default:
 	  cp_parser_error (parser, "expected primary-expression");
 	  return error_mark_node;
Index: testsuite/g++.dg/parse/template25.C
===================================================================
--- testsuite/g++.dg/parse/template25.C	(revision 0)
+++ testsuite/g++.dg/parse/template25.C	(revision 0)
@@ -0,0 +1,8 @@ 
+// PR c++/22318
+// { dg-do compile }
+void f(void)
+{
+  template<typename T> class A /* { dg-error "a template declaration cannot appear at block scope" } */
+  {
+  };
+}