Re: [PATCH] c++: outer 'this' leaking into local class [PR106969]

[Jason Merrill <jason@redhat.com>]

On 3/23/23 11:00, Patrick Palka wrote:
> Here when resolving the implicit object for '&wrapped' within the
> local class Foo, we expect to obtain a dummy object of type Foo& since
> there's no 'this' available in this context.  And yet at this point
> current_class_ref still corresponds to the outer class Context (and is
> const), which confuses maybe_dummy_object into propagating the cv-quals
> of current_class_ref and returning an object of type const Foo&.  Thus
> decltype(&wrapped) wrongly yields const int* instead of int*.
> 
> The problem ultimately seems to be that the 'this' from the enclosing
> class appears available for use when parsing the local class, but 'this'
> shouldn't leak across classes like that.  This patch fixes this by
> clearing current_class_ptr/ref when parsing a class definition.
> 
> After this change, for name-clash11.C in C++98 mode we would now
> complain about an invalid use of 'this' for e.g.
> 
>    ASSERT (sizeof (this->A) == 16);
> 
> due to the way the ASSERT macro is defined using a local class.  This
> patch redefines it using a local typedef instead.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?

OK.

> 	PR c++/106969
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_class_specifier): Clear current_class_ptr
> 	and current_class_ref when parsing a class definition.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/lookup/name-clash11.C: New test.
> 	* g++.dg/lookup/this2.C: New test.
> ---
>   gcc/cp/parser.cc                           | 13 +++++++++----
>   gcc/testsuite/g++.dg/lookup/name-clash11.C |  2 +-
>   gcc/testsuite/g++.dg/lookup/this2.C        | 22 ++++++++++++++++++++++
>   3 files changed, 32 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/lookup/this2.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index a277003ea58..be9c77b415e 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -26151,6 +26151,11 @@ cp_parser_class_specifier (cp_parser* parser)
>     saved_in_unbraced_linkage_specification_p
>       = parser->in_unbraced_linkage_specification_p;
>     parser->in_unbraced_linkage_specification_p = false;
> +  /* 'this' from an enclosing non-static member function is unvailable.  */
> +  tree saved_ccp = current_class_ptr;
> +  tree saved_ccr = current_class_ref;
> +  current_class_ptr = NULL_TREE;
> +  current_class_ref = NULL_TREE;
>   
>     /* Start the class.  */
>     if (nested_name_specifier_p)
> @@ -26369,8 +26374,6 @@ cp_parser_class_specifier (cp_parser* parser)
>         /* If there are noexcept-specifiers that have not yet been processed,
>   	 take care of them now.  Do this before processing NSDMIs as they
>   	 may depend on noexcept-specifiers already having been processed.  */
> -      tree save_ccp = current_class_ptr;
> -      tree save_ccr = current_class_ref;
>         FOR_EACH_VEC_SAFE_ELT (unparsed_noexcepts, ix, decl)
>   	{
>   	  tree ctx = DECL_CONTEXT (decl);
> @@ -26496,8 +26499,8 @@ cp_parser_class_specifier (cp_parser* parser)
>   	}
>         vec_safe_truncate (unparsed_contracts, 0);
>   
> -      current_class_ptr = save_ccp;
> -      current_class_ref = save_ccr;
> +      current_class_ptr = NULL_TREE;
> +      current_class_ref = NULL_TREE;
>         if (pushed_scope)
>   	pop_scope (pushed_scope);
>   
> @@ -26529,6 +26532,8 @@ cp_parser_class_specifier (cp_parser* parser)
>       = saved_num_template_parameter_lists;
>     parser->in_unbraced_linkage_specification_p
>       = saved_in_unbraced_linkage_specification_p;
> +  current_class_ptr = saved_ccp;
> +  current_class_ref = saved_ccr;
>   
>     return type;
>   }
> diff --git a/gcc/testsuite/g++.dg/lookup/name-clash11.C b/gcc/testsuite/g++.dg/lookup/name-clash11.C
> index bc63645e8d3..2ae9a65264d 100644
> --- a/gcc/testsuite/g++.dg/lookup/name-clash11.C
> +++ b/gcc/testsuite/g++.dg/lookup/name-clash11.C
> @@ -7,7 +7,7 @@
>   #  define ASSERT(e) static_assert (e, #e)
>   #else
>   #  define ASSERT(e)                                             \
> -  do { struct S { bool: !!(e); } asrt; (void)&asrt; } while (0)
> +  do { typedef int asrt[bool(e) ? 1 : -1]; } while (0)
>   #endif
>   
>   
> diff --git a/gcc/testsuite/g++.dg/lookup/this2.C b/gcc/testsuite/g++.dg/lookup/this2.C
> new file mode 100644
> index 00000000000..1450c563d92
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/this2.C
> @@ -0,0 +1,22 @@
> +// PR c++/106969
> +// { dg-do compile { target c++11 } }
> +
> +struct Context
> +{
> +    void
> +    action() const
> +    {
> +        struct Foo
> +        {
> +            int wrapped;
> +            decltype( &wrapped ) get() { return &wrapped; }
> +        } t;
> +
> +        *t.get()= 42; // OK, get() returns int* not const int*
> +
> +        struct Bar
> +        {
> +            using type = decltype(this); // { dg-error "invalid use of 'this'" }
> +        };
> +    }
> +};