[prev in list] [next in list] [prev in thread] [next in thread] 

List:       perl6-internals
Subject:    Re: [svn:parrot] r22585 - trunk/src
From:       "Paul Cochrane" <paultcochrane () gmail ! com>
Date:       2007-10-30 21:50:42
Message-ID: 8e8ee0d40710301450l68c82b32jae9b02021b585b6a () mail ! gmail ! com
[Download RAW message or body]

On 30/10/2007, jerry gay <jerry.gay@gmail.com> wrote:
> On Oct 30, 2007 1:58 PM, Paul Cochrane <paultcochrane@gmail.com> wrote:
> > > Modified: trunk/src/inter_create.c
> > > ==============================================================================
> > > --- trunk/src/inter_create.c    (original)
> > > +++ trunk/src/inter_create.c    Mon Oct 29 05:59:52 2007
> > > @@ -135,7 +135,7 @@
> > >      create_initial_context(interp);
> > >      interp->resume_flag = RESUME_INITIAL;
> > >      /* main is called as a Sub too - this will get depth 0 then */
> > > -    CONTEXT(interp->ctx)->recursion_depth = -1;
> > > +    CONTEXT(interp->ctx)->recursion_depth = (UINTVAL)-1;
> > >      interp->recursion_limit = RECURSION_LIMIT;
> > >
> >
> > Does casting -1 to unint make any sense?  It looks here like if
> > recursion depth has a negative number then something has gone wrong
> > (in which case I don't know why it was declared unsigned int to begin
> > with).  I've seen these errors/warnings for a while and wondered
> > whether or not to make the cast, but I believe there is a deeper
> > problem.  Why are we wanting to assign -1 to recursion_depth to start
> > with?  This is also the case where the variable 'c' is set to -1
> > above.  'c' is ostensibly a character, so why is it being set to -1?
> > I agree with getting rid of the warnings (there many others which need
> > attention) but I think we should get rid of them in the right way (and
> > yes, I do realise this is *me* the one who writes terrible C code
> > talking here ;-)  ).
> >
> recursion_depth is a UINTVAL, so it can't contain a negative number.
> -1, when assigned to a UINTVAL (which translates to unsigned long, at
> least on win32) becomes 4294967295. so, setting an unsigned thing to
> -1 is a C hack to set to it's maximum value, due to the way numbers
> are represented internally.
>
> since we have warnings turned way up on parrot source, we get a
> warning unless we cast the -1 to the proper type. this makes the -1
> hack much less elegant looking, and more confusing. it's probably time
> to change all the 'foo = (type)-1' to 'foo = MAXTYPE' to reduce the
> magic number count in the source, and make the intent of the code more
> clear.

I agree with this sentiment most definitely.  Clarity is a Good Thing.

Paul
[prev in list] [next in list] [prev in thread] [next in thread]