[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] %nil-handling optimization and fixes v1
From: |
Andy Wingo |
Subject: |
Re: [PATCH] %nil-handling optimization and fixes v1 |
Date: |
Thu, 23 Jul 2009 23:38:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.92 (gnu/linux) |
Hi Mark,
This is also not a patch review yet :)
On Thu 09 Jul 2009 18:11, Mark H Weaver <address@hidden> writes:
> I added the following macros, whose names explicitly state how %nil
> should be handled. See the comments in the patch for more information
> about these.
>
> scm_is_false_assume_not_lisp_nil scm_is_true_assume_not_lisp_nil
> scm_is_false_and_not_lisp_nil scm_is_true_or_lisp_nil
> scm_is_false_or_lisp_nil scm_is_true_and_not_lisp_nil
>
> scm_is_lisp_false scm_is_lisp_true
>
> scm_is_null_assume_not_lisp_nil
> scm_is_null_and_not_lisp_nil
> scm_is_null_or_lisp_nil
>
> scm_is_bool_and_not_lisp_nil
> scm_is_bool_or_lisp_nil
These are terrible names. But they seem to be the best names for the
concepts we're trying to express. I don't understand all of them yet,
will wait for a review -- unless Neil takes care of that before I do ;-)
> The following already-existing macros are defined as aliases, such
> that their semantics is unchanged (although scm_is_bool used to be a
> function and is now a macro).
>
> scm_is_null --> scm_is_null_and_not_lisp_nil
> scm_is_false --> scm_is_false_and_not_lisp_nil
> scm_is_true --> scm_is_true_or_lisp_nil
> scm_is_bool --> scm_is_bool_and_not_lisp_nil
This part sounds right to me, based on the current semantics.
> (I still believe that these should be changed to versions that handle
> %nil properly, but await approval on that point, so these patches do
> not make those changes)
Yes, this also sounds right to me.
> Also, if the preprocessor macro SCM_ENABLE_ELISP is not true (this
> macro already existed and was used in lang.h), all overheads
> associated with %nil handling are eliminated from the above macros.
Excellent. Hacks like this are excellent :-)
> vm-fixes.patch changes semantics, by fixing %nil handling in the
> following instructions: br-if, br-if-not, br-if-null, br-if-not-null,
> not, not-not, null?, and not-null?
Sounds great in principle, though i have not looked at it
> srfi-1-fixes.patch changes semantics, by fixing %nil handling in
> several functions. Note that this patch (and several other large
> forthcoming patches) will be unnecessary if the scm_is_false,
> scm_is_true, and scm_is_null macros are changed to handle %nil as I
> proposed.
Hm. Perhaps we should decide first.
> non-essential.patch is the last and least important. It doesn't
> change any functionality or implementation. It changes two
> occurrences of scm_is_bool and scm_is_null, in which %nil must *not*
> be treated as a boolean or null, to use newly-added equivalent macros
> which are explicit about how nil should be handled. These changes
> will be needed if scm_is_null is changed as I proposed. It also adds
> a few comments related to %nil handling.
>
> I've run "make check" on recent git master (c4b681fd) with these
> patches applied, and everything seems to work.
Sounds good too.
> I haven't yet run any benchmarks, because I'm not sure how to best do
> that. I doubt the changes will make any noticeable difference except
> possibly in C code which does a lot of tests which include %nil.
Yeah I don't expect too many differences either. Still, nice to clean
up.
Regards,
Andy
--
http://wingolog.org/