Re: [C++ Patch] [PR c++/88146] do not crash synthesizing inherited ctor(...)

[Alexandre Oliva <aoliva@redhat.com>]

On Dec 20, 2018, Jason Merrill <jason@redhat.com> wrote:

> I think the bug is in calling instantiate_constexpr_fns in this case.
> I think that should only happen when ctx->manifestly_const_eval.

FTR, changing that breaks cpp2a/constexpr-init1.C (P0859),
unfortunately.  I don't see why the operand of decltype or of int{}
would be manifestly const eval.  Plus, I don't quite follow why the two
cases covered in the constexpr-init1.C testcase have different
consequences, constexpr-wise, even after rereading the verbiage about it
in the standard development tree a few times; I guess I still need to
fill in other gaps to in my knowledge to try and make sense of it.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index cea414d33def..88bee7aa1fed 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5076,7 +5076,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
 	r = TARGET_EXPR_INITIAL (r);
     }
 
-  instantiate_constexpr_fns (r);
+  if (ctx.manifestly_const_eval)
+    instantiate_constexpr_fns (r);
   r = cxx_eval_constant_expression (&ctx, r,
 				    false, &non_constant_p, &overflow_p);
 



But, really, should varying but user-invisible cdtor return types really
ever play a role in determining whether a program is well-formed, like
they do ATM?  I mean, is my suggested change wrong or undesirable for
some reason I can't grasp, aside from its incorrect implementation,
using operand0 of CALL_EXPR instead of operand0 of the ADDR_EXPR that
will be operand1 of CALL_EXPR if it's not NULL?

diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index f758f2d9bc8f..9b3b4d039a16 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -1118,7 +1118,8 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
         error_at (loc, "pseudo-destructor is not called");
       return error_mark_node;
     }
-  if (VOID_TYPE_P (TREE_TYPE (expr)))
+  if (VOID_TYPE_P (TREE_TYPE (expr))
+      && TREE_CODE (expr) != CALL_EXPR)
     return expr;
   switch (TREE_CODE (expr))
     {
@@ -1175,6 +1176,26 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
       break;
 
     case CALL_EXPR:   /* We have a special meaning for volatile void fn().  */
+      /* cdtors may return this or void, depending on
+	 targetm.cxx.cdtor_returns_this, but this shouldn't affect our
+	 decisions here: nodiscard cdtors are nonsensical, and we
+	 don't want to call maybe_warn_nodiscard because it may
+	 trigger constexpr or template instantiation in a way that
+	 changes their instantiaton nesting.  This changes the way
+	 contexts are printed in diagnostics, with bad consequences
+	 for the testsuite, but there may be other undesirable
+	 consequences of visiting referenced ctors too soon.  */
+      if (TREE_OPERAND (expr, 1)
+	  && TREE_CODE (TREE_OPERAND (expr, 1)) == ADDR_EXPR
+	  && DECL_P (TREE_OPERAND (TREE_OPERAND (expr, 1), 0))
+	  && IDENTIFIER_CDTOR_P (DECL_NAME (TREE_OPERAND (TREE_OPERAND (expr, 1), 0))))
+	return expr;
+      /* FIXME: Move this test before the one above, after a round of
+	 testing as it is, to get coverage of the behavior we'd get on
+	 ARM.  */
+      else if (VOID_TYPE_P (TREE_TYPE (expr)))
+	return expr;
+
       maybe_warn_nodiscard (expr, implicit);
       break;
 


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe