From patchwork Wed Aug 1 10:38:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paolo Carlini X-Patchwork-Id: 952067 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-482862-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="xnS4T0rM"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=oracle.com header.i=@oracle.com header.b="aCn7l8LN"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41gVC31CpMz9s1x for ; Wed, 1 Aug 2018 20:38:21 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to:cc :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=rWxvr699EiR/KE9lLnHIyR25U4ERmWcgqXSSbMuQa4VFYwlp2G ontR+ZTV6InkwD0GoJMdhU0HaJkwIMOKt17wacwTZ9FvyvnYsF4mxgOiutoHvjXe 1vcifxgYkChBQ18fnht0m5sWLMz069GVdEf+BSl158to2HmMnFJttDWaM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to:cc :from:subject:message-id:date:mime-version:content-type; s= default; bh=+O9bEDmrnizwvnZJEOrQ/kwJ/R4=; b=xnS4T0rMgg7bYyjdAYi/ lJKsyIOEAll0vWFP87CFYNJIa2yZ356PjKtBAjBUdw2JWOEWhWU0cnxCX5HoOMkp DuFgBDXHZlv7lh8CxvfNqxxNy/6TVBHDkR5DzUNGXBmNgmYDsUuJERpnfLuVyuQ/ SsGqIO7f0uJR87NGTxZjQAs= Received: (qmail 31371 invoked by alias); 1 Aug 2018 10:38:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 31351 invoked by uid 89); 1 Aug 2018 10:38:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, SPF_HELO_PASS, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy=Per, templates, H*Ad:D*oracle.com, sk:check_n X-HELO: aserp2130.oracle.com Received: from aserp2130.oracle.com (HELO aserp2130.oracle.com) (141.146.126.79) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Aug 2018 10:38:09 +0000 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w71ASw2r160425; Wed, 1 Aug 2018 10:38:08 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=to : cc : from : subject : message-id : date : mime-version : content-type; s=corp-2018-07-02; bh=Kqe0XIb9OW+IJyrW5qmn6Q6Xf6YFYfqArqF5TqSaOqg=; b=aCn7l8LND7Gl7Po3DpBnLxlj1iSfeUZHIEXlVaONVclD51gix2JbLSu1/kMr/mLE6F5I NbPCT0RvmfPVGQNnqSWv6qV2H7nzdAvQr++UKM2CgNZUCL5U+1cjlcfjXeTpUk2tFKiw snc9wXYoAIPBnFrtZdWZkgQKQaNVve16VXwpEs20kkUuh1ECEVyXCT7pBSYBMNWi9wGD 2lJ8b8Ptp+WbgK85ft2FfGV9RyHW9Hrx+IDtHo/d+paehka5gH28xSepjXokXF31fV3H 2esXYNglN4zniA/LqsP9U5m7rqBB0c3+JAfL/GAepAcaDsetFyXo3+8lzEF4yGfSW39a 2g== Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by aserp2130.oracle.com with ESMTP id 2kge0d54vk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 01 Aug 2018 10:38:07 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w71Ac7iX006129 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 1 Aug 2018 10:38:07 GMT Received: from abhmp0018.oracle.com (abhmp0018.oracle.com [141.146.116.24]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w71Ac7NX004044; Wed, 1 Aug 2018 10:38:07 GMT Received: from [192.168.1.4] (/79.36.21.168) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 01 Aug 2018 03:38:06 -0700 To: "gcc-patches@gcc.gnu.org" Cc: Jason Merrill From: Paolo Carlini Subject: =?utf-8?q?=5BC++_Patch=5D_PR_59480_=28=22Missing_error_diagnosti?= =?utf-8?q?c=3A_friend_declaration_specifying_a_default_argument_mu?= =?utf-8?b?c3QgYmUgYSBkZWZpbml0aW9uIinigIsgKFRha2UgMik=?= Message-ID: <562e7947-7f88-a40b-55b4-96c7c1fdfd76@oracle.com> Date: Wed, 1 Aug 2018 12:38:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 X-IsSubscribed: yes 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." */ + 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 friend int bar(bool = true) { return 1; } // { dg-message "33:previous" } +}; + +int foo(bool); // { dg-error "5:friend declaration" } +template 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 friend int bar(bool = true) { return 1; } // { dg-message "33:previous" } + template 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 +class test { + friend int foo(bool = true) { return 1; } // { dg-message "14:previous" } + friend int foo(bool); // { dg-error "14:friend declaration" } + template friend int bar(bool = true) { return 1; } // { dg-message "33:previous" } + template friend int bar(bool); // { dg-error "33:friend declaration" } +}; + +template class test; 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 Matrix rot90_ (const Matrix& a, int k = 1); + +class Matrix { + friend Matrix rot90 (const Matrix&, int); + template friend Matrix rot90_ (const Matrix&, int); +}; + +Matrix rot90 (const Matrix& a, int k) { return Matrix(); } +template 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 void rot90_ (const Matrix& a, int k = 1) { } + +class Matrix { + friend void rot90 (const Matrix&, int); + template friend void rot90_ (const Matrix&, int); +}; + +void rot90 (const Matrix& a, int k); +template 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 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 +class test { + friend int foo(bool = true); // { dg-error "14:friend declaration" } + template friend int bar(bool = true); // { dg-error "33:friend declaration" } +}; + +template class test; 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 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"