Re: [C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition") (Take 2)
OK.
On Wed, Aug 1, 2018 at 8:38 PM, Paolo Carlini wrote:
> 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.
>
> ///
[C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition") (Take 2)
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
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
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." */
+
Re: [C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition")
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.
Re: [C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition")
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 -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition")
OK. On Thu, Jul 12, 2018 at 7:52 PM, Paolo Carlini 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. > > / >
[C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition")
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
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
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.
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
