* 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).