[PATCH] c++: Fix ICE from op_unqualified_lookup [PR97582]

[Jason Merrill]

On 2/9/21 5:12 PM, Patrick Palka wrote:
> On Tue, 2 Feb 2021, Jason Merrill wrote:
> 
>> On 2/2/21 12:19 AM, Patrick Palka wrote:
>>> In this testcase, we're crashing because the lookup of operator+ from
>>> within the generic lambda via lookup_name finds multiple bindings
>>> (namely C1::operator+ and C2::operator+) and returns a TREE_LIST
>>> thereof, something which maybe_save_operator_binding isn't prepared to
>>> handle.
>>>
>>> Since we already discard the result of lookup_name when it returns a
>>> class-scope binding here, it seems cleaner (and equivalent) to instead
>>> communicate to lookup_name that we don't want such bindings in the first
>>> place.  While this change seems like an improvement on its own, it also
>>> fixes the mentioned PR, because the call to lookup_name now returns
>>> NULL_TREE rather than a TREE_LIST of (unwanted) class-scope bindings.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk/9/10?
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	PR c++/97582
>>> 	* name-lookup.c (op_unqualified_lookup): Pass BLOCK_NAMESPACE to
>>> 	lookup_name in order to ignore class-scope bindings, rather
>>> 	than discarding them after the fact.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	PR c++/97582
>>> 	* g++.dg/cpp0x/lambda/lambda-template17.C: New test.
>>> ---
>>>    gcc/cp/name-lookup.c                                  | 11 +++--------
>>>    gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template17.C |  8 ++++++++
>>>    2 files changed, 11 insertions(+), 8 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template17.C
>>>
>>> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
>>> index 52e4a630e25..46d6cc0dfa4 100644
>>> --- a/gcc/cp/name-lookup.c
>>> +++ b/gcc/cp/name-lookup.c
>>> @@ -9213,17 +9213,12 @@ op_unqualified_lookup (tree fnname)
>>>    	return NULL_TREE;
>>>        }
>>>    -  tree fns = lookup_name (fnname);
>>> +  /* We don't need to remember class-scope functions or declarations,
>>> +     normal unqualified lookup will find them again.  */
>>> +  tree fns = lookup_name (fnname, LOOK_where::BLOCK_NAMESPACE);
>>
>> Hmm, I'd expect this to look past class-scope declarations to find
>> namespace-scope declarations, but we want class decls to hide decls in an
>> outer scope.
> 
> D'oh, good point.  But IIUC, even if we did return (and later inject at
> instantiation time) namespace-scope declarations that were hidden by
> class-scope declarations, wouldn't the lookup at instantiation time
> still find and prefer the class-scope bindings (as desired)?  It seems
> to me that the end result might be the same, but I'm not sure.

The injection happens in the function parameter binding level, so I'd 
expect it to be found before class bindings.

> Alternatively, would it be safe to assume that if lookup_name returns an
> ambiguous result, then the result must consist of class-scope
> declarations and so we can discard it?

That isn't true in general:

inline namespace A { int i; }
inline namespace B { int i; }
int main() { return i; } // ambiguous lookup

though I think it is true for functions.  But if the result is 
ambiguous, you can look at the first element to see if it's from class 
scope.

>>
>>>      if (!fns)
>>>        /* Remember we found nothing!  */
>>>        return error_mark_node;
>>> -
>>> -  tree d = is_overloaded_fn (fns) ? get_first_fn (fns) : fns;
>>> -  if (DECL_CLASS_SCOPE_P (d))
>>> -    /* We don't need to remember class-scope functions or declarations,
>>> -       normal unqualified lookup will find them again.  */
>>> -    fns = NULL_TREE;
>>> -
>>>      return fns;
>>>    }
>>>    diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template17.C
>>> b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template17.C
>>> new file mode 100644
>>> index 00000000000..6cafbab8cb0
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template17.C
>>> @@ -0,0 +1,8 @@
>>> +// PR c++/97582
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +struct C1 { void operator+(); };
>>> +struct C2 { void operator+(); };
>>> +struct C3 : C1, C2 {
>>> +  template <class T> void get() { [] (T x) { +x; }; }
>>> +};
>>>
>>
>>
>