Re: [PATCH] [PR85569] skip constexpr target_expr constructor dummy type conversion

[Jason Merrill <jason@redhat.com>]

On 11/22/18 6:39 PM, Alexandre Oliva wrote:
> The testcase is the work-around testcase for the PR; even that had
> started failing.  The problem was that, when unqualifying the type of
> a TARGET_EXPR, we'd create a variant of the type, then request the
> conversion of the TARGET_EXPR_INITIAL to that variant type.  Though
> the types are different pointer-wise, they're the same_type_p, so the
> resulting modified expr compares cp_tree_equal to the original, which
> maybe_constant_value flags as an error.  There's no reason to
> construct an alternate TARGET_EXPR or CONSTRUCTOR just because of an
> equivalent type, except for another spot that expected pointer
> equality that would no longer be satisfied.  Without relaxing the
> assert in constexpr_call_hasher::equal, g++.robertl/eb73.C would
> trigger an assertion failure.
> 
> Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?
> 
> 
> for  gcc/cp/ChangeLog
> 
> 	PR c++/85569
> 	* constexpr.c (adjust_temp_type): Test for type equality with
> 	same_type_p.
> 
> for  gcc/testsuite
> 
> 	PR c++/85569
> 	* g++.dg/cpp1z/pr85569.C: New.
> ---
>   gcc/cp/constexpr.c                   |    4 +
>   gcc/testsuite/g++.dg/cpp1z/pr85569.C |   93 ++++++++++++++++++++++++++++++++++
>   2 files changed, 95 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr85569.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 92fd2b2d9d59..bb5d1301b332 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -1060,7 +1060,7 @@ constexpr_call_hasher::equal (constexpr_call *lhs, constexpr_call *rhs)
>       {
>         tree lhs_arg = TREE_VALUE (lhs_bindings);
>         tree rhs_arg = TREE_VALUE (rhs_bindings);
> -      gcc_assert (TREE_TYPE (lhs_arg) == TREE_TYPE (rhs_arg));
> +      gcc_assert (same_type_p (TREE_TYPE (lhs_arg), TREE_TYPE (rhs_arg)));
>         if (!cp_tree_equal (lhs_arg, rhs_arg))
>           return false;
>         lhs_bindings = TREE_CHAIN (lhs_bindings);
> @@ -1276,7 +1276,7 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
>   static tree
>   adjust_temp_type (tree type, tree temp)
>   {
> -  if (TREE_TYPE (temp) == type)
> +  if (TREE_TYPE (temp) == type || same_type_p (TREE_TYPE (temp), type))
>       return temp;
>     /* Avoid wrapping an aggregate value in a NOP_EXPR.  */

Hmm, I'm a bit uneasy about this change, but it does make sense to 
follow cp_tree_equal.

Let's replace the == comparison rather than supplement it.  OK with that 
change.

Jason