Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Umesh Kalappa]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Marek Polacek]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Jason Merrill]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Umesh Kalappa]

[-- 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)

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Umesh Kalappa]

>>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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Marek Polacek]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Umesh Kalappa]

[-- 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;
+}
+

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Marek Polacek]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Umesh Kalappa]

[-- 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;} 
+};

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Marek Polacek]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Marek Polacek]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Umesh Kalappa]

[-- 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;
+}

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Marek Polacek]

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

Re: Bug 52869 - [DR 1207] "this" not being allowed in noexcept clauses

[Jason Merrill]

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