On 2/10/21 5:25 PM, Patrick Palka wrote:
On Wed, 10 Feb 2021, Jason Merrill wrote:
On 2/10/21 12:32 PM, Patrick Palka wrote:
On Wed, 10 Feb 2021, Jason Merrill wrote:
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.
Oops, I didn't look at push_operator_bindings closely enough. Never
mind about that idea then.
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.
I see, that's good to know. So something like this?
-- >8 --
Subject: [PATCH] c++: Fix ICE from op_unqualified_lookup [PR97582]
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 op_unqualified_lookup (and
push_operator_bindings) isn't prepared to handle.
This patch makes op_unqualified_lookup and push_operator_bindings handle
an ambiguous lookup result appropriately.
gcc/cp/ChangeLog:
PR c++/97582
* name-lookup.c (op_unqualified_lookup): Handle an ambiguous
lookup result by discarding it if the first element is
a class-scope declaration, otherwise return it.
(push_operator_bindings): Handle an ambiguous lookup result by
doing push_local_binding on each element in the list.
gcc/testsuite/ChangeLog:
PR c++/97582
* g++.dg/cpp0x/lambda/lambda-template17.C: New test.
---
gcc/cp/name-lookup.c | 16 ++++++++++++----
.../g++.dg/cpp0x/lambda/lambda-template17.C | 8 ++++++++
2 files changed, 20 insertions(+), 4 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 1f4a7ac1d0c..27af21d9ac9 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -9219,11 +9219,15 @@ op_unqualified_lookup (tree fnname)
/* 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))
+ tree fn = fns;
+ if (TREE_CODE (fn) == TREE_LIST)
+ fn = TREE_VALUE (fn);
+ if (is_overloaded_fn (fn))
+ fn = get_first_fn (fn);
+ if (DECL_CLASS_SCOPE_P (fn))
/* We don't need to remember class-scope functions or declarations,
normal unqualified lookup will find them again. */
- fns = NULL_TREE;
+ return NULL_TREE;
return fns;
}
@@ -9302,7 +9306,11 @@ push_operator_bindings ()
if (tree val = TREE_VALUE (binds))
{
tree name = TREE_PURPOSE (binds);
- push_local_binding (name, val, /*using*/true);
+ if (TREE_CODE (val) == TREE_LIST)
+ for (tree v = val; v; v = TREE_CHAIN (v))
+ push_local_binding (name, TREE_VALUE (v), /*using*/true);
+ else
+ push_local_binding (name, val, /*using*/true);
}
}
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; }; }
+};
The testcase should instantiate get() and check that we get the appropriate
error.
Done in the patch below:
OK, thanks.
-- >8 --
Subject: [PATCH] c++: Fix ICE from op_unqualified_lookup [PR97582]
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 op_unqualified_lookup (and
push_operator_bindings) isn't prepared to handle.
This patch makes op_unqualified_lookup and push_operator_bindings handle
an ambiguous lookup result appropriately.
gcc/cp/ChangeLog:
PR c++/97582
* name-lookup.c (op_unqualified_lookup): Handle an ambiguous
lookup result by discarding it if the first element is a
class-scope declaration, otherwise return it.
(push_operator_bindings): Handle an ambiguous lookup result by
doing push_local_binding on each element in the list.
gcc/testsuite/ChangeLog:
PR c++/97582
* g++.dg/cpp0x/lambda/lambda-template17.C: New test.
---
gcc/cp/name-lookup.c | 16 ++++++++++++----
.../g++.dg/cpp0x/lambda/lambda-template17.C | 10 ++++++++++
2 files changed, 22 insertions(+), 4 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 1f4a7ac1d0c..27af21d9ac9 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -9219,11 +9219,15 @@ op_unqualified_lookup (tree fnname)
/* 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))
+ tree fn = fns;
+ if (TREE_CODE (fn) == TREE_LIST)
+ fn = TREE_VALUE (fn);
+ if (is_overloaded_fn (fn))
+ fn = get_first_fn (fn);
+ if (DECL_CLASS_SCOPE_P (fn))
/* We don't need to remember class-scope functions or declarations,
normal unqualified lookup will find them again. */
- fns = NULL_TREE;
+ return NULL_TREE;
return fns;
}
@@ -9302,7 +9306,11 @@ push_operator_bindings ()
if (tree val = TREE_VALUE (binds))
{
tree name = TREE_PURPOSE (binds);
- push_local_binding (name, val, /*using*/true);
+ if (TREE_CODE (val) == TREE_LIST)
+ for (tree v = val; v; v = TREE_CHAIN (v))
+ push_local_binding (name, TREE_VALUE (v), /*using*/true);
+ else
+ push_local_binding (name, val, /*using*/true);
}
}
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..34dd07c983d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template17.C
@@ -0,0 +1,10 @@
+// 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; }; } // { dg-error
"ambiguous" }
+};
+
+template void C3::get<C3>();
