| Message ID | 4EA8879F.40906@oracle.com |
|---|---|
| State | New |
| Headers | show |
... forgot the testcase, sorry. Paolo. //////////////////////
On 10/26/2011 06:20 PM, Paolo Carlini wrote: > I re-opened this one because: 1- We may want to fix it in 4_6-branch > too, it's a regression there too; 2- We are still handling incorrectly > the template impl case. For the latter a variant of my old idea still > works, fwiw. > - object_type = TREE_TYPE (object); > + object_type = (TREE_CODE (object) == ARROW_EXPR > + ? TREE_OPERAND (object, 0) : TREE_TYPE (object)); This is still wrong. Why not use the same patch for 4.6 as for trunk? Jason
On 10/27/2011 12:48 AM, Jason Merrill wrote: > On 10/26/2011 06:20 PM, Paolo Carlini wrote: >> I re-opened this one because: 1- We may want to fix it in 4_6-branch >> too, it's a regression there too; 2- We are still handling incorrectly >> the template impl case. For the latter a variant of my old idea still >> works, fwiw. > >> - object_type = TREE_TYPE (object); >> + object_type = (TREE_CODE (object) == ARROW_EXPR >> + ? TREE_OPERAND (object, 0) : TREE_TYPE (object)); > This is still wrong. Why not use the same patch for 4.6 as for trunk? I don't understand, sorry: both mainline and 4_6-branch do not handle correctly both testcases, template impl and not. If you want to look into it personally, I can unassign myself, no problem, really. Paolo.
.. maybe my message wasn't clear, sorry, I'm a bit tired (here it's late): I meant to say that the non_reference tweak fixes the non-template impl class case, but something more is needed for a template impl (thus the new testcase). And, additionally, this issue is a [4.6/4.7 Regression], thus, post 4.6.2, we may be interested in back porting something. Let me know if you need additional details, or whatelse. Or we want me to look into a different way to attack the template case. Paolo.
On 10/26/2011 07:00 PM, Paolo Carlini wrote: > .. maybe my message wasn't clear, sorry, I'm a bit tired (here it's > late): I meant to say that the non_reference tweak fixes the > non-template impl class case, but something more is needed for a > template impl (thus the new testcase). And, additionally, this issue is > a [4.6/4.7 Regression], thus, post 4.6.2, we may be interested in back > porting something. Ah. > Let me know if you need additional details, or what else. How is the template case failing? > Or we want me to look into a different way to attack the template case. Please. Jason
Hi, > > Let me know if you need additional details, or what else. > > How is the template case failing? Some preliminary notes, maybe you can already make some sense out of this, more later, if you like: Thus we have this testcase: template <class V> struct impl { template <class T> static T create(); }; template <class T, class U, class V, class = decltype(impl<V>::template create<T>() -> impl<V>::template create<U>())> struct tester { }; tester<impl<float>*, int, float> ti; ////////////////// We are in tsubst_copy_and_build, COMPONENT_REF case. Argument t is: <component_ref 0x7ffff57810e0 arg 0 <arrow_expr 0x7ffff5769f00 arg 0 <call_expr 0x7ffff57810a8 fn <scope_ref 0x7ffff563db40 tree_0 tree_1 arg 0 <record_type 0x7ffff577f540 impl> arg 1 <template_id_expr 0x7ffff563da80 arg 0 <identifier_node 0x7ffff577e790 create bindings <(nil)> local bindings <(nil)>> arg 1 <tree_vec 0x7ffff5769e38 elt 0 <template_type_parm 0x7ffff577f348 T>>>>>> arg 1 <scope_ref 0x7ffff563db70 tree_0 tree_1 arg 0 <record_type 0x7ffff577f540 impl type_0 type_5 type_6 VOID align 8 symtab 0 alias set -1 canonical type 0x7ffff577f540 context <translation_unit_decl 0x7ffff5646170 D.1> full-name "struct impl<V>" no-binfo use_template=1 interface-unknown chain <type_decl 0x7ffff5780170 impl>> arg 1 <template_id_expr 0x7ffff563dae0 arg 0 <identifier_node 0x7ffff577e790 create> arg 1 <tree_vec 0x7ffff5769ed8 elt 0 <template_type_parm 0x7ffff577f3f0 U>>>>> Everything begins like the non-template case: member is computed, we get to: else if (TREE_CODE (member) == SCOPE_REF && TREE_CODE (TREE_OPERAND (member, 1)) == TEMPLATE_ID_EXPR) in here we have a call of lookup_qualified_name (TREE_OPERAND (member, 0), tmpl, ... which, in this case, fails, returns error_mark_node (Seg Fault immediately after). When lookup_qualified_name is called, this is member: <scope_ref 0x7ffff563dd20 tree_0 tree_1 arg 0 <record_type 0x7ffff577f9d8 impl type_0 type_5 type_6 VOID align 8 symtab 0 alias set -1 canonical type 0x7ffff577f9d8 context <translation_unit_decl 0x7ffff5646170 D.1> full-name "struct impl<V>" no-binfo use_template=1 interface-unknown chain <type_decl 0x7ffff57802e0 impl>> arg 1 <template_id_expr 0x7ffff563dcf0 type <lang_type 0x7ffff5765888 unknown type type <lang_type 0x7ffff5765888 unknown type> VOID align 1 symtab 0 alias set -1 canonical type 0x7ffff5765888 pointer_to_this <lang_type 0x7ffff5765888 unknown type> reference_to_this <lang_type 0x7ffff5765888 unknown type>> arg 0 <identifier_node 0x7ffff577e790 create bindings <(nil)> local bindings <(nil)>> arg 1 <tree_vec 0x7ffff5783140 elt 0 <template_type_parm 0x7ffff577f7e0 U>>>> and this is tmpl: <identifier_node 0x7ffff577e790 create bindings <(nil)> local bindings <(nil)>> For comparison, in the non-template case, path inside COMPONENT_REF is rather different after member: none of the if branches is executed, we just get to return finish_class_member_access_expr at the end of case COMPONENT_REF. Thanks! Paolo.
Looks like > qualified_name_lookup_error (object_type, tmpl, member, > input_location); is passing the wrong first argument; the scope argument should be the same as we passed to lookup_qualified_name. Jason
Hi, > Looks like > >> qualified_name_lookup_error (object_type, tmpl, member, >> input_location); > > is passing the wrong first argument; the scope argument should be the same as we passed to lookup_qualified_name. Ok, thanks, I'll work more on this later today. Then, if we fix that we don't error out at all, right? Because this is a reject valid, AFAIK. Paolo
On 10/27/2011 08:57 AM, Paolo Carlini wrote:
> Ok, thanks, I'll work more on this later today. Then, if we fix that we don't error out at all, right? Because this is a reject valid, AFAIK.
In that case, lookup_qualified_name shouldn't be returning error_mark_node.
Jason
Hi > On 10/27/2011 08:57 AM, Paolo Carlini wrote: >> Ok, thanks, I'll work more on this later today. Then, if we fix that we don't error out at all, right? Because this is a reject valid, AFAIK. > > In that case, lookup_qualified_name shouldn't be returning error_mark_node. Yes, I suspected that. I'll try to look a bit into it but I don't make promises, if you consider the issue urgent, definitely don't wait on me. Thanks again, Paolo
Hi again, >> On 10/27/2011 08:57 AM, Paolo Carlini wrote: >>> Ok, thanks, I'll work more on this later today. Then, if we fix that we don't error out at all, right? Because this is a reject valid, AFAIK. >> In that case, lookup_qualified_name shouldn't be returning error_mark_node. A little more information, maybe more to come today: lookup_qualified_name calls lookup_member which actually gives up *very* early, because its first argument, xbasetype, aka TREE_OPERAND (member, 0) in tsubst_copy_and_build, has no binfo: <record_type 0x7ffff577f9d8 impl type_0 type_5 type_6 VOID align 8 symtab 0 alias set -1 canonical type 0x7ffff577f9d8 context <translation_unit_decl 0x7ffff5646170 D.1> full-name "struct impl<V>" no-binfo use_template=1 interface-unknown chain <type_decl 0x7ffff57802e0 impl>> thus (type is xbasetype completed): 1192 if (!basetype_path) 1193 basetype_path = TYPE_BINFO (type); 1194 1195 if (!basetype_path) 1196 return NULL_TREE; NULL_TREE becomes error_mark_node in lookup_qualified_name. For comparison, in 4_5-branch we have, for xbasetype, a very different story: <record_type 0x7ffff5a15690 impl type_5 type_6 QI size <integer_cst 0x7ffff7e6d730 type <integer_type 0x7ffff7e820a8 bit_size_type> constant 8> unit size <integer_cst 0x7ffff7e6d758 type <integer_type 0x7ffff7e82000 long unsigned int> constant 1> align 8 symtab 0 alias set -1 canonical type 0x7ffff5a15690 fields <type_decl 0x7ffff5a17170 impl type <record_type 0x7ffff5a157e0 impl type_5 type_6 QI size <integer_cst 0x7ffff7e6d730 8> unit size <integer_cst 0x7ffff7e6d758 1> align 8 symtab 0 alias set -1 canonical type 0x7ffff5a15690 fields <type_decl 0x7ffff5a17170 impl> full-name "struct impl<float>" X() X(constX&) this=(X&) n_parents=0 use_template=1 interface-unknown chain <type_decl 0x7ffff5a170b8 impl>> external nonlocal suppress-debug decl_4 VOID file 50870.C line 6 col 3 align 8 context <record_type 0x7ffff5a15690 impl> result <record_type 0x7ffff5a15690 impl> > full-name "struct impl<float>" X() X(constX&) this=(X&) n_parents=0 use_template=1 interface-unknown pointer_to_this <pointer_type 0x7ffff5a15738> chain <type_decl 0x7ffff5a170b8 impl>> Paolo.
.. thus now I'm investigating, comparing 4.5 to mainline, where the
former gets the TYPE_BINFO of the TREE_OPERAND (member, 0). And the
answer is "simple": at the beginning of tsubst_copy_and_build case
COMPONENT_REF itself! Where:
member = TREE_OPERAND (t, 1);
if (BASELINK_P (member))
member = tsubst_baselink (member,
non_reference (TREE_TYPE (object)),
args, complain, in_decl);
else
member = tsubst_copy (member, args, complain, in_decl);
the tsubst_copy call changes very little member in mainline. From this:
<scope_ref 0x7ffff563db70 tree_0 tree_1
arg 0 <record_type 0x7ffff577f540 impl type_0 type_5 type_6 VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff577f540
context <translation_unit_decl 0x7ffff5646170 D.1>
full-name "struct impl<V>"
no-binfo use_template=1 interface-unknown
chain <type_decl 0x7ffff5780170 impl>>
arg 1 <template_id_expr 0x7ffff563dae0
arg 0 <identifier_node 0x7ffff577e790 create
bindings <(nil)>
local bindings <(nil)>>
arg 1 <tree_vec 0x7ffff5769ed8 elt 0 <template_type_parm
0x7ffff577f3f0 U>>>>
to this:
<scope_ref 0x7ffff563dd20 tree_0 tree_1
arg 0 <record_type 0x7ffff577f9d8 impl type_0 type_5 type_6 VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff577f9d8
context <translation_unit_decl 0x7ffff5646170 D.1>
full-name "struct impl<V>"
no-binfo use_template=1 interface-unknown
chain <type_decl 0x7ffff57802e0 impl>>
arg 1 <template_id_expr 0x7ffff563dcf0
type <lang_type 0x7ffff5765888 unknown type type <lang_type
0x7ffff5765888 unknown type>
VOID
align 1 symtab 0 alias set -1 canonical type 0x7ffff5765888
pointer_to_this <lang_type 0x7ffff5765888 unknown type>
reference_to_this <lang_type 0x7ffff5765888 unknown type>>
arg 0 <identifier_node 0x7ffff577e790 create
bindings <(nil)>
local bindings <(nil)>>
arg 1 <tree_vec 0x7ffff5783140 elt 0 <template_type_parm
0x7ffff577f7e0 U>>>>
note: no changes for arg0, no-binfo.
Instead, in the 4.5 case, from:
<scope_ref 0x7ffff7f8d188 tree_0
arg 0 <record_type 0x7ffff5a15348 impl type_0 type_5 type_6 VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff5a15348
full-name "struct impl<V>"
no-binfo use_template=1 interface-unknown
chain <type_decl 0x7ffff5a01c38 impl>>
arg 1 <template_id_expr 0x7ffff7f8d0e0
arg 0 <identifier_node 0x7ffff5a11840 create
bindings <(nil)>
local bindings <(nil)>>
arg 1 <tree_vec 0x7ffff5a164d8
elt 0 <template_type_parm 0x7ffff5a151f8 U>>>>
To:
<scope_ref 0x7ffff7f8d268 tree_0
arg 0 <record_type 0x7ffff5a15690 impl type_5 type_6 QI
size <integer_cst 0x7ffff7e6d730 constant 8>
unit size <integer_cst 0x7ffff7e6d758 constant 1>
align 8 symtab 0 alias set -1 canonical type 0x7ffff5a15690
fields <type_decl 0x7ffff5a17170 impl type <record_type
0x7ffff5a157e0 impl>
external nonlocal suppress-debug decl_4 VOID file 50870.C
line 6 col 3
align 8 context <record_type 0x7ffff5a15690 impl> result
<record_type 0x7ffff5a15690 impl>
>
full-name "struct impl<float>"
X() X(constX&) this=(X&) n_parents=0 use_template=1
interface-unknown
pointer_to_this <pointer_type 0x7ffff5a15738> chain <type_decl
0x7ffff5a170b8 impl>>
arg 1 <template_id_expr 0x7ffff7f8d230
type <lang_type 0x7ffff59f8bd0 unknown type type <lang_type
0x7ffff59f8bd0 unknown type>
VOID
align 1 symtab 0 alias set -1 canonical type 0x7ffff59f8bd0
pointer_to_this <lang_type 0x7ffff59f8bd0 unknown type>
reference_to_this <lang_type 0x7ffff59f8bd0 unknown type>>
arg 0 <identifier_node 0x7ffff5a11840 create
bindings <(nil)>
local bindings <(nil)>>
arg 1 <tree_vec 0x7ffff5a16b18
elt 0 <integer_type 0x7ffff7e82498 int>>>>
arg0 gets its fields. Now, a big difference between the two - 4.5 vs
mainline - tsubst_copy calls, is the args argument.
In 4.5, a very simple and understandable:
<tree_vec 0x7ffff59f67c0
elt 0 <pointer_type 0x7ffff5a15738>
elt 1 <integer_type 0x7ffff7e82498 int>
elt 2 <real_type 0x7ffff7e96150 float>>
in mainline:
<tree_vec 0x7ffff5758fc0
elt 0 <template_type_parm 0x7ffff577f738 T VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff577f738
index 0 level 1 orig_level 1
chain <type_decl 0x7ffff576df18 T>>
elt 1 <template_type_parm 0x7ffff577f7e0 U VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff577f7e0
index 1 level 1 orig_level 1
chain <type_decl 0x7ffff5780000 U>>
elt 2 <template_type_parm 0x7ffff577f888 V type_0 type_6 VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff577f888
index 2 level 1 orig_level 1
chain <type_decl 0x7ffff57800b8 V>>
elt 3 <template_type_parm 0x7ffff577f930 VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff577f930
index 3 level 1 orig_level 1
chain <type_decl 0x7ffff5780228 D.1853>>>
args comes directly from the tsubst_copy_and_build arguments, looks like
we reach case COMPONENT_REF with the template arguments still
unsubstituted, something is simply not done earlier in mainline.
I guess I had better stopping here in terms of details on the mailing
list today. If you have some further hints...
Thanks!
Paolo.
.. maybe one final piece of information for today. I'm trying to figure out where the very different args argument is coming from. Both in 4.5 and in mainline we have the same final chain of tusbst_copy_build, preceded by tsubst_copy_and_build, tusbst_expr, tsubt, tsubst_template_arg, the args argument remains the same all the way. Earlier than that things are different: in mainline the same args, as arglist, comes from fixup_template_parm, and earlier we have fixup_template_parms which creates the arglist itself, even earlier the parser. In 4.5 before tsubst_template_arg we have coerce_template_parms, which changes its args argument to the final value, and before that lookup_template_class, finish_template_type, the parser. Paolo.
On 10/27/2011 08:22 PM, Paolo Carlini wrote: > I'm trying to figure out where the very different args argument is > coming from. > Earlier than that things are different: in mainline the same args, as > arglist, comes from fixup_template_parm, and earlier we have > fixup_template_parms which creates the arglist itself Right, this is new. I guess the COMPONENT_REF code needs to be fixed to handle partial instantiation. Jason
On 10/28/2011 06:07 PM, Jason Merrill wrote: > On 10/27/2011 08:22 PM, Paolo Carlini wrote: >> I'm trying to figure out where the very different args argument is >> coming from. > >> Earlier than that things are different: in mainline the same args, as >> arglist, comes from fixup_template_parm, and earlier we have >> fixup_template_parms which creates the arglist itself > > Right, this is new. I guess the COMPONENT_REF code needs to be fixed > to handle partial instantiation. I see. Something I didn't tell you yesterday, is that, in 4_5-branch, tsubst_template_arg is called like this, by coerce_template_parms: /* There must be a default arg in this case. */ arg = tsubst_template_arg (TREE_PURPOSE (parm), new_args, complain, in_decl); which, what can I say, looks right ;) By the way, your hint that probably we are also passing a wrong first argument to qualified_name_lookup_error may be useful for the other issue, the ice-on-invalid, which I was trying to fix in the parser ;) Let me test something... Paolo.
Index: pt.c =================================================================== --- pt.c (revision 180532) +++ pt.c (working copy) @@ -13706,12 +13706,13 @@ tsubst_copy_and_build (tree t, /* Remember that there was a reference to this entity. */ if (DECL_P (object)) mark_used (object); - object_type = TREE_TYPE (object); + object_type = (TREE_CODE (object) == ARROW_EXPR + ? TREE_OPERAND (object, 0) : TREE_TYPE (object)); member = TREE_OPERAND (t, 1); if (BASELINK_P (member)) member = tsubst_baselink (member, - non_reference (TREE_TYPE (object)), + non_reference (object_type), args, complain, in_decl); else member = tsubst_copy (member, args, complain, in_decl);
On 10/26/2011 06:27 PM, Jason Merrill wrote: > OK. I re-opened this one because: 1- We may want to fix it in 4_6-branch too, it's a regression there too; 2- We are still handling incorrectly the template impl case. For the latter a variant of my old idea still works, fwiw. Thanks, Paolo. /////////////////////