"Marco Antoniotti" marcoxa@cs.nyu.edu sayeth thusly:
Hello there.
thanks a lot for all the work. I have read through the messages and I agree with your analysis with a few comments.
I think all the changes you propose are worth including.
Let me comment on a few of these.
Woo! Glad to know I didn't get myself lost digging around. :)
On Jan 16, 2010, at 06:43 , Pixel // pinterface wrote:
[snip]
Patches 0 and 2 help somewhat.
- Extract template handling of MATCH[ING] into %TEMPLATE-FOR-MATCH
Should be fairly self-explanatory; not much of a win in lines.
- Extract the bits that wrap forms with bindings for template variables
This patch extracts the assorted flet GENERATE-VAR-BINDINGS into a single common function. It's slightly less straightforward than that, however, because it also extracts the (let ...) forms which actually used those bindings.
This results in a decent win for code size, but in some cases it swaps the order of execution of %TEMPLATE-FOR-MATCH and COLLECT-TEMPLATE-VARS. I'm pretty sure this doesn't have any noticable effect, but thorough testing is probably wise.
This is good. However, I would set up a bunch of regression tests before moving on and change things. I usually use the test suite from Franz, but I'd be willing to change to another test suite.
Agreed. More tests are needed. I'll add that to my TODO list. :)
I have no preference for any particular test suite; largely because I (regrettably) have no experience with any of the myriad of test suites available. I'm good with whatever.
[snip]
** MATCH
Though documented, MATCH's behavior when its body forms signal UNIFICATION-FAILURE seems at odds with expectations, for pretty much the same reason the behavior seems nonsensical in MATCH-CASE. Was it a deliberate design decision, or a side-effect of how MATCH was implemented?
Never attribute to malice what can be attributed to incompetence. Of course it is a side-effect of the implementation :) I knew I needed a test suite :)
Well, "incompetence" hardly seems fair to yourself! It's a tricky bit, that.
** :MATCH-NAMED, :MATCHING-NAMED, :MATCH-CASE-NAMED
It seems a bit redundant to have <macro-name>-named arguments for block names rather than simply having a :named argument as in, say, LOOP. Any particular reason it wasn't done that way?
Having the :NAMED option would be preferable, with NIL or the macro name as fall-back.
Should :<macro-name>-NAMED also work, for backwards-compatibility? If yes, should it warn about deprecation during compilation? (A warning that would undoubtedly be lost in the scroll of compilation output, but c'est la vie.)
** Apologies
Sorry about the massive brain dump. I /may/ have gotten carried away. :)
Absolutely not. All of this is most welcome. I am just a little swamped (what an euphemism!) and maybe not all that reactive. But let's keep the discussion going.
Oh, good. I was worried. :)