[C++ Patch] PR 53856

[C++ Patch] PR 53856

[Paolo Carlini]

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

Hi,

today I noticed that the accepts-invalid half this bug report is already 
fixed in 5+, but the rejects-valid second half is still an issue: I 
think we can easily use CLASSTYPE_IS_TEMPLATE on current_class_type to 
skip members of non-template classes. Tested x86_64-linux.

Thanks,
Paolo.

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

[-- Attachment #2: CL_53856 --]
[-- Type: text/plain, Size: 393 bytes --]

/cp
2015-09-22  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53856
	* pt.c (check_default_tmpl_args): Per [temp.param]/9, do not
	reject default template arguments in out of class definitions
	of members of non-template classes.

/testsuite
2015-09-22  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53856
	* g++.dg/template/defarg19.C: New.
	* g++.dg/template/defarg20.C: Likewise.

[-- Attachment #3: patch_53856 --]
[-- Type: text/plain, Size: 2516 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 228020)
+++ cp/pt.c	(working copy)
@@ -4793,8 +4793,8 @@ bool
 check_default_tmpl_args (tree decl, tree parms, bool is_primary,
                          bool is_partial, int is_friend_decl)
 {
-  const char *msg;
-  int last_level_to_check;
+  const char *msg = 0;
+  int last_level_to_check = 0;   /* By default check everything.  */
   tree parm_level;
   bool no_errors = true;
 
@@ -4940,6 +4940,13 @@ check_default_tmpl_args (tree decl, tree parms, bo
   else if (is_partial)
     msg = G_("default template arguments may not be used in "
 	     "partial specializations");
+  else if (current_class_type && !CLASSTYPE_IS_TEMPLATE (current_class_type))
+    /* Per [temp.param]/9, "A default template-argument shall not be
+       specified in the template-parameter-lists of the definition of
+       a member of a class template that appears outside of the member's
+       class.", thus if we aren't handling a member of a class template
+       there is no need to examine the parameters.  */
+    last_level_to_check = template_class_depth (current_class_type) + 1;
   else
     msg = G_("default argument for template parameter for class enclosing %qD");
 
@@ -4953,9 +4960,6 @@ check_default_tmpl_args (tree decl, tree parms, bo
        Here the default argument for `S' has no bearing on the
        declaration of `f'.  */
     last_level_to_check = template_class_depth (current_class_type) + 1;
-  else
-    /* Check everything.  */
-    last_level_to_check = 0;
 
   for (parm_level = parms;
        parm_level && TMPL_PARMS_DEPTH (parm_level) >= last_level_to_check;
Index: testsuite/g++.dg/template/defarg19.C
===================================================================
--- testsuite/g++.dg/template/defarg19.C	(revision 0)
+++ testsuite/g++.dg/template/defarg19.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/53856
+
+template<typename T>
+struct A
+{
+  struct B;
+};
+
+template<typename T = int>
+struct A<T>::B  // { dg-error "default argument" }
+{
+  int i;
+};
+
+A<int>::B b = { };
Index: testsuite/g++.dg/template/defarg20.C
===================================================================
--- testsuite/g++.dg/template/defarg20.C	(revision 0)
+++ testsuite/g++.dg/template/defarg20.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/53856
+
+struct A
+{
+  template<typename T>
+  struct B;
+};
+
+template<typename T = int>
+struct A::B
+{
+  int i;
+};
+
+A::B<int> b = { };

Re: [C++ Patch] PR 53856

[Jason Merrill]

On 09/22/2015 03:31 PM, Paolo Carlini wrote:
>      msg = G_("default template arguments may not be used in "
>  	     "partial specializations");
> +  else if (current_class_type && !CLASSTYPE_IS_TEMPLATE (current_class_type))
> +    /* Per [temp.param]/9, "A default template-argument shall not be
> +       specified in the template-parameter-lists of the definition of
> +       a member of a class template that appears outside of the member's
> +       class.", thus if we aren't handling a member of a class template
> +       there is no need to examine the parameters.  */
> +    last_level_to_check = template_class_depth (current_class_type) + 1;
>    else
>      msg = G_("default argument for template parameter for class enclosing %qD");

Why not handle this below, with the other code that sets 
last_level_to_check?

Jason

Re: [C++ Patch] PR 53856

[Paolo Carlini]

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

Hi,

On 09/24/2015 03:24 PM, Jason Merrill wrote:
> On 09/22/2015 03:31 PM, Paolo Carlini wrote:
>>      msg = G_("default template arguments may not be used in "
>>           "partial specializations");
>> +  else if (current_class_type && !CLASSTYPE_IS_TEMPLATE 
>> (current_class_type))
>> +    /* Per [temp.param]/9, "A default template-argument shall not be
>> +       specified in the template-parameter-lists of the definition of
>> +       a member of a class template that appears outside of the 
>> member's
>> +       class.", thus if we aren't handling a member of a class template
>> +       there is no need to examine the parameters.  */
>> +    last_level_to_check = template_class_depth (current_class_type) 
>> + 1;
>>    else
>>      msg = G_("default argument for template parameter for class 
>> enclosing %qD");
>
> Why not handle this below, with the other code that sets 
> last_level_to_check?
First, sorry for late replying (a few days of vacations)...

In general, the rationale behind changing that earlier conditional was 
restricting it to the specific case at issue and avoid affecting other 
default-related diagnostic. That said, the patch was subtly wrong 
anyway, because relied on msg remaining zero for the second call of 
check_default_tmpl_args for defarg20.C, thus the testcase was correctly 
accepted but with a zeroed default.

Today, it occurred to me that maybe we can even more directly avoid 
emitting a meaningless "default argument for template parameter for 
class enclosing" for an enclosing class which isn't a template. Thus the 
below, which definitely passes the testsuite and also complexified 
variants (deeper nestings) of the new testcases. Does it make sense to you?

Thanks,
Paolo.

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

[-- Attachment #2: patch_53856_2 --]
[-- Type: text/plain, Size: 1909 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 228467)
+++ cp/pt.c	(working copy)
@@ -4940,8 +4940,15 @@ check_default_tmpl_args (tree decl, tree parms, bo
   else if (is_partial)
     msg = G_("default template arguments may not be used in "
 	     "partial specializations");
+  else if (!current_class_type || CLASSTYPE_IS_TEMPLATE (current_class_type))
+    msg = G_("default argument for template parameter for class enclosing %qD");
   else
-    msg = G_("default argument for template parameter for class enclosing %qD");
+    /* Per [temp.param]/9, "A default template-argument shall not be
+       specified in the template-parameter-lists of the definition of
+       a member of a class template that appears outside of the member's
+       class.", thus if we aren't handling a member of a class template
+       there is no need to examine the parameters.  */
+    return true;
 
   if (current_class_type && TYPE_BEING_DEFINED (current_class_type))
     /* If we're inside a class definition, there's no need to
Index: testsuite/g++.dg/template/defarg19.C
===================================================================
--- testsuite/g++.dg/template/defarg19.C	(revision 0)
+++ testsuite/g++.dg/template/defarg19.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/53856
+
+template<typename T>
+struct A
+{
+  struct B;
+};
+
+template<typename T = int>
+struct A<T>::B  // { dg-error "default argument" }
+{
+  int i;
+};
+
+A<int>::B b = { };
Index: testsuite/g++.dg/template/defarg20.C
===================================================================
--- testsuite/g++.dg/template/defarg20.C	(revision 0)
+++ testsuite/g++.dg/template/defarg20.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/53856
+
+struct A
+{
+  template<typename T>
+  struct B;
+};
+
+template<typename T = int>
+struct A::B
+{
+  int i;
+};
+
+A::B<int> b = { };

Re: [C++ Patch] PR 53856

[Jason Merrill]

On 10/05/2015 12:50 PM, Paolo Carlini wrote:
> +  else if (!current_class_type || CLASSTYPE_IS_TEMPLATE (current_class_type))
> +    msg = G_("default argument for template parameter for class enclosing %qD");

Why would this be right when !current_class_type?

Jason

Re: [C++ Patch] PR 53856

[Paolo Carlini]

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

Hi,

On 10/05/2015 07:10 PM, Jason Merrill wrote:
> On 10/05/2015 12:50 PM, Paolo Carlini wrote:
>> +  else if (!current_class_type || CLASSTYPE_IS_TEMPLATE 
>> (current_class_type))
>> +    msg = G_("default argument for template parameter for class 
>> enclosing %qD");
>
> Why would this be right when !current_class_type?
Yes, it doesn't make much sense, but at least it's conservatively 
correct ;) I'm finishing regtesting the below, everything seems fine so 
far. Ok if it passes?

Thanks,
Paolo.

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

[-- Attachment #2: patch_53856_2b --]
[-- Type: text/plain, Size: 1908 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 228467)
+++ cp/pt.c	(working copy)
@@ -4940,8 +4940,15 @@ check_default_tmpl_args (tree decl, tree parms, bo
   else if (is_partial)
     msg = G_("default template arguments may not be used in "
 	     "partial specializations");
+  else if (current_class_type && CLASSTYPE_IS_TEMPLATE (current_class_type))
+    msg = G_("default argument for template parameter for class enclosing %qD");
   else
-    msg = G_("default argument for template parameter for class enclosing %qD");
+    /* Per [temp.param]/9, "A default template-argument shall not be
+       specified in the template-parameter-lists of the definition of
+       a member of a class template that appears outside of the member's
+       class.", thus if we aren't handling a member of a class template
+       there is no need to examine the parameters.  */
+    return true;
 
   if (current_class_type && TYPE_BEING_DEFINED (current_class_type))
     /* If we're inside a class definition, there's no need to
Index: testsuite/g++.dg/template/defarg19.C
===================================================================
--- testsuite/g++.dg/template/defarg19.C	(revision 0)
+++ testsuite/g++.dg/template/defarg19.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/53856
+
+template<typename T>
+struct A
+{
+  struct B;
+};
+
+template<typename T = int>
+struct A<T>::B  // { dg-error "default argument" }
+{
+  int i;
+};
+
+A<int>::B b = { };
Index: testsuite/g++.dg/template/defarg20.C
===================================================================
--- testsuite/g++.dg/template/defarg20.C	(revision 0)
+++ testsuite/g++.dg/template/defarg20.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/53856
+
+struct A
+{
+  template<typename T>
+  struct B;
+};
+
+template<typename T = int>
+struct A::B
+{
+  int i;
+};
+
+A::B<int> b = { };

Re: [C++ Patch] PR 53856

[Jason Merrill]

OK.

Jason