* Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses @ 2018-11-13 6:20 Umesh Kalappa 2018-11-13 15:40 ` Marek Polacek 0 siblings, 1 reply; 14+ messages in thread From: Umesh Kalappa @ 2018-11-13 6:20 UTC (permalink / raw) To: gcc-patches Hi All, the following patch fix the subjected issue Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 266026) +++ gcc/cp/parser.c (working copy) @@ -24615,6 +24615,8 @@ { tree expr; cp_lexer_consume_token (parser->lexer); + + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) { ok to commit along the testcase with changelog update ? Thank you ~Umesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-13 6:20 Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses Umesh Kalappa @ 2018-11-13 15:40 ` Marek Polacek 2018-11-13 21:25 ` Jason Merrill 0 siblings, 1 reply; 14+ messages in thread From: Marek Polacek @ 2018-11-13 15:40 UTC (permalink / raw) To: Umesh Kalappa; +Cc: gcc-patches On Tue, Nov 13, 2018 at 11:49:55AM +0530, Umesh Kalappa wrote: > Hi All, > > the following patch fix the subjected issue > > Index: gcc/cp/parser.c > =================================================================== > --- gcc/cp/parser.c (revision 266026) > +++ gcc/cp/parser.c (working copy) > @@ -24615,6 +24615,8 @@ > { > tree expr; > cp_lexer_consume_token (parser->lexer); > + > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) > { > > > ok to commit along the testcase with changelog update ? Thanks for the patch. Please also include the testcase along with the patch (and I think it should also test noexcept in a template). Please also include a ChangeLog entry in the patch submission. Can you describe how this patch has been tested? Further, wouldn't it be better to call inject_this_parameter inside the CPP_OPEN_PAREN block? If noexcept doesn't have any expression, then it can't refer to "this". Marek ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-13 15:40 ` Marek Polacek @ 2018-11-13 21:25 ` Jason Merrill 2018-11-14 11:49 ` Umesh Kalappa 0 siblings, 1 reply; 14+ messages in thread From: Jason Merrill @ 2018-11-13 21:25 UTC (permalink / raw) To: Marek Polacek; +Cc: Umesh Kalappa, gcc-patches List On Tue, Nov 13, 2018 at 10:40 AM Marek Polacek <polacek@redhat.com> wrote: > On Tue, Nov 13, 2018 at 11:49:55AM +0530, Umesh Kalappa wrote: > > Hi All, > > > > the following patch fix the subjected issue > > > > Index: gcc/cp/parser.c > > =================================================================== > > --- gcc/cp/parser.c (revision 266026) > > +++ gcc/cp/parser.c (working copy) > > @@ -24615,6 +24615,8 @@ > > { > > tree expr; > > cp_lexer_consume_token (parser->lexer); > > + > > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > > > if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) > > { > > > > > > ok to commit along the testcase with changelog update ? > > Thanks for the patch. > > Please also include the testcase along with the patch (and I think it should > also test noexcept in a template). Please also include a ChangeLog entry > in the patch submission. > > Can you describe how this patch has been tested? > > Further, wouldn't it be better to call inject_this_parameter inside the > CPP_OPEN_PAREN block? If noexcept doesn't have any expression, then it > can't refer to "this". Agreed, thanks. You also need to restore the old current_class_{ptr,ref} at the end of the noexcept-specifier. Jason ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-13 21:25 ` Jason Merrill @ 2018-11-14 11:49 ` Umesh Kalappa 2018-11-14 13:23 ` Umesh Kalappa 2018-11-14 14:33 ` Marek Polacek 0 siblings, 2 replies; 14+ messages in thread From: Umesh Kalappa @ 2018-11-14 11:49 UTC (permalink / raw) To: jason; +Cc: polacek, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1850 bytes --] Thank you Jason and Marek for the suggestions . Attached patch(pr86512.patch) along the Changelog . and also please note tested the patch for x86_64 only with "make -k check-gcc RUNTESTFLAGS=dg.exp=g++.dg" and see no regressions. We are runing the make check-gcc(x86_64) and will let know for any regressions . Meanwhile ,Please let us know your thoughts on the patch. Thank you ~Umesh On Wed, Nov 14, 2018 at 2:55 AM Jason Merrill <jason@redhat.com> wrote: > > On Tue, Nov 13, 2018 at 10:40 AM Marek Polacek <polacek@redhat.com> wrote: > > On Tue, Nov 13, 2018 at 11:49:55AM +0530, Umesh Kalappa wrote: > > > Hi All, > > > > > > the following patch fix the subjected issue > > > > > > Index: gcc/cp/parser.c > > > =================================================================== > > > --- gcc/cp/parser.c (revision 266026) > > > +++ gcc/cp/parser.c (working copy) > > > @@ -24615,6 +24615,8 @@ > > > { > > > tree expr; > > > cp_lexer_consume_token (parser->lexer); > > > + > > > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > > > > > if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) > > > { > > > > > > > > > ok to commit along the testcase with changelog update ? > > > > Thanks for the patch. > > > > Please also include the testcase along with the patch (and I think it should > > also test noexcept in a template). Please also include a ChangeLog entry > > in the patch submission. > > > > Can you describe how this patch has been tested? > > > > Further, wouldn't it be better to call inject_this_parameter inside the > > CPP_OPEN_PAREN block? If noexcept doesn't have any expression, then it > > can't refer to "this". > > Agreed, thanks. You also need to restore the old > current_class_{ptr,ref} at the end of the noexcept-specifier. > > Jason [-- Attachment #2: pr86512.patch --] [-- Type: application/octet-stream, Size: 2084 bytes --] Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 262850) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2018-07-18 Umesh Kalappa <umesh.kalappa0@gmail.com> + + PR libgcc/86512 + * gcc.target/arm/pr86512.c :New test. + 2018-07-18 Richard Biener <rguenther@suse.de> PR debug/86523 Index: gcc/testsuite/gcc.target/arm/pr86512.c =================================================================== --- gcc/testsuite/gcc.target/arm/pr86512.c (nonexistent) +++ gcc/testsuite/gcc.target/arm/pr86512.c (working copy) @@ -0,0 +1,30 @@ +/* { dg-do run } */ +/* { dg-options "-O0 -msoft-float" } */ + +#include<stdlib.h> +#include<stdint.h> +#define PRIx64 "llx" + +typedef union +{ + double d; + uint64_t i; +} u; + +int main() +{ + u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result; + + smallesPositiveSubnormal.i = 1; + + smallestPositiveNormal.i = 0x0010000000000000; + expectedResult.i = 0x000fffffffffffff; + result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d; + + + if (result.i != expectedResult.i) + abort(); + + return 0; +} + Index: libgcc/ChangeLog =================================================================== --- libgcc/ChangeLog (revision 262850) +++ libgcc/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-07-18 Umesh Kalappa <umesh.kalappa0@gmail.com> + + PR libgcc/86512 + * config/arm/ieee754-df.S :Don't normalise the denormal result. + + 2018-07-05 James Clarke <jrtc27@jrtc27.com> * configure: Regenerated. Index: libgcc/config/arm/ieee754-df.S =================================================================== --- libgcc/config/arm/ieee754-df.S (revision 262850) +++ libgcc/config/arm/ieee754-df.S (working copy) @@ -203,8 +203,11 @@ #endif @ Determine how to normalize the result. + @ if result is denormal i.e (exp)=0,then don't normalise the result, LSYM(Lad_p): cmp xh, #0x00100000 + blt LSYM(Lad_e) + cmp xh, #0x00100000 bcc LSYM(Lad_a) cmp xh, #0x00200000 bcc LSYM(Lad_e) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-14 11:49 ` Umesh Kalappa @ 2018-11-14 13:23 ` Umesh Kalappa 2018-11-14 14:33 ` Marek Polacek 1 sibling, 0 replies; 14+ messages in thread From: Umesh Kalappa @ 2018-11-14 13:23 UTC (permalink / raw) To: jason; +Cc: polacek, gcc-patches >>We are runing the make check-gcc(x86_64) and will let know for any regressions . No regress found . ~Umesh On Wed, Nov 14, 2018 at 5:18 PM Umesh Kalappa <umesh.kalappa0@gmail.com> wrote: > > Thank you Jason and Marek for the suggestions . > > Attached patch(pr86512.patch) along the Changelog . > > and also please note tested the patch for x86_64 only with "make -k > check-gcc RUNTESTFLAGS=dg.exp=g++.dg" and see no regressions. > > We are runing the make check-gcc(x86_64) and will let know for any regressions . > > Meanwhile ,Please let us know your thoughts on the patch. > > Thank you > ~Umesh > > On Wed, Nov 14, 2018 at 2:55 AM Jason Merrill <jason@redhat.com> wrote: > > > > On Tue, Nov 13, 2018 at 10:40 AM Marek Polacek <polacek@redhat.com> wrote: > > > On Tue, Nov 13, 2018 at 11:49:55AM +0530, Umesh Kalappa wrote: > > > > Hi All, > > > > > > > > the following patch fix the subjected issue > > > > > > > > Index: gcc/cp/parser.c > > > > =================================================================== > > > > --- gcc/cp/parser.c (revision 266026) > > > > +++ gcc/cp/parser.c (working copy) > > > > @@ -24615,6 +24615,8 @@ > > > > { > > > > tree expr; > > > > cp_lexer_consume_token (parser->lexer); > > > > + > > > > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > > > > > > > if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) > > > > { > > > > > > > > > > > > ok to commit along the testcase with changelog update ? > > > > > > Thanks for the patch. > > > > > > Please also include the testcase along with the patch (and I think it should > > > also test noexcept in a template). Please also include a ChangeLog entry > > > in the patch submission. > > > > > > Can you describe how this patch has been tested? > > > > > > Further, wouldn't it be better to call inject_this_parameter inside the > > > CPP_OPEN_PAREN block? If noexcept doesn't have any expression, then it > > > can't refer to "this". > > > > Agreed, thanks. You also need to restore the old > > current_class_{ptr,ref} at the end of the noexcept-specifier. > > > > Jason ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-14 11:49 ` Umesh Kalappa 2018-11-14 13:23 ` Umesh Kalappa @ 2018-11-14 14:33 ` Marek Polacek 2018-11-14 16:25 ` Umesh Kalappa 1 sibling, 1 reply; 14+ messages in thread From: Marek Polacek @ 2018-11-14 14:33 UTC (permalink / raw) To: Umesh Kalappa; +Cc: jason, gcc-patches On Wed, Nov 14, 2018 at 05:18:40PM +0530, Umesh Kalappa wrote: > Thank you Jason and Marek for the suggestions . > > Attached patch(pr86512.patch) along the Changelog . It seems you've attached the wrong patch. Marek ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-14 14:33 ` Marek Polacek @ 2018-11-14 16:25 ` Umesh Kalappa 2018-11-14 18:53 ` Marek Polacek 0 siblings, 1 reply; 14+ messages in thread From: Umesh Kalappa @ 2018-11-14 16:25 UTC (permalink / raw) To: polacek; +Cc: jason, gcc-patches [-- Attachment #1: Type: text/plain, Size: 114 bytes --] My bad Marek and thank you for pointing that out. Please find the attached correct one (pr52869.patch) . ~Umesh [-- Attachment #2: pr52869.patch --] [-- Type: application/octet-stream, Size: 2421 bytes --] Index: gcc/cp/ChangeLog =================================================================== --- gcc/cp/ChangeLog (revision 266026) +++ gcc/cp/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + *parser.c () : restore the old current_class_{ptr,ref} by + inject_this_parameter(). + 2018-11-09 Jakub Jelinek <jakub@redhat.com> * parser.c (cp_parser_omp_clause_final, cp_parser_omp_clause_if): Use Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 266026) +++ gcc/cp/parser.c (working copy) @@ -24615,11 +24615,24 @@ { tree expr; cp_lexer_consume_token (parser->lexer); - + if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) { matching_parens parens; parens.consume_open (parser); + + if (current_class_type) + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); + else + { + /*clear the current_class_ptr for non class type , like + int foo() noexcept(*this) + { + return 1; + } + */ + current_class_ptr = NULL_TREE; + } if (require_constexpr) { Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 266026) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR g++.dg/52869 + * g++.dg/pr52869.C: New. + 2018-11-11 Xianmiao Qu <xianmiao_qu@c-sky.com> * gcc.target/csky/fnmul-1.c: New. Index: gcc/testsuite/g++.dg/pr52869.C =================================================================== --- gcc/testsuite/g++.dg/pr52869.C (nonexistent) +++ gcc/testsuite/g++.dg/pr52869.C (working copy) @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -g" } */ + +struct S { + void f() { } + void g() noexcept(noexcept(f())) { } + void h() noexcept(noexcept(this->f())) { } +}; + +struct Nyan { + constexpr Nyan &operator++() noexcept { return *this; } + constexpr void omg() noexcept(noexcept(++*this)) {} +}; + +template< typename T > +T sine( T const& a, T const& b ) noexcept +{ + static_assert( noexcept( T(a / sqrt(a * a + b * b)) ), "throwing expr" ); + return a / sqrt(a * a + b * b); +} + +int foo() noexcept +{ + return 1; +} + ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-14 16:25 ` Umesh Kalappa @ 2018-11-14 18:53 ` Marek Polacek 2018-11-15 8:56 ` Umesh Kalappa 0 siblings, 1 reply; 14+ messages in thread From: Marek Polacek @ 2018-11-14 18:53 UTC (permalink / raw) To: Umesh Kalappa; +Cc: jason, gcc-patches On Wed, Nov 14, 2018 at 09:55:39PM +0530, Umesh Kalappa wrote: > My bad Marek and thank you for pointing that out. > > Please find the attached correct one (pr52869.patch) . Index: gcc/cp/ChangeLog =================================================================== --- gcc/cp/ChangeLog (revision 266026) +++ gcc/cp/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + *parser.c () : restore the old current_class_{ptr,ref} by + inject_this_parameter(). + So the correct CL entry would look like 2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> DR 1207 PR c++/52869 * parser.c (cp_parser_noexcept_specification_opt): Call inject_this_parameter. or so. Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 266026) +++ gcc/cp/parser.c (working copy) @@ -24615,11 +24615,24 @@ { tree expr; cp_lexer_consume_token (parser->lexer); - + You're adding whitespaces where they shouldn't be. Let's avoid changes like these. if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) { matching_parens parens; parens.consume_open (parser); + + if (current_class_type) + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); + else + { + /*clear the current_class_ptr for non class type , like + int foo() noexcept(*this) + { + return 1; + } + */ + current_class_ptr = NULL_TREE; + } I don't believe that's what Jason meant by restoring; I think you want tree save_ccp = current_class_ptr; tree save_ccr = current_class_ref; inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); [...] current_class_ptr = save_ccp; current_class_ref = save_ccr; In the future, if using diff, please also use the -p option. Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 266026) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR g++.dg/52869 + * g++.dg/pr52869.C: New. Should be "PR c++/52869". Index: gcc/testsuite/g++.dg/pr52869.C =================================================================== --- gcc/testsuite/g++.dg/pr52869.C (nonexistent) +++ gcc/testsuite/g++.dg/pr52869.C (working copy) Maybe move the test to testsuite/g++.dg/DRs? @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -g" } */ Why these options? I don't think you need -g. +struct S { + void f() { } + void g() noexcept(noexcept(f())) { } + void h() noexcept(noexcept(this->f())) { } +}; + +struct Nyan { + constexpr Nyan &operator++() noexcept { return *this; } + constexpr void omg() noexcept(noexcept(++*this)) {} +}; I was hoping you'd add also a test with 'this' in noexcept in a class template. This test doesn't compile on all dialects: FAIL: g++.dg/pr52869.C -std=gnu++98 (test for excess errors) FAIL: g++.dg/pr52869.C -std=gnu++11 (test for excess errors) You can run just the new test in all dialects using: GXX_TESTSUITE_STDS=98,11,14,17,2a make check-c++ RUNTESTFLAGS=dg.exp=pr52869.C The noexcept specifier is only in C++11 and newer I think. +template< typename T > +T sine( T const& a, T const& b ) noexcept +{ + static_assert( noexcept( T(a / sqrt(a * a + b * b)) ), "throwing expr" ); + return a / sqrt(a * a + b * b); +} + +int foo() noexcept +{ + return 1; +} + I don't understand what this part of the test is testing. It compiles even without the patch. Let's drop it. Marek ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-14 18:53 ` Marek Polacek @ 2018-11-15 8:56 ` Umesh Kalappa 2018-11-15 15:56 ` Marek Polacek 0 siblings, 1 reply; 14+ messages in thread From: Umesh Kalappa @ 2018-11-15 8:56 UTC (permalink / raw) To: polacek; +Cc: jason, gcc-patches, kamlesh kumar [-- Attachment #1: Type: text/plain, Size: 4408 bytes --] Thank you Marek for the inputs . >>In the future, if using diff, please also use the -p option. We are using svn diif and other comments are addressed . please let us know your take on the revised attached patch . Thank you ~Umesh On Thu, Nov 15, 2018 at 12:23 AM Marek Polacek <polacek@redhat.com> wrote: > > On Wed, Nov 14, 2018 at 09:55:39PM +0530, Umesh Kalappa wrote: > > My bad Marek and thank you for pointing that out. > > > > Please find the attached correct one (pr52869.patch) . > > Index: gcc/cp/ChangeLog > =================================================================== > --- gcc/cp/ChangeLog (revision 266026) > +++ gcc/cp/ChangeLog (working copy) > @@ -1,3 +1,9 @@ > +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> > + > + PR c++/52869 > + *parser.c () : restore the old current_class_{ptr,ref} by > + inject_this_parameter(). > + > > So the correct CL entry would look like > > 2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> > > DR 1207 > PR c++/52869 > * parser.c (cp_parser_noexcept_specification_opt): Call > inject_this_parameter. > > or so. > > Index: gcc/cp/parser.c > =================================================================== > --- gcc/cp/parser.c (revision 266026) > +++ gcc/cp/parser.c (working copy) > @@ -24615,11 +24615,24 @@ > { > tree expr; > cp_lexer_consume_token (parser->lexer); > - > + > > You're adding whitespaces where they shouldn't be. Let's avoid changes like these. > > if (cp_lexer_peek_token (parser->lexer)->type == CPP_OPEN_PAREN) > { > matching_parens parens; > parens.consume_open (parser); > + > + if (current_class_type) > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > + else > + { > + /*clear the current_class_ptr for non class type , like > + int foo() noexcept(*this) > + { > + return 1; > + } > + */ > + current_class_ptr = NULL_TREE; > + } > > I don't believe that's what Jason meant by restoring; I think you want > > tree save_ccp = current_class_ptr; > tree save_ccr = current_class_ref; > > inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > [...] > > current_class_ptr = save_ccp; > current_class_ref = save_ccr; > > In the future, if using diff, please also use the -p option. > > Index: gcc/testsuite/ChangeLog > =================================================================== > --- gcc/testsuite/ChangeLog (revision 266026) > +++ gcc/testsuite/ChangeLog (working copy) > @@ -1,3 +1,8 @@ > +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> > + > + PR g++.dg/52869 > + * g++.dg/pr52869.C: New. > > Should be "PR c++/52869". > > Index: gcc/testsuite/g++.dg/pr52869.C > =================================================================== > --- gcc/testsuite/g++.dg/pr52869.C (nonexistent) > +++ gcc/testsuite/g++.dg/pr52869.C (working copy) > > Maybe move the test to testsuite/g++.dg/DRs? > > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O0 -g" } */ > > Why these options? I don't think you need -g. > > +struct S { > + void f() { } > + void g() noexcept(noexcept(f())) { } > + void h() noexcept(noexcept(this->f())) { } > +}; > + > +struct Nyan { > + constexpr Nyan &operator++() noexcept { return *this; } > + constexpr void omg() noexcept(noexcept(++*this)) {} > +}; > > I was hoping you'd add also a test with 'this' in noexcept in a class template. > > This test doesn't compile on all dialects: > FAIL: g++.dg/pr52869.C -std=gnu++98 (test for excess errors) > FAIL: g++.dg/pr52869.C -std=gnu++11 (test for excess errors) > > You can run just the new test in all dialects using: > GXX_TESTSUITE_STDS=98,11,14,17,2a make check-c++ RUNTESTFLAGS=dg.exp=pr52869.C > > The noexcept specifier is only in C++11 and newer I think. > > +template< typename T > > +T sine( T const& a, T const& b ) noexcept > +{ > + static_assert( noexcept( T(a / sqrt(a * a + b * b)) ), "throwing expr" ); > + return a / sqrt(a * a + b * b); > +} > + > +int foo() noexcept > +{ > + return 1; > +} > + > > I don't understand what this part of the test is testing. It compiles even > without the patch. Let's drop it. > > Marek [-- Attachment #2: pr52869.patch --] [-- Type: application/octet-stream, Size: 2251 bytes --] Index: cp/ChangeLog =================================================================== --- cp/ChangeLog (revision 266026) +++ cp/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + *parser.c () : restore the old current_class_{ptr,ref} by + inject_this_parameter(). + 2018-11-09 Jakub Jelinek <jakub@redhat.com> * parser.c (cp_parser_omp_clause_final, cp_parser_omp_clause_if): Use Index: cp/parser.c =================================================================== --- cp/parser.c (revision 266026) +++ cp/parser.c (working copy) @@ -24620,6 +24620,12 @@ { matching_parens parens; parens.consume_open (parser); + + tree save_ccp = current_class_ptr; + tree save_ccr = current_class_ref; + + if (current_class_type) + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); if (require_constexpr) { @@ -24640,6 +24646,9 @@ } parens.require_close (parser); + + save_ccp = current_class_ptr = save_ccp; + save_ccr = current_class_ref = save_ccr; } else { Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 266026) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + * g++.dg//DRs/dr52869.C: New. + 2018-11-11 Xianmiao Qu <xianmiao_qu@c-sky.com> * gcc.target/csky/fnmul-1.c: New. Index: testsuite/g++.dg/DRs/dr52869.C =================================================================== --- testsuite/g++.dg/DRs/dr52869.C (nonexistent) +++ testsuite/g++.dg/DRs/dr52869.C (working copy) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -std=c++11" } */ + +struct S { + void f() { } + void g() noexcept(noexcept(f())) { } + void h() noexcept(noexcept(this->f())) { } +}; + +struct Nyan { + Nyan &operator++() noexcept { return *this; } + void omg() noexcept(noexcept(++*this)) {} +}; + +template <class T> +class Test{ + T count; + Test (T arg) {count=arg;} + void fetch() { } + T inc () noexcept(noexcept(this->fetch())) {return ++count;} + T dec () noexcept(noexcept(fetch())) { return --count;} +}; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-15 8:56 ` Umesh Kalappa @ 2018-11-15 15:56 ` Marek Polacek 2018-11-15 15:59 ` Marek Polacek 2018-11-16 6:53 ` Umesh Kalappa 0 siblings, 2 replies; 14+ messages in thread From: Marek Polacek @ 2018-11-15 15:56 UTC (permalink / raw) To: Umesh Kalappa; +Cc: jason, gcc-patches, kamlesh kumar On Thu, Nov 15, 2018 at 02:26:24PM +0530, Umesh Kalappa wrote: > Thank you Marek for the inputs . > >>In the future, if using diff, please also use the -p option. > We are using svn diif and other comments are addressed . Thanks, but it doesn't seem like the -p option was used. > please let us know your take on the revised attached patch . Index: cp/ChangeLog =================================================================== --- cp/ChangeLog (revision 266026) +++ cp/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + *parser.c () : restore the old current_class_{ptr,ref} by + inject_this_parameter(). + This is still the same; can you adjust it according to my last suggestion? Index: cp/parser.c =================================================================== --- cp/parser.c (revision 266026) +++ cp/parser.c (working copy) @@ -24620,6 +24620,12 @@ { matching_parens parens; parens.consume_open (parser); + + tree save_ccp = current_class_ptr; + tree save_ccr = current_class_ref; + Watch out for trailing whitespace in the blank lines. + if (current_class_type) + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); I think you can remove the if here. if (require_constexpr) { @@ -24640,6 +24646,9 @@ } parens.require_close (parser); + + save_ccp = current_class_ptr = save_ccp; + save_ccr = current_class_ref = save_ccr; You don't need to set save_cc[pr] to itself here. Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 266026) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + * g++.dg//DRs/dr52869.C: New. + So DR != PR. Please name the test dr1207.C Index: testsuite/g++.dg/DRs/dr52869.C =================================================================== --- testsuite/g++.dg/DRs/dr52869.C (nonexistent) +++ testsuite/g++.dg/DRs/dr52869.C (working copy) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -std=c++11" } */ Instead of this, do: // { dg-do compile { target c++11 } } Also, I wrote a test that fails if current_class_{ptr,ref} aren't properly restored: // DR 1207 // PR c++/52869 // { dg-do compile { target c++11 } } void fn () { struct S { bool operator!() noexcept(false); } s; S t = s; } So you can add that one too, e.g. testsuite/g++.dg/DRs/dr1207-2.C. Thanks, Marek ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-15 15:56 ` Marek Polacek @ 2018-11-15 15:59 ` Marek Polacek 2018-11-16 6:53 ` Umesh Kalappa 1 sibling, 0 replies; 14+ messages in thread From: Marek Polacek @ 2018-11-15 15:59 UTC (permalink / raw) To: Umesh Kalappa; +Cc: jason, gcc-patches, kamlesh kumar On Thu, Nov 15, 2018 at 10:56:15AM -0500, Marek Polacek wrote: > + if (current_class_type) > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > I think you can remove the if here. Actually you probably need it after all. Marek ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-15 15:56 ` Marek Polacek 2018-11-15 15:59 ` Marek Polacek @ 2018-11-16 6:53 ` Umesh Kalappa 2018-11-16 18:18 ` Marek Polacek 1 sibling, 1 reply; 14+ messages in thread From: Umesh Kalappa @ 2018-11-16 6:53 UTC (permalink / raw) To: Marek Polacek; +Cc: jason, gcc-patches, kamlesh kumar [-- Attachment #1: Type: text/plain, Size: 3055 bytes --] Thank you Marek,Appreciate your valuable feedback on the patch . Attached the latest ,please do let us know your thoughts. ~Umesh On Thu, Nov 15, 2018 at 9:26 PM Marek Polacek <polacek@redhat.com> wrote: > > On Thu, Nov 15, 2018 at 02:26:24PM +0530, Umesh Kalappa wrote: > > Thank you Marek for the inputs . > > >>In the future, if using diff, please also use the -p option. > > We are using svn diif and other comments are addressed . > > Thanks, but it doesn't seem like the -p option was used. > > > please let us know your take on the revised attached patch . > > > Index: cp/ChangeLog > =================================================================== > --- cp/ChangeLog (revision 266026) > +++ cp/ChangeLog (working copy) > @@ -1,3 +1,9 @@ > +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> > + > + PR c++/52869 > + *parser.c () : restore the old current_class_{ptr,ref} by > + inject_this_parameter(). > + > > This is still the same; can you adjust it according to my last suggestion? > > Index: cp/parser.c > =================================================================== > --- cp/parser.c (revision 266026) > +++ cp/parser.c (working copy) > @@ -24620,6 +24620,12 @@ > { > matching_parens parens; > parens.consume_open (parser); > + > + tree save_ccp = current_class_ptr; > + tree save_ccr = current_class_ref; > + > > Watch out for trailing whitespace in the blank lines. > > + if (current_class_type) > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > I think you can remove the if here. > > if (require_constexpr) > { > @@ -24640,6 +24646,9 @@ > } > > parens.require_close (parser); > + > + save_ccp = current_class_ptr = save_ccp; > + save_ccr = current_class_ref = save_ccr; > > You don't need to set save_cc[pr] to itself here. > > Index: testsuite/ChangeLog > =================================================================== > --- testsuite/ChangeLog (revision 266026) > +++ testsuite/ChangeLog (working copy) > @@ -1,3 +1,8 @@ > +2018-11-14 Kamlesh Kumar <kamleshbhalui@gmail.com> > + > + PR c++/52869 > + * g++.dg//DRs/dr52869.C: New. > + > > So DR != PR. Please name the test dr1207.C > > Index: testsuite/g++.dg/DRs/dr52869.C > =================================================================== > --- testsuite/g++.dg/DRs/dr52869.C (nonexistent) > +++ testsuite/g++.dg/DRs/dr52869.C (working copy) > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O0 -std=c++11" } */ > > Instead of this, do: > > // { dg-do compile { target c++11 } } > > Also, I wrote a test that fails if current_class_{ptr,ref} aren't properly > restored: > > // DR 1207 > // PR c++/52869 > // { dg-do compile { target c++11 } } > > void > fn () > { > struct S { > bool operator!() noexcept(false); > } s; > S t = s; > } > > So you can add that one too, e.g. testsuite/g++.dg/DRs/dr1207-2.C. > > Thanks, > Marek [-- Attachment #2: pr52869.patch --] [-- Type: application/octet-stream, Size: 2827 bytes --] Index: gcc/cp/ChangeLog =================================================================== --- gcc/cp/ChangeLog (revision 266026) +++ gcc/cp/ChangeLog (working copy) @@ -1,3 +1,10 @@ +2018-11-16 Kamlesh Kumar <kamleshbhalui@gmail.com> + + DR 1207 + PR c++/52869 + * parser.c (cp_parser_noexcept_specification_opt): Call + inject_this_parameter. + 2018-11-09 Jakub Jelinek <jakub@redhat.com> * parser.c (cp_parser_omp_clause_final, cp_parser_omp_clause_if): Use Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 266026) +++ gcc/cp/parser.c (working copy) @@ -24620,6 +24620,12 @@ cp_parser_noexcept_specification_opt (cp_parser* p { matching_parens parens; parens.consume_open (parser); + + tree save_ccp = current_class_ptr; + tree save_ccr = current_class_ref; + + if (current_class_type) + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); if (require_constexpr) { @@ -24640,6 +24646,9 @@ cp_parser_noexcept_specification_opt (cp_parser* p } parens.require_close (parser); + + current_class_ptr = save_ccp; + current_class_ref = save_ccr; } else { Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 266026) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-11-16 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + * g++.dg//DRs/dr1207-1.C: New. + * g++.dg//DRs/dr1207-2.C: New. + 2018-11-11 Xianmiao Qu <xianmiao_qu@c-sky.com> * gcc.target/csky/fnmul-1.c: New. Index: gcc/testsuite/g++.dg/DRs/dr1207-1.C =================================================================== --- gcc/testsuite/g++.dg/DRs/dr1207-1.C (nonexistent) +++ gcc/testsuite/g++.dg/DRs/dr1207-1.C (working copy) @@ -0,0 +1,25 @@ +// DR 1207 +// PR c++/52869 +// { dg-do compile { target c++11 } } + +struct S { + void f() { } + void g() noexcept(noexcept(f())) { } + void h() noexcept(noexcept(this->f())) { } +}; + +struct Nyan { + Nyan &operator++() noexcept { return *this; } + void omg() noexcept(noexcept(++*this)) {} +}; + +template <class T> +class Test{ + T count; + Test (T arg) {count=arg;} + void fetch() { } + T inc () noexcept(noexcept(this->fetch())) {return ++count;} + T dec () noexcept(noexcept(fetch())) { return --count;} +}; + + Index: gcc/testsuite/g++.dg/DRs/dr1207-2.C =================================================================== --- gcc/testsuite/g++.dg/DRs/dr1207-2.C (nonexistent) +++ gcc/testsuite/g++.dg/DRs/dr1207-2.C (working copy) @@ -0,0 +1,12 @@ +// DR 1207 +// PR c++/52869 +// { dg-do compile { target c++11 } } + +void +fn () +{ + struct S { + bool operator!() noexcept(false); + } s; + S t = s; +} ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-16 6:53 ` Umesh Kalappa @ 2018-11-16 18:18 ` Marek Polacek 2018-11-16 21:55 ` Jason Merrill 0 siblings, 1 reply; 14+ messages in thread From: Marek Polacek @ 2018-11-16 18:18 UTC (permalink / raw) To: Umesh Kalappa; +Cc: jason, gcc-patches, kamlesh kumar On Fri, Nov 16, 2018 at 12:23:42PM +0530, Umesh Kalappa wrote: > Thank you Marek,Appreciate your valuable feedback on the patch . > > Attached the latest ,please do let us know your thoughts. Thanks, this version looks good! Just some small nits: --- gcc/cp/parser.c (revision 266026) +++ gcc/cp/parser.c (working copy) @@ -24620,6 +24620,12 @@ cp_parser_noexcept_specification_opt (cp_parser* p { matching_parens parens; parens.consume_open (parser); + + tree save_ccp = current_class_ptr; + tree save_ccr = current_class_ref; + + if (current_class_type) + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); This is indented a bit too much: the inject_this_parameter call should be two spaces to the right of the if. I.e.: if (current_class_type) inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); +2018-11-16 Kamlesh Kumar <kamleshbhalui@gmail.com> + + PR c++/52869 + * g++.dg//DRs/dr1207-1.C: New. + * g++.dg//DRs/dr1207-2.C: New. Just one / instead of two. --- gcc/testsuite/g++.dg/DRs/dr1207-1.C (nonexistent) +++ gcc/testsuite/g++.dg/DRs/dr1207-1.C (working copy) @@ -0,0 +1,25 @@ +// DR 1207 +// PR c++/52869 +// { dg-do compile { target c++11 } } + +struct S { + void f() { } + void g() noexcept(noexcept(f())) { } + void h() noexcept(noexcept(this->f())) { } +}; + +struct Nyan { + Nyan &operator++() noexcept { return *this; } + void omg() noexcept(noexcept(++*this)) {} +}; + +template <class T> +class Test{ + T count; + Test (T arg) {count=arg;} + void fetch() { } + T inc () noexcept(noexcept(this->fetch())) {return ++count;} + T dec () noexcept(noexcept(fetch())) { return --count;} +}; + + There are two extra newlines you can remove but otherwise the tests look fine now. I'll let Jason review the last version. Thanks for contributing to GCC. Marek ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses 2018-11-16 18:18 ` Marek Polacek @ 2018-11-16 21:55 ` Jason Merrill 0 siblings, 0 replies; 14+ messages in thread From: Jason Merrill @ 2018-11-16 21:55 UTC (permalink / raw) To: Marek Polacek, Umesh Kalappa; +Cc: gcc-patches, kamlesh kumar On 11/16/18 1:18 PM, Marek Polacek wrote: > On Fri, Nov 16, 2018 at 12:23:42PM +0530, Umesh Kalappa wrote: >> Thank you Marek,Appreciate your valuable feedback on the patch . >> >> Attached the latest ,please do let us know your thoughts. > > Thanks, this version looks good! Just some small nits: > > --- gcc/cp/parser.c (revision 266026) > +++ gcc/cp/parser.c (working copy) > @@ -24620,6 +24620,12 @@ cp_parser_noexcept_specification_opt (cp_parser* p > { > matching_parens parens; > parens.consume_open (parser); > + > + tree save_ccp = current_class_ptr; > + tree save_ccr = current_class_ref; > + > + if (current_class_type) > + inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > This is indented a bit too much: the inject_this_parameter call should be two > spaces to the right of the if. I.e.: > > if (current_class_type) > inject_this_parameter (current_class_type, TYPE_UNQUALIFIED); > > +2018-11-16 Kamlesh Kumar <kamleshbhalui@gmail.com> > + > + PR c++/52869 > + * g++.dg//DRs/dr1207-1.C: New. > + * g++.dg//DRs/dr1207-2.C: New. > > Just one / instead of two. > > --- gcc/testsuite/g++.dg/DRs/dr1207-1.C (nonexistent) > +++ gcc/testsuite/g++.dg/DRs/dr1207-1.C (working copy) > @@ -0,0 +1,25 @@ > +// DR 1207 > +// PR c++/52869 > +// { dg-do compile { target c++11 } } > + > +struct S { > + void f() { } > + void g() noexcept(noexcept(f())) { } > + void h() noexcept(noexcept(this->f())) { } > +}; > + > +struct Nyan { > + Nyan &operator++() noexcept { return *this; } > + void omg() noexcept(noexcept(++*this)) {} > +}; > + > +template <class T> > +class Test{ > + T count; > + Test (T arg) {count=arg;} > + void fetch() { } > + T inc () noexcept(noexcept(this->fetch())) {return ++count;} > + T dec () noexcept(noexcept(fetch())) { return --count;} > +}; > + > + > > There are two extra newlines you can remove but otherwise the tests look fine now. > > I'll let Jason review the last version. Thanks for contributing to GCC. Yes, looks good; I've committed the patch with the changes Marek mentioned. I see that you don't seem to have a copyright assignment on file with the FSF; this patch is small enough that it doesn't need it, but you might want to get one submitted now to ease future patches. Thanks! Jason ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-11-16 21:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-13 6:20 Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses Umesh Kalappa 2018-11-13 15:40 ` Marek Polacek 2018-11-13 21:25 ` Jason Merrill 2018-11-14 11:49 ` Umesh Kalappa 2018-11-14 13:23 ` Umesh Kalappa 2018-11-14 14:33 ` Marek Polacek 2018-11-14 16:25 ` Umesh Kalappa 2018-11-14 18:53 ` Marek Polacek 2018-11-15 8:56 ` Umesh Kalappa 2018-11-15 15:56 ` Marek Polacek 2018-11-15 15:59 ` Marek Polacek 2018-11-16 6:53 ` Umesh Kalappa 2018-11-16 18:18 ` Marek Polacek 2018-11-16 21:55 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).