This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Make canonical type failures ICE


Hello,

The attached patch makes failures in the canonical type system ICE the
compiler. I had been waiting to submit this oft-requested patch until
I was relatively comfortable that we've flushed out most of the bugs
in the canonical type system.

The patch is straightforward. When checking is enabled, we verify
canonical types. Failures cause an ICE.

There is a workaround, which I suggest we keep for a while. It's the
"use-canonical-types" parameter. When 1 (the default), we use the
canonical type system, checking the results only when ENABLE_CHECKING
is defined and going fast when ENABLE_CHECKING is not defined. When 0,
we do the normal (slow) structural type comparisons that we've always
done. If we manage to miss bugs in the canonical type system, we can
tell users to add --param use-canonical-types=0 as a workaround.

I have bootstrapped C/C++/ObjC/Obj-C++/Java, regtested them, and
regtested libstdc++, all on i686-pc-linux-gnu. At this point, there
are no outstanding PRs against the canonical type system. That said,
this patch *will* cause ICEs where we previously only warned; I'll try
to react quickly to fix those problems that surface. You may want to
consider giving me permission to commit fixes to the canonical type
system without waiting for approval, to minimize down-time.

Okay for mainline?

- Doug

2007-07-04 Douglas Gregor <doug.gregor@gmail.com>

	* params.def (PARAM_VERIFY_CANONICAL_TYPES): Remove.
	(PARAM_USE_CANONICAL_TYPES): New; decides whether to use canonical
	types or not.
	* params.h (VERIFY_CANONICAL_TYPES): Remove.
	(USE_CANONICAL_TYPES): New.

2007-07-04 Douglas Gregor <doug.gregor@gmail.com>

	* invoke.texi (verify-canonical-types): Remove.
	(use-canonical-types): Add.

2007-07-04 Douglas Gregor <doug.gregor@gmail.com>

	* typeck.c (comptypes): When USE_CANONICAL_TYPES, use the
	canonical types; otherwise, fall back to structural type
	comparisons. If ENABLE_CHECKING and USE_CANONICAL_TYPES, give an
	internal compiler error if the canonical types are wrong.
Index: params.def
===================================================================
--- params.def	(revision 126302)
+++ params.def	(working copy)
@@ -673,19 +673,16 @@ DEFPARAM (PARAM_L1_CACHE_LINE_SIZE,
 	  "The size of L1 cache line",
 	  32, 0, 0)
 
-#ifdef ENABLE_CHECKING
-# define GCC_CANONICAL_TYPES_DEFAULT 1
-#else
-# define GCC_CANONICAL_TYPES_DEFAULT 0
-#endif
+/* Whether we should use canonical types rather than deep "structural"
+   type checking.  Setting this value to 1 (the default) improves
+   compilation performance in the C++ and Objective-C++ front end;
+   this value should only be set to zero to work around bugs in the
+   canonical type system by disabling it.  */
 
-/* Whether we should verify that the canonical types in the system are
-   consistent with the "structural" typing. */
-
-DEFPARAM (PARAM_VERIFY_CANONICAL_TYPES,
-	  "verify-canonical-types",
-	  "Whether to verify canonical types",
-	  GCC_CANONICAL_TYPES_DEFAULT, 0, 1)
+DEFPARAM (PARAM_USE_CANONICAL_TYPES,
+	  "use-canonical-types",
+	  "Whether to use canonical types",
+	  1, 0, 1)
 /*
 Local variables:
 mode:c
Index: params.h
===================================================================
--- params.h	(revision 126302)
+++ params.h	(working copy)
@@ -168,6 +168,6 @@ typedef enum compiler_param
   PARAM_VALUE (PARAM_L1_CACHE_SIZE)
 #define L1_CACHE_LINE_SIZE \
   PARAM_VALUE (PARAM_L1_CACHE_LINE_SIZE)
-#define VERIFY_CANONICAL_TYPES \
-  PARAM_VALUE (PARAM_VERIFY_CANONICAL_TYPES)
+#define USE_CANONICAL_TYPES \
+  PARAM_VALUE (PARAM_USE_CANONICAL_TYPES)
 #endif /* ! GCC_PARAMS_H */
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 126302)
+++ doc/invoke.texi	(working copy)
@@ -6954,12 +6954,12 @@ The size of cache line in L1 cache, in b
 @item l1-cache-size
 The number of cache lines in L1 cache.
 
-@item verify-canonical-types
-Whether the compiler should verify the ``canonical'' types used for
-type equality comparisons within the C++ and Objective-C++ front
-ends. Set to 1 (the default when GCC is configured with
---enable-checking) to enable verification, 0 to disable verification
-(the default when GCC is configured with --disable-checking).
+@item use-canonical-types
+Whether the compiler should use the ``canonical'' type system.  By
+default, this should always be 1, which uses a more efficient internal
+mechanism for comparing types in C++ and Objective-C++.  However, if
+bugs in the canonical type system are causing compilation failures,
+set this value to 0 to disable canonical types.
 
 @end table
 @end table
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 126302)
+++ cp/typeck.c	(working copy)
@@ -1108,8 +1108,6 @@ comptypes (tree t1, tree t2, int strict)
 {
   if (strict == COMPARE_STRICT)
     {
-      bool result;
-
       if (t1 == t2)
 	return true;
 
@@ -1121,37 +1119,34 @@ comptypes (tree t1, tree t2, int strict)
 	   perform a deep check. */
 	return structural_comptypes (t1, t2, strict);
 
-      if (VERIFY_CANONICAL_TYPES)
+#ifdef ENABLE_CHECKING
+      if (USE_CANONICAL_TYPES)
 	{
-	  result = structural_comptypes (t1, t2, strict);
-
+	  bool result = structural_comptypes (t1, t2, strict);
+	  
 	  if (result && TYPE_CANONICAL (t1) != TYPE_CANONICAL (t2))
-	    {
-	      /* The two types are structurally equivalent, but their
-		 canonical types were different. This is a failure of the
-		 canonical type propagation code.*/
-	      warning(0,
-		      "canonical types differ for identical types %T and %T", 
-		      t1, t2);
-	      debug_tree (t1);
-	      debug_tree (t2);
-	    }
+	    /* The two types are structurally equivalent, but their
+	       canonical types were different. This is a failure of the
+	       canonical type propagation code.*/
+	    internal_error 
+	      ("canonical types differ for identical types %T and %T", 
+	       t1, t2);
 	  else if (!result && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2))
-	    {
-	      /* Two types are structurally different, but the canonical
-		 types are the same. This means we were over-eager in
-		 assigning canonical types. */
-	      warning (0, 
-		       "same canonical type node for different types %T and %T",
-		       t1, t2);
-	      debug_tree (t1);
-	      debug_tree (t2);
-	    }
+	    /* Two types are structurally different, but the canonical
+	       types are the same. This means we were over-eager in
+	       assigning canonical types. */
+	    internal_error 
+	      ("same canonical type node for different types %T and %T",
+	       t1, t2);
 	  
 	  return result;
 	}
-      else
+#else
+      if (USE_CANONICAL_TYPES)
 	return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
+#endif
+      else
+	return structural_comptypes (t1, t2, strict);
     }
   else if (strict == COMPARE_STRUCTURAL)
     return structural_comptypes (t1, t2, COMPARE_STRICT);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]