On 30/10/2007, jerry gay <[EMAIL PROTECTED]> wrote: > On Oct 30, 2007 1:58 PM, Paul Cochrane <[EMAIL PROTECTED]> 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
