[PING^2] [PR46003, C++] Fix the assertion failure in build_target_expr

[Yufeng Zhang]

Thank you very much for the reply.

On 02/26/2011 05:38 PM, Jason Merrill wrote:
> The problem seems to be with the calls to build_cplus_new in
> convert_like_real, which seem to be redundant; we should have already
> called from build_cxx_call when we built up the call in the first place
> What happens if we just remove all those calls?

I tried to remove one of calls to build_cplus_new in convert_like_real,
the one just after the call to build_temp. For the following test code:

struct A
{
  A(int);
};

template<int> void foo(A x)
{ 0 ? x : 0; }

the compiler would complain (for both arm and x86 targets):

test.cpp: In function 'void foo(A)':
test.cpp:7:11: error: no match for ternary 'operator?:' in '(bool)0 ? x
: 0u->A::A(A(0))'


I looked into it a bit further. It seems that without the call to
build_cplus_new, a CALL_EXPR node rather than a TARGET_EXPR node is
returned to convert_like_real's caller build_conditional_expr, and
then this CALL_EXPR node is used as the converted 3rd operand for the
conditional expression. I don't think this is right, since the node is
just an overloaded constructor call.

The calls to build_cplus_new in convert_like_real don't seem to be
redundant. build_cxx_call won't be called, e.g. from build_over_call,
when processing_template_decl is non-zero. Furthermore, build_cxx_call
calls build_cplus_new only if MAYBE_CLASS_TYPE_P holds of the function
return type, but this is not the case in constructor calls.

Also the comment above build_special_member_call says:

   If NAME indicates a complete object constructor, INSTANCE may be
   NULL_TREE.  In this case, the caller will call build_cplus_new to
   store the newly constructed object into a VAR_DECL.

since build_temp calls build_special_member_call without calling
build_cplus_new, it seems reasonable to me that build_cplus_new should
be called after the call to build_temp in convert_like_real.

> We shouldn't be calling build_aggr_init_expr for such a CALL_EXPR.

Can you please provide a bit more details about the reason? I don't really
understand why, even after having spent some time further understanding
the related code.

The internal doc says: "An AGGR_INIT_EXPR represents the initialization
as the return value of a function call, or as the result of a
constructor. ..." It doesn't seem to breach the concept by calling
build_aggr_init_expr for a pre-overload-resolution CALL_EXPR.


Moreover, I'm thinking to further constrain the proposed change by also
considering processing_template_decl, since the problem only happens
when a template declaration is being processed. This should make the
patch more conservative and less possible in causing troubles.

Please let me know how you think about it. If you would like more analysis
or other experiments to be done, please do let me know as well.


Regards,
Yufeng


2011-03-02  Yufeng Zhang  <yufeng.zhang@arm.com>

        PR c++/46003
        * tree.c (build_aggr_init_expr): Also consider overloaded ctors
        when determining is_ctor.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 1a1f150..ae33e58 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -391,9 +391,12 @@ build_aggr_init_expr (tree type, tree init)
   else
     return convert (type, init);
 
-  is_ctor = (TREE_CODE (fn) == ADDR_EXPR
-	     && TREE_CODE (TREE_OPERAND (fn, 0)) == FUNCTION_DECL
-	     && DECL_CONSTRUCTOR_P (TREE_OPERAND (fn, 0)));
+  is_ctor = ((TREE_CODE (fn) == ADDR_EXPR
+	      && TREE_CODE (TREE_OPERAND (fn, 0)) == FUNCTION_DECL
+	      && DECL_CONSTRUCTOR_P (TREE_OPERAND (fn, 0)))
+	     || (processing_template_decl
+		 && is_overloaded_fn (fn)
+		 && DECL_CONSTRUCTOR_P (get_first_fn (fn))));
 
   /* We split the CALL_EXPR into its function and its arguments here.
      Then, in expand_expr, we put them back together.  The reason for