c++: Fix ICE with lambda in operator function [PR93597]

Message ID 20200205213103.2340184-1-polacek@redhat.com
State New
Headers show
Series
  • c++: Fix ICE with lambda in operator function [PR93597]
Related show

Commit Message

Marek Polacek Feb. 5, 2020, 9:31 p.m.
If we are going to use get_first_fn let's make sure we operate
on is_overloaded_fn, as the rest of the codebase does.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

	PR c++/93597 - ICE with lambda in operator function.
	* name-lookup.c (maybe_save_operator_binding): Check is_overloaded_fn.

	* g++.dg/cpp0x/lambda/lambda-93597.C: New test.
---
 gcc/cp/name-lookup.c                             | 13 ++++++++-----
 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C |  8 ++++++++
 2 files changed, 16 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C


base-commit: fa0c6e297b22d5883857d0db4a6a8be0967cb16f
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Comments

Jason Merrill Feb. 5, 2020, 9:40 p.m. | #1
On 2/5/20 4:31 PM, Marek Polacek wrote:
> If we are going to use get_first_fn let's make sure we operate

> on is_overloaded_fn, as the rest of the codebase does.

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

> 

> 	PR c++/93597 - ICE with lambda in operator function.

> 	* name-lookup.c (maybe_save_operator_binding): Check is_overloaded_fn.

> 

> 	* g++.dg/cpp0x/lambda/lambda-93597.C: New test.

> ---

>   gcc/cp/name-lookup.c                             | 13 ++++++++-----

>   gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C |  8 ++++++++

>   2 files changed, 16 insertions(+), 5 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> 

> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c

> index 2447166a444..aa539926332 100644

> --- a/gcc/cp/name-lookup.c

> +++ b/gcc/cp/name-lookup.c

> @@ -7624,11 +7624,14 @@ maybe_save_operator_binding (tree e)

>   

>     if (!fns && (fns = op_unqualified_lookup (fnname)))

>       {

> -      tree fn = get_first_fn (fns);

> -      if (DECL_CLASS_SCOPE_P (fn))

> -	/* We don't need to remember class-scope functions, normal unqualified

> -	   lookup will find them again.  */

> -	return;

> +      if (is_overloaded_fn (fns))

> +	{

> +	  tree fn = get_first_fn (fns);

> +	  if (DECL_CLASS_SCOPE_P (fn))

> +	    /* We don't need to remember class-scope functions, normal

> +	       unqualified lookup will find them again.  */

> +	    return;

> +	}


I think we want to return early in this testcase, too, i.e. if lookup 
finds any class-scope declaration.

>         bindings = tree_cons (fnname, fns, bindings);

>         if (attr)

> diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> new file mode 100644

> index 00000000000..257d9c7cdfd

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> @@ -0,0 +1,8 @@

> +// PR c++/93597 - ICE with lambda in operator function.

> +// { dg-do compile { target c++11 } }

> +

> +template <typename T>

> +struct S {

> +  using T ::operator<;

> +  void operator==(T x) { [x] { 0 < x; }; }

> +};

> 

> base-commit: fa0c6e297b22d5883857d0db4a6a8be0967cb16f

>
Marek Polacek Feb. 5, 2020, 10:04 p.m. | #2
On Wed, Feb 05, 2020 at 04:40:50PM -0500, Jason Merrill wrote:
> On 2/5/20 4:31 PM, Marek Polacek wrote:

> > If we are going to use get_first_fn let's make sure we operate

> > on is_overloaded_fn, as the rest of the codebase does.

> > 

> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > 

> > 	PR c++/93597 - ICE with lambda in operator function.

> > 	* name-lookup.c (maybe_save_operator_binding): Check is_overloaded_fn.

> > 

> > 	* g++.dg/cpp0x/lambda/lambda-93597.C: New test.

> > ---

> >   gcc/cp/name-lookup.c                             | 13 ++++++++-----

> >   gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C |  8 ++++++++

> >   2 files changed, 16 insertions(+), 5 deletions(-)

> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> > 

> > diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c

> > index 2447166a444..aa539926332 100644

> > --- a/gcc/cp/name-lookup.c

> > +++ b/gcc/cp/name-lookup.c

> > @@ -7624,11 +7624,14 @@ maybe_save_operator_binding (tree e)

> >     if (!fns && (fns = op_unqualified_lookup (fnname)))

> >       {

> > -      tree fn = get_first_fn (fns);

> > -      if (DECL_CLASS_SCOPE_P (fn))

> > -	/* We don't need to remember class-scope functions, normal unqualified

> > -	   lookup will find them again.  */

> > -	return;

> > +      if (is_overloaded_fn (fns))

> > +	{

> > +	  tree fn = get_first_fn (fns);

> > +	  if (DECL_CLASS_SCOPE_P (fn))

> > +	    /* We don't need to remember class-scope functions, normal

> > +	       unqualified lookup will find them again.  */

> > +	    return;

> > +	}

> 

> I think we want to return early in this testcase, too, i.e. if lookup finds

> any class-scope declaration.


Ah okay, how about this, if it passes the usual testing?

-- >8 --
If we are going to use get_first_fn let's make sure we operate on
is_overloaded_fn, as the rest of the codebase does, and if lookup finds
any class-scope declaration, return early too.

	PR c++/93597 - ICE with lambda in operator function.
	* name-lookup.c (maybe_save_operator_binding): Check is_overloaded_fn.

	* g++.dg/cpp0x/lambda/lambda-93597.C: New test.
---
 gcc/cp/name-lookup.c                             | 8 ++++----
 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C | 8 ++++++++
 2 files changed, 12 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2447166a444..8853c39f0eb 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -7624,10 +7624,10 @@ maybe_save_operator_binding (tree e)
 
   if (!fns && (fns = op_unqualified_lookup (fnname)))
     {
-      tree fn = get_first_fn (fns);
-      if (DECL_CLASS_SCOPE_P (fn))
-	/* We don't need to remember class-scope functions, normal unqualified
-	   lookup will find them again.  */
+      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.  */
 	return;
 
       bindings = tree_cons (fnname, fns, bindings);
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C
new file mode 100644
index 00000000000..257d9c7cdfd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C
@@ -0,0 +1,8 @@
+// PR c++/93597 - ICE with lambda in operator function.
+// { dg-do compile { target c++11 } }
+
+template <typename T>
+struct S {
+  using T ::operator<;
+  void operator==(T x) { [x] { 0 < x; }; }
+};

base-commit: d10cddeaad2a315c114063b7c1ff11c6a356ab65
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Jason Merrill Feb. 6, 2020, 2:55 a.m. | #3
On 2/5/20 5:04 PM, Marek Polacek wrote:
> On Wed, Feb 05, 2020 at 04:40:50PM -0500, Jason Merrill wrote:

>> On 2/5/20 4:31 PM, Marek Polacek wrote:

>>> If we are going to use get_first_fn let's make sure we operate

>>> on is_overloaded_fn, as the rest of the codebase does.

>>>

>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?

>>>

>>> 	PR c++/93597 - ICE with lambda in operator function.

>>> 	* name-lookup.c (maybe_save_operator_binding): Check is_overloaded_fn.

>>>

>>> 	* g++.dg/cpp0x/lambda/lambda-93597.C: New test.

>>> ---

>>>    gcc/cp/name-lookup.c                             | 13 ++++++++-----

>>>    gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C |  8 ++++++++

>>>    2 files changed, 16 insertions(+), 5 deletions(-)

>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

>>>

>>> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c

>>> index 2447166a444..aa539926332 100644

>>> --- a/gcc/cp/name-lookup.c

>>> +++ b/gcc/cp/name-lookup.c

>>> @@ -7624,11 +7624,14 @@ maybe_save_operator_binding (tree e)

>>>      if (!fns && (fns = op_unqualified_lookup (fnname)))

>>>        {

>>> -      tree fn = get_first_fn (fns);

>>> -      if (DECL_CLASS_SCOPE_P (fn))

>>> -	/* We don't need to remember class-scope functions, normal unqualified

>>> -	   lookup will find them again.  */

>>> -	return;

>>> +      if (is_overloaded_fn (fns))

>>> +	{

>>> +	  tree fn = get_first_fn (fns);

>>> +	  if (DECL_CLASS_SCOPE_P (fn))

>>> +	    /* We don't need to remember class-scope functions, normal

>>> +	       unqualified lookup will find them again.  */

>>> +	    return;

>>> +	}

>>

>> I think we want to return early in this testcase, too, i.e. if lookup finds

>> any class-scope declaration.

> 

> Ah okay, how about this, if it passes the usual testing?

> 

> -- >8 --

> If we are going to use get_first_fn let's make sure we operate on

> is_overloaded_fn, as the rest of the codebase does, and if lookup finds

> any class-scope declaration, return early too.

> 

> 	PR c++/93597 - ICE with lambda in operator function.

> 	* name-lookup.c (maybe_save_operator_binding): Check is_overloaded_fn.

> 

> 	* g++.dg/cpp0x/lambda/lambda-93597.C: New test.

> ---

>   gcc/cp/name-lookup.c                             | 8 ++++----

>   gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C | 8 ++++++++

>   2 files changed, 12 insertions(+), 4 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> 

> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c

> index 2447166a444..8853c39f0eb 100644

> --- a/gcc/cp/name-lookup.c

> +++ b/gcc/cp/name-lookup.c

> @@ -7624,10 +7624,10 @@ maybe_save_operator_binding (tree e)

>   

>     if (!fns && (fns = op_unqualified_lookup (fnname)))

>       {

> -      tree fn = get_first_fn (fns);

> -      if (DECL_CLASS_SCOPE_P (fn))

> -	/* We don't need to remember class-scope functions, normal unqualified

> -	   lookup will find them again.  */

> +      tree d = is_overloaded_fn (fns) ? get_first_fn (fns) : fns;

> +      if (DECL_CLASS_SCOPE_P (d))


We likely want to guard this with DECL_P (d).  OK with that change.

Jason

> +	/* We don't need to remember class-scope functions or declarations,

> +	   normal unqualified lookup will find them again.  */

>   	return;

>   

>         bindings = tree_cons (fnname, fns, bindings);

> diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> new file mode 100644

> index 00000000000..257d9c7cdfd

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C

> @@ -0,0 +1,8 @@

> +// PR c++/93597 - ICE with lambda in operator function.

> +// { dg-do compile { target c++11 } }

> +

> +template <typename T>

> +struct S {

> +  using T ::operator<;

> +  void operator==(T x) { [x] { 0 < x; }; }

> +};

> 

> base-commit: d10cddeaad2a315c114063b7c1ff11c6a356ab65

>

Patch

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2447166a444..aa539926332 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -7624,11 +7624,14 @@  maybe_save_operator_binding (tree e)
 
   if (!fns && (fns = op_unqualified_lookup (fnname)))
     {
-      tree fn = get_first_fn (fns);
-      if (DECL_CLASS_SCOPE_P (fn))
-	/* We don't need to remember class-scope functions, normal unqualified
-	   lookup will find them again.  */
-	return;
+      if (is_overloaded_fn (fns))
+	{
+	  tree fn = get_first_fn (fns);
+	  if (DECL_CLASS_SCOPE_P (fn))
+	    /* We don't need to remember class-scope functions, normal
+	       unqualified lookup will find them again.  */
+	    return;
+	}
 
       bindings = tree_cons (fnname, fns, bindings);
       if (attr)
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C
new file mode 100644
index 00000000000..257d9c7cdfd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-93597.C
@@ -0,0 +1,8 @@ 
+// PR c++/93597 - ICE with lambda in operator function.
+// { dg-do compile { target c++11 } }
+
+template <typename T>
+struct S {
+  using T ::operator<;
+  void operator==(T x) { [x] { 0 < x; }; }
+};