From patchwork Tue Feb 4 16:25:58 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Martin X-Patchwork-Id: 105975 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D96E33858424 for ; Tue, 4 Feb 2025 16:27:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D96E33858424 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=nasilyan.com header.i=@nasilyan.com header.a=rsa-sha256 header.s=tey23rxsjton5kop5bydp3vc5ylkyjkg header.b=iX0jJ8a3; dkim=pass (1024-bit key, unprotected) header.d=amazonses.com header.i=@amazonses.com header.a=rsa-sha256 header.s=uku4taia5b5tsbglxyj6zym32efj7xqv header.b=EIfm9M2Q X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from a2-45.smtp-out.eu-west-1.amazonses.com (a2-45.smtp-out.eu-west-1.amazonses.com [54.240.2.45]) by sourceware.org (Postfix) with ESMTPS id 8E814385840F for ; Tue, 4 Feb 2025 16:25:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8E814385840F Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=nasilyan.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eu-west-1.amazonses.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8E814385840F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=54.240.2.45 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738686359; cv=none; b=YXQLA8MgiJ6TedmN5yhNmQCCU8zUclQPCOpT25cP9VukLBo0aGa6pEyuBi1tIyu4c1p08fAL6pJlONa/lkOqoZn740a5DDQ9j+WyZNoo3VdyJB1DCti7g7Kw4uveNCt2B6Yf4Lk4mTLJ4wx5w3Zcf/IJmrlC8TSgPwoUKQJXoC4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738686359; c=relaxed/simple; bh=dN3PyXvVRbJH0GzNgvp6hpD+BuNsrqGWe9Reygj35pg=; h=DKIM-Signature:DKIM-Signature:Subject:From:To:Date:Mime-Version: Message-ID; b=h+xb/RSgiHmADluTBab9zfUwZVH/Y7vYj4fwvGkZfymMt1uBE0n+hW892Jtu3Fzd53asdHfHhUYpzGNQkTNg7s3axRlv4//59IINwIvjprGX7XiPbog/p+9e6giqM+87T2t9Dm6IjkUCuqkYJ9J+T8xxTaFwAsOXyXvSOu+PWj0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8E814385840F DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=tey23rxsjton5kop5bydp3vc5ylkyjkg; d=nasilyan.com; t=1738686358; h=Subject:From:To:Cc:Date:Mime-Version:Content-Type:In-Reply-To:References:Message-Id; bh=dN3PyXvVRbJH0GzNgvp6hpD+BuNsrqGWe9Reygj35pg=; b=iX0jJ8a3dQTpWuHtQ/JogrwvKJSALYHSmAY9dFe1mnnsFiEJpqO8EY3EDvMQaZ7K J15m/TEwwuIHuCa+vNho6iJgEGm0U2te7E/7nQg+ZHnEa/U0SnN9ibGrs32z3ZZB7n/ RwWI5u1mC9eYus5QoVwUSK9w+TvftHdlw2ALPHzw= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=uku4taia5b5tsbglxyj6zym32efj7xqv; d=amazonses.com; t=1738686358; h=Subject:From:To:Cc:Date:Mime-Version:Content-Type:In-Reply-To:References:Message-Id:Feedback-ID; bh=dN3PyXvVRbJH0GzNgvp6hpD+BuNsrqGWe9Reygj35pg=; b=EIfm9M2QqvWaOoHjT+np/PoW25BYpxlNxqVvi/xUjKPNVE8lTpSQ1aUVIJClKz2H fpOW2+GxW+jf4i7j4SZ6fJnh7z0Nux+l6VCGo13HbLGtOwpXjwoxI2qaP2woWu1um44 7XUptixYyUckHoYmaJtWLjCpNcYX0o6mLHjlk7c4= Subject: [PATCH v3] c++: Reject default arguments for template class friend functions [PR118319] From: =?utf-8?q?Simon_Martin?= To: =?utf-8?q?Jason_Merrill?= Cc: =?utf-8?q?gcc-patches=40gcc=2Egnu=2Eorg?= Date: Tue, 4 Feb 2025 16:25:58 +0000 Mime-Version: 1.0 In-Reply-To: <263bb2c0-98c8-4886-8455-e9ceaaf9f481@redhat.com> References: <010201944b3e44d9-b7ddcbdc-d7aa-4d6f-954b-411c953c4b08-000000@eu-west-1.amazonses.com> <9d634544-fcc9-4466-9af7-3d292296606f@redhat.com> <25C41CD3-AB9A-4A34-8257-D08EC150439E@nasilyan.com> <01020194bcd9bbc9-fb5282a8-69dc-455a-996d-905407828130-000000@eu-west-1.amazonses.com> <1c966c76-cbf1-4862-a331-59bc91fa0a5f@redhat.com> <01020194bd224283-52da0dd7-2ce2-4a5c-8046-7718e2f79d92-000000@eu-west-1.amazonses.com> <263bb2c0-98c8-4886-8455-e9ceaaf9f481@redhat.com> X-Mailer: Amazon WorkMail Thread-Index: AQHbYppYSFt4kKslTHi5/KqJjxcUeQARwLV9BFVda58EVrUWzwRYIaeBBP/ViHoFIcf7eg== Thread-Topic: [PATCH v3] c++: Reject default arguments for template class friend functions [PR118319] X-Original-Mailer: MailMate (1.13.2r5673) X-Wm-Sent-Timestamp: 1738686357 Message-ID: <01020194d1c862e4-f51d3dd8-c5b9-4228-ae55-c2af1293427e-000000@eu-west-1.amazonses.com> Feedback-ID: ::1.eu-west-1.b24dn6frgCi6dh20skzbuMRr7UL8M6Soir/3ogtEjHQ=:AmazonSES X-SES-Outgoing: 2025.02.04-54.240.2.45 X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, PDS_BAD_THREAD_QP_64, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, THREAD_INDEX_BAD, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org Hi Jason, On 4 Feb 2025, at 1:11, Jason Merrill wrote: > On 1/31/25 11:12 AM, Simon Martin wrote: >> Hi Jason, >> >> On 31 Jan 2025, at 16:29, Jason Merrill wrote: >> >>> On 1/31/25 9:52 AM, Simon Martin wrote: >>>> Hi Jason, >>>> >>>> On 9 Jan 2025, at 22:55, Jason Merrill wrote: >>>> >>>>> On 1/9/25 8:25 AM, Simon Martin wrote: >>>>>> We segfault upon the following invalid code >>>>>> >>>>>> === cut here === >>>>>> template struct S { >>>>>>       friend void foo (int a = []{}()); >>>>>> }; >>>>>> void foo (int a) {} >>>>>> int main () { >>>>>>       S<0> t; >>>>>>       foo (); >>>>>> } >>>>>> === cut here === >>>>>> >>>>>> The problem is that we end up with a LAMBDA_EXPR callee in >>>>>> set_flags_from_callee, and dereference its NULL_TREE >>>>>> TREE_TYPE (TREE_TYPE ( )). >>>>>> >>>>>> This patch simply sets the default argument to error_mark_node >>>>>> for >>>>>> friend functions that do not meet the requirement in C++17 >>>>>> 11.3.6/4. >>>>>> >>>>>> Successfully tested on x86_64-pc-linux-gnu. >>>>>> >>>>>> PR c++/118319 >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> * decl.cc (grokfndecl): Inspect all friend function parameters, >>>>>> and set them to error_mark_node if invalid. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> * g++.dg/parse/defarg18.C: New test. >>>>>> >>>>>> --- >>>>>>      >>>>>> gcc/cp/decl.cc                        | 13 >>>>>> +++++--- >>>>>>      gcc/testsuite/g++.dg/parse/defarg18.C | 48 >>>>>> +++++++++++++++++++++++++++ >>>>>>      2 files changed, 57 insertions(+), 4 deletions(-) >>>>>>      create mode 100644 gcc/testsuite/g++.dg/parse/defarg18.C >>>>>> >>>>>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc >>>>>> index 503ecd9387e..b2761c23d3e 100644 >>>>>> --- a/gcc/cp/decl.cc >>>>>> +++ b/gcc/cp/decl.cc >>>>>> @@ -11134,14 +11134,19 @@ grokfndecl (tree ctype, >>>>>>           expression, that declaration shall be a >>>>>> definition..."  */ >>>>>>        if (friendp && !funcdef_flag) >>>>>>          { >>>>>> +      bool has_permerrored = false; >>>>>>            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 (!has_permerrored) >>>>>> +       { >>>>>> + has_permerrored = true; >>>>>> + permerror (DECL_SOURCE_LOCATION (decl), >>>>>> +    "friend declaration of %qD specifies default " >>>>>> +    "arguments and isn%'t a definition", decl); >>>>>> +       } >>>>>> +     TREE_PURPOSE (t) = error_mark_node; >>>>> >>>>> If we're going to unconditionally change TREE_PURPOSE, then >>>>> permerror >>>>> needs to strengthen to error.  But I'd think we could leave the >>>>> current state in a non-template class, only changing the template >>>>> case. >>>> Thanks. It’s true that setting the argument to error_mark_node is >>>> contradictory with the fact that we accept the code with >>>> -fpermissive, >>>> even if only under processing_template_decl, so I checked if >>>> there’s >>>> not a better way of approaching this PR. >>>> >>>> After a bit of investigation, I think that the real problem is that >>>> duplicate_decls tries to merge the two declarations, even though >>>> they >>>> don’t meet the constraint about friend functions and default >>>> arguments. >>> >>> I disagree; in this testcase the friend is the (lexically) first >>> declaration, the problem is that it's a non-defining friend (in a >>> template) that specifies default args, as addressed by your first >>> patch. >> Fair. >> >>> I still think my earlier comments are the way forward here: leave >>> the >>> non-template case alone (permerror, don't change TREE_PURPOSE), in a >>> template give a hard error and change to error_mark_node. >> Thanks, understood. The reason I looked for another “solution” is >> that it felt strange to be permissive in non-templates and stricter >> in >> templates. For example, if we do so, we’ll regress the case I added >> in >> defarg19.C in -fpermissive (also available at >> https://godbolt.org/z/YT3dexGjM). >> >> I’m probably splitting hair, and I’m happy to go ahead with your >> suggestion if you think it’s fine. Otherwise I’ll see if I find >> some >> better fix. > > That's fine, it's common to be stricter in templates. Ok, cool. The attached updated patch does this, and has been successfully tested on x86_64-pc-linux-gnu. OK for trunk? Thanks, Simon From bfa8949814b15d1c143b0928e8fbb7741bb93ac3 Mon Sep 17 00:00:00 2001 From: Simon Martin Date: Tue, 4 Feb 2025 11:17:35 +0100 Subject: [PATCH] c++: Reject default arguments for template class friend functions [PR118319] We segfault upon the following invalid code === cut here === template struct S { friend void foo (int a = []{}()); }; void foo (int a) {} int main () { S<0> t; foo (); } === cut here === The problem is that we end up with a LAMBDA_EXPR callee in set_flags_from_callee, and dereference its NULL_TREE TREE_TYPE (TREE_TYPE (..)). This patch sets the default argument to error_mark_node for template class friend functions that do not meet the requirement in C++17 11.3.6/4 (the change is restricted to templates per discussion with Jason). Successfully tested on x86_64-pc-linux-gnu. PR c++/118319 gcc/cp/ChangeLog: * decl.cc (grokfndecl): Inspect all friend function parameters. Set their default value to error_mark_node if processing a template and it's not valid for them to have any. gcc/testsuite/ChangeLog: * g++.dg/parse/defarg18.C: New test. --- gcc/cp/decl.cc | 14 +++++--- gcc/testsuite/g++.dg/parse/defarg18.C | 48 +++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/parse/defarg18.C -- 2.44.0 diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index b7af33b3231..d9c79e6f620 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -11213,14 +11213,20 @@ grokfndecl (tree ctype, expression, that declaration shall be a definition..." */ if (friendp && !funcdef_flag) { + bool has_permerrored = false; 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 (!has_permerrored) + { + has_permerrored = true; + permerror (DECL_SOURCE_LOCATION (decl), + "friend declaration of %qD specifies default " + "arguments and isn%'t a definition", decl); + } + if (processing_template_decl) + TREE_PURPOSE (t) = error_mark_node; } } diff --git a/gcc/testsuite/g++.dg/parse/defarg18.C b/gcc/testsuite/g++.dg/parse/defarg18.C new file mode 100644 index 00000000000..62c8f15f284 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/defarg18.C @@ -0,0 +1,48 @@ +// PR c++/118319 +// { dg-do "compile" { target c++11 } } + +// Template case, that used to crash. + +template +struct S { + friend void foo1 (int a = []{}()); // { dg-error "specifies default|only declaration" } + friend void foo3 (int a, // { dg-error "specifies default|only declaration" } + int b = []{}(), + int c = []{}()); +}; + +void foo1 (int a) {} +void foo3 (int a, int b, int c) {} + +void hello (){ + S<0> t; + foo1 (); + foo3 (1, 2); +} + + +// Template case, that already worked. + +template +struct T { + friend void bar (int a = []{}()); // { dg-error "specifies default|only declaration" } +}; + +void hallo (){ + T<0> t; + bar (); // { dg-error "not declared" } +} + + +// Non template case, that already worked. + +struct NoTemplate { + friend void baz (int a = []{}()); // { dg-error "specifies default|could not convert" } +}; + +void baz (int a) {} // { dg-error "only declaration" } + +void ola (){ + NoTemplate t; + baz (); // { dg-error "void value not ignored" } +}