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 <pola...@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 <kamleshbha...@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 <kamleshbha...@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 <kamleshbha...@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
pr52869.patch
Description: Binary data
