On Tue, 30 Jul 2024, Jason Merrill wrote:
> On 7/29/24 5:32 PM, Patrick Palka wrote:
> > On Mon, 29 Jul 2024, Jakub Jelinek wrote:
> >
> > > On Fri, Jul 26, 2024 at 06:00:12PM -0400, Patrick Palka wrote:
> > > > On Fri, 26 Jul 2024, Jakub Jelinek wrote:
> > > >
> > > > > On Fri, Jul 26, 2024 at 04:42:36PM -0400, Patrick Palka wrote:
> > > > > > > // P2963R3 - Ordering of constraints involving fold expressions
> > > > > > > // { dg-do compile { target c++20 } }
> > > > > > >
> > > > > > > template <class ...T> concept C = (__is_same (T, int) && ...);
> > > > > > > template <typename V>
> > > > > > > struct S {
> > > > > > > template <class ...U> requires (C<U...>)
> > > > > > > static constexpr bool foo () { return true; }
> > > > > > > };
> > > > > > >
> > > > > > > static_assert (S<void>::foo <int, int, int, int> ());
> > > > > > >
> > > > > > > somehow the template parameter mapping needs to be remembered even
> > > > > > > for the
> > > > > > > fold expanded constraint, right now the patch will see the pack is
> > > > > > > T,
> > > > > > > which is level 1 index 0, but args aren't arguments of the C
> > > > > > > concept,
> > > > > > > but of the foo function template.
> > > > > > > One can also use requires (C<int, int, int>) etc., no?
> > > > > >
> > > > > > It seems the problem is FOLD_EXPR_PACKS is currently set to the
> > > > > > parameter packs used inside the non-normalized constraints, but I
> > > > > > think
> > > > > > what we really need are the packs used in the normalized
> > > > > > constraints,
> > > > > > specifically the packs used in the target of each parameter mapping
> > > > > > of
> > > > > > each atomic constraint?
> > > > >
> > > > > But in that case there might be no packs at all.
> > > > >
> > > > > template <class T> C = true;
> > > > > template <class ...U> requires (C<T> && ...)
> > > > > constexpr bool foo () { return true; }
> > > > >
> > > > > If normalized C<T> is just true, it doesn't use any packs.
> > > > > But the [temp.constr.fold] wording assumes it is a pack expansion and
> > > > > that
> > > > > there is at least one pack expansion parameter, otherwise N wouldn't
> > > > > be
> > > > > defined.
> > > >
> > > > Hmm yeah, I see what you mean. That seems to be an edge case that's not
> > > > fully accounted for by the wording.
>
> I agree the wording is unclear, but it seems necessary to me that T is a pack
> expansion parameter, even if it isn't mentioned by the normalized constraint.
>
> > > > One thing that's unclear to me in that wording is what are the pack
> > > > expansion parameters of a fold expanded constraint.
> > > >
> > > > In
> > > >
> > > > template<class... T> concept C = (__is_same (T, int) && ...);
> > > > template<class U, class... V>
> > > > void f() requires C<V...>;
> > > >
> > > > is the pack expansion parameter T or V? In
> > > >
> > > > template<class... T> concept C = (__is_same (T, int) && ...);
> > > > template<class U>
> > > > void g() requires C<U>;
> > > >
> > > > it must be T. So I guess in both cases it must be T. But then I reckon
> > > > when [temp.constr.fold] mentions "pack expansion parameter(s)" what it
> > > > really means is "target of each pack expansion parameter within the
> > > > parameter mapping"...
>
> Yeah.
>
> In the paper a fold expanded constraint doesn't have a parameter mapping, only
> atomic constraints do. Within the normal form of (__is_same (T, int) && ...)
> we have a single atomic constraint with parameter mapping T -> T, which only
> comes into play when we're checking satisfaction for each element.
>
> But that doesn't specify how the packs are established. For many cases it's a
> simple matter of connecting one pack to another, so you could kind of handwave
> it, but it isn't that hard to come up with a testcase that isn't so simple,
> say
>
> template<class... T> concept C = (__is_same (T, int) && ...);
> template <class...T> struct A { };
> template <class...U, class...V>
> void g(A<U...>, A<V...>) requires C<U..., V...>;
>
> How is <U..., V...> expressed in the normalized constraints of g?
Couldn't the parameter mapping be just T -> {U..., V...}? Ah but
then during satisfaction we somehow need to know to substitute the
elements of U and V serially instead of in parallel, i.e. not conflate
it with `requires (C<U, V>) && ...)', while also respecting short
circuiting and all that...
>
> > > So, shall we file some https://github.com/cplusplus/CWG/ issue about this?
> > > Whether the packs [temp.constr.fold] talks about are the normalized ones
> > > only (in that case what happens if there are no packs), or all packs
> > > mentioned (in that case, whether there shouldn't be also template
> > > parameter
> > > mappings on the fold expanded constraints like there are on the atomic
> > > constraints (for the unexpanded packs only)?
>
> I think there should be parameter mappings for all parameter packs named in
> the fold-expression. And I suppose for the other template parameters as well.
>
> > Seems worth submitting an issue, but I'm not 100% sure about my
> > understanding of the paper's wording.. I wonder what Jason thinks.
> >
> > >
> > > Interesting testcases could be also:
> > > struct A <class ...T> {};
> > > template <class T> C = true;
> > > template <class T> D = __is_same (T, int);
> > > template <class ...U, class ... V> requires ((C<U> && D<V>) && ...)
> > > constexpr bool foo (A<U...>, A<V...>) { return true; }
> > > static_assert (foo (A<int, int>, A<int, int, int>));
> > > // Is this valid because only V unexpanded pack from the normalized
> > > // constraint is considered, or invalid because there are 2 packs
> > > // and have different length?
>
> IMO ill-formed.
>
> > > Anyway, I'm afraid on the implementation side, ARGUMENT_PACK_SELECT
> > > didn't help almost at all. The problem e.g. on fold-constr7.C testcase
> > > is that the ARGUMENT_PACK_SELECT is optimized away before it could be
> > > used.
> > > tsubst_parameter_mapping (where I could remove the
> > > if (cxx_dialect >= cxx26 && ARGUMENT_PACK_P (arg))
> > > hack without any behavior change) just tsubsts it into int type.
> > > With the hack removed, it will go through
> > > if (ARGUMENT_PACK_P (arg))
> > > new_arg = tsubst_argument_pack (arg, args, complain, in_decl);
> > > but that still sets new_arg to int INTEGER_TYPE; while if a pack is used
> > > in some nested pack expansion as well as outside of it, we'd need to
> > > arrange
> > > to reconstruct ARGUMENT_PACK_SELECT in what tsubst_parameter_mapping
> > > arranges.
> >
> > Ah right, because of the double substitution -- first satisfy_atom
> > substitutes into the parameter mapping, and then it substitutes this
> > substituted parameter mapping into the atomic constraint expression.
> > So after the first substitution the APS might already have gotten
> > "resolved", I think..
> >
> > IIUC the normal form of the constraint in fold-constr7.C will have
> > the identity parameter mapping Ts -> {Ts...}. And you'll be passing
> > Ts=APS<{int,int,...}, 0> etc to the recursive satisfy_constraint_r call
> > in satisfy_fold.
> >
> > Does it work if you wrap the ARGUMENT_PACK_SELECT in a single-element
> > TYPE/NONTYPE_ARGUMENT_PACK?
>
> I think trying to play games with APS in the normalized form is a mistake; I'd
> think we should only use it it when substituting elements of the argument pack
> into the atomic constraint's parameter mapping.
Indeed I was thinking of using ARGUMENT_PACK_SELECT during satisfaction.
The problem with using a bare APS is that satisfaction needs to
substitute twice, first into the parameter mapping and then into the
constraint, and this first substitution could prematurely resolve the
APS. To work around that we could wrap each APS in a single-element
argument pack and then treat the constraint as a standalone pack
expansion. So e.g. for
template<class... Ts>
void f() requires ((2*Ts::value < (Ts::value + ...)) && ...)
// normal form has the identity mapping Ts -> {Ts...}
f<A, B, C>();
satisfy_fold would get called with Ts={A, B, C} (the template args)
satisfy_fold would recurse into satisfy_atom with Ts={APS<{A, B, C}, N} for
N=0,1,2
satisfy_atom would substitute into the parameter mapping yielding
the instantiated mapping Ts -> {APS<{A, B, C}, N>}
satisfy_atom would call make_pack_expansion on the constraint of
the atom, turning it into a standalone pack expansion, and
substitute into it with the instantiated mapping yielding
{2*A::value < (A::value + B::value + C::value)}
That way the APS doesn't get prematurely resolved during the first
substitution in satisfy_atom. Not sure how this would work in
more complicated examples, though...
By the way, for one of the examples from the paper
template <typename X, typename... T>
concept environment_of = (... && requires (X& x) { { get<T>(x) } ->
std::same_as<T&>; } );
auto f(sender auto&& s, environment_of<std::stop_token> auto env); // #1
auto f(sender auto&& s, environment_of<std::stop_token, std::pmr::allocator>
auto env); // #2
I don't see how the paper now allows #2 to be more constrained than #1.
Wouldn't determining that require expanding the fold during normalization
which is certainly not what the wording says? Seems the Clang
implementation doesn't accept the example either:
https://godbolt.org/z/dG1Gc1h4a
