On Mon, 3 Apr 2023, Jason Merrill wrote:
> On 4/3/23 10:49, Patrick Palka wrote:
> > This testcase demonstrates we can legitimately enter satisfaction with
> > an ARGUMENT_PACK_SELECT argument, which is problematic because we can't
> > store such arguments in the satisfaction cache (or any other hash table).
> >
> > Since this appears to be possible only during constrained auto deduction
> > for a return-type-requirement, the most appropriate spot to fix this seems
> > to be from do_auto_deduction, by calling preserve_args to strip A_P_S args
> > before entering satisfaction.
> >
> > +++ b/gcc/cp/pt.cc
> > @@ -30965,6 +30965,12 @@ do_auto_deduction (tree type, tree init, tree
> > auto_node,
> > return type;
> > }
> > + /* We can see an ARGUMENT_PACK_SELECT argument when evaluating
> > + a return-type-requirement. Get rid of them before entering
> > + satisfaction, since the satisfaction cache can't handle them. */
> > + if (context == adc_requirement)
> > + outer_targs = preserve_args (outer_targs);
>
> I'd like to get do_auto_deduction out of the business of handling
> return-type-requirements, since there is no longer any actual deduction
> involved (as there was in the TS). So I'd prefer not to add any more tweaks
> there.
>
> Maybe this should happen higher up, in tsubst_requires_expr? Maybe just
> before the call to add_extra_args?
Interestingly, calling preserve_args from tsubst_requires_expr breaks
cases where a requires-expression contains an inner pack expansion over
the same pack as the outer expansion, such as 'Ts... ts' in
template<class... Ts>
concept C = (requires (Ts... ts) { Ts(); } && ...);
static_assert(C<int, char>);
and 'sizeof...(Ts)' in
template<int> struct A;
template<class... Ts>
concept C = (requires { typename A<sizeof...(Ts)>; } && ...);
static_assert(C<int, char>);
because we need to hold on to the ARGUMENT_PACK_SELECT version of the
argument in order to properly substitute 'Ts... ts' and 'sizeof...(Ts)'
during each outer expansion, but calling preserve_args gets rid of
the A_P_S.
And on second thought, narrowly calling preserve_args from
do_auto_deduction (or type_deducible_p) isn't quite correct either,
consider:
template<class T, class U, int N>
concept C = __is_same(T, U) && N > 1;
template<class... Ts>
concept D = (requires { { Ts() } -> C<Ts, sizeof...(Ts)>; } && ...);
static_assert(D<int, char>);
We need to hold on to the A_P_S in order to check the
return-type-requirement C<Ts, sizeof...(Ts)>, but the satisfaction
cache can't handle A_P_S!
The following non-requires-expr version of that example works:
template<class T, class U, int N>
concept C = __is_same(T, U) && N > 1;
template<class... Ts>
concept D = (C<Ts, Ts, sizeof...(Ts)> && ...);
static_assert(D<int, char>);
because here we directly substitute into the concept-id
C<Ts, Ts, sizeof...(Ts)>, so we effectively end up checking
satisfaction of C<int, int, 2> and C<char, char, 2> directly
and never enter satisfaction with an A_P_S argument.
So ISTM the satisfaction cache might need to be able to cache A_P_S
arguments perhaps by changing preserve_args to instead make a copy of
each A_P_S and set a flag on this copy to indicate it's "stable"
and therefore suitable for hashing etc.
