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 <int> 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.
Simon
