diff mbox series

[C++] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition")

Message ID 1acff21c-37d1-480a-be4a-bb9f14fc887e@oracle.com
State New
Headers show
Series [C++] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition") | expand

Commit Message

Paolo Carlini July 12, 2018, 9:52 a.m. UTC
Hi,

the below resolves the bug report and its duplicates by implementing - 
in a rather straightforward way, I believe - the resolution of DR 136, 
which also made into C++17. Note that in the patch I used permerror 
instead of a plain error for consistency with the other check 
(check_redeclaration_no_default_args) which I added (rather) recently, 
and I'm exploiting that to allow two existing testcases to compile as 
they are. Tested x86_64-linux.

Thanks, Paolo.

/////////////////
/cp
2019-07-12  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/59480, DR 136
	* decl.c (check_no_redeclaration_friend_default_args): New.
	(duplicate_decls): Use the latter; also check that a friend
	declaration specifying default arguments is a definition.

/testsuite
2019-07-12  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/59480, DR 136
	* g++.dg/other/friend8.C: New.
	* g++.dg/other/friend9.C: Likewise.
	* g++.dg/other/friend10.C: Likewise.
	* g++.dg/other/friend11.C: Likewise.
	* g++.dg/other/friend12.C: Likewise.
	* g++.dg/parse/defarg4.C: Compile with -fpermissive -w.
	* g++.dg/parse/defarg8.C: Likewise.

Comments

Jason Merrill July 18, 2018, 8:09 a.m. UTC | #1
OK.

On Thu, Jul 12, 2018 at 7:52 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> the below resolves the bug report and its duplicates by implementing - in a
> rather straightforward way, I believe - the resolution of DR 136, which also
> made into C++17. Note that in the patch I used permerror instead of a plain
> error for consistency with the other check
> (check_redeclaration_no_default_args) which I added (rather) recently, and
> I'm exploiting that to allow two existing testcases to compile as they are.
> Tested x86_64-linux.
>
> Thanks, Paolo.
>
> /////////////////
>
Rainer Orth July 19, 2018, 8:43 a.m. UTC | #2
Hi Paolo,

> the below resolves the bug report and its duplicates by implementing - 
> in a rather straightforward way, I believe - the resolution of DR 136,
> which also made into C++17. Note that in the patch I used permerror instead
> of a plain error for consistency with the other check
> (check_redeclaration_no_default_args) which I added (rather) recently, and
> I'm exploiting that to allow two existing testcases to compile as they
> are. Tested x86_64-linux.

this patch caused

+FAIL: g++.old-deja/g++.mike/p784.C  -std=gnu++11 (test for excess errors)      
+FAIL: g++.old-deja/g++.mike/p784.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.old-deja/g++.mike/p784.C  -std=gnu++98 (test for excess errors)

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.mike/p784.C:1185:21: error: friend declaration of 'String common_prefix(const String&, const String&, int)' specifies default arguments and isn't a definition [-fpermissive]
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.mike/p784.C:1187:21: error: friend declaration of 'String common_suffix(const String&, const String&, int)' specifies default arguments and isn't a definition [-fpermissive]
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.mike/p784.C:1226:21: error: friend declaration of 'int readline(istream&, String&, char, int)' specifies default arguments and isn't a definition [-fpermissive]

I'm seeing it on i386-pc-solaris2.11 and sparc-sun-solaris2.11 with
-m32, but according to gcc-testresults it also happens on
i686-pc-linux-gnu, x86_64-pc-linux-gnu and a few others.

	Rainer
Paolo Carlini July 19, 2018, 9:37 a.m. UTC | #3
Hi,

On 19/07/2018 10:43, Rainer Orth wrote:
> Hi Paolo,
>
>> the below resolves the bug report and its duplicates by implementing -
>> in a rather straightforward way, I believe - the resolution of DR 136,
>> which also made into C++17. Note that in the patch I used permerror instead
>> of a plain error for consistency with the other check
>> (check_redeclaration_no_default_args) which I added (rather) recently, and
>> I'm exploiting that to allow two existing testcases to compile as they
>> are. Tested x86_64-linux.
> this patch caused
>
> +FAIL: g++.old-deja/g++.mike/p784.C  -std=gnu++11 (test for excess errors)
> +FAIL: g++.old-deja/g++.mike/p784.C  -std=gnu++14 (test for excess errors)
> +FAIL: g++.old-deja/g++.mike/p784.C  -std=gnu++98 (test for excess errors)
>
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.mike/p784.C:1185:21: error: friend declaration of 'String common_prefix(const String&, const String&, int)' specifies default arguments and isn't a definition [-fpermissive]
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.mike/p784.C:1187:21: error: friend declaration of 'String common_suffix(const String&, const String&, int)' specifies default arguments and isn't a definition [-fpermissive]
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.mike/p784.C:1226:21: error: friend declaration of 'int readline(istream&, String&, char, int)' specifies default arguments and isn't a definition [-fpermissive]
>
> I'm seeing it on i386-pc-solaris2.11 and sparc-sun-solaris2.11 with
> -m32, but according to gcc-testresults it also happens on
> i686-pc-linux-gnu, x86_64-pc-linux-gnu and a few others.
Thanks. I'm wondering why I didn't notice that. Anyway, I'm going to 
simply add -fpermissive to this testcase too.

Thanks again,
Paolo.
diff mbox series

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 262577)
+++ cp/decl.c	(working copy)
@@ -1280,6 +1280,39 @@  check_redeclaration_no_default_args (tree decl)
       }
 }
 
+/* NEWDECL is a redeclaration of a function or function template OLDDECL.
+   If either the declaration or the redeclaration is a friend declaration
+   and specifies default arguments issue a diagnostic.   Note: this is to
+   enforce C++17 11.3.6/4: "If a friend declaration specifies a default
+   argument expression, that declaration... shall be the only declaration
+   of the function or function template in the translation unit."  */
+
+static void
+check_no_redeclaration_friend_default_args (tree olddecl, tree newdecl)
+{
+  bool olddecl_friend_p = DECL_FRIEND_P (STRIP_TEMPLATE (olddecl));
+  bool newdecl_friend_p = DECL_FRIEND_P (STRIP_TEMPLATE (newdecl));
+
+  if (!olddecl_friend_p && !newdecl_friend_p)
+    return;
+
+  tree t1 = FUNCTION_FIRST_USER_PARMTYPE (olddecl);
+  tree t2 = FUNCTION_FIRST_USER_PARMTYPE (newdecl);
+
+  for (; t1 && t1 != void_list_node;
+       t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2))
+    if ((olddecl_friend_p && TREE_PURPOSE (t1))
+	|| (newdecl_friend_p && TREE_PURPOSE (t2)))
+      {
+	if (permerror (DECL_SOURCE_LOCATION (newdecl),
+		       "friend declaration of %q#D specifies default "
+		       "arguments and isn't the only declaration", newdecl))
+	  inform (DECL_SOURCE_LOCATION (olddecl),
+		  "previous declaration of %q#D", olddecl);
+	return;
+      }
+}
+
 /* Merge tree bits that correspond to attributes noreturn, nothrow,
    const,  malloc, and pure from NEWDECL with those of OLDDECL.  */
 
@@ -1876,6 +1909,12 @@  next_arg:;
 				olddecl);
 		      }
 		  }
+
+	      /* C++17 11.3.6/4: "If a friend declaration specifies a default
+		 argument expression, that declaration... shall be the only
+		 declaration of the function or function template in the
+		 translation unit."  */
+	      check_no_redeclaration_friend_default_args (olddecl, newdecl);
 	    }
 	}
     }
@@ -2008,11 +2047,18 @@  next_arg:;
 
       if (DECL_FUNCTION_TEMPLATE_P (newdecl))
 	{
-	  /* Per C++11 8.3.6/4, default arguments cannot be added in later
-	     declarations of a function template.  */
 	  if (DECL_SOURCE_LOCATION (newdecl)
 	      != DECL_SOURCE_LOCATION (olddecl))
-	    check_redeclaration_no_default_args (newdecl);
+	    {
+	      /* 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);
+	      /* C++17 11.3.6/4: "If a friend declaration specifies a default
+		 argument expression, that declaration... shall be the only
+		 declaration of the function or function template in the
+		 translation unit."  */
+	      check_no_redeclaration_friend_default_args (olddecl, newdecl);
+	    }
 
 	  check_default_args (newdecl);
 
@@ -8742,6 +8788,18 @@  grokfndecl (tree ctype,
 	}
     }
 
+  /* C++17 11.3.6/4: "If a friend declaration specifies a default argument
+     expression, that declaration shall be a definition..."  */
+  if (friendp && !funcdef_flag)
+    {
+      for (tree t = FUNCTION_FIRST_USER_PARMTYPE (decl);
+	   t && t != void_list_node; t = TREE_CHAIN (t))
+       if (TREE_PURPOSE (t))
+	 permerror (DECL_SOURCE_LOCATION (decl),
+		    "friend declaration of %qD specifies default "
+		    "arguments and isn't a definition", decl);
+    }
+
   /* If this decl has namespace scope, set that up.  */
   if (in_namespace)
     set_decl_namespace (decl, in_namespace, friendp);
Index: testsuite/g++.dg/other/friend10.C
===================================================================
--- testsuite/g++.dg/other/friend10.C	(nonexistent)
+++ testsuite/g++.dg/other/friend10.C	(working copy)
@@ -0,0 +1,9 @@ 
+// PR c++/59480
+
+class test {
+  friend int foo(bool = true) { return 1; }  // { dg-message "14:previous" }
+  template<typename> friend int bar(bool = true) { return 1; }  // { dg-message "33:previous" }
+};
+
+int foo(bool);  // { dg-error "5:friend declaration" }
+template<typename> int bar(bool);  // { dg-error "24:friend declaration" }
Index: testsuite/g++.dg/other/friend11.C
===================================================================
--- testsuite/g++.dg/other/friend11.C	(nonexistent)
+++ testsuite/g++.dg/other/friend11.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/59480
+
+class test {
+  friend int foo(bool = true) { return 1; }  // { dg-message "14:previous" }
+  friend int foo(bool);  // { dg-error "14:friend declaration" }
+  template<typename> friend int bar(bool = true) { return 1; }  // { dg-message "33:previous" }
+  template<typename> friend int bar(bool);  // { dg-error "33:friend declaration" }
+};
Index: testsuite/g++.dg/other/friend12.C
===================================================================
--- testsuite/g++.dg/other/friend12.C	(nonexistent)
+++ testsuite/g++.dg/other/friend12.C	(working copy)
@@ -0,0 +1,11 @@ 
+// PR c++/59480
+
+template<typename>
+class test {
+  friend int foo(bool = true) { return 1; }  // { dg-message "14:previous" }
+  friend int foo(bool);  // { dg-error "14:friend declaration" }
+  template<typename> friend int bar(bool = true) { return 1; }  // { dg-message "33:previous" }
+  template<typename> friend int bar(bool);  // { dg-error "33:friend declaration" }
+};
+
+template class test<bool>;
Index: testsuite/g++.dg/other/friend8.C
===================================================================
--- testsuite/g++.dg/other/friend8.C	(nonexistent)
+++ testsuite/g++.dg/other/friend8.C	(working copy)
@@ -0,0 +1,6 @@ 
+// PR c++/59480
+
+class test {
+  friend int foo(bool = true);  // { dg-error "14:friend declaration" }
+  template<typename> friend int bar(bool = true);  // { dg-error "33:friend declaration" }
+};
Index: testsuite/g++.dg/other/friend9.C
===================================================================
--- testsuite/g++.dg/other/friend9.C	(nonexistent)
+++ testsuite/g++.dg/other/friend9.C	(working copy)
@@ -0,0 +1,9 @@ 
+// PR c++/59480
+
+template<typename>
+class test {
+  friend int foo(bool = true);  // { dg-error "14:friend declaration" }
+  template<typename> friend int bar(bool = true);  // { dg-error "33:friend declaration" }
+};
+
+template class test<bool>;
Index: testsuite/g++.dg/parse/defarg4.C
===================================================================
--- testsuite/g++.dg/parse/defarg4.C	(revision 262577)
+++ testsuite/g++.dg/parse/defarg4.C	(working copy)
@@ -1,4 +1,4 @@ 
-// { dg-do compile }
+// { dg-options "-fpermissive -w" }
 
 // Copyright (C) 2003 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 3 Jul 2003 <nathan@codesourcery.com>
Index: testsuite/g++.dg/parse/defarg8.C
===================================================================
--- testsuite/g++.dg/parse/defarg8.C	(revision 262577)
+++ testsuite/g++.dg/parse/defarg8.C	(working copy)
@@ -1,3 +1,5 @@ 
+// { dg-options "-fpermissive -w" }
+
 struct A {
   static void g(int);
 };