| Message ID | 20190823203740.GP14737@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/91521 - wrong error with operator-> | expand |
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
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; +}
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
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
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; +}
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.