Re: A whole bunch of bug reports and patches

[Trent Lange <lange@CS.UCLA.EDU>]

William --

Thanks for your response:

>Trent Lange <lange@CS.UCLA.EDU> writes:
>> - Added single-float, double-float, long-float, and short-float to
>>   the list of types that ONLY-SIMPLE-TYPES recognizes, to allow
>>   loop variables to use them as type specs.
>
>Well, page 743 of CLtL2 says that the ``of-type'' can be left out if
>the type spec contains only uses of fixnum, float, t, nil.  I guess
>enumerating the subtypes of float is reasonable.

You're right;  I missed that part of the specification.  Maybe it
would be best to leave it the way you had it -- I got in trouble
with this in the first place because Lucid's implementation of
LOOP let me get away with the SINGLE-FLOAT simple type-spec.

>> - Modified PARSE-REPEAT to declare the temporary variable used in
>>   the REPEAT iteration construct as type fixnum (where before it
>>   was undeclared).  Because the user's count form might not be a fixnum,
>>   PARSE-REPEAT also sets up the form to check that, using the FLOOR of
>>   its value if it is not.
>
>But it might be larger than MOST-POSITIVE-FIXNUM.

Good point.  I don't know *why* anybody would ever have a repeat greater
than MOST-POSITIVE-FIXNUM, but they might.  However, the efficiency
gained in declaring the repeat variable as a fixnum means that something
should be done.  (A bare repeat loop declared as fixnum is almost ten
times faster in unsafe code than when it is undeclared).

At a minimum the first test that directly checks whether the COUNT-FORM
is a fixnum or a (THE FIXNUM ...) form could be safely kept.  (This is
one of those places I wish common lisp had a function that let you query
the compiler to determine a form's result type, which would make the
above test far more robust).

As for the other tests;  well, it would definitely be more efficient to
have the generated code test whether the repeat count is a FIXNUM at
runtime, and have it use a FIXNUM declared variable if so, and a INTEGER
declared variable otherwise.  But I could see where that would get a
little hairy.

Too bad the specifications didn't take this into account and just specify
that the repeat count must be a fixnum, and have users use the FOR ..  FROM
construct if they wanted to do something strange.  But at least the first
case can be optimized, when the LOOP macro can determine in advance that
the count is a FIXNUM.

>The problem is that it pops the last value off the list, sets j to
>(car nil), then tests to see if the list is empty.
>
>I could change it to move the endp test before assigning J, but what
>would I do for the first iteration?  I would have to decide there was
>nothing to do before J was ever bound.  But it's not clear that the
>LOOP semantics allow that.  Ack.

What I did when writing my own optimized version of DOLIST was to check
ENDP before assigning J, as you suggest, and have the code look approximately
like so:

(let ((list-ptr <list>)
      (j <any-default-of-j's-type>))
  ...
  (tagbody
    (if (null list-ptr)
        (loop-finish))
    again (setf j (car list-ptr))
	  ... body-stuff ...
	  (if (null (setf list-ptr (cdr list-ptr)))
	      (loop-finish))
         (go again)
    loop-finish
    ...))

(My FAST-DOLIST has other optimizations, like not allocating a variable
for J if it's used only once in the body, but they probably wouldn't
applicable here for portability's sake.)

So it does explicitly decide that there was nothing to do before J was
ever bound (to an element of the list).  This should conform to the spec,
because I don't believe the value of the var is undefined at the end of
the list (or when the list was null in the first place).  It also has
the (very slight) advantage that it ends up calling CAR one less time
in every case.

I'd also be tempted to not even allocate the J temporary var until
after the first test to see if the list was null initially.  This
would speed things up in the null-list case, but would of course
mean that J wouldn't be defined in the scope of the LOOP-FINISH --
which is probably the way it should be, gien it's value is undefined
anyway.  But this may be going too far.

----------

And finally, thanks much for adding SYMBOL-MACROLET to the walker; I'll
pick up your changes and include them in PCL once I finally get this
revision out.

- Trent