[C++ Patch] PR 65370

[C++ Patch] PR 65370

[Paolo Carlini]

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

Hi,

my fix for c++/15339 caused this regression, where we now reject the 
below valid testcase. I think we can handle the problem by adding an 
early return to check_redeclaration_no_default_args.

Tested x86_64-linux.

Thanks,
Paolo.

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

[-- Attachment #2: CL_65370 --]
[-- Type: text/plain, Size: 322 bytes --]

/cp
2015-03-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/65370
	* decl.c (check_redeclaration_no_default_args): Avoid spurious
	errors for member template functions of specialized class templates.

/testsuite
2015-03-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/65370
	* g++.dg/other/default11.C: New.

[-- Attachment #3: patch_65370 --]
[-- Type: text/plain, Size: 1195 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 221317)
+++ cp/decl.c	(working copy)
@@ -1271,6 +1271,22 @@ check_redeclaration_no_default_args (tree decl)
 {
   gcc_assert (DECL_DECLARES_FUNCTION_P (decl));
 
+  /* Don't get fooled by, eg:
+
+     template <typename> class C
+     {
+       template <typename U>
+       C(const C<U>&, bool = false);
+     };
+
+     template <>
+     template <typename U>
+     C<int>::C(const C<U>&, bool);  */
+
+  if (DECL_FUNCTION_MEMBER_P (decl)
+      && CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (DECL_CONTEXT (decl)))
+    return;
+
   for (tree t = FUNCTION_FIRST_USER_PARMTYPE (decl);
        t && t != void_list_node; t = TREE_CHAIN (t))
     if (TREE_PURPOSE (t))
Index: testsuite/g++.dg/other/default11.C
===================================================================
--- testsuite/g++.dg/other/default11.C	(revision 0)
+++ testsuite/g++.dg/other/default11.C	(working copy)
@@ -0,0 +1,11 @@
+// PR c++/65370
+
+template <typename> class C
+{
+  template <typename U>
+  C(const C<U>&, bool = false);
+};
+
+template <>
+template <typename U>
+C<int>::C(const C<U>&, bool);

Re: [C++ Patch] PR 65370

[Jason Merrill]

On 03/10/2015 11:50 AM, Paolo Carlini wrote:
> +  /* Don't get fooled by, eg:
> +
> +     template <typename> class C
> +     {
> +       template <typename U>
> +       C(const C<U>&, bool = false);
> +     };
> +
> +     template <>
> +     template <typename U>
> +     C<int>::C(const C<U>&, bool);  */
> +
> +  if (DECL_FUNCTION_MEMBER_P (decl)
> +      && CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (DECL_CONTEXT (decl)))
> +    return;

Wouldn't this also allow

template<>
template<typename U>
C<int>::C(const C<U>&, bool = false);

?

Would it work to avoid calling this function if the DECL_SOURCE_LOCATION 
of the new decl matches the location of the old decl?

Jason

Re: [C++ Patch] PR 65370

[Paolo Carlini]

Hi,

On 03/10/2015 05:19 PM, Jason Merrill wrote:
> On 03/10/2015 11:50 AM, Paolo Carlini wrote:
>> +  /* Don't get fooled by, eg:
>> +
>> +     template <typename> class C
>> +     {
>> +       template <typename U>
>> +       C(const C<U>&, bool = false);
>> +     };
>> +
>> +     template <>
>> +     template <typename U>
>> +     C<int>::C(const C<U>&, bool);  */
>> +
>> +  if (DECL_FUNCTION_MEMBER_P (decl)
>> +      && CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P 
>> (DECL_CONTEXT (decl)))
>> +    return;
>
> Wouldn't this also allow
>
> template<>
> template<typename U>
> C<int>::C(const C<U>&, bool = false);
>
> ?
Good question, but we don't have this issue, because for that we emit 
anyway:

65370.C:11:36: error: default argument specified in explicit 
specialization [-fpermissive]
  C<int>::C(const C<U>&, bool = false);

nothing changes about that kind of testcase, usual behavior.

Thanks,
Paolo.

Re: [C++ Patch] PR 65370

[Paolo Carlini]

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

... in any case, I can confirm that the below also passes testing. Not 
sure if it makes sense to add the check also to the other call site, 
can't figure out a testcase...

Thanks,
Paolo.

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

[-- Attachment #2: patch_65370_2 --]
[-- Type: text/plain, Size: 978 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 221317)
+++ cp/decl.c	(working copy)
@@ -1922,7 +1922,9 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 	{
 	  /* Per C++11 8.3.6/4, default arguments cannot be added in later
 	     declarations of a function template.  */
-	  check_redeclaration_no_default_args (newdecl);
+	  if (DECL_SOURCE_LOCATION (newdecl)
+	      != DECL_SOURCE_LOCATION (olddecl))
+	    check_redeclaration_no_default_args (newdecl);
 
 	  check_default_args (newdecl);
 
Index: testsuite/g++.dg/other/default11.C
===================================================================
--- testsuite/g++.dg/other/default11.C	(revision 0)
+++ testsuite/g++.dg/other/default11.C	(working copy)
@@ -0,0 +1,11 @@
+// PR c++/65370
+
+template <typename> class C
+{
+  template <typename U>
+  C(const C<U>&, bool = false);
+};
+
+template <>
+template <typename U>
+C<int>::C(const C<U>&, bool);

Re: [C++ Patch] PR 65370

[Jason Merrill]

On 03/10/2015 01:03 PM, Paolo Carlini wrote:
> Good question, but we don't have this issue, because for that we emit
> anyway:
>
> 65370.C:11:36: error: default argument specified in explicit
> specialization [-fpermissive]
>   C<int>::C(const C<U>&, bool = false);
>
> nothing changes about that kind of testcase, usual behavior.

Ah.  So here we can ignore any template instantiation or specialization, 
with a comment that check_explicit_specialization will handle them.  But 
I suspect that checking the decl itself will be better; I would expect 
checking the context to lead you to accept

template<> class C<int> {
   template <typename U>
   C(const C<U>&, bool);
};

template <typename U> C<int>::C(const C<U>&, bool = false);

Since here C<int> is a specialization of C, but the constructor is not 
itself a partial instantiation.

Jason

Re: [C++ Patch] PR 65370

[Paolo Carlini]

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

Hi,

On 03/10/2015 07:10 PM, Jason Merrill wrote:
> Ah.  So here we can ignore any template instantiation or 
> specialization, with a comment that check_explicit_specialization will 
> handle them.  But I suspect that checking the decl itself will be 
> better; I would expect checking the context to lead you to accept
>
> template<> class C<int> {
>   template <typename U>
>   C(const C<U>&, bool);
> };
>
> template <typename U> C<int>::C(const C<U>&, bool = false);
>
> Since here C<int> is a specialization of C, but the constructor is not 
> itself a partial instantiation.
Indeed you are right. Therefore I'm finishing testing the below.

Thanks!
Paolo.

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

[-- Attachment #2: CL_65370_3 --]
[-- Type: text/plain, Size: 369 bytes --]

/cp
2015-03-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/65370
	* decl.c (duplicate_decls): Call check_redeclaration_no_default_args
	only if the location of newdecl doesn't match the location of olddecl.

/testsuite
2015-03-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/65370
	* g++.dg/other/default11.C: New.
	* g++.dg/other/default12.C: Likewise.

[-- Attachment #3: patch_65370_3 --]
[-- Type: text/plain, Size: 1512 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 221317)
+++ cp/decl.c	(working copy)
@@ -1922,7 +1922,9 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 	{
 	  /* Per C++11 8.3.6/4, default arguments cannot be added in later
 	     declarations of a function template.  */
-	  check_redeclaration_no_default_args (newdecl);
+	  if (DECL_SOURCE_LOCATION (newdecl)
+	      != DECL_SOURCE_LOCATION (olddecl))
+	    check_redeclaration_no_default_args (newdecl);
 
 	  check_default_args (newdecl);
 
Index: testsuite/g++.dg/other/default11.C
===================================================================
--- testsuite/g++.dg/other/default11.C	(revision 0)
+++ testsuite/g++.dg/other/default11.C	(working copy)
@@ -0,0 +1,11 @@
+// PR c++/65370
+
+template <typename> class C
+{
+  template <typename U>
+  C(const C<U>&, bool = false);
+};
+
+template <>
+template <typename U>
+C<int>::C(const C<U>&, bool);
Index: testsuite/g++.dg/other/default12.C
===================================================================
--- testsuite/g++.dg/other/default12.C	(revision 0)
+++ testsuite/g++.dg/other/default12.C	(working copy)
@@ -0,0 +1,16 @@
+// PR c++/65370
+
+template <typename> class C
+{
+  template <typename U>
+  C(const C<U>&, bool = false);
+};
+
+template<>
+class C<int>
+{
+  template <typename U>
+  C(const C<U>&, bool);
+};
+
+template <typename U> C<int>::C(const C<U>&, bool = false) { }  // { dg-error "default arguments" }

Re: [C++ Patch] PR 65370

[Jason Merrill]

OK.

Jason