diff mbox

PR54442 build_qualified_type produces a non-canonical type

Message ID alpine.DEB.2.02.1406091604020.13093@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse June 9, 2014, 2:18 p.m. UTC 
Hello,

in this PR, we end up with 3 types A, B and C such that 
TYPE_CANONICAL(A)=B and TYPE_CANONICAL(B)=C. I don't think that is 
supposed to happen. Here, build_qualified_type fills in TYPE_CANONICAL(A) 
by calling build_qualified_type (so get_qualified_type), which afaics 
isn't guaranteed to return a canonical type.

I doubt the patch can be wrong, but it may be that this is a situation 
that is not supposed to happen and should be fixed elsewhere?

Bootstrap+testsuite on x86_64-linux-gnu.

2014-06-09  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/54442
gcc/
 	* tree.c (build_qualified_type): Use a canonical type for
 	TYPE_CANONICAL.
gcc/testsuite/
 	* g++.dg/pr54442.C: New file.

Comments

Jason Merrill June 9, 2014, 2:24 p.m. UTC | #1 
On 06/09/2014 10:18 AM, Marc Glisse wrote:
> I doubt the patch can be wrong, but it may be that this is a situation
> that is not supposed to happen and should be fixed elsewhere?

Seems likely.  What is the difference between the type returned from 
build_qualified_type (TYPE_CANONICAL and it's TYPE_CANONICAL?  I would 
expect them to be the same.

Jason
Marc Glisse June 9, 2014, 2:32 p.m. UTC | #2 
On Mon, 9 Jun 2014, Jason Merrill wrote:

> On 06/09/2014 10:18 AM, Marc Glisse wrote:
>> I doubt the patch can be wrong, but it may be that this is a situation
>> that is not supposed to happen and should be fixed elsewhere?
>
> Seems likely.  What is the difference between the type returned from 
> build_qualified_type (TYPE_CANONICAL and it's TYPE_CANONICAL?  I would expect 
> them to be the same.

     throws <tree_list 0x7ffff660e5c8
         purpose <integer_cst 0x7ffff64d6ba0 constant 1>>>

(in what build_qualified_type returns)
Jason Merrill June 9, 2014, 2:46 p.m. UTC | #3 
On 06/09/2014 10:32 AM, Marc Glisse wrote:
> On Mon, 9 Jun 2014, Jason Merrill wrote:
>
>> On 06/09/2014 10:18 AM, Marc Glisse wrote:
>>> I doubt the patch can be wrong, but it may be that this is a situation
>>> that is not supposed to happen and should be fixed elsewhere?
>>
>> Seems likely.  What is the difference between the type returned from
>> build_qualified_type (TYPE_CANONICAL and it's TYPE_CANONICAL?  I would
>> expect them to be the same.
>
>      throws <tree_list 0x7ffff660e5c8
>          purpose <integer_cst 0x7ffff64d6ba0 constant 1>>>
>
> (in what build_qualified_type returns)

I guess that makes sense, given that the exception specification isn't 
really part of the type.  The patch is OK.

Jason
Paolo Carlini Jan. 13, 2015, 6:46 p.m. UTC | #4 
Hi,

On 06/09/2014 04:46 PM, Jason Merrill wrote:
> On 06/09/2014 10:32 AM, Marc Glisse wrote:
>> On Mon, 9 Jun 2014, Jason Merrill wrote:
>>
>>> On 06/09/2014 10:18 AM, Marc Glisse wrote:
>>>> I doubt the patch can be wrong, but it may be that this is a situation
>>>> that is not supposed to happen and should be fixed elsewhere?
>>>
>>> Seems likely.  What is the difference between the type returned from
>>> build_qualified_type (TYPE_CANONICAL and it's TYPE_CANONICAL? I would
>>> expect them to be the same.
>>
>>      throws <tree_list 0x7ffff660e5c8
>>          purpose <integer_cst 0x7ffff64d6ba0 constant 1>>>
>>
>> (in what build_qualified_type returns)
>
> I guess that makes sense, given that the exception specification isn't 
> really part of the type.  The patch is OK.

In fact, I noticed today that this is a 4.8/4.9 Regression too. Shall I 
try to apply the patchlet to 4_9-branch too and, if testing passes, 
commit there and close the bug?

Thanks,
Paolo.
Jason Merrill Jan. 13, 2015, 9:14 p.m. UTC | #5 
On 01/13/2015 01:46 PM, Paolo Carlini wrote:
> In fact, I noticed today that this is a 4.8/4.9 Regression too. Shall I
> try to apply the patchlet to 4_9-branch too and, if testing passes,
> commit there and close the bug?

OK.

Jason
diff mbox

Patch

Index: gcc/testsuite/g++.dg/pr54442.C
===================================================================
--- gcc/testsuite/g++.dg/pr54442.C	(revision 0)
+++ gcc/testsuite/g++.dg/pr54442.C	(working copy)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+
+struct S
+{
+  void s (int) const throw ();
+  void s (int) throw ();
+};
+
+typedef int index_t;
+
+void (S::*f) (index_t)       = &S::s;
+void (S::*g) (index_t) const = &S::s;
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 211374)
+++ gcc/tree.c	(working copy)
@@ -6319,22 +6319,25 @@  build_qualified_type (tree type, int typ
 		TYPE_ALIGN (t) = TYPE_ALIGN (atomic_type);
 	    }
 	}
 
       if (TYPE_STRUCTURAL_EQUALITY_P (type))
 	/* Propagate structural equality. */
 	SET_TYPE_STRUCTURAL_EQUALITY (t);
       else if (TYPE_CANONICAL (type) != type)
 	/* Build the underlying canonical type, since it is different
 	   from TYPE. */
-	TYPE_CANONICAL (t) = build_qualified_type (TYPE_CANONICAL (type),
-						   type_quals);
+	{
+	  tree c = build_qualified_type (TYPE_CANONICAL (type),
+					 type_quals);
+	  TYPE_CANONICAL (t) = TYPE_CANONICAL (c);
+	}
       else
 	/* T is its own canonical type. */
 	TYPE_CANONICAL (t) = t;
 
     }
 
   return t;
 }
 
 /* Create a variant of type T with alignment ALIGN.  */