Re: [PATCH] c++: implicitly_declare_fn and access checks [PR113908]

[Jason Merrill <jason@redhat.com>]

On 2/14/24 08:46, Patrick Palka wrote:
> On Tue, 13 Feb 2024, Jason Merrill wrote:
> 
>> On 2/13/24 11:49, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, are one of
>>> both of these fixes OK for trunk?
>>>
>>> -- >8 --
>>>
>>> Here during ahead of time checking of the non-dependent new-expr we
>>> synthesize B's copy constructor, which should be defined as deleted
>>> due to A's inaccessible copy constructor.  But enforce_access incorrectly
>>> decides to defer the (silent) access check for A::A(const A&) during
>>> synthesization since current_template_parms is still set (before r14-557
>>> it checked processing_template_decl which got cleared from
>>> implicitly_declare_fn), which leads to the access check leaking out to
>>> the template context that needed the synthesization.
>>>
>>> This patch narrowly fixes this regression in two sufficient ways:
>>>
>>> 1. Clear current_template_parms alongside processing_template_decl
>>>      in implicitly_declare_fn so that it's more independent of context.
>>
>> Hmm, perhaps it or synthesized_method_walk should use maybe_push_to_top_level?
> 
> That works nicely, and also fixes the other regression PR113332.  There
> the lambda context triggering synthesization of a default ctor was
> causing maybe_dummy_object to misbehave during overload resolution of
> one of its member's default ctors, and now synthesization is context
> independent.
> 
>>
>>> 2. Don't defer a silent access check when in a template context,
>>>      since such deferred checks will be replayed noisily at instantiation
>>>      time which may not be what the caller intended.
>>
>> True, but returning a possibly incorrect 'false' is probably also not what the
>> caller intended.  It would be better to see that we never call enforce_access
>> with tf_none in a template.  If that's not feasible, I think we should still
>> conservatively return true.
> 
> Makes sense, I can experiment with that enforce_access access change as
> a follow-up.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK.

> -- >8 --
> 
> Subject: [PATCH] c++: synthesized_method_walk context independence [PR113908]
> 
> 	PR c++/113908
> 	PR c++/113332
> 
> gcc/cp/ChangeLog:
> 
> 	* method.cc (synthesized_method_walk): Use maybe_push_to_top_level.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/non-dependent31.C: New test.
> 	* g++.dg/template/non-dependent32.C: New test.
> ---
>   gcc/cp/method.cc                              |  2 ++
>   .../g++.dg/template/non-dependent31.C         | 18 +++++++++++++++++
>   .../g++.dg/template/non-dependent32.C         | 20 +++++++++++++++++++
>   3 files changed, 40 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/template/non-dependent31.C
>   create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
> 
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index 957496d3e18..98c10e6a8b5 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -2760,6 +2760,7 @@ synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
>   	return;
>       }
>   
> +  bool push_to_top = maybe_push_to_top_level (TYPE_NAME (ctype));
>     ++cp_unevaluated_operand;
>     ++c_inhibit_evaluation_warnings;
>     push_deferring_access_checks (dk_no_deferred);
> @@ -2857,6 +2858,7 @@ synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
>     pop_deferring_access_checks ();
>     --cp_unevaluated_operand;
>     --c_inhibit_evaluation_warnings;
> +  maybe_pop_from_top_level (push_to_top);
>   }
>   
>   /* DECL is a defaulted function whose exception specification is now
> diff --git a/gcc/testsuite/g++.dg/template/non-dependent31.C b/gcc/testsuite/g++.dg/template/non-dependent31.C
> new file mode 100644
> index 00000000000..3fa68f40fe1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/non-dependent31.C
> @@ -0,0 +1,18 @@
> +// PR c++/113908
> +// { dg-do compile { target c++11 } }
> +
> +struct A {
> +  A();
> +private:
> +  A(const A&);
> +};
> +
> +struct B {
> +  A a;
> +
> +  template<class T>
> +  static void f() { new B(); }
> +};
> +
> +template void B::f<int>();
> +static_assert(!__is_constructible(B, const B&), "");
> diff --git a/gcc/testsuite/g++.dg/template/non-dependent32.C b/gcc/testsuite/g++.dg/template/non-dependent32.C
> new file mode 100644
> index 00000000000..246654c5b50
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/non-dependent32.C
> @@ -0,0 +1,20 @@
> +// PR c++/113332
> +// { dg-do compile { target c++11 } }
> +
> +struct tuple {
> +  template<class _Tp>
> +  static constexpr bool __is_implicitly_default_constructible() { return true; }
> +
> +  template<class _Tp = void,
> +           bool = __is_implicitly_default_constructible<_Tp>()>
> +  tuple();
> +};
> +
> +struct DBusStruct {
> +private:
> +  tuple data_;
> +};
> +
> +struct IBusService {
> +  int m = [] { DBusStruct{}; return 42; }();
> +};