Re: [PATCH] Re: Canonical type nodes, or, comptypes considered harmful

[Mike Stump <mrs at apple dot com>]

On Nov 21, 2006, at 11:06 AM, Doug Gregor wrote:
> Make DEPTH=6, we get an 85% speedup:

Yes, this mirrors the type of speed up I expect for _some_ types of  
template code.  I'd love to see us go in this direction.  Anyway, I  
endorse this type of work.

Anyway, on to the review...

Any thoughts of enabling the checking code with --enable-checking and  
running it that way.  The speed type people already know how to  
disable it for speed testing, and releases turn it off automatically,  
and stage 1 runs the checker.  Something like:

#ifdef ENABLE_CHECKING
#define VERIFY_CANONICAL_TYPES 1
#endif

I'd like to see some random larger C++ code shoved though it to  
ensure if doesn't fall over, if you structure it that way, however,  
if instead you just warn:

+#ifdef VERIFY_CANONICAL_TYPES
+      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, "internal: canonical types differ for identical  
types %T and %T",
+               t1, t2);
+         debug_tree (t1);
+         debug_tree (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, "internal: same canonical type node for  
different types %T and %T",
+                t1, t2);
+         debug_tree (t1);
+         debug_tree (t2);
+       }
+
+      return result;
+#else

or even maybe just a note, it'll make it just a bit safer in the  
short term.  People can then watch for these messages and report  
them.  I've been known to do this type of code before and the warning  
definitely was nicer as the complex cases came in after release in my  
case.

Other than that, this looks like a nice step in the right direction,  
thanks.