Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]

[Jason Merrill <jason@redhat.com>]

On 3/22/20 9:21 PM, Patrick Palka wrote:
> This patch relaxes an assertion in tsubst_default_argument that exposes a latent
> bug in how we substitute an array type into a cv-qualified wildcard function
> parameter type.  Concretely, the latent bug is that given the function template
> 
>    template<typename T> void foo(const T t);
> 
> one would expect the type of foo<int[]> to be void(const int*), but we
> (seemingly prematurely) strip function parameter types of their top-level
> cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end up
> obtaining void(int*) as the type of foo<int[]> after substitution and decaying.
> 
> We still however correctly substitute into and decay the formal parameter type,
> obtaining const int* as the type of t after substitution.  But this then leads
> to us tripping over the assert in tsubst_default_argument that verifies the
> formal parameter type and the function type are consistent.
> 
> Assuming it's too late at this stage to fix the substitution bug, we can still
> relax the assertion like so.  Tested on x86_64-pc-linux-gnu, does this look OK?

This is core issues 1001/1322, which have not been resolved.  Clang does 
the substitution the way you suggest; EDG rejects the testcase because 
the two substitutions produce different results.  I think it would make 
sense to follow the EDG behavior until this issue is actually resolved.

> gcc/cp/ChangeLog:
> 
> 	PR c++/92010
> 	* pt.c (tsubst_default_argument): Relax assertion to permit
> 	disagreements between the function type and the parameter type
> 	about the cv-qualification of the pointed-to type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/92010
> 	* g++.dg/template/defarg22.C: New test.
> ---
>   gcc/cp/pt.c                              | 17 ++++++++++++++++-
>   gcc/testsuite/g++.dg/template/defarg22.C | 11 +++++++++++
>   2 files changed, 27 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 9e1eb9416c9..923166276b8 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -13337,7 +13337,22 @@ tsubst_default_argument (tree fn, int parmnum, tree type, tree arg,
>     if (parmtype == error_mark_node)
>       return error_mark_node;
>   
> -  gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
> +  if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
> +    ;
> +  else if (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype))
> +    {
> +      /* The function type and the parameter type can disagree about the
> +	 cv-qualification of the pointed-to type; see PR92010.  */
> +      gcc_assert (same_type_p (TREE_TYPE (type),
> +			       strip_top_quals (TREE_TYPE (parmtype))));
> +      /* Verify that this happens only when the dependent parameter type is a
> +	 cv-qualified wildcard type.  */
> +      tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
> +      gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
> +		  && cv_qualified_p (TREE_TYPE (pattern_parm)));
> +    }
> +  else
> +    gcc_unreachable ();
>   
>     tree *slot;
>     if (defarg_inst && (slot = defarg_inst->get (parm)))
> diff --git a/gcc/testsuite/g++.dg/template/defarg22.C b/gcc/testsuite/g++.dg/template/defarg22.C
> new file mode 100644
> index 00000000000..cf6261916d8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/defarg22.C
> @@ -0,0 +1,11 @@
> +// PR c++/92010
> +// { dg-do compile }
> +
> +template <typename T>
> +void foo(const T t = 0)
> +{ }
> +
> +int main()
> +{
> +  foo<char[]>();
> +}
>