Patchwork C++/24314 (was: Re: [C++ Patch] PR 18747)

login
register
mail settings
Submitter Paolo Carlini
Date Sept. 7, 2012, 9:07 p.m.
Message ID <504A6228.5030005@oracle.com>
Download mbox | patch
Permalink /patch/182446/
State New
Headers show

Comments

Paolo Carlini - Sept. 7, 2012, 9:07 p.m.
On 09/07/2012 09:44 PM, Jason Merrill wrote:
> On 09/06/2012 06:29 PM, Paolo Carlini wrote:
>> Ok, I did that in the below, also passes testing.
>
> This patch is OK.
Great, applied.

In the meanwhile I looked a bit more into C++/24314 itself, and figured 
out something, I'm attaching a draft-draft patch ("p4") which passes the 
testsuite (+ all the 24314-like testcases I could imagine, including 
member template cases) modulo the regression of template/crash83.C 
because of excess errors. The latter happens because we have code in 
cp_parser_class_head which tries to improve error recovery for cases of 
missing 'template <>' in explicit specializations, and, for that rather 
broken testcase, it confuses the code I'm tentatively adding, resulting 
in the additional error message below:

crash83.C:5:27: error: an explicit specialization must be preceded by 
‘template <>’
  template<typename = class A<0>: > struct B {}; // { dg-error "explicit 
specialization|expected" }
                            ^
crash83.C:5:27: error: too many template-parameter-lists for A<0> 
(should be 1)

More importantly, I'm having troubles removing the early 
cp_parser_check_template_parameters check in cp_parser_class_head: one 
issue is that it also checks for too few template-parameter-lists not 
just too many, and thus catches rather broken testcases like, eg, 
template/error7.C or template/class2.C:

template <class T>
struct A {
   struct B;
   struct A::B { }; // { dg-error "" }
};

but there are definitely more problems with just checking 
template_header_count < wanted too in 
maybe_process_partial_specialization and removing the early check, I'm 
seeing many, many fails if I just try to extend "p4" like that...

Suggestions?

Thanks!
Paolo.
Jason Merrill - Sept. 7, 2012, 9:36 p.m.
On 09/07/2012 05:07 PM, Paolo Carlini wrote:
> the regression of template/crash83.C because of excess errors. The latter happens because we have code in cp_parser_class_head which tries to improve error recovery for cases of missing 'template <>' in explicit specializations, and, for that rather broken testcase, it confuses the code I'm tentatively adding, resulting in the additional error message below:
>
> crash83.C:5:27: error: an explicit specialization must be preceded by ‘template <>’
>  template<typename = class A<0>: > struct B {}; // { dg-error "explicit specialization|expected" }
>                            ^
> crash83.C:5:27: error: too many template-parameter-lists for A<0> (should be 1)

So we're adding a second irrelevant diagnostic to go with the first one. 
  The error in this testcase is a simple stray ':' causing a syntax 
error, it has nothing to do with explicit specialization.  It would be 
nice to improve this, but given that we're already not giving the right 
error, adding a second wrong error doesn't seem like a big problem.

> More importantly, I'm having troubles removing the early
> cp_parser_check_template_parameters check in cp_parser_class_head: one
> issue is that it also checks for too few template-parameter-lists not
> just too many, and thus catches rather broken testcases like, eg,
> template/error7.C or template/class2.C:
>
> template <class T>
> struct A {
>    struct B;
>    struct A::B { }; // { dg-error "" }
> };

This testcase is indeed ill-formed, but its ill-formedness has nothing 
to do with too few template-parameter-lists.  The problem is that clause 
9 says

If a class-head-name contains a nested-name-specifier, the 
class-specifier shall refer to a class that was previously declared 
directly in the class or namespace to which the nested-name-specifier 
refers, ... and the class-specifier shall appear in a namespace 
enclosing the previous declaration.

The definition of A::B does not appear at namespace scope, so it is 
ill-formed.  Removing the incorrect error is an improvement, but then we 
need to add the correct error rather than accept the testcase.

> but there are definitely more problems with just checking
> template_header_count < wanted too in
> maybe_process_partial_specialization and removing the early check, I'm
> seeing many, many fails if I just try to extend "p4" like that...

Like what?

Jason

Patch

Index: pt.c
===================================================================
--- pt.c	(revision 191082)
+++ pt.c	(working copy)
@@ -835,6 +835,15 @@  maybe_process_partial_specialization (tree type)
 	  && !COMPLETE_TYPE_P (type))
 	{
 	  check_specialization_namespace (CLASSTYPE_TI_TEMPLATE (type));
+
+	  int wanted = num_template_headers_for_class (type);
+	  if (template_header_count > wanted)
+	    {
+	      error ("too many template-parameter-lists for %E "
+		     "(should be %d)", type, wanted);
+	      return error_mark_node;
+	    }
+
 	  SET_CLASSTYPE_TEMPLATE_SPECIALIZATION (type);
 	  if (processing_template_decl)
 	    {
@@ -882,6 +891,14 @@  maybe_process_partial_specialization (tree type)
 	  tree t;
 	  tree tmpl = CLASSTYPE_TI_TEMPLATE (type);
 
+	  int wanted = num_template_headers_for_class (type);
+	  if (template_header_count > wanted)
+	    {
+	      error ("too many template-parameter-lists for %E "
+		     "(should be %d)", type, wanted);
+	      return error_mark_node;
+	    }
+
 	  if (current_namespace
 	      != decl_namespace_context (tmpl))
 	    {