Hi,
thus, as you may or may not have noticed I reverted my first try, when
Tobias noticed that in his large codebase we were rejecting code like:
class Matrix;
Matrix rot90 (const Matrix& a, int k = 1);
class Matrix {
friend Matrix rot90 (const Matrix&, int);
};
Matrix rot90 (const Matrix& a, int k) { return Matrix(); }
this was happening because, when duplicate_decls saw the friend
declaration, smashed together the first two rot90 declaration and we
ended up with a friend declaration with defaults (taken from the first
rot90 declaration), as such rejected the third time we saw rot90. I
think we can elegantly handle this kind of problem by exploiting the
DECL_HIDDEN_FRIEND_P information, thus, in
check_no_redeclaration_friend_default_args, as called by
duplicate_decls, if the newdecl doesn't have DECL_FRIEND_P set,
disregard an olddecl which doesn't have DECL_HIDDEN_FRIEND_P set (we
can't do this directly because duplicate_decls is already smashing decls
together at that point, thus we need to save the info and pass it as an
argument). I added quite a few additional tests an also asked Tobias to
double check on his code base. All good.
As a general arguments of why I think moving from DECL_FRIEND_P to
DECL_HIDDEN_FRIEND_P for the olddecl is the right thing to do, if the
olddecl is a friend but doesn't have DECL_HIDDEN_FRIEND_P set, there was
a declaration preceding it, thus, either the friend declaration has
default arguments and we already diagnosed that, or it doesn't , thus
isn't interesting anymore for the diagnostic at issue.
Thanks! Paolo.
///////////////////////////
/cp
2019-08-01 Paolo Carlini <paolo.carl...@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-08-01 Paolo Carlini <paolo.carl...@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/other/friend13.C: Likewise.
* g++.dg/other/friend14.C: Likewise.
* g++.dg/other/friend15.C: Likewise.
* g++.dg/parse/defarg4.C: Compile with -fpermissive -w.
* g++.dg/parse/defarg8.C: Likewise.
Index: cp/decl.c
===================================================================
--- cp/decl.c (revision 263197)
+++ cp/decl.c (working copy)
@@ -1280,6 +1280,38 @@ check_redeclaration_no_default_args (tree decl)
}
}
+/* NEWDECL is a redeclaration of a function or function template OLDDECL,
+ in any case represented as FUNCTION_DECLs (the DECL_TEMPLATE_RESULTs of
+ the TEMPLATE_DECLs in case of function templates). This function is used
+ to enforce the final part of C++17 11.3.6/4, about a single declaration:
+ "If a friend declaration specifies a default argument expression, that
+ declaration shall be a definition and 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_hidden_friend_p)
+{
+ if (!olddecl_hidden_friend_p && !DECL_FRIEND_P (newdecl))
+ 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_hidden_friend_p && TREE_PURPOSE (t1))
+ || (DECL_FRIEND_P (newdecl) && 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. */
@@ -1318,6 +1350,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool
{
unsigned olddecl_uid = DECL_UID (olddecl);
int olddecl_friend = 0, types_match = 0, hidden_friend = 0;
+ int olddecl_hidden_friend = 0;
int new_defines_function = 0;
tree new_template_info;
location_t olddecl_loc = DECL_SOURCE_LOCATION (olddecl);
@@ -1876,6 +1909,13 @@ 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, DECL_HIDDEN_FRIEND_P (olddecl));
}
}
}
@@ -1982,6 +2022,7 @@ next_arg:;
if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl))
{
olddecl_friend = DECL_FRIEND_P (olddecl);
+ olddecl_hidden_friend = DECL_HIDDEN_FRIEND_P (olddecl);
hidden_friend = (DECL_ANTICIPATED (olddecl)
&& DECL_HIDDEN_FRIEND_P (olddecl)
&& newdecl_is_friend);
@@ -1994,10 +2035,8 @@ next_arg:;
if (TREE_CODE (newdecl) == TEMPLATE_DECL)
{
- tree old_result;
- tree new_result;
- old_result = DECL_TEMPLATE_RESULT (olddecl);
- new_result = DECL_TEMPLATE_RESULT (newdecl);
+ tree old_result = DECL_TEMPLATE_RESULT (olddecl);
+ tree new_result = DECL_TEMPLATE_RESULT (newdecl);
TREE_TYPE (olddecl) = TREE_TYPE (old_result);
DECL_TEMPLATE_SPECIALIZATIONS (olddecl)
= chainon (DECL_TEMPLATE_SPECIALIZATIONS (olddecl),
@@ -2008,11 +2047,19 @@ 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
+ (old_result, new_result, olddecl_hidden_friend);
+ }
check_default_args (newdecl);
@@ -8780,6 +8827,21 @@ 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);
+ break;
+ }
+ }
+
/* 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/friend13.C
===================================================================
--- testsuite/g++.dg/other/friend13.C (nonexistent)
+++ testsuite/g++.dg/other/friend13.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/59480
+
+void f(int, int, int=0); // { dg-message "6:previous" }
+class C {
+ friend void f(int, int=0, int) {} // { dg-error "15:friend declaration" }
+};
Index: testsuite/g++.dg/other/friend14.C
===================================================================
--- testsuite/g++.dg/other/friend14.C (nonexistent)
+++ testsuite/g++.dg/other/friend14.C (working copy)
@@ -0,0 +1,14 @@
+// PR c++/59480
+
+class Matrix;
+
+Matrix rot90 (const Matrix& a, int k = 1);
+template<typename> Matrix rot90_ (const Matrix& a, int k = 1);
+
+class Matrix {
+ friend Matrix rot90 (const Matrix&, int);
+ template<typename> friend Matrix rot90_ (const Matrix&, int);
+};
+
+Matrix rot90 (const Matrix& a, int k) { return Matrix(); }
+template<typename> Matrix rot90_ (const Matrix& a, int k) { return Matrix(); }
Index: testsuite/g++.dg/other/friend15.C
===================================================================
--- testsuite/g++.dg/other/friend15.C (nonexistent)
+++ testsuite/g++.dg/other/friend15.C (working copy)
@@ -0,0 +1,14 @@
+// PR c++/59480
+
+class Matrix;
+
+void rot90 (const Matrix& a, int k = 1) { }
+template<typename> void rot90_ (const Matrix& a, int k = 1) { }
+
+class Matrix {
+ friend void rot90 (const Matrix&, int);
+ template<typename> friend void rot90_ (const Matrix&, int);
+};
+
+void rot90 (const Matrix& a, int k);
+template<typename> void rot90_ (const Matrix& a, int k);
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 263197)
+++ 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 <nat...@codesourcery.com>
Index: testsuite/g++.dg/parse/defarg8.C
===================================================================
--- testsuite/g++.dg/parse/defarg8.C (revision 263197)
+++ testsuite/g++.dg/parse/defarg8.C (working copy)
@@ -1,3 +1,5 @@
+// { dg-options "-fpermissive -w" }
+
struct A {
static void g(int);
};
Index: testsuite/g++.old-deja/g++.mike/p784.C
===================================================================
--- testsuite/g++.old-deja/g++.mike/p784.C (revision 263197)
+++ testsuite/g++.old-deja/g++.mike/p784.C (working copy)
@@ -1,6 +1,6 @@
// { dg-do assemble }
// { dg-require-effective-target ilp32 } */
-// { dg-options "-w" }
+// { dg-options "-w -fpermissive" }
// prms-id: 784
//# 1 "GctSymbol.GctSymbol.CHMap.cc"
