[C++ PATCH] Battle of the comptypes (PR c++/35049)

[C++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

[-- Attachment #1: Type: text/plain, Size: 2441 bytes --]

PR c++/35049 is a 4.3 regression where we are failing to emit an error
when performing addition of vector types that don't have the same base
type. The bug seems to have been triggered by a recent change in the
canonical types system, but in fact it's been a ticking time bomb for
a while. The bug shows up on i386-apple-darwin9.1.0 and (for some
people) on i686-pc-linux-gnu

The C front end declares the function comptypes like this, in c-tree.h:

  extern int comptypes(tree, tree);

Parts of the C front end and the C/C++ commit bits use this comptypes.
However, this comptypes actually resides in c-typeck.c, which is not
linked into the C++ compiler.

The C++ front end declares comptypes like this, in cp-tree.h:

  extern int comptypes(tree, tree, int);

That int is the "strict" parameter, that tells the routine how to
compare the types, e.g., strict comparison, checking for base and
derived, derived and base, or redeclarations.

The fun comes in where the C/C++ common bits call comptypes with 2
arguments (as they should), but that ends up calling the C++
comptypes... with random data for the "strict" argument. So sometimes
we'll get strict comparisons (if that parameter is zero), sometimes
we'll get comparisons based on derived/base matching, etc. Most likely
this didn't matter before because the C bits don't deal with such C++
types often, but in the case of this bug we're thwarting the canonical
type system (which is only enabled with strict==COMPARE_STRICT) and
getting the wrong answer from comptypes.

This patch just renames "comptypes" in the C++ front end to
"cp_comptypes", and adds a 2-argument "comptypes" function to the C++
front end that accepts calls from the C/C++ common bits and forwards
on to cp_comptypes.

Tested i386-apple-darwin9.1.0; okay for mainline?

  - Doug

2008-02-05  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/35049
	* typeck.c (structural_comptypes): Call cp_comptypes.
	(comptypes): New; called from the C/C++ common bits to perform
	strict checks.
	(cp_comptypes): Renamed from comptypes, which is already used,
	with a different signature, by the C++ front end.
	(build_reinterpret_cast_1): Call cp_comptypes.
	(ptr_reasonably_similar): Ditto.
	* decl.c (decls_match): Ditto.
	* cvt.c (convert_to_reference): Ditto.
	* cp-tree.h (same_type_p): Ditto.
	(same_or_base_type_p): Ditto.
	(comptypes): Rename to cp_comptypes.
	* pt.c (canonical_type_parameter): Call cp_comptypes.

[-- Attachment #2: cp_comptypes.patch --]
[-- Type: application/octet-stream, Size: 8111 bytes --]

Index: typeck.c
===================================================================
--- typeck.c	(revision 132121)
+++ typeck.c	(working copy)
@@ -160,7 +160,7 @@ type_unknown_p (const_tree exp)
 
 \f
 /* Return the common type of two parameter lists.
-   We assume that comptypes has already been done and returned 1;
+   We assume that cp_comptypes has already been done and returned 1;
    if that isn't so, this may crash.
 
    As an optimization, free the space we allocate if the parameter
@@ -573,7 +573,7 @@ composite_pointer_type (tree t1, tree t2
 }
 
 /* Return the merged type of two types.
-   We assume that comptypes has already been done and returned 1;
+   We assume that cp_comptypes has already been done and returned 1;
    if that isn't so, this may crash.
 
    This just combines attributes and default arguments; any other
@@ -735,7 +735,7 @@ merge_types (tree t1, tree t2)
 }
 
 /* Return the common type of two types.
-   We assume that comptypes has already been done and returned 1;
+   We assume that cp_comptypes has already been done and returned 1;
    if that isn't so, this may crash.
 
    This is the type for the result of most arithmetic operations
@@ -926,7 +926,7 @@ comp_array_types (const_tree t1, const_t
   return true;
 }
 
-/* Subroutine in comptypes.  */
+/* Subroutine in cp_comptypes.  */
 
 static bool
 structural_comptypes (tree t1, tree t2, int strict)
@@ -1034,8 +1034,8 @@ structural_comptypes (tree t1, tree t2, 
       return false;
 
     case OFFSET_TYPE:
-      if (!comptypes (TYPE_OFFSET_BASETYPE (t1), TYPE_OFFSET_BASETYPE (t2),
-		      strict & ~COMPARE_REDECLARATION))
+      if (!cp_comptypes (TYPE_OFFSET_BASETYPE (t1), TYPE_OFFSET_BASETYPE (t2),
+                         strict & ~COMPARE_REDECLARATION))
 	return false;
       if (!same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
 	return false;
@@ -1123,11 +1123,22 @@ structural_comptypes (tree t1, tree t2, 
   return targetm.comp_type_attributes (t1, t2);
 }
 
+extern int comptypes (tree, tree);
+
+/* Type comparison function that matches the signature of comptypes
+   from c-tree.h, which is used by the C front end and some of the
+   C/C++ common bits.  */
+int
+comptypes (tree t1, tree t2)
+{
+  return cp_comptypes (t1, t2, COMPARE_STRICT);
+}
+
 /* Return true if T1 and T2 are related as allowed by STRICT.  STRICT
    is a bitwise-or of the COMPARE_* flags.  */
 
 bool
-comptypes (tree t1, tree t2, int strict)
+cp_comptypes (tree t1, tree t2, int strict)
 {
   if (strict == COMPARE_STRICT)
     {
@@ -1224,7 +1235,7 @@ comp_cv_qual_signature (tree type1, tree
     return 0;
 }
 \f
-/* Subroutines of `comptypes'.  */
+/* Subroutines of `cp_comptypes'.  */
 
 /* Return true if two parameter type lists PARMS1 and PARMS2 are
    equivalent in the sense that functions with those parameter types
@@ -5249,8 +5260,8 @@ build_reinterpret_cast_1 (tree type, tre
 	 "B" are related class types; the reinterpret_cast does not
 	 adjust the pointer.  */
       if (TYPE_PTR_P (intype)
-	  && (comptypes (TREE_TYPE (intype), TREE_TYPE (type),
-			 COMPARE_BASE | COMPARE_DERIVED)))
+	  && (cp_comptypes (TREE_TYPE (intype), TREE_TYPE (type),
+                            COMPARE_BASE | COMPARE_DERIVED)))
 	warning (0, "casting %qT to %qT does not dereference pointer",
 		 intype, type);
 
@@ -6913,9 +6924,9 @@ ptr_reasonably_similar (const_tree to, c
 	return 0;
 
       if (TREE_CODE (from) == OFFSET_TYPE
-	  && comptypes (TYPE_OFFSET_BASETYPE (to),
-			TYPE_OFFSET_BASETYPE (from),
-			COMPARE_BASE | COMPARE_DERIVED))
+	  && cp_comptypes (TYPE_OFFSET_BASETYPE (to),
+                           TYPE_OFFSET_BASETYPE (from),
+                           COMPARE_BASE | COMPARE_DERIVED))
 	continue;
 
       if (TREE_CODE (to) == VECTOR_TYPE
@@ -6930,7 +6941,7 @@ ptr_reasonably_similar (const_tree to, c
 	return 1;
 
       if (TREE_CODE (to) != POINTER_TYPE)
-	return comptypes
+	return cp_comptypes
 	  (TYPE_MAIN_VARIANT (to), TYPE_MAIN_VARIANT (from),
 	   COMPARE_BASE | COMPARE_DERIVED);
     }
Index: decl.c
===================================================================
--- decl.c	(revision 132121)
+++ decl.c	(working copy)
@@ -1015,9 +1015,9 @@ decls_match (tree newdecl, tree olddecl)
       else if (TREE_TYPE (newdecl) == NULL_TREE)
 	types_match = 0;
       else
-	types_match = comptypes (TREE_TYPE (newdecl),
-				 TREE_TYPE (olddecl),
-				 COMPARE_REDECLARATION);
+	types_match = cp_comptypes (TREE_TYPE (newdecl),
+                                    TREE_TYPE (olddecl),
+                                    COMPARE_REDECLARATION);
     }
 
   return types_match;
Index: cvt.c
===================================================================
--- cvt.c	(revision 132121)
+++ cvt.c	(working copy)
@@ -465,8 +465,8 @@ convert_to_reference (tree reftype, tree
       /* B* bp; A& ar = (A&)bp; is valid, but it's probably not what they
 	 meant.  */
       if (TREE_CODE (intype) == POINTER_TYPE
-	  && (comptypes (TREE_TYPE (intype), type,
-			 COMPARE_BASE | COMPARE_DERIVED)))
+	  && (cp_comptypes (TREE_TYPE (intype), type,
+                            COMPARE_BASE | COMPARE_DERIVED)))
 	warning (0, "casting %qT to %qT does not dereference pointer",
 		 intype, reftype);
 
@@ -604,7 +604,7 @@ ocp_convert (tree type, tree expr, int c
 	/* The call to fold will not always remove the NOP_EXPR as
 	   might be expected, since if one of the types is a typedef;
 	   the comparison in fold is just equality of pointers, not a
-	   call to comptypes.  We don't call fold in this case because
+	   call to cp_comptypes.  We don't call fold in this case because
 	   that can result in infinite recursion; fold will call
 	   convert, which will call ocp_convert, etc.  */
 	return e;
Index: cp-tree.h
===================================================================
--- cp-tree.h	(revision 132121)
+++ cp-tree.h	(working copy)
@@ -281,7 +281,7 @@ typedef struct ptrmem_cst * ptrmem_cst_t
 /* Returns nonzero iff TYPE1 and TYPE2 are the same type, in the usual
    sense of `same'.  */
 #define same_type_p(TYPE1, TYPE2) \
-  comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
+  cp_comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
 
 /* Returns nonzero iff TYPE1 and TYPE2 are the same type, ignoring
    top-level qualifiers.  */
@@ -3744,7 +3744,7 @@ enum overload_flags { NO_SPECIAL = 0, DT
 #define WANT_VECTOR    32 /* vector types */
 #define WANT_ARITH	(WANT_INT | WANT_FLOAT | WANT_VECTOR)
 
-/* Used with comptypes, and related functions, to guide type
+/* Used with cp_comptypes, and related functions, to guide type
    comparison.  */
 
 #define COMPARE_STRICT	      0 /* Just check if the types are the
@@ -3780,7 +3780,7 @@ enum overload_flags { NO_SPECIAL = 0, DT
    is derived from TYPE1, or if TYPE2 is a pointer (reference) to a
    class derived from the type pointed to (referred to) by TYPE1.  */
 #define same_or_base_type_p(TYPE1, TYPE2) \
-  comptypes ((TYPE1), (TYPE2), COMPARE_BASE)
+  cp_comptypes ((TYPE1), (TYPE2), COMPARE_BASE)
 
 /* These macros are used to access a TEMPLATE_PARM_INDEX.  */
 #define TEMPLATE_PARM_INDEX_CAST(NODE) \
@@ -4764,7 +4764,7 @@ extern tree complete_type			(tree);
 extern tree complete_type_or_else		(tree, tree);
 extern int type_unknown_p			(const_tree);
 extern bool comp_except_specs			(const_tree, const_tree, bool);
-extern bool comptypes				(tree, tree, int);
+extern bool cp_comptypes			(tree, tree, int);
 extern bool compparms				(const_tree, const_tree);
 extern int comp_cv_qualification		(const_tree, const_tree);
 extern int comp_cv_qual_signature		(tree, tree);
Index: pt.c
===================================================================
--- pt.c	(revision 132121)
+++ pt.c	(working copy)
@@ -2905,7 +2905,7 @@ canonical_type_parameter (tree type)
     VEC_safe_push (tree, gc, canonical_template_parms, NULL_TREE);
 
   list = VEC_index (tree, canonical_template_parms, idx);
-  while (list && !comptypes (type, TREE_VALUE (list), COMPARE_STRUCTURAL))
+  while (list && !cp_comptypes (type, TREE_VALUE (list), COMPARE_STRUCTURAL))
     list = TREE_CHAIN (list);
 
   if (list)

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[H.J. Lu]

Hi Doug,

Can you include a testcase from

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35096


H.J.
On Feb 5, 2008 9:52 AM, Doug Gregor <doug.gregor@gmail.com> wrote:
> PR c++/35049 is a 4.3 regression where we are failing to emit an error
> when performing addition of vector types that don't have the same base
> type. The bug seems to have been triggered by a recent change in the
> canonical types system, but in fact it's been a ticking time bomb for
> a while. The bug shows up on i386-apple-darwin9.1.0 and (for some
> people) on i686-pc-linux-gnu
>
> The C front end declares the function comptypes like this, in c-tree.h:
>
>   extern int comptypes(tree, tree);
>
> Parts of the C front end and the C/C++ commit bits use this comptypes.
> However, this comptypes actually resides in c-typeck.c, which is not
> linked into the C++ compiler.
>
> The C++ front end declares comptypes like this, in cp-tree.h:
>
>   extern int comptypes(tree, tree, int);
>
> That int is the "strict" parameter, that tells the routine how to
> compare the types, e.g., strict comparison, checking for base and
> derived, derived and base, or redeclarations.
>
> The fun comes in where the C/C++ common bits call comptypes with 2
> arguments (as they should), but that ends up calling the C++
> comptypes... with random data for the "strict" argument. So sometimes
> we'll get strict comparisons (if that parameter is zero), sometimes
> we'll get comparisons based on derived/base matching, etc. Most likely
> this didn't matter before because the C bits don't deal with such C++
> types often, but in the case of this bug we're thwarting the canonical
> type system (which is only enabled with strict==COMPARE_STRICT) and
> getting the wrong answer from comptypes.
>
> This patch just renames "comptypes" in the C++ front end to
> "cp_comptypes", and adds a 2-argument "comptypes" function to the C++
> front end that accepts calls from the C/C++ common bits and forwards
> on to cp_comptypes.
>
> Tested i386-apple-darwin9.1.0; okay for mainline?
>
>   - Doug
>
> 2008-02-05  Douglas Gregor  <doug.gregor@gmail.com>
>
>         PR c++/35049
>         * typeck.c (structural_comptypes): Call cp_comptypes.
>         (comptypes): New; called from the C/C++ common bits to perform
>         strict checks.
>         (cp_comptypes): Renamed from comptypes, which is already used,
>         with a different signature, by the C++ front end.
>         (build_reinterpret_cast_1): Call cp_comptypes.
>         (ptr_reasonably_similar): Ditto.
>         * decl.c (decls_match): Ditto.
>         * cvt.c (convert_to_reference): Ditto.
>         * cp-tree.h (same_type_p): Ditto.
>         (same_or_base_type_p): Ditto.
>         (comptypes): Rename to cp_comptypes.
>         * pt.c (canonical_type_parameter): Call cp_comptypes.
>

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

Once I get the O.K. from a C++ front end maintainer to commit this,
yes; I'll include that new test case.

  - Doug

On Feb 6, 2008 10:50 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Hi Doug,
>
> Can you include a testcase from
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35096
>
>
> H.J.
>
> On Feb 5, 2008 9:52 AM, Doug Gregor <doug.gregor@gmail.com> wrote:
> > PR c++/35049 is a 4.3 regression where we are failing to emit an error
> > when performing addition of vector types that don't have the same base
> > type. The bug seems to have been triggered by a recent change in the
> > canonical types system, but in fact it's been a ticking time bomb for
> > a while. The bug shows up on i386-apple-darwin9.1.0 and (for some
> > people) on i686-pc-linux-gnu
> >
> > The C front end declares the function comptypes like this, in c-tree.h:
> >
> >   extern int comptypes(tree, tree);
> >
> > Parts of the C front end and the C/C++ commit bits use this comptypes.
> > However, this comptypes actually resides in c-typeck.c, which is not
> > linked into the C++ compiler.
> >
> > The C++ front end declares comptypes like this, in cp-tree.h:
> >
> >   extern int comptypes(tree, tree, int);
> >
> > That int is the "strict" parameter, that tells the routine how to
> > compare the types, e.g., strict comparison, checking for base and
> > derived, derived and base, or redeclarations.
> >
> > The fun comes in where the C/C++ common bits call comptypes with 2
> > arguments (as they should), but that ends up calling the C++
> > comptypes... with random data for the "strict" argument. So sometimes
> > we'll get strict comparisons (if that parameter is zero), sometimes
> > we'll get comparisons based on derived/base matching, etc. Most likely
> > this didn't matter before because the C bits don't deal with such C++
> > types often, but in the case of this bug we're thwarting the canonical
> > type system (which is only enabled with strict==COMPARE_STRICT) and
> > getting the wrong answer from comptypes.
> >
> > This patch just renames "comptypes" in the C++ front end to
> > "cp_comptypes", and adds a 2-argument "comptypes" function to the C++
> > front end that accepts calls from the C/C++ common bits and forwards
> > on to cp_comptypes.
> >
> > Tested i386-apple-darwin9.1.0; okay for mainline?
> >
> >   - Doug
> >
> > 2008-02-05  Douglas Gregor  <doug.gregor@gmail.com>
> >
> >         PR c++/35049
> >         * typeck.c (structural_comptypes): Call cp_comptypes.
> >         (comptypes): New; called from the C/C++ common bits to perform
> >         strict checks.
> >         (cp_comptypes): Renamed from comptypes, which is already used,
> >         with a different signature, by the C++ front end.
> >         (build_reinterpret_cast_1): Call cp_comptypes.
> >         (ptr_reasonably_similar): Ditto.
> >         * decl.c (decls_match): Ditto.
> >         * cvt.c (convert_to_reference): Ditto.
> >         * cp-tree.h (same_type_p): Ditto.
> >         (same_or_base_type_p): Ditto.
> >         (comptypes): Rename to cp_comptypes.
> >         * pt.c (canonical_type_parameter): Call cp_comptypes.
> >
>

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[H.J. Lu]

On Feb 6, 2008 7:59 AM, Doug Gregor <doug.gregor@gmail.com> wrote:
> Once I get the O.K. from a C++ front end maintainer to commit this,
> yes; I'll include that new test case.
>

FWIW, your patch works on Linux/ia32, Linux/Intel64 and Linux/ia64

http://gcc.gnu.org/ml/gcc-testresults/2008-02/msg00395.html
http://gcc.gnu.org/ml/gcc-testresults/2008-02/msg00392.html
http://gcc.gnu.org/ml/gcc-testresults/2008-02/msg00398.html

H.J.

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Andreas Tobler]

Doug Gregor wrote:

> Tested i386-apple-darwin9.1.0; okay for mainline?

But not with obj-c++. That's broken now!
I do not have a full insight.
But the below brings me a step further. Might be wrong....
Me is wondering about the test results and also how people test.... 
before committing a patch.

Andreas


Index: objcp-decl.c
===================================================================
--- objcp-decl.c	(revision 132159)
+++ objcp-decl.c	(working copy)
@@ -95,7 +95,7 @@
  int
  objcp_comptypes (tree type1, tree type2)
  {
-  return comptypes (type1, type2, COMPARE_STRICT);
+  return cp_comptypes (type1, type2, COMPARE_STRICT);
  }

  tree

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Manuel López-Ibáñez]

On 06/02/2008, Andreas Tobler <andreast-list@fgznet.ch> wrote:
> Doug Gregor wrote:
>
> > Tested i386-apple-darwin9.1.0; okay for mainline?
>
> But not with obj-c++. That's broken now!
> I do not have a full insight.
> But the below brings me a step further. Might be wrong....
> Me is wondering about the test results and also how people test....
> before committing a patch.
>

I test using --enable-languages=all, which I guess is what most people
use. That doesn't include obj-c++ or ada for some reason...

Cheers,

Manuel.

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Andrew Pinski]

On Feb 6, 2008 2:54 PM, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
>
> I test using --enable-languages=all, which I guess is what most people
> use. That doesn't include obj-c++ or ada for some reason...

Because are not enabled by default.  Also the testcase at
g++.dg/ext/vector13.C still fails for me.

-- Pinski

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

> The fun comes in where the C/C++ common bits call comptypes with 2
> arguments (as they should), but that ends up calling the C++
> comptypes... with random data for the "strict" argument.

Summing up, we have:

C comptypes => C++ same_type_p

C lang_hooks.types_compatible_p => C++ 
same_type_ignoring_top_level_qualifiers_p

C++ lang_hooks.types_compatible_p => equivalence between pointers and 
reference + same_type_ignoring_top_level_qualifiers_p


So I have an alternate plan:

1) bulk-rename the calls to comptypes in the C front-end to same_type_p

2) add an argument to the C comptypes that will always be COMPARE_STRICT 
for C

3) move same_type_p, same_type_ignoring_top_level_qualifiers_p, 
COMPARE_STRICT from cp-tree.h to c-common.h

I'm testing a patch.

For 4.4, additionally, we can add the C++ lang_hooks.types_compatible_p 
special casing of pointers & references to the C langhook, thus killing 
the C++ lang_hooks.types_compatible_p

Paolo

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

Here it is.  The problem is that C and C++ call two different things 
"comptypes".  What C calls comptypes, C++ calls same_type_p.  In turn, 
C++ does have a comptypes but it has an additional third argument; 
same_type_p (a, b) == comptypes (a, b, 0) == <C front-end comptypes> (a, b).

The solution is to give the same signature to the C and C++ front-end's 
comptypes functions.  Furthermore, I moved the same_type_p macro from 
C++ to c-common.h so that the whole family of languages can use the same 
name for the same thing.  Therefore, a large part of the patch renames 
comptypes calls to same_type_p or same_type_ignoring_top_level_qualifiers_p.

The patch may seem a little more complicated than Doug's because it 
touches multiple languages, but actually it kills more code than it 
adds!  In particular, it removes an ugly hack in the ObjC++ front-end to 
mediate between objc/objc-act.c (which uses a two-argument comptypes) 
and cp/typeck.c (which implements a three-argument comptypes).

Bootstrapped/regtested i686-pc-linux-gnu, 
C/C++/ObjC/ObjC++/Fortran/Java.  Ok for mainline together with reverting 
Doug's changes?

Paolo

[-- Attachment #2: pr35049-mine.patch --]
[-- Type: text/plain, Size: 18817 bytes --]

2008-02-07  Paolo Bonzini  <bonzini@gnu.org>

	* c-objc-common.c (c_types_compatible_p): Don't use comptypes directly.
	* c-tree.h (comptypes): Move prototype...
	* c-common.h (comptypes): ... here, adding a third argument.
	(same_type_p, same_type_ignoring_top_level_qualifiers_p,
	COMPARE_STRICT): Move here from cp/cp-tree.h.
	* c-common.c: Don't use comptypes directly.
	* c-decl.c: Don't use comptypes directly.
	* c-parser.c: Don't use comptypes directly.
	* c-typeck.c: Don't use comptypes directly.
	(comptypes): Add a third, unused argument for compatibility with C++.

cp:
2008-02-07  Paolo Bonzini  <bonzini@gnu.org>

	* typeck.c (comptypes): Return int.
	* cp-tree.h (same_type_p, same_type_ignoring_top_level_qualifiers_p,
	COMPARE_STRICT, comptypes): Remove.

objc:
2008-02-07  Paolo Bonzini  <bonzini@gnu.org>

	* objc-act.c (check_ivars): Use same_type_p instead of
	comptypes.

objcp:
2008-02-07  Paolo Bonzini  <bonzini@gnu.org>

	* objcp-decl.c (objcp_comptypes): Delete.
	* objcp-decl.h (objcp_comptypes): Delete prototype.
	(comptypes): Delete.

Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c	(revision 131960)
+++ objc/objc-act.c	(working copy)
@@ -4983,7 +4983,7 @@ check_ivars (tree inter, tree imp)
 
       t1 = TREE_TYPE (intdecls); t2 = TREE_TYPE (impdecls);
 
-      if (!comptypes (t1, t2)
+      if (!same_type_p (t1, t2)
 	  || !tree_int_cst_equal (DECL_INITIAL (intdecls),
 				  DECL_INITIAL (impdecls)))
 	{
Index: objcp/objcp-decl.c
===================================================================
--- objcp/objcp-decl.c	(revision 131960)
+++ objcp/objcp-decl.c	(working copy)
@@ -92,12 +92,6 @@ objcp_xref_tag (enum tree_code code ATTR
   return xref_tag (record_type, name, ts_global, false);
 }
 
-int
-objcp_comptypes (tree type1, tree type2)
-{     
-  return comptypes (type1, type2, COMPARE_STRICT);
-}
-
 tree
 objcp_begin_compound_stmt (int flags ATTRIBUTE_UNUSED)
 {
Index: objcp/objcp-decl.h
===================================================================
--- objcp/objcp-decl.h	(revision 131960)
+++ objcp/objcp-decl.h	(working copy)
@@ -28,7 +28,6 @@ extern tree objcp_finish_struct (tree, t
 extern void objcp_finish_function (void);
 extern tree objcp_build_function_call (tree, tree);
 extern tree objcp_xref_tag (enum tree_code, tree);
-extern int objcp_comptypes (tree, tree);
 extern tree objcp_begin_compound_stmt (int);
 extern tree objcp_end_compound_stmt (tree, int);
 
@@ -45,8 +44,6 @@ extern tree objcp_end_compound_stmt (tre
 	objcp_finish_function ()
 #define xref_tag(code, name) \
 	objcp_xref_tag (code, name)
-#define comptypes(type1, type2) \
-	objcp_comptypes (type1, type2)
 #define c_begin_compound_stmt(flags) \
 	objcp_begin_compound_stmt (flags)
 #define c_end_compound_stmt(stmt, flags) \
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 131960)
+++ cp/typeck.c	(working copy)
@@ -1102,7 +1102,7 @@ structural_comptypes (tree t1, tree t2, 
 /* Return true if T1 and T2 are related as allowed by STRICT.  STRICT
    is a bitwise-or of the COMPARE_* flags.  */
 
-bool
+int
 comptypes (tree t1, tree t2, int strict)
 {
   if (strict == COMPARE_STRICT)
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 131960)
+++ cp/cp-tree.h	(working copy)
@@ -278,16 +278,6 @@ typedef struct ptrmem_cst * ptrmem_cst_t
 #define STMT_EXPR_NO_SCOPE(NODE) \
    TREE_LANG_FLAG_0 (STMT_EXPR_CHECK (NODE))
 
-/* Returns nonzero iff TYPE1 and TYPE2 are the same type, in the usual
-   sense of `same'.  */
-#define same_type_p(TYPE1, TYPE2) \
-  comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
-
-/* Returns nonzero iff TYPE1 and TYPE2 are the same type, ignoring
-   top-level qualifiers.  */
-#define same_type_ignoring_top_level_qualifiers_p(TYPE1, TYPE2) \
-  same_type_p (TYPE_MAIN_VARIANT (TYPE1), TYPE_MAIN_VARIANT (TYPE2))
-
 /* Nonzero if we are presently building a statement tree, rather
    than expanding each statement as we encounter it.  */
 #define building_stmt_tree()  (cur_stmt_list != NULL_TREE)
@@ -3747,8 +3737,6 @@ enum overload_flags { NO_SPECIAL = 0, DT
 /* Used with comptypes, and related functions, to guide type
    comparison.  */
 
-#define COMPARE_STRICT	      0 /* Just check if the types are the
-				   same.  */
 #define COMPARE_BASE	      1 /* Check to see if the second type is
 				   derived from the first.  */
 #define COMPARE_DERIVED	      2 /* Like COMPARE_BASE, but in
@@ -4763,7 +4751,6 @@ extern tree complete_type			(tree);
 extern tree complete_type_or_else		(tree, tree);
 extern int type_unknown_p			(const_tree);
 extern bool comp_except_specs			(const_tree, const_tree, bool);
-extern bool comptypes				(tree, tree, int);
 extern bool compparms				(const_tree, const_tree);
 extern int comp_cv_qualification		(const_tree, const_tree);
 extern int comp_cv_qual_signature		(tree, tree);
Index: c-objc-common.c
===================================================================
--- c-objc-common.c	(revision 131960)
+++ c-objc-common.c	(working copy)
@@ -187,7 +187,7 @@ c_initialize_diagnostics (diagnostic_con
 int
 c_types_compatible_p (tree x, tree y)
 {
-  return comptypes (TYPE_MAIN_VARIANT (x), TYPE_MAIN_VARIANT (y));
+  return same_type_ignoring_top_level_qualifiers_p (x, y);
 }
 
 /* Determine if the type is a vla type for the backend.  */
Index: c-tree.h
===================================================================
--- c-tree.h	(revision 131960)
+++ c-tree.h	(working copy)
@@ -542,7 +542,6 @@ extern struct c_label_context_vm *label_
 
 extern tree require_complete_type (tree);
 extern int same_translation_unit_p (const_tree, const_tree);
-extern int comptypes (tree, tree);
 extern bool c_vla_type_p (const_tree);
 extern bool c_mark_addressable (tree);
 extern void c_incomplete_type_error (const_tree, const_tree);
Index: c-decl.c
===================================================================
--- c-decl.c	(revision 131960)
+++ c-decl.c	(working copy)
@@ -1021,7 +1021,7 @@ diagnose_arglist_conflict (tree newdecl,
   tree t;
 
   if (TREE_CODE (olddecl) != FUNCTION_DECL
-      || !comptypes (TREE_TYPE (oldtype), TREE_TYPE (newtype))
+      || !same_type_p (TREE_TYPE (oldtype), TREE_TYPE (newtype))
       || !((TYPE_ARG_TYPES (oldtype) == 0 && DECL_INITIAL (olddecl) == 0)
 	   ||
 	   (TYPE_ARG_TYPES (newtype) == 0 && DECL_INITIAL (newdecl) == 0)))
@@ -1098,7 +1098,7 @@ validate_proto_after_old_defn (tree newd
 
       /* Type for passing arg must be consistent with that declared
 	 for the arg.  */
-      else if (!comptypes (oldargtype, newargtype))
+      else if (!same_type_p (oldargtype, newargtype))
 	{
 	  error ("prototype for %q+D declares argument %d"
 		 " with incompatible type",
@@ -1193,7 +1193,7 @@ diagnose_mismatched_decls (tree newdecl,
       return false;
     }
 
-  if (!comptypes (oldtype, newtype))
+  if (!same_type_p (oldtype, newtype))
     {
       if (TREE_CODE (olddecl) == FUNCTION_DECL
 	  && DECL_BUILT_IN (olddecl) && !C_DECL_DECLARED_BUILTIN (olddecl))
@@ -1202,7 +1202,7 @@ diagnose_mismatched_decls (tree newdecl,
 	     This is for the ffs and fprintf builtins.  */
 	  tree trytype = match_builtin_function_types (newtype, oldtype);
 
-	  if (trytype && comptypes (newtype, trytype))
+	  if (trytype && same_type_p (newtype, trytype))
 	    *oldtypep = oldtype = trytype;
 	  else
 	    {
@@ -1649,7 +1649,7 @@ merge_decls (tree newdecl, tree olddecl,
     = composite_type (newtype, oldtype);
 
   /* Lay the type out, unless already done.  */
-  if (!comptypes (oldtype, TREE_TYPE (newdecl)))
+  if (!same_type_p (oldtype, TREE_TYPE (newdecl)))
     {
       if (TREE_TYPE (newdecl) != error_mark_node)
 	layout_type (TREE_TYPE (newdecl));
@@ -2138,7 +2138,7 @@ pushdecl (tree x)
 	      /* Save the updated type in the external scope and
 		 restore the proper type for this scope.  */
 	      tree thistype;
-	      if (comptypes (vistype, type))
+	      if (same_type_p (vistype, type))
 		thistype = composite_type (vistype, type);
 	      else
 		thistype = TREE_TYPE (b_use->decl);
@@ -2240,7 +2240,7 @@ pushdecl (tree x)
 	  tree thistype;
 	  if (vistype)
 	    {
-	      if (comptypes (vistype, type))
+	      if (same_type_p (vistype, type))
 		thistype = composite_type (vistype, type);
 	      else
 		thistype = TREE_TYPE (b->decl);
@@ -2411,7 +2411,7 @@ implicitly_declare (tree functionid)
 	      newtype = build_type_attribute_variant (newtype,
 						      TYPE_ATTRIBUTES
 						      (TREE_TYPE (decl)));
-	      if (!comptypes (newtype, TREE_TYPE (decl)))
+	      if (!same_type_p (newtype, TREE_TYPE (decl)))
 		{
 		  warning (0, "incompatible implicit declaration of built-in"
 			   " function %qD", decl);
@@ -2420,7 +2420,7 @@ implicitly_declare (tree functionid)
 	    }
 	  else
 	    {
-	      if (!comptypes (newtype, TREE_TYPE (decl)))
+	      if (!same_type_p (newtype, TREE_TYPE (decl)))
 		{
 		  error ("incompatible implicit declaration of function %qD",
 			 decl);
@@ -6146,8 +6146,8 @@ start_function (struct c_declspecs *decl
   if (TYPE_ARG_TYPES (TREE_TYPE (decl1)) == 0)
     {
       if (old_decl != 0 && TREE_CODE (TREE_TYPE (old_decl)) == FUNCTION_TYPE
-	  && comptypes (TREE_TYPE (TREE_TYPE (decl1)),
-			TREE_TYPE (TREE_TYPE (old_decl))))
+	  && same_type_p (TREE_TYPE (TREE_TYPE (decl1)),
+			  TREE_TYPE (TREE_TYPE (old_decl))))
 	{
 	  TREE_TYPE (decl1) = composite_type (TREE_TYPE (old_decl),
 					      TREE_TYPE (decl1));
@@ -6175,8 +6175,8 @@ start_function (struct c_declspecs *decl
 	      ext_decl = b->decl;
 	      ext_type = b->type ? b->type : TREE_TYPE (ext_decl);
 	      if (TREE_CODE (ext_type) == FUNCTION_TYPE
-		  && comptypes (TREE_TYPE (TREE_TYPE (decl1)),
-				TREE_TYPE (ext_type)))
+		  && same_type_p (TREE_TYPE (TREE_TYPE (decl1)),
+				  TREE_TYPE (ext_type)))
 		{
 		  current_function_prototype_locus
 		    = DECL_SOURCE_LOCATION (ext_decl);
@@ -6493,8 +6493,8 @@ store_parm_decls_oldstyle (tree fndecl, 
 	  /* Type for passing arg must be consistent with that
 	     declared for the arg.  ISO C says we take the unqualified
 	     type for parameters declared with qualified type.  */
-	  if (!comptypes (TYPE_MAIN_VARIANT (DECL_ARG_TYPE (parm)),
-			  TYPE_MAIN_VARIANT (TREE_VALUE (type))))
+	  if (!same_type_ignoring_top_level_qualifiers_p (DECL_ARG_TYPE (parm),
+			  				  TREE_VALUE (type)))
 	    {
 	      if (TYPE_MAIN_VARIANT (TREE_TYPE (parm))
 		  == TYPE_MAIN_VARIANT (TREE_VALUE (type)))
Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 131960)
+++ c-typeck.c	(working copy)
@@ -466,7 +466,7 @@ composite_type (tree t1, tree t2)
 		    if (mv3 && mv3 != error_mark_node
 			&& TREE_CODE (mv3) != ARRAY_TYPE)
 		      mv3 = TYPE_MAIN_VARIANT (mv3);
-		    if (comptypes (mv3, mv2))
+		    if (same_type_p (mv3, mv2))
 		      {
 			TREE_VALUE (n) = composite_type (TREE_TYPE (memb),
 							 TREE_VALUE (p2));
@@ -491,7 +491,7 @@ composite_type (tree t1, tree t2)
 		    if (mv3 && mv3 != error_mark_node
 			&& TREE_CODE (mv3) != ARRAY_TYPE)
 		      mv3 = TYPE_MAIN_VARIANT (mv3);
-		    if (comptypes (mv3, mv1))
+		    if (same_type_p (mv3, mv1))
 		      {
 			TREE_VALUE (n) = composite_type (TREE_TYPE (memb),
 							 TREE_VALUE (p1));
@@ -858,7 +858,7 @@ common_type (tree t1, tree t2)
    but a warning may be needed if you use them together.  */
 
 int
-comptypes (tree type1, tree type2)
+comptypes (tree type1, tree type2, int strict ATTRIBUTE_UNUSED)
 {
   const struct tagged_tu_seen_cache * tagged_tu_seen_base1 = tagged_tu_seen_base;
   int val;
@@ -1037,7 +1037,7 @@ comp_target_types (tree ttl, tree ttr)
     mvl = TYPE_MAIN_VARIANT (mvl);
   if (TREE_CODE (mvr) != ARRAY_TYPE)
     mvr = TYPE_MAIN_VARIANT (mvr);
-  val = comptypes (mvl, mvr);
+  val = same_type_p (mvl, mvr);
 
   if (val == 2 && pedantic)
     pedwarn ("types are not quite compatible");
@@ -2403,7 +2403,7 @@ build_function_call (tree function, tree
        || TREE_CODE (function) == CONVERT_EXPR)
       && TREE_CODE (tem = TREE_OPERAND (function, 0)) == ADDR_EXPR
       && TREE_CODE (tem = TREE_OPERAND (tem, 0)) == FUNCTION_DECL
-      && !comptypes (fntype, TREE_TYPE (tem)))
+      && !same_type_p (fntype, TREE_TYPE (tem)))
     {
       tree return_type = TREE_TYPE (fntype);
       tree trap = build_function_call (built_in_decls[BUILT_IN_TRAP],
@@ -3628,8 +3628,8 @@ build_c_cast (tree type, tree expr)
       tree field;
 
       for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
-	if (comptypes (TYPE_MAIN_VARIANT (TREE_TYPE (field)),
-		       TYPE_MAIN_VARIANT (TREE_TYPE (value))))
+	if (same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (field),
+						       TREE_TYPE (value)))
 	  break;
 
       if (field)
@@ -4030,7 +4030,7 @@ convert_for_assignment (tree type, tree 
      This code doesn't fully support references, it's just for the
      special case of va_start and va_copy.  */
   if (codel == REFERENCE_TYPE
-      && comptypes (TREE_TYPE (type), TREE_TYPE (rhs)) == 1)
+      && same_type_p (TREE_TYPE (type), TREE_TYPE (rhs)) == 1)
     {
       if (!lvalue_p (rhs))
 	{
@@ -4070,7 +4070,7 @@ convert_for_assignment (tree type, tree 
   /* Aggregates in different TUs might need conversion.  */
   if ((codel == RECORD_TYPE || codel == UNION_TYPE)
       && codel == coder
-      && comptypes (type, rhstype))
+      && same_type_p (type, rhstype))
     return convert_and_check (type, rhs);
 
   /* Conversion to a transparent union from its member types.
@@ -4084,8 +4084,7 @@ convert_for_assignment (tree type, tree 
 	{
 	  tree memb_type = TREE_TYPE (memb);
 
-	  if (comptypes (TYPE_MAIN_VARIANT (memb_type),
-			 TYPE_MAIN_VARIANT (rhstype)))
+	  if (same_type_ignoring_top_level_qualifiers_p (memb_type, rhstype))
 	    break;
 
 	  if (TREE_CODE (memb_type) != POINTER_TYPE)
@@ -4704,7 +4703,7 @@ digest_init (tree type, tree init, bool 
       bool char_array = (typ1 == char_type_node
 			 || typ1 == signed_char_type_node
 			 || typ1 == unsigned_char_type_node);
-      bool wchar_array = !!comptypes (typ1, wchar_type_node);
+      bool wchar_array = !!same_type_p (typ1, wchar_type_node);
       if (char_array || wchar_array)
 	{
 	  struct c_expr expr;
@@ -4717,8 +4716,8 @@ digest_init (tree type, tree init, bool 
 	    = (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (inside_init)))
 	       == char_type_node);
 
-	  if (comptypes (TYPE_MAIN_VARIANT (TREE_TYPE (inside_init)),
-			 TYPE_MAIN_VARIANT (type)))
+	  if (same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (inside_init),
+							 type))
 	    return inside_init;
 
 	  if (!wchar_array && !char_string)
@@ -4767,8 +4766,8 @@ digest_init (tree type, tree init, bool 
       && TREE_CONSTANT (inside_init))
     {
       if (TREE_CODE (inside_init) == VECTOR_CST
-	  && comptypes (TYPE_MAIN_VARIANT (TREE_TYPE (inside_init)),
-			TYPE_MAIN_VARIANT (type)))
+	  && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (inside_init),
+							type))
 	return inside_init;
 
       if (TREE_CODE (inside_init) == CONSTRUCTOR)
@@ -4796,16 +4795,16 @@ digest_init (tree type, tree init, bool 
      from an expression of the same type, optionally with braces.  */
 
   if (inside_init && TREE_TYPE (inside_init) != 0
-      && (comptypes (TYPE_MAIN_VARIANT (TREE_TYPE (inside_init)),
-		     TYPE_MAIN_VARIANT (type))
+      && (same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (inside_init),
+						     type)
 	  || (code == ARRAY_TYPE
-	      && comptypes (TREE_TYPE (inside_init), type))
+	      && same_type_p (TREE_TYPE (inside_init), type))
 	  || (code == VECTOR_TYPE
-	      && comptypes (TREE_TYPE (inside_init), type))
+	      && same_type_p (TREE_TYPE (inside_init), type))
 	  || (code == POINTER_TYPE
 	      && TREE_CODE (TREE_TYPE (inside_init)) == ARRAY_TYPE
-	      && comptypes (TREE_TYPE (TREE_TYPE (inside_init)),
-			    TREE_TYPE (type)))))
+	      && same_type_p (TREE_TYPE (TREE_TYPE (inside_init)),
+			      TREE_TYPE (type)))))
     {
       if (code == POINTER_TYPE)
 	{
@@ -6246,8 +6245,7 @@ output_init_element (tree value, bool st
       && !(TREE_CODE (value) == STRING_CST
 	   && TREE_CODE (type) == ARRAY_TYPE
 	   && INTEGRAL_TYPE_P (TREE_TYPE (type)))
-      && !comptypes (TYPE_MAIN_VARIANT (TREE_TYPE (value)),
-		     TYPE_MAIN_VARIANT (type)))
+      && !same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (value), type))
     value = array_to_pointer_conversion (value);
 
   if (TREE_CODE (value) == COMPOUND_LITERAL_EXPR
Index: c-common.c
===================================================================
--- c-common.c	(revision 131960)
+++ c-common.c	(working copy)
@@ -1194,7 +1194,7 @@ vector_types_convertible_p (const_tree t
     return convertible_lax;
 
   if (TYPE_VECTOR_SUBPARTS (t1) == TYPE_VECTOR_SUBPARTS (t2)
-      && comptypes (TREE_TYPE (t1), TREE_TYPE (t2)))
+      && same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
     return true;
 
   if (emit_lax_note && !emitted_lax_note)
Index: c-common.h
===================================================================
--- c-common.h	(revision 131960)
+++ c-common.h	(working copy)
@@ -826,6 +826,7 @@ extern tree finish_label_address_expr (t
 
 /* Same function prototype, but the C and C++ front ends have
    different implementations.  Used in c-common.c.  */
+extern int comptypes (tree, tree, int);
 extern tree lookup_label (tree);
 extern tree lookup_name (tree);
 
@@ -881,6 +882,22 @@ enum lvalue_use {
   lv_asm
 };
 
+/* Returns nonzero iff TYPE1 and TYPE2 are the same type, in the usual
+   sense of `same'.  */
+#define same_type_p(TYPE1, TYPE2) \
+  comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
+
+/* Returns nonzero iff TYPE1 and TYPE2 are the same type, ignoring
+   top-level qualifiers.  */
+#define same_type_ignoring_top_level_qualifiers_p(TYPE1, TYPE2) \
+  same_type_p (TYPE_MAIN_VARIANT (TYPE1), TYPE_MAIN_VARIANT (TYPE2))
+
+/* Used with comptypes, and related functions, to guide type
+   comparison.  */
+
+#define COMPARE_STRICT	      0 /* Just check if the types are the
+				   same.  */
+
 extern void lvalue_error (enum lvalue_use);
 
 extern int complete_array_type (tree *, tree, bool);
Index: c-parser.c
===================================================================
--- c-parser.c	(revision 131960)
+++ c-parser.c	(working copy)
@@ -5456,7 +5456,7 @@ c_parser_postfix_expression (c_parser *p
 	    e1 = TYPE_MAIN_VARIANT (groktypename (t1));
 	    e2 = TYPE_MAIN_VARIANT (groktypename (t2));
 
-	    expr.value = comptypes (e1, e2)
+	    expr.value = same_type_p (e1, e2)
 	      ? build_int_cst (NULL_TREE, 1)
 	      : build_int_cst (NULL_TREE, 0);
 	    expr.original_code = ERROR_MARK;

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[H.J. Lu]

On Thu, Feb 07, 2008 at 02:09:45PM +0100, Paolo Bonzini wrote:
> Here it is.  The problem is that C and C++ call two different things 
> "comptypes".  What C calls comptypes, C++ calls same_type_p.  In turn, C++ 
> does have a comptypes but it has an additional third argument; same_type_p 
> (a, b) == comptypes (a, b, 0) == <C front-end comptypes> (a, b).
>
> The solution is to give the same signature to the C and C++ front-end's 
> comptypes functions.  Furthermore, I moved the same_type_p macro from C++ 
> to c-common.h so that the whole family of languages can use the same name 
> for the same thing.  Therefore, a large part of the patch renames comptypes 
> calls to same_type_p or same_type_ignoring_top_level_qualifiers_p.
>
> The patch may seem a little more complicated than Doug's because it touches 
> multiple languages, but actually it kills more code than it adds!  In 
> particular, it removes an ugly hack in the ObjC++ front-end to mediate 
> between objc/objc-act.c (which uses a two-argument comptypes) and 
> cp/typeck.c (which implements a three-argument comptypes).
>
> Bootstrapped/regtested i686-pc-linux-gnu, C/C++/ObjC/ObjC++/Fortran/Java.  
> Ok for mainline together with reverting Doug's changes?
>

Will your patch work with g++.dg/ext/vector13.C for PR c++/35096,
which is the part for Doug's changes?

BTW, could you please mention PR c++/35049 and PR c++/35096 in your
ChangeLog?

Thanks.

H.J.

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

> Will your patch work with g++.dg/ext/vector13.C for PR c++/35096,
> which is the part for Doug's changes?

It works with whatever vector13.C is already in svn.

> BTW, could you please mention PR c++/35049 and PR c++/35096 in your
> ChangeLog?

Yes.

Paolo

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[H.J. Lu]

On Thu, Feb 07, 2008 at 03:06:23PM +0100, Paolo Bonzini wrote:
>
>> Will your patch work with g++.dg/ext/vector13.C for PR c++/35096,
>> which is the part for Doug's changes?
>
> It works with whatever vector13.C is already in svn.

I tried your patch and it doesn't work for me:

bash-3.2$ /export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../g++ -B/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../ /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C -nostdinc++ -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include -I/export/gnu/src/gcc/gcc/libstdc++-v3/libsupc++ -I/export/gnu/src/gcc/gcc/libstdc++-v3/include/backward -I/export/gnu/src/gcc/gcc/libstdc++-v3/testsuite/util -fmessage-length=0   -ansi -pedantic-errors -Wno-long-long -fno-show-column -S -o foo.s
/export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C:6: error:
zero-size array ‘x’
bash-3.2$ 

It is

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35113



H.J.

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

On Feb 7, 2008 9:58 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Feb 07, 2008 at 03:06:23PM +0100, Paolo Bonzini wrote:
> >
> >> Will your patch work with g++.dg/ext/vector13.C for PR c++/35096,
> >> which is the part for Doug's changes?
> >
> > It works with whatever vector13.C is already in svn.
>
> I tried your patch and it doesn't work for me:
>
> bash-3.2$ /export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../g++ -B/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../ /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C -nostdinc++ -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include -I/export/gnu/src/gcc/gcc/libstdc++-v3/libsupc++ -I/export/gnu/src/gcc/gcc/libstdc++-v3/include/backward -I/export/gnu/src/gcc/gcc/libstdc++-v3/testsuite/util -fmessage-length=0   -ansi -pedantic-errors -Wno-long-long -fno-show-column -S -o foo.s
> /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C:6: error:
> zero-size array 'x'

Sorry, that was my mistake; I'd copied the testcase from the PR
directly, tested it in isolation, and committed it. Then I later
realized the -pedantic failure due to  the zero-length array, and
committed a quick fix:

  typedef const int X __attribute((vector_size(8)));
  extern const int x[] __attribute((vector_size(8)));
  X x[] = { 5 };

  - Doug

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Jakub Jelinek]

On Thu, Feb 07, 2008 at 06:58:52AM -0800, H.J. Lu wrote:
> On Thu, Feb 07, 2008 at 03:06:23PM +0100, Paolo Bonzini wrote:
> >
> >> Will your patch work with g++.dg/ext/vector13.C for PR c++/35096,
> >> which is the part for Doug's changes?
> >
> > It works with whatever vector13.C is already in svn.
> 
> I tried your patch and it doesn't work for me:
> 
> bash-3.2$ /export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../g++ -B/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../ /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C -nostdinc++ -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include -I/export/gnu/src/gcc/gcc/libstdc++-v3/libsupc++ -I/export/gnu/src/gcc/gcc/libstdc++-v3/include/backward -I/export/gnu/src/gcc/gcc/libstdc++-v3/testsuite/util -fmessage-length=0   -ansi -pedantic-errors -Wno-long-long -fno-show-column -S -o foo.s
> /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C:6: error:
> zero-size array ‘x’
> bash-3.2$ 
> 
> It is
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35113

Maybe you missed
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132154
?

	Jakub

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Richard Guenther]

On Feb 7, 2008 4:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Feb 07, 2008 at 06:58:52AM -0800, H.J. Lu wrote:
> > On Thu, Feb 07, 2008 at 03:06:23PM +0100, Paolo Bonzini wrote:
> > >
> > >> Will your patch work with g++.dg/ext/vector13.C for PR c++/35096,
> > >> which is the part for Doug's changes?
> > >
> > > It works with whatever vector13.C is already in svn.
> >
> > I tried your patch and it doesn't work for me:
> >
> > bash-3.2$ /export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../g++ -B/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/../../ /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C -nostdinc++ -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include -I/export/gnu/src/gcc/gcc/libstdc++-v3/libsupc++ -I/export/gnu/src/gcc/gcc/libstdc++-v3/include/backward -I/export/gnu/src/gcc/gcc/libstdc++-v3/testsuite/util -fmessage-length=0   -ansi -pedantic-errors -Wno-long-long -fno-show-column -S -o foo.s
> > /export/gnu/src/gcc/gcc/gcc/testsuite/g++.dg/ext/vector13.C:6: error:
> > zero-size array 'x'
> > bash-3.2$
> >
> > It is
> >
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35113
>
> Maybe you missed
> http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132154
> ?

But now it ICEs for me again...

/space/rguenther/src/svn/trunk/gcc/testsuite/g++.dg/ext/vector13.C:6:
internal compiler error: canonical types differ for identical types
const int __vector__ [] and const int __vector__ []^M
Please submit a full bug report,^M
with preprocessed source if appropriate.^M
See <http://gcc.gnu.org/bugs.html> for instructions.^M

FAIL: g++.dg/ext/vector13.C (internal compiler error)
FAIL: g++.dg/ext/vector13.C (test for excess errors)

Richard.

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

On Feb 7, 2008 10:11 AM, Richard Guenther <richard.guenther@gmail.com> wrote:
> But now it ICEs for me again...
>
> /space/rguenther/src/svn/trunk/gcc/testsuite/g++.dg/ext/vector13.C:6:
> internal compiler error: canonical types differ for identical types
> const int __vector__ [] and const int __vector__ []^M
> Please submit a full bug report,^M
> with preprocessed source if appropriate.^M
> See <http://gcc.gnu.org/bugs.html> for instructions.^M
>
> FAIL: g++.dg/ext/vector13.C (internal compiler error)
> FAIL: g++.dg/ext/vector13.C (test for excess errors)

I'm seeing this on i686-pc-linux-gnu, but not i386-apple-darwin9. This
looks like a typical canonical-types failure, where we have two
equivalent ARRAY_TYPE nodes whose canonical types are different.  I'm
looking into it.

The issue is independent of the issue with comptypes, and I happen to
like Paolo's patch better than my own. Despite its size, it's a far
cleaner approach.

  - Doug

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

> I'm seeing this on i686-pc-linux-gnu, but not i386-apple-darwin9. This
> looks like a typical canonical-types failure, where we have two
> equivalent ARRAY_TYPE nodes whose canonical types are different.  I'm
> looking into it.

Thanks, I took a look for half an hour but I'm raising white flag.

> The issue is independent of the issue with comptypes, and I happen to
> like Paolo's patch better than my own. Despite its size, it's a far
> cleaner approach.

Thanks for confirming it (that the issue is independent, not that you 
like my patch though I appreciate it). :-)

Paolo

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Andreas Krebbel]

> 2008-02-05  Douglas Gregor  <doug.gregor@gmail.com>
> 
> 	PR c++/35049
> 	* typeck.c (structural_comptypes): Call cp_comptypes.
> 	(comptypes): New; called from the C/C++ common bits to perform
> 	strict checks.
> 	(cp_comptypes): Renamed from comptypes, which is already used,
> 	with a different signature, by the C++ front end.
> 	(build_reinterpret_cast_1): Call cp_comptypes.
> 	(ptr_reasonably_similar): Ditto.
> 	* decl.c (decls_match): Ditto.
> 	* cvt.c (convert_to_reference): Ditto.
> 	* cp-tree.h (same_type_p): Ditto.
> 	(same_or_base_type_p): Ditto.
> 	(comptypes): Rename to cp_comptypes.
> 	* pt.c (canonical_type_parameter): Call cp_comptypes.

This breaks objcp bootstrap for me.  You probably forgot to adjust the
objcp front end, did you?

Bye,

-Andreas-

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

On Feb 7, 2008 1:52 PM, Andreas Krebbel <Andreas.Krebbel@de.ibm.com> wrote:
> > 2008-02-05  Douglas Gregor  <doug.gregor@gmail.com>
> >
> >       PR c++/35049
> >       * typeck.c (structural_comptypes): Call cp_comptypes.
> >       (comptypes): New; called from the C/C++ common bits to perform
> >       strict checks.
> >       (cp_comptypes): Renamed from comptypes, which is already used,
> >       with a different signature, by the C++ front end.
> >       (build_reinterpret_cast_1): Call cp_comptypes.
> >       (ptr_reasonably_similar): Ditto.
> >       * decl.c (decls_match): Ditto.
> >       * cvt.c (convert_to_reference): Ditto.
> >       * cp-tree.h (same_type_p): Ditto.
> >       (same_or_base_type_p): Ditto.
> >       (comptypes): Rename to cp_comptypes.
> >       * pt.c (canonical_type_parameter): Call cp_comptypes.
>
> This breaks objcp bootstrap for me.  You probably forgot to adjust the
> objcp front end, did you?

Yes, I did. I've just committed Andreas Tobler's patch to clear this
bootstrap issue with Objective-C++, and submitted a patch for the
issue with g++.dg/ext/vector13.C

  - Doug

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Joseph S. Myers]

On Thu, 7 Feb 2008, Paolo Bonzini wrote:

> 1) bulk-rename the calls to comptypes in the C front-end to same_type_p

That name would be misleading.  The test is for compatibility, not 
sameness; int[] and int[5] are compatible, not the same.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

Joseph S. Myers wrote:
> On Thu, 7 Feb 2008, Paolo Bonzini wrote:
> 
>> 1) bulk-rename the calls to comptypes in the C front-end to same_type_p
> 
> That name would be misleading.  The test is for compatibility, not 
> sameness; int[] and int[5] are compatible, not the same.

Is the same true for C++?  If we settle on a name like comp_type_p, we 
should change it for both C and C++.  I can commit a patch that chooses 
a different name, but the one I posted would anyway be much smaller and 
easier to review; so I'd prefer to have the renaming of same_type_p 
preapproved on top of the patch I already posted.

Paolo

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Joseph S. Myers]

On Fri, 8 Feb 2008, Paolo Bonzini wrote:

> Joseph S. Myers wrote:
> > On Thu, 7 Feb 2008, Paolo Bonzini wrote:
> > 
> > > 1) bulk-rename the calls to comptypes in the C front-end to same_type_p
> > 
> > That name would be misleading.  The test is for compatibility, not sameness;
> > int[] and int[5] are compatible, not the same.
> 
> Is the same true for C++?  If we settle on a name like comp_type_p, we should
> change it for both C and C++.  I can commit a patch that chooses a different
> name, but the one I posted would anyway be much smaller and easier to review;
> so I'd prefer to have the renaming of same_type_p preapproved on top of the
> patch I already posted.

I can't speak for the semantics of functions in the C++ front end (or how 
many different but related concepts there may be in the C++ language).  
I'd prefer a more precise name such as types_compatible_p, rather than an 
abbreviated "comp" ("compatible"? "compare"?).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

On Feb 8, 2008 10:33 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Fri, 8 Feb 2008, Paolo Bonzini wrote:
> > Joseph S. Myers <joseph@codesourcery.com> wrote:
> > > That name would be misleading.  The test is for compatibility, not
> > > sameness; int[] and int[5] are compatible, not the same.
> > Is the same true for C++?  If we settle on a name like comp_type_p, we should
> > change it for both C and C++.  I can commit a patch that chooses a different
> > name, but the one I posted would anyway be much smaller and easier to review;
> > so I'd prefer to have the renaming of same_type_p preapproved on top of the
> > patch I already posted.
>
> I can't speak for the semantics of functions in the C++ front end (or how
> many different but related concepts there may be in the C++ language).
> I'd prefer a more precise name such as types_compatible_p, rather than an
> abbreviated "comp" ("compatible"? "compare"?).

C++'s comptypes (with its "strict" argument equal to 0) is a
comparison to determine whether two types are exactly the same, so
int[] and int[5] would be different.Of course, that's C++'s notion of
compatibility; C++ handles assignment from one type to another by
introducing implicit conversions to make the types the same.

So, comptypes is still the right name, I think, although "comp" is
rather ambiguous: is it "compare" or "compatible"?

  - Doug

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

>> I'd prefer a more precise name such as types_compatible_p, rather than an
>> abbreviated "comp" ("compatible"? "compare"?).

Fine; the disadvantage is that a lot of lines would wrap.

> C++'s comptypes (with its "strict" argument equal to 0) is a
> comparison to determine whether two types are exactly the same, so
> int[] and int[5] would be different.  Of course, that's C++'s notion of
> compatibility; C++ handles assignment from one type to another by
> introducing implicit conversions to make the types the same.
> 
> So, comptypes is still the right name, I think, although "comp" is
> rather ambiguous: is it "compare" or "compatible"?

So we would have to add a ", 0" to every comptypes call in the C front-end.

Paolo

Re: [C++ PATCH] Battle of the comptypes (PR c++/35049)

[Joseph S. Myers]

On Fri, 8 Feb 2008, Doug Gregor wrote:

> On Feb 8, 2008 10:33 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > On Fri, 8 Feb 2008, Paolo Bonzini wrote:
> > > Joseph S. Myers <joseph@codesourcery.com> wrote:
> > > > That name would be misleading.  The test is for compatibility, not
> > > > sameness; int[] and int[5] are compatible, not the same.
> > > Is the same true for C++?  If we settle on a name like comp_type_p, we should
> > > change it for both C and C++.  I can commit a patch that chooses a different
> > > name, but the one I posted would anyway be much smaller and easier to review;
> > > so I'd prefer to have the renaming of same_type_p preapproved on top of the
> > > patch I already posted.
> >
> > I can't speak for the semantics of functions in the C++ front end (or how
> > many different but related concepts there may be in the C++ language).
> > I'd prefer a more precise name such as types_compatible_p, rather than an
> > abbreviated "comp" ("compatible"? "compare"?).
> 
> C++'s comptypes (with its "strict" argument equal to 0) is a
> comparison to determine whether two types are exactly the same, so
> int[] and int[5] would be different.Of course, that's C++'s notion of
> compatibility; C++ handles assignment from one type to another by
> introducing implicit conversions to make the types the same.

C also has different rules for assignment; compatibility is only one case 
where assignment is permitted, there are several others.

If the underlying concepts or their names are too divergent in the two 
standards, perhaps we should not be thinking in terms of naming functions 
the same in the two front ends; C++ should use names based on the terms in 
the C++ standard, and C should used names based on the terms in the C 
standard.  Instead, we should work out a definition of the semantics 
required by the common code that is meaningful for both C and C++, give a 
function name (not "comptypes") to that definition, and add 
implementations of that definition to both the C and C++ front ends; the 
common code can then use that function.

We should also eliminate includes of c-tree.h from all files shared 
between C and C++ (including some files outside the front ends altogether, 
I see); much of c-tree.h is specific to C, such as the declaration of 
comptypes that caused the present problems.  Anything needed by the shared 
C/C++ files and genuinely common can move to c-common.h; this includes 
definitions of functions with a shared interface but two different 
implementations.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Mark Mitchell]

Paolo Bonzini wrote:

> Bootstrapped/regtested i686-pc-linux-gnu, 
> C/C++/ObjC/ObjC++/Fortran/Java.  Ok for mainline together with reverting 
> Doug's changes?

Like Doug, I think this is the right idea.

I also agree with Joseph that we need to make the names for these 
functions map onto the concepts used in the language standards -- but 
perhaps we can do that later?  So, I suggest that we just change the C 
front-end version of comptypes to add the extra parameter (as in your 
patch) but leave out the same_type_p and 
same_type_ignoring_top_level_qualifiers_p changes.

Joseph, what do you think of that?

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Joseph S. Myers]

On Sun, 10 Feb 2008, Mark Mitchell wrote:

> Paolo Bonzini wrote:
> 
> > Bootstrapped/regtested i686-pc-linux-gnu, C/C++/ObjC/ObjC++/Fortran/Java.
> > Ok for mainline together with reverting Doug's changes?
> 
> Like Doug, I think this is the right idea.
> 
> I also agree with Joseph that we need to make the names for these functions
> map onto the concepts used in the language standards -- but perhaps we can do
> that later?  So, I suggest that we just change the C front-end version of
> comptypes to add the extra parameter (as in your patch) but leave out the
> same_type_p and same_type_ignoring_top_level_qualifiers_p changes.
> 
> Joseph, what do you think of that?

My suggestion is not to change names or signatures of front-end-internal 
functions such as comptypes, but to add a new function with a different 
name that does what's required by the common code (it only appears to be 
one call to comptypes in c-common.c) and define it in each front end as a 
wrapper to the front-end-internal function.  Then we can deal separately 
with such things as the naming of functions within the front ends and 
stopping common code from using c-tree.h.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

> My suggestion is not to change names or signatures of front-end-internal 
> functions such as comptypes, but to add a new function with a different 
> name that does what's required by the common code (it only appears to be 
> one call to comptypes in c-common.c) and define it in each front end as a 
> wrapper to the front-end-internal function.  Then we can deal separately 
> with such things as the naming of functions within the front ends and 
> stopping common code from using c-tree.h.

So, for 4.3 we could start with this patch:

Index: c-common.c
===================================================================
--- c-common.c  (revision 131976)
+++ c-common.c  (working copy)
@@ -1194,7 +1194,7 @@ vector_types_convertible_p (const_tree t
      return convertible_lax;

    if (TYPE_VECTOR_SUBPARTS (t1) == TYPE_VECTOR_SUBPARTS (t2)
-      && comptypes (TREE_TYPE (t1), TREE_TYPE (t2)))
+      && lang_hooks.types_compatible_p (TREE_TYPE (t1), TREE_TYPE (t2)))
      return true;

    if (emit_lax_note && !emitted_lax_note)

Could someone please bootstrap/regtest it for me?

Paolo

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Doug Gregor]

On Feb 11, 2008 8:40 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> > It bootstraps on i686-pc-linux-gnu, but g++.dg/ext/vector13.C test still fails.
>
> The other half of the patch was approved a few hours ago, but has not
> been committed yet.

That other half of the patch has now been committed .

  - Doug

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Paolo Bonzini]

Uros Bizjak wrote:
> Hello!
> 
>> So, for 4.3 we could start with this patch:
> 
> ...
> 
>> Could someone please bootstrap/regtest it for me?
> 
> It bootstraps on i686-pc-linux-gnu, but g++.dg/ext/vector13.C test still fails.

The other half of the patch was approved a few hours ago, but has not 
been committed yet.

Paolo

Re: [C/C++/ObjC/ObjC++ PATCH] Battle of the comptypes (PR c++/35049)

[Uros Bizjak]

Hello!

> So, for 4.3 we could start with this patch:

...

> Could someone please bootstrap/regtest it for me?

It bootstraps on i686-pc-linux-gnu, but g++.dg/ext/vector13.C test still fails.

Uros.