diff mbox

[C++1y,3/4] Ensure implicit template parameters have distinct canonical types.

Message ID 1379615867-21555-4-git-send-email-adam@jessamine.co.uk
State New
Headers show

Commit Message

Adam Butcher Sept. 19, 2013, 6:37 p.m. UTC 
* parser.c (add_implicit_template_parms): Set the canonical type of a
	generic parameter to be that of the newly generated type such that it is
	unique.
---
 gcc/cp/parser.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jason Merrill Sept. 20, 2013, 5:47 p.m. UTC | #1 
Why is canonical_type_parameter not doing the right thing here?  I don't 
see a reason we should need to treat these differently from normal 
template parms.

Jason
Adam Butcher Sept. 22, 2013, 1:07 p.m. UTC | #2 
On 20.09.2013 18:47, Jason Merrill wrote:
> Why is canonical_type_parameter not doing the right thing here?  I
> don't see a reason we should need to treat these differently from
> normal template parms.
>
The issue only happens with indirect parms.  The type 'auto' is given a 
canonical type in make_auto_1.  When the parm type is plain unqualified 
'auto', the implicit template code replaces it with the generated type 
and all's well.  When the parm is 'auto&' the implicit template code 
only replaces the template_type_parm 'auto' with the generated type, the 
wrapping reference_type is left with the canonical type of 'auto&'.

Example of two parm types in the same list (chain field removed):

  <reference_type 0x7f27b2ff32a0
     type <template_type_parm 0x7f27b2ff31f8 auto VOID
        align 8 symtab 0 alias set -1
        canonical type 0x7f27b2fe0f18
        index 0 level 1 orig_level 1
        reference_to_this <reference_type 0x7f27b2ff32a0>>
     unsigned DI
     size <...> unit size <...>
     align 64 symtab 0 alias set -1
     canonical type 0x7f27b2ff3150>

  <reference_type 0x7f27b2ff33f0
     type <template_type_parm 0x7f27b2ff3348 auto VOID
        align 8 symtab 0 alias set -1
        canonical type 0x7f27b2fe0f18
        index 0 level 1 orig_level 1
        reference_to_this <reference_type 0x7f27b2ff33f0>>
     unsigned DI
     size <...> unit size <...>
     align 64 symtab 0 alias set -1
     canonical type 0x7f27b2ff3150>

After replacement:

  <reference_type 0x7f27b2ff32a0
     type <template_type_parm 0x7f27b2ff3540 <auto2> VOID
        align 8 symtab 0 alias set -1
        canonical type 0x7f27b2ff3540
        index 1 level 1 orig_level 1>>
     unsigned DI
     size <...> unit size <...>
     align 64 symtab 0 alias set -1
     canonical type 0x7f27b2ff3150>

  <reference_type 0x7f27b2ff33f0
     type <template_type_parm 0x7f27b2ff35e8 <auto3> VOID
        align 8 symtab 0 alias set -1
        canonical type 0x7f27b2ff35e8
        index 2 level 1 orig_level 1>>
     unsigned DI
     size <...> unit size <...>
     align 64 symtab 0 alias set -1
     canonical type 0x7f27b2ff3150>

Looks like I'll need to expose canonical_type_parameter and push it 
through from the top of the parameter rather than just replace the 
template_type_parm.   Or leave this as is and arrange for this scenario 
to never occur.

I guess this wouldn't happen if we were replacing on the fly.  Maybe 
make_auto_1 could differ in behavior when in a parameter list (would not 
setting a canonical type get us anywhere?) or just call a different 
function?

I'll have a further look when I get some more time.

Cheers,
Adam
Adam Butcher Sept. 22, 2013, 5:57 p.m. UTC | #3 
On 22.09.2013 14:07, Adam Butcher wrote:
> On 20.09.2013 18:47, Jason Merrill wrote:
>> Why is canonical_type_parameter not doing the right thing here?  I
>> don't see a reason we should need to treat these differently from
>> normal template parms.
>>
> The issue only happens with indirect parms.  The type 'auto' is given
> a canonical type in make_auto_1.  When the parm type is plain
> unqualified 'auto', the implicit template code replaces it with the
> generated type and all's well.  When the parm is 'auto&' the implicit
> template code only replaces the template_type_parm 'auto' with the
> generated type, the wrapping reference_type is left with the 
> canonical
> type of 'auto&'.
>
> Example of two parm types in the same list (chain field removed):
>
>  <reference_type 0x7f27b2ff32a0
>     type <template_type_parm 0x7f27b2ff31f8 auto VOID
>        align 8 symtab 0 alias set -1
>        canonical type 0x7f27b2fe0f18
>        index 0 level 1 orig_level 1
>        reference_to_this <reference_type 0x7f27b2ff32a0>>
>     unsigned DI
>     size <...> unit size <...>
>     align 64 symtab 0 alias set -1
>     canonical type 0x7f27b2ff3150>
>
>  <reference_type 0x7f27b2ff33f0
>     type <template_type_parm 0x7f27b2ff3348 auto VOID
>        align 8 symtab 0 alias set -1
>        canonical type 0x7f27b2fe0f18
>        index 0 level 1 orig_level 1
>        reference_to_this <reference_type 0x7f27b2ff33f0>>
>     unsigned DI
>     size <...> unit size <...>
>     align 64 symtab 0 alias set -1
>     canonical type 0x7f27b2ff3150>
>
> After replacement:
>
>  <reference_type 0x7f27b2ff32a0
>     type <template_type_parm 0x7f27b2ff3540 <auto2> VOID
>        align 8 symtab 0 alias set -1
>        canonical type 0x7f27b2ff3540
>        index 1 level 1 orig_level 1>>
>     unsigned DI
>     size <...> unit size <...>
>     align 64 symtab 0 alias set -1
>     canonical type 0x7f27b2ff3150>
>
>  <reference_type 0x7f27b2ff33f0
>     type <template_type_parm 0x7f27b2ff35e8 <auto3> VOID
>        align 8 symtab 0 alias set -1
>        canonical type 0x7f27b2ff35e8
>        index 2 level 1 orig_level 1>>
>     unsigned DI
>     size <...> unit size <...>
>     align 64 symtab 0 alias set -1
>     canonical type 0x7f27b2ff3150>
>
> Looks like I'll need to expose canonical_type_parameter and push it
> through from the top of the parameter rather than just replace the
> template_type_parm.   Or leave this as is and arrange for this
> scenario to never occur.
>
> I guess this wouldn't happen if we were replacing on the fly.  Maybe
> make_auto_1 could differ in behavior when in a parameter list (would
> not setting a canonical type get us anywhere?) or just call a
> different function?
>
> I'll have a further look when I get some more time.
>
The following solves the canonical type issue in the general case 
(pointers and refs) and makes it equivalent to the explicit template 
case -- in terms of canonical type at least.

       for (tree t = TREE_TYPE (TREE_VALUE (p)); t; t = TREE_CHAIN (t))
         TYPE_CANONICAL (t) = t;

But I'm not necessarily proposing it as a good final solution.  It is 
yet another type chain loop, and I'm not sure about preserving other 
fields that are set in the explicit template case (e.g. 
reference_to_this).

So far test cases appear to work as expected but I'm thinking that 
getting the 'auto' replaced earlier might solve these issues more 
naturally (and others such as auto... packs)

Cheers,
Adam
diff mbox

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7e9ade2..148e2f2 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -29005,6 +29005,9 @@  add_implicit_template_parms (cp_parser *parser, size_t expect_count,
 	cur_type = cp_build_qualified_type (new_type, TYPE_QUALS (cur_type));
       else
 	cur_type = new_type;
+
+      /* Make the canonical type of the parameter distinct.  */
+      TYPE_CANONICAL (TREE_TYPE (TREE_VALUE (p))) = cur_type;
     }
 
   gcc_assert (synth_count == expect_count);