Re: [AC] Re: [PATCH 21.5] Allow autoloaded DEFUN

"Stephen J. Turnbull" <stephen(a)xemacs.org&gt; wrote: Hmmmm. We need to do different things for builtin functions and for functions defined in modules. Builtin functions: The check_sane_subr() call is a noop unless DEBUG_XEMACS is defined. (Incidentally, this means that no new DEFUNs should be added to the stable XEmacs branch, unless somebody does a build with DEBUG_XEMACS defined first.) The original assert is correct in that case; no builtin should be defining a symbol that already has an autoload cookie, since we should call this code *before* any Lisp code runs. Module functions: These DEFUNs should *always* be checked with the DEBUG_XEMACS version of check_sane_subr, even if DEBUG_XEMACS is not defined. At the very least, we should be sure that a DEFUN is not stomping on an existing function. This way, we avoid loading a module that defines a function already defined by some other module. (This will be a real possibility very soon, by the way. Stay tuned.) However, in this case, we do not want to use an assert(). We should signal an error, which will be caught back in the module loader in emodules.c (see module_load_unwind), causing the module loading process to be halted. One possibility is to do this in check_module_subr(). So now I think we need something like this patch. The idea is to restore check_sane_subr to its original form, but only apply it if !initialized. That way, builtin functions are checked, but not module functions, thereby avoiding the assertion failure that got me started on all this. The body of check_module_subr was already wrapped in a check for initialized, so it is only applied to module DEFUNs. Then we basically duplicate the checks in check_sane_subr in check_module_subr, but signal an error instead of asserting if something fails. Also, the extern declaration of Qdll_error is gross; I'll do something about that for the final version of this patch. Also also, I added a parameter to check_module_subr(), because I abhor macros that assume the existence of variables with a certain name. Index: src/symbols.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/symbols.c,v retrieving revision 1.38 diff -d -u -r1.38 symbols.c --- src/symbols.c 2002/07/08 08:22:00 1.38 +++ src/symbols.c 2002/07/09 16:28:52 ＠＠ -3358,25 +3358,24 ＠＠ } #ifdef DEBUG_XEMACS -/* Check that nobody spazzed writing a DEFUN. */ +/* Check that nobody spazzed writing a builtin (non-module) DEFUN. */ static void check_sane_subr (Lisp_Subr *subr, Lisp_Object sym) { - Lisp_Object f; - - assert (subr->min_args >= 0); - assert (subr->min_args <= SUBR_MAX_ARGS); + if (!initialized) { + assert (subr->min_args >= 0); + assert (subr->min_args <= SUBR_MAX_ARGS); - if (subr->max_args != MANY && - subr->max_args != UNEVALLED) - { - /* Need to fix lisp.h and eval.c if SUBR_MAX_ARGS too small */ - assert (subr->max_args <= SUBR_MAX_ARGS); - assert (subr->min_args <= subr->max_args); - } + if (subr->max_args != MANY && + subr->max_args != UNEVALLED) + { + /* Need to fix lisp.h and eval.c if SUBR_MAX_ARGS too small */ + assert (subr->max_args <= SUBR_MAX_ARGS); + assert (subr->min_args <= subr->max_args); + } - f = XSYMBOL (sym)->function; - assert (UNBOUNDP (f) || (CONSP (f) && EQ (XCAR (f), Qautoload))); + assert (UNBOUNDP (XSYMBOL (sym)->function)); + } } #else #define check_sane_subr(subr, sym) /* nothing */ ＠＠ -3407,17 +3406,44 ＠＠ * FIXME: Should newsubr be staticpro()'ed? I don't think so but I need * a guru to check. */ -#define check_module_subr() \ -do { \ - if (initialized) { \ - Lisp_Subr *newsubr = (Lisp_Subr *) xmalloc (sizeof (Lisp_Subr)); \ - memcpy (newsubr, subr, sizeof (Lisp_Subr)); \ - subr->doc = (const char *)newsubr; \ - subr = newsubr; \ - } \ +extern Lisp_Object Qdll_error; /* Defined in emodules.c */ +#define check_module_subr(subr) \ +do { \ + if (initialized) { \ + Lisp_Subr *newsubr; \ + Lisp_Object f; \ + \ + if (subr->min_args < 0) \ + signal_ferror (Qdll_error, "%s min_args (%hd) too small", \ + subr_name (subr), subr->min_args); \ + if (subr->min_args > SUBR_MAX_ARGS) \ + signal_ferror (Qdll_error, "%s min_args (%hd) too big (max = %d)", \ + subr_name (subr), subr->min_args, SUBR_MAX_ARGS); \ + \ + if (subr->max_args != MANY && \ + subr->max_args != UNEVALLED) \ + { \ + /* Need to fix lisp.h and eval.c if SUBR_MAX_ARGS too small */ \ + if (subr->max_args > SUBR_MAX_ARGS) \ + signal_ferror (Qdll_error, "%s max_args (%hd) too big (max = %d)", \ + subr_name (subr), subr->max_args, SUBR_MAX_ARGS); \ + if (subr->min_args > subr->max_args) \ + signal_ferror (Qdll_error, "%s min_args (%hd) > max_args (%hd)", \ + subr_name (subr), subr->min_args, subr->max_args); \ + } \ + \ + f = XSYMBOL (sym)->function; \ + if (!UNBOUNDP (f) && (!CONSP (f) || !EQ (XCAR (f), Qautoload))) \ + signal_ferror (Qdll_error, "Attempt to redefine %s", subr_name (subr)); \ + \ + newsubr = (Lisp_Subr *) xmalloc (sizeof (Lisp_Subr)); \ + memcpy (newsubr, subr, sizeof (Lisp_Subr)); \ + subr->doc = (const char *)newsubr; \ + subr = newsubr; \ + } \ } while (0) #else /* ! HAVE_SHLIB */ -#define check_module_subr() +#define check_module_subr(subr) #endif void ＠＠ -3427,7 +3453,7 ＠＠ Lisp_Object fun; check_sane_subr (subr, sym); - check_module_subr (); + check_module_subr (subr); fun = wrap_subr (subr); XSYMBOL (sym)->function = fun; ＠＠ -3441,7 +3467,7 ＠＠ Lisp_Object fun; check_sane_subr (subr, sym); - check_module_subr(); + check_module_subr (subr); fun = wrap_subr (subr); XSYMBOL (sym)->function = Fcons (Qmacro, fun); -- Jerry James http://www.ittc.ku.edu/~james/