On Fri, Jan 31, 2003 at 02:44:47PM -0600, Robert E. Brown wrote:
> Here are the changes for list.lisp that I did not send to the sbcl-devel
> mailing list because of the message size constraint.
(I should straighten out or work around the stupid interoperability
issues with my browser and SourceForge list admin CGI so that I can
let messages like this through again when appropriate. Mea culpa.)
Other than that, I have several remarks.
> First, the patch gets rid of all warnings like these from list.lisp:
> ; note: unable to
> ; optimize away possible call to FDEFINITION at runtime
> ; due to type uncertainty:
> ; The first argument is a (OR FUNCTION SYMBOL), not a FUNCTION.
> To remove them, I added type declarations and also introduced new lexical
> bindings. Here's what I did to subst:
Optimization notes aren't really style warnings. I don't ordinarily
add code just to remove them if they're complaining about having to
use a general-purpose expansion when, in fact, what I really want is a
general-purpose expansion. (This happens not only in things like
FUNCALL, but also in AREF of general arrays and non-fixnum INTEGER
arithmetic and so forth.)
However, that doesn't mean that I don't approve of most or all the
changes in the patch, since they seem to be actual efficiency
improvements, lifting the coercions to FUNCTION out of loop bodies so
that they're only done once. Even if I'm not motivated to add code
simply to get rid of optimization notes about generalized operations,
it seems reasonable to add or rearrange code in order to move the
expensive general operations somewhere where they get executed less
> I use symbol-function in functionize instead of (coerce ... 'function),
> because coerce handles function names, but the arguments to all the list
> manipulation functions are required to be function designators.
> (defmacro functionize (x)
> `(if (functionp ,x) ,x (symbol-function ,x)))
There is already a built-in operator %COERCE-CALLABLE-TO-FUN
which does something like this, and my first impression is that
it's appropriate here. A bit of poking around in the Hyperspec
suggests that we're dealing with "function designators" and not
"generalized function designators" in most or all cases here, so
we don't need to worry about sick corner cases like
(tree-equal x y :test '(setf foo))
so maybe either of them would work, but I won't know for sure
without looking at it more carefully.
As a minor style nit, by and large we try to do things like this as
functions (possibly inline functions) instead of macros. There's a lot
of gratuitous macrology in the code, but mostly it dates from very old
CMU CL code, and shouldn't be taken as something to emulate. Today
when we can express something just as well as a function, we almost
always use a function. This is typical modern Lisp style, since for
ordinary app code DECLAIM INLINE works nicely, and functions are
easier to read and less error-prone. Within SBCL itself can either
use DECLAIM INLINE, or use things like DEFTRANSFORM to give us
finer-grained control, e.g. expanding inline conditional on (> SPEED
SPACE) optimization policy, or expanding differently depending on the
inferred types of the arguments.
> The test for the presence of both :test and :test-not,
> (when (and testp notp)
> (error ":TEST and :TEST-NOT were both supplied."))
> was present in the old verion of the with-set-keys macro and in the old
> version of union. I added the test to the beginning of all the functions
> where it made sense and removed the test from the with-set-keys macro.
That sounds fairly reasonable, although I haven't yet turned the patch
into a context diff to walk through what it means in practice.
(Context or universal diffs, "diff -c" or "diff -u", are usually
easier to follow.)
> Finally, I applied a FIXME suggestion to the assoc-guts macro.
Probably sometime in the next next week I'll go through your patch,
perhaps turning FUNCTIONIZE into something like %COERCE-CALLABLE-TO-FUN
and perhaps tweaking some other things too, and merge at least most of
it into SBCL.
William Harold Newman <william.newman@...>
Furious activity is no substitute for understanding. -- H. H. Williams
PGP key fingerprint 85 CE 1C BA 79 8D 51 8C B9 25 FB EE E0 C3 E5 7C