| Message ID | 000401cbd209$c85e6840$591b38c0$@Zhang@arm.com |
|---|---|
| State | New |
| Headers | show |
On 2/21/2011 12:56 PM, Yufeng Zhang wrote: > http://gcc.gnu.org/ml/gcc-patches/2011-02/msg00689.html > Any comment will be appreciated. I would think that when we call the constructor on ARM we should cast the result to "void". Then we would presumably not have to change build_aggr_init_expr. Does that work?
On 02/21/2011 03:56 PM, Yufeng Zhang wrote: > The patch tries to patch cp/tree.c:build_aggr_init_expr(). > build_aggr_init_expr() wraps a ctor-calling CALL_EXPR tree node with an > AGGR_INIT_EXPR tree node, whose expression type is always VOID_TYPE. When > it determines whether the routine being called is a ctor or not, it only > looks into simple cases, but not the case of overloaded ctors in a tree > of COMPONENT_REF (INDIRECT_REF (something, BASELINK (OVERLOAD))). This > patch expands the is_ctor determining logic. I don't understand how we can get here without having already done overload resolution. At this point we should have a simple function pointer. Jason
On 02/23/2011 00:14 AM, Jason Merrill wrote: >> build_aggr_init_expr() wraps a ctor-calling CALL_EXPR tree node >> with an AGGR_INIT_EXPR tree node, whose expression type is always >> VOID_TYPE. When it determines whether the routine being called is >> a ctor or not, it only looks into simple cases, but not the case >> of overloaded ctors in a tree of >> COMPONENT_REF (INDIRECT_REF (something, BASELINK (OVERLOAD))). >> This patch expands the is_ctor determining logic. > > I don't understand how we can get here without having already done > overload resolution. At this point we should have a simple function > pointer. Good point. I looked into the issue a bit further and here is some more observation. Given the following code: struct A { A(int); }; template<int> void foo(A x) { 0 ? x : 0; } for the conditional expression in the function template, the following user-defined conversion sequence is determined in order to get the type of the 3rd operand converted to that of the 2nd one: ck_identity ck_user ck_rvalue int -----------> int -----------> struct A -----------> struct A A::A(int) A::A(const A&) Note that above the arrows are annotated with conversion_kind and below the arrows are the functions involved in the conversions. When cp/call.c:convert_like_real() performs the conversion sequence, something interesting happens during the last step, i.e. ck_rvalue. Here is the backtrace at that point: #0 build_new_method_call #1 build_special_member_call #2 build_temp #3 convert_like_real #4 build_conditional_expr #5 build_x_conditional_expr ... I can see that build_new_method_call does the overload resolution correctly, selects the compiler-generated copy constructor A::A(const A&), and builds up a CALL_EXPR node with a simple function pointer to A::A(const A&), i.e. CALL_EXPR (ADDR_EXPR (FUNCTION_DECL (A::A(const A&)))) _However_, just before returning this CALL_EXPR node, build_new_method_call has some special handling of situations if the parser is in the middle of template declaration processing, i.e. when processing_template_dec != 0 I don't fully understand that piece of code, but it seems related with the handling of non-dependent expressions (in order to make template instantiation more like ordinary parsing?). If the parser is still processing a template declaration, just as in our case, build_new_method_call reverts to use the original set of candidate functions to construct and return the following node: CALL_EXPR (CONPONENT_REF (INDIRECT_REF ((A*)0), BASELINK (OVERLOAD))) Eventually this CALL_EXPR node is returned to convert_like_real, which calls build_cplus_new to declare and initialize a temporary variable using this node. When build_target_expr is further called inside build_cplus_new, the assertion failure occurs due to the 'incompatible' types, as the temporary variable is a RECORD_TYPE of A and the CALL_EXPR has a type of POINTER_TYPE to A. So this is how we get into build_target_expr without the overload resolution performed. Is it an expected behaviour? Thanks, Yufeng
On 2/22/2011 03:59 AM, Mark Mitchell wrote: > >> http://gcc.gnu.org/ml/gcc-patches/2011-02/msg00689.html > >> Any comment will be appreciated. > > I would think that when we call the constructor on ARM we should cast > the result to "void". Then we would presumably not have to change > build_aggr_init_expr. Does that work? Do you mean casting the result of _all_ constructor calls on ARM to "void" or just for that particular case? I'm not sure whether this works or whether this will introduce any inconsistency. I wonder if there is any particular concern about changing build_aggr_init_expr, or any reason that it's not necessary for the 'is_ctor' to consider overloaded constructor calls. Many thanks, Yufeng
On 02/25/2011 11:12 AM, Yufeng Zhang wrote: > So this is how we get into build_target_expr without the overload > resolution performed. Is it an expected behaviour? Thanks for the analysis. We shouldn't be calling build_aggr_init_expr for such a CALL_EXPR. 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? Jason
Index: gcc/cp/tree.c =================================================================== --- gcc/cp/tree.c (revision 169974) +++ gcc/cp/tree.c (working copy) @@ -391,9 +391,11 @@ 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))) + || ((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
Hi, This patch fixes g++.dg/template/cond5.C failure with arm-eabi; it's a fix in the C++ frontend. http://gcc.gnu.org/ml/gcc-patches/2011-02/msg00689.html http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01168.html Any comment will be appreciated. Thanks, Yufeng -----Original Message----- The test ./g++.dg/template/cond5.C fails for testing with arm-eabi with an assertion failure, which occurs in cp/tree.c:build_target_expr(). (Please see the bugzilla page for more information on the analysis: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46003) Note that the assertion does not fail in x86-targeted (or any of many other targets targeted) compiler, because of the difference in the ARM C ++ ABI on constructor return values. The ARM C++ ABI requires C1 and C2 constructors to return this (instead of being void functions). The patch tries to patch cp/tree.c:build_aggr_init_expr(). build_aggr_init_expr() wraps a ctor-calling CALL_EXPR tree node with an AGGR_INIT_EXPR tree node, whose expression type is always VOID_TYPE. When it determines whether the routine being called is a ctor or not, it only looks into simple cases, but not the case of overloaded ctors in a tree of COMPONENT_REF (INDIRECT_REF (something, BASELINK (OVERLOAD))). This patch expands the is_ctor determining logic. The test with arm-eabi before and after the patch shows that the previously failing test (cond5.C) now passes, and all the other tests are unaffected. There is no change in the x86 test result. OK for the trunk? Thanks, Yufeng 2011-02-10 Yufeng Zhang <yufeng.zhang@arm.com> PR c++/46003 * tree.c (build_aggr_init_expr): Also consider overloaded ctors when determining is_ctor.