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]

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


On 11/22/06, Mike Stump <mrs@apple.com> wrote:
ensure if doesn't fall over, if you structure it that way, however,
if instead you just warn:
[snip code using warnings]
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.

I like the idea of making it a warning. To get the warning, of course, we need to slow down compilation significantly to do the checking, hence ENABLE_CHECKING. Now, one thing we need to keep in mind here is that the failure mode is pretty bad when the canonical types are wrong. I am concerned that, no matter how much code we cram through with this checking enabled, we're going to miss some cases. Most likely, we'll end up rejecting valid code that used to work; when we're unlucky, we'll miscompile code that used to work. Ugh.

So, here's a variant that might just work: add a flag variable
flag_check_canonical_types.  When it's true, we do the complete
structural checking, verify it against the canonical types result, and
warn if they differ. (This is what we do now when
VERIFY_CANONICAL_TYPES is defined).

When it's false, we just use canonical types for fast comparison of types.

So, we pay for one extra branch in comptypes, but when we miscompile
the user's code we can tell them to add "-fcheck-canonical-types" to
their command line and report the resulting warnings. Their code will
now compile correctly (albeit with warnings, since we're using
structural type comparison), and the warnings will tell us what we
need to fix for the next version.

For the hook into --enable-checking, we use:

 int flag_check_canonical_types =
 #ifdef ENABLE_CHECKING
   1
 #else
   0
 #endif
 ;

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

Thanks!


 Cheers,
 Doug


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