On Thu, Apr 29, 2021 at 7:48 AM Patrick Palka <ppa...@redhat.com> wrote:
>
> On Wed, 28 Apr 2021, Jason Merrill wrote:
>
> > On 4/28/21 2:24 PM, Patrick Palka wrote:
> > > This makes tsubst_arg_types substitute into a function's parameter types
> > > in left-to-right order instead of right-to-left order, in accordance with
> > > DR 1227.
> > >
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > trunk? [ diff generated with -w to hide noisy whitespace changes ]
> >
> > OK. We'll still substitute lambda default args in reverse order, but that
> > shouldn't happen in sfinae context, so that shouldn't be a problem. Maybe
> > add
> > an assert (complain & tf_error) in the lambda case?
>
> Apparently cpp2a/lambda-uneval2.C trips over such an assert during
> deduction for:
>
> template <class T>
> auto j(T t) -> decltype([](auto x) -> decltype(x.invalid) { } (t));
>
> which makes sense, I suppose.
>
> It looks like we can just move up the default argument processing to
> before the recursive call to tsubst_arg_types, as in the below.
> Bootstrapped and regtested on x86_64-pc-linux-gnu.
Ping
>
> BTW, now that LAMBDA_EXPR has LAMBDA_EXPR_REGEN_INFO I think we have
> enough information to defer substituting into lambda default arguments
> until they're actually needed. Would that be something to look into as
> a followup patch?
>
> -- >8 --
>
> Subject: [PATCH] c++: Substitute into function parms in lexical order
> [PR96560]
>
> This makes tsubst_arg_types substitute into a function's parameter types
> in left-to-right order instead of right-to-left order, in accordance with
> DR 1227.
>
> gcc/cp/ChangeLog:
>
> DR 1227
> PR c++/96560
> * pt.c (tsubst_arg_types): Rearrange so that we substitute into
> TYPE_ARG_TYPES in forward order while short circuiting
> appropriately. Adjust formatting.
>
> gcc/testsuite/ChangeLog:
>
> DR 1227
> PR c++/96560
> * g++.dg/template/sfinae-dr1227.C: New test.
> ---
> gcc/cp/pt.c | 51 ++++++++++---------
> gcc/testsuite/g++.dg/template/sfinae-dr1227.C | 23 +++++++++
> 2 files changed, 51 insertions(+), 23 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/template/sfinae-dr1227.C
>
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index eaf46659f85..e6d65595e2f 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -15068,20 +15068,13 @@ tsubst_arg_types (tree arg_types,
> tsubst_flags_t complain,
> tree in_decl)
> {
> - tree remaining_arg_types;
> tree type = NULL_TREE;
> - int i = 1;
> + int len = 1;
> tree expanded_args = NULL_TREE;
> - tree default_arg;
>
> if (!arg_types || arg_types == void_list_node || arg_types == end)
> return arg_types;
>
> - remaining_arg_types = tsubst_arg_types (TREE_CHAIN (arg_types),
> - args, end, complain, in_decl);
> - if (remaining_arg_types == error_mark_node)
> - return error_mark_node;
> -
> if (PACK_EXPANSION_P (TREE_VALUE (arg_types)))
> {
> /* For a pack expansion, perform substitution on the
> @@ -15092,7 +15085,7 @@ tsubst_arg_types (tree arg_types,
>
> if (TREE_CODE (expanded_args) == TREE_VEC)
> /* So that we'll spin through the parameters, one by one. */
> - i = TREE_VEC_LENGTH (expanded_args);
> + len = TREE_VEC_LENGTH (expanded_args);
> else
> {
> /* We only partially substituted into the parameter
> @@ -15101,14 +15094,15 @@ tsubst_arg_types (tree arg_types,
> expanded_args = NULL_TREE;
> }
> }
> + else
> + type = tsubst (TREE_VALUE (arg_types), args, complain, in_decl);
>
> - while (i > 0) {
> - --i;
> -
> + /* Check if a substituted type is erroneous before substituting into
> + the rest of the chain. */
> + for (int i = 0; i < len; i++)
> + {
> if (expanded_args)
> type = TREE_VEC_ELT (expanded_args, i);
> - else if (!type)
> - type = tsubst (TREE_VALUE (arg_types), args, complain, in_decl);
>
> if (type == error_mark_node)
> return error_mark_node;
> @@ -15122,15 +15116,12 @@ tsubst_arg_types (tree arg_types,
> }
> return error_mark_node;
> }
> -
> - /* Do array-to-pointer, function-to-pointer conversion, and ignore
> - top-level qualifiers as required. */
> - type = cv_unqualified (type_decays_to (type));
> + }
>
> /* We do not substitute into default arguments here. The standard
> mandates that they be instantiated only when needed, which is
> done in build_over_call. */
> - default_arg = TREE_PURPOSE (arg_types);
> + tree default_arg = TREE_PURPOSE (arg_types);
>
> /* Except that we do substitute default arguments under tsubst_lambda_expr,
> since the new op() won't have any associated template arguments for us
> @@ -15139,20 +15130,34 @@ tsubst_arg_types (tree arg_types,
> default_arg = tsubst_copy_and_build (default_arg, args, complain,
> in_decl,
> false/*fn*/, false/*constexpr*/);
>
> + tree remaining_arg_types = tsubst_arg_types (TREE_CHAIN (arg_types),
> + args, end, complain, in_decl);
> + if (remaining_arg_types == error_mark_node)
> + return error_mark_node;
> +
> + for (int i = len-1; i >= 0; i--)
> + {
> + if (expanded_args)
> + type = TREE_VEC_ELT (expanded_args, i);
> +
> + /* Do array-to-pointer, function-to-pointer conversion, and ignore
> + top-level qualifiers as required. */
> + type = cv_unqualified (type_decays_to (type));
> +
> if (default_arg && TREE_CODE (default_arg) == DEFERRED_PARSE)
> {
> /* We've instantiated a template before its default arguments
> have been parsed. This can happen for a nested template
> class, and is not an error unless we require the default
> argument in a call of this function. */
> - remaining_arg_types =
> - tree_cons (default_arg, type, remaining_arg_types);
> + remaining_arg_types
> + = tree_cons (default_arg, type, remaining_arg_types);
> vec_safe_push (DEFPARSE_INSTANTIATIONS (default_arg),
> remaining_arg_types);
> }
> else
> - remaining_arg_types =
> - hash_tree_cons (default_arg, type, remaining_arg_types);
> + remaining_arg_types
> + = hash_tree_cons (default_arg, type, remaining_arg_types);
> }
>
> return remaining_arg_types;
> diff --git a/gcc/testsuite/g++.dg/template/sfinae-dr1227.C
> b/gcc/testsuite/g++.dg/template/sfinae-dr1227.C
> new file mode 100644
> index 00000000000..821ff0313b6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/sfinae-dr1227.C
> @@ -0,0 +1,23 @@
> +// PR c++/96560
> +// DR 1227
> +// Test that we substitute function parameter types in lexical order.
> +
> +template <class T>
> +struct A { typedef typename T::type type; }; // { dg-error "void" }
> +
> +template <class T> void f(typename T::type, typename A<T>::type);
> +template <class T> long f(...);
> +
> +long x = f<int>(0, 0); // { dg-bogus "" } OK
> +
> +
> +template <class T> void g(T, typename A<T>::type);
> +template <class T> long g(...);
> +
> +long y = g<void>(0, 0); // { dg-bogus "" } OK
> +
> +
> +template <class T> void h(typename A<T>::type, T);
> +template <class T> long h(...);
> +
> +long z = h<void>(0, 0); // { dg-message "required from here" } hard error
> --
> 2.31.1.362.g311531c9de
>
