Re: clisp - general interest in code cleanup

[Daniel J. <dan...@gm...>]

Hi Don,

Don Cohen writes:
>
> Daniel Jour writes:
>  > > > Looking at the code base, I think this low activity stems
> My interpretation has been that the low activity is largely due to the
> code being GOOD - although I've found things I'd like to change, these
> have been easy enough to work around to make the cost/benefit favor
> leaving them.

Please don't get me wrong: The code is good. It's implementing good logic,
making mostly sensible use of the available tools of the C language (I'm
focusing on the runtime implementation). But:

 * The files are huge and contain way too much. Related stuff, but in
principle stuff that makes up separate parts, and could and should be
kept separate.

 * Many functions are also huge, and contain really a lot sections that
are subject to conditional compilation, depending on the target platform
and the selected configuration.

The implication is that in order to make well informed changes to one
part of the code, you also need to know how it interacts with the rest
of the code. If there would be a clean interface then this is not a problem,
because the interface should clearly describe these interactions.

Assume a hobbyist user found a bug in the parser. She's presented with
a 10k+ lines src/io.d which includes (the result of preprocessing) a 17k+
lines src/lispbibl.d. That's a lot of code to even quickly skim over, in order
to make sure not to introduce any unwanted side effects. The
implementation notes help a lot, but provide more of a high level overview,
not so much the interactions between the various code parts.

I think this difficulty to get working with the code is demonstrated by the
low development activity, the age and number of open bugs and the low
number of feature requests (general tail call optimization?).

 * There seems to be a lot of code determining the "available features"
and the "configuration" of the target machine which should be, to at
least some extend, handled by the C library respectively gnulib.

 * Undefined behavior, probably mostly in spvw* and also sort.d:
Stuff like comparing (less, greater) pointers, out of bounds indexing
and some cast just aren't legal C, and can lead to all sorts of nasty bugs,
especially when using an optimizing compiler. I'd guess that's the reason
that (e.g.) my distributions build instructions pass in CFLAGS=-O0.

There's probably more in the direction of general code readability. (A
fun thing to do is the following: Collect the names of all local variables
per function of a source file and the names of the functions. Then shuffle
and try to figure out which set of local variables is part of which function.)

I hope the above didn't make a harsh or arrogant impression, it's basically
how I start when e.g. reviewing code on stackoverflow, and based on the
experiences I made in the last 14 years. I don't assume it to be the "only
way", and I'm completely open to revising my view :) After all, I've never
received any mentoring or formal teaching in programming (I'm not
counting my university ... I can tell with certainty now that universities are
not places that teach "how to code").

> Over the years I've suggested a few small changes on the mailing
> list that never made it as far as patches, partly cause I didn't even
> know that patches were being collected.
> I'd appreciate if some of those could be included when the patches are
> incorporated.

I will search for them in the mailing lists. The more input, the better, IMO.
SF doesn't support searching for names, though, and the search provided
by gmane seems to be working, but "open ended", so it's probable that
I miss some posts. Do you have any "record" of your suggestions?


> Just wondering, how much interest is there in different linux versions,
> and especially old ones?  Some of my nightly test builds are on
> Fedora Core release 4 (2.6.17-1.2142_FC4).

AFAIK the "core" of CLISP doesn't have much dependencies. libsigsegv,
libffi and libffcall (and perhaps readline) are listed as dependencies in
my package description, ldd only shows glibc and pthreads. Assuming
proper use of gnulib CLISP should probably be able to run on most of the
platforms supported by gnulib, which would also include older linux
versions.

CLISP is also known for its portability (and availability, IIRC it was part of
the default installation of debian in the past. At least it's on every machine
in my universities computer pools, and AFAIK the system administrator
hasn't installed it on purpose) so IMO it would be good if a new release
wouldn't break this backwards compability.

But, being completely new here, that's just me guessing :)


>  > How "extensive" are these self-tests?
> You'll see.
> I consider them to be very extensive.
> Sometimes they begin to feel interminable.

Yes, I had the time to read over some of them now. That's basically a huge
suite of regression tests, and that's awesome :)

Daniel