C++ PATCH for c++/91521 - wrong error with operator->

Message ID 20190823203740.GP14737@redhat.com
State New
Headers show
Series
  • C++ PATCH for c++/91521 - wrong error with operator->
Related show

Commit Message

Marek Polacek Aug. 23, 2019, 8:37 p.m.
Since r263836 we enter the "a late-specified return type" block in
grokdeclarator when inner_declarator is null:

        /* Handle a late-specified return type.  */
        tree late_return_type = declarator->u.function.late_return_type;
-       if (funcdecl_p)
+       if (funcdecl_p
+       /* This is the case e.g. for
+          using T = auto () -> int.  */
+       || inner_declarator == NULL)
          {

inner_declarator is null in this testcase too, but here we don't have
a real trailing return type; it's just an overloaded operator ->.  That
means that late_return_type is non-null, but is error_mark_node.  In
that case I think it's not sensible to complain about "trailing return
type only available" or "function with trailing return type not declared
with auto".  In this case that made us reject valid code.

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

2019-08-23  Marek Polacek  <polacek@redhat.com>

	PR c++/91521 - wrong error with operator->.
	* decl.c (grokdeclarator): Don't consider error_mark_node a valid
	trailing return type.

	* g++.dg/parse/operator8.C: New test.

Comments

Jason Merrill Aug. 23, 2019, 9:24 p.m. | #1
On 8/23/19 1:37 PM, Marek Polacek wrote:
> Since r263836 we enter the "a late-specified return type" block in

> grokdeclarator when inner_declarator is null:

> 

>          /* Handle a late-specified return type.  */

>          tree late_return_type = declarator->u.function.late_return_type;

> -       if (funcdecl_p)

> +       if (funcdecl_p

> +       /* This is the case e.g. for

> +          using T = auto () -> int.  */

> +       || inner_declarator == NULL)

>            {

> 

> inner_declarator is null in this testcase too, but here we don't have

> a real trailing return type; it's just an overloaded operator ->.  That

> means that late_return_type is non-null, but is error_mark_node.  In

> that case I think it's not sensible to complain about "trailing return

> type only available" or "function with trailing return type not declared

> with auto".  In this case that made us reject valid code.

> 

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

> 

> 2019-08-23  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/91521 - wrong error with operator->.

> 	* decl.c (grokdeclarator): Don't consider error_mark_node a valid

> 	trailing return type.

> 

> 	* g++.dg/parse/operator8.C: New test.

> 

> diff --git gcc/cp/decl.c gcc/cp/decl.c

> index 88aa69ce5de..ea5752a249e 100644

> --- gcc/cp/decl.c

> +++ gcc/cp/decl.c

> @@ -11546,6 +11546,7 @@ grokdeclarator (const cp_declarator *declarator,

>   		      }

>   		  }

>   		else if (late_return_type

> +			 && late_return_type != error_mark_node

>   			 && sfk != sfk_conversion)

>   		  {


What if instead of this change we check for error_mark_node inside the 
block and return without an error?

Jason
Marek Polacek Aug. 23, 2019, 11:13 p.m. | #2
On Fri, Aug 23, 2019 at 02:24:17PM -0700, Jason Merrill wrote:
> On 8/23/19 1:37 PM, Marek Polacek wrote:

> > Since r263836 we enter the "a late-specified return type" block in

> > grokdeclarator when inner_declarator is null:

> > 

> >          /* Handle a late-specified return type.  */

> >          tree late_return_type = declarator->u.function.late_return_type;

> > -       if (funcdecl_p)

> > +       if (funcdecl_p

> > +       /* This is the case e.g. for

> > +          using T = auto () -> int.  */

> > +       || inner_declarator == NULL)

> >            {

> > 

> > inner_declarator is null in this testcase too, but here we don't have

> > a real trailing return type; it's just an overloaded operator ->.  That

> > means that late_return_type is non-null, but is error_mark_node.  In

> > that case I think it's not sensible to complain about "trailing return

> > type only available" or "function with trailing return type not declared

> > with auto".  In this case that made us reject valid code.

> > 

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

> > 

> > 2019-08-23  Marek Polacek  <polacek@redhat.com>

> > 

> > 	PR c++/91521 - wrong error with operator->.

> > 	* decl.c (grokdeclarator): Don't consider error_mark_node a valid

> > 	trailing return type.

> > 

> > 	* g++.dg/parse/operator8.C: New test.

> > 

> > diff --git gcc/cp/decl.c gcc/cp/decl.c

> > index 88aa69ce5de..ea5752a249e 100644

> > --- gcc/cp/decl.c

> > +++ gcc/cp/decl.c

> > @@ -11546,6 +11546,7 @@ grokdeclarator (const cp_declarator *declarator,

> >   		      }

> >   		  }

> >   		else if (late_return_type

> > +			 && late_return_type != error_mark_node

> >   			 && sfk != sfk_conversion)

> >   		  {

> 

> What if instead of this change we check for error_mark_node inside the block

> and return without an error?


That works too.  Take your pick.

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

2019-08-23  Marek Polacek  <polacek@redhat.com>

	PR c++/91521 - wrong error with operator->.
	* decl.c (grokdeclarator): Return error_mark_node for an invalid
	trailing return type.

	* g++.dg/parse/operator8.C: New test.

diff --git gcc/cp/decl.c gcc/cp/decl.c
index cb5571e4f24..9f7923871db 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -11549,6 +11549,8 @@ grokdeclarator (const cp_declarator *declarator,
 		else if (late_return_type
 			 && sfk != sfk_conversion)
 		  {
+		    if (late_return_type == error_mark_node)
+		      return error_mark_node;
 		    if (cxx_dialect < cxx11)
 		      /* Not using maybe_warn_cpp0x because this should
 			 always be an error.  */
diff --git gcc/testsuite/g++.dg/parse/operator8.C gcc/testsuite/g++.dg/parse/operator8.C
new file mode 100644
index 00000000000..c5ee3eb934a
--- /dev/null
+++ gcc/testsuite/g++.dg/parse/operator8.C
@@ -0,0 +1,13 @@
+// PR c++/91521 - wrong error with operator->.
+// { dg-do compile }
+
+struct foo {
+	int bar() { return 0; }
+	foo* operator->() { return this; }
+};
+
+int main()
+{
+	int pt(foo()->bar());
+	return pt;
+}
Jason Merrill Aug. 23, 2019, 11:20 p.m. | #3
On Fri, Aug 23, 2019 at 4:13 PM Marek Polacek <polacek@redhat.com> wrote:
> On Fri, Aug 23, 2019 at 02:24:17PM -0700, Jason Merrill wrote:

> > On 8/23/19 1:37 PM, Marek Polacek wrote:

> > > Since r263836 we enter the "a late-specified return type" block in

> > > grokdeclarator when inner_declarator is null:

> > >

> > >          /* Handle a late-specified return type.  */

> > >          tree late_return_type = declarator->u.function.late_return_type;

> > > -       if (funcdecl_p)

> > > +       if (funcdecl_p

> > > +       /* This is the case e.g. for

> > > +          using T = auto () -> int.  */

> > > +       || inner_declarator == NULL)

> > >            {

> > >

> > > inner_declarator is null in this testcase too, but here we don't have

> > > a real trailing return type; it's just an overloaded operator ->.  That

> > > means that late_return_type is non-null, but is error_mark_node.  In

> > > that case I think it's not sensible to complain about "trailing return

> > > type only available" or "function with trailing return type not declared

> > > with auto".  In this case that made us reject valid code.

> > >

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

> > >

> > > 2019-08-23  Marek Polacek  <polacek@redhat.com>

> > >

> > >     PR c++/91521 - wrong error with operator->.

> > >     * decl.c (grokdeclarator): Don't consider error_mark_node a valid

> > >     trailing return type.

> > >

> > >     * g++.dg/parse/operator8.C: New test.

> > >

> > > diff --git gcc/cp/decl.c gcc/cp/decl.c

> > > index 88aa69ce5de..ea5752a249e 100644

> > > --- gcc/cp/decl.c

> > > +++ gcc/cp/decl.c

> > > @@ -11546,6 +11546,7 @@ grokdeclarator (const cp_declarator *declarator,

> > >                   }

> > >               }

> > >             else if (late_return_type

> > > +                    && late_return_type != error_mark_node

> > >                      && sfk != sfk_conversion)

> > >               {

> >

> > What if instead of this change we check for error_mark_node inside the block

> > and return without an error?

>

> That works too.  Take your pick.

>

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


> 2019-08-23  Marek Polacek  <polacek@redhat.com>

>

>         PR c++/91521 - wrong error with operator->.

>         * decl.c (grokdeclarator): Return error_mark_node for an invalid

>         trailing return type.


OK, thanks.

Jason
Marek Polacek Aug. 23, 2019, 11:23 p.m. | #4
On Fri, Aug 23, 2019 at 04:20:47PM -0700, Jason Merrill wrote:
> > 2019-08-23  Marek Polacek  <polacek@redhat.com>

> >

> >         PR c++/91521 - wrong error with operator->.

> >         * decl.c (grokdeclarator): Return error_mark_node for an invalid

> >         trailing return type.

> 

> OK, thanks.


Thanks!  Applying to 9 too, BTW.

Marek

Patch

diff --git gcc/cp/decl.c gcc/cp/decl.c
index 88aa69ce5de..ea5752a249e 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -11546,6 +11546,7 @@  grokdeclarator (const cp_declarator *declarator,
 		      }
 		  }
 		else if (late_return_type
+			 && late_return_type != error_mark_node
 			 && sfk != sfk_conversion)
 		  {
 		    if (cxx_dialect < cxx11)
diff --git gcc/testsuite/g++.dg/parse/operator8.C gcc/testsuite/g++.dg/parse/operator8.C
new file mode 100644
index 00000000000..c5ee3eb934a
--- /dev/null
+++ gcc/testsuite/g++.dg/parse/operator8.C
@@ -0,0 +1,13 @@ 
+// PR c++/91521 - wrong error with operator->.
+// { dg-do compile }
+
+struct foo {
+	int bar() { return 0; }
+	foo* operator->() { return this; }
+};
+
+int main()
+{
+	int pt(foo()->bar());
+	return pt;
+}