Bytecode format changed for function signatures?

Question

(defun magit-max-args-internal (function)
    "Return the maximum number of arguments accepted by FUNCTION."
    (if (symbolp function)
        (setq function (symbol-function function)))
    (if (subrp function)
        (let ((max (cdr (subr-arity function))))
          (if (eq 'many max)
              most-positive-fixnum
            max))
      (if (eq 'macro (car-safe function))
          (setq function (cdr function)))
      (let ((arglist (if (byte-code-function-p function)
                         (aref function 0) ; <--------- format changed
                       (cadr function))))
        (if (memq '&rest arglist)
            most-positive-fixnum
          (length (remq '&optional arglist))))))

I had to recompile magit.el and discovered this problem in their code. If I follow the code correctly, then what they were after here is the function's arity, but instead they are getting some "strange" number. Any ideas what happened?

In addition, this post: Elisp get function arity? offers a better solution (which does the job just fine, the answer by Andreas Röhler. So I will probably try to suggest it to magit maintainers.

At this opportunity, you may probably want to ask why they even need to determine a function's arity at runtime. That's a strange thing to do, and usually a good indication of doing something generally wrong. — , Oct 01 '13 at 19:58
@lunaryorn I kind of see the reason. Looks like they are trying to cater for some older versions of some built-ins, which changed the number of arguments. It would take some effort to unearth the earlier versions of `delete-directory`, but from looking at it, there is a good chance that it had a different number of arguments once, the `trash` argument meaning "move to trash" would probably appear later then the function itself. — , Oct 01 '13 at 20:09
Yes, `delete-directory` originally had only what is now its first arg. When code is expected to work for different Emacs versions it is not unusual to have to call the same function with a different calling sequence. Checking the arity is one way to try to deal with this, but sometimes the number of arguments is not different but their types or meanings are different. Other approaches are (a) checking the Emacs version and (b) trying different calls inside `conditon-case` or `ignore-errors`. — Drew, Oct 01 '13 at 22:33
@wvxvw: You mention "built-ins". FWIW, if it is only the arity of a built-in that they want to check then they can do so directly, using `subr-arity`. (Perhaps you meant something different from built-in -- perhaps you meant a function provided by the vanilla code?) — Drew, Oct 01 '13 at 22:35
@Drew yup, I didn't mean C-only, all code that says "part of GNU Emacs". Why isn't `help-function-arglist` a good option? It seems straight-forward? (To check Emacs version you would need to maintain a historical list of functions with their signatures, which seems less generic). — , Oct 02 '13 at 04:22
@wvxvw: I didn't suggest that `help-function-arglist` isn't a good option. I only meant to say that (a) yes, as you said, there is no predefined Emacs function to get the arity, (b) yes, as you said, there are times when checking the arity is useful, and (c) using `condition-case` and `wrong-number-of-arguments` handling is another way to deal with the problem of a changed calling sequence. `help-function-arglist` is yet another approach, but it is not available in some older Emacs versions. — Drew, Oct 02 '13 at 05:30
@Drew It looks like though it has already been fixed, I just had some earlier version of `magit`. https://github.com/magit/magit/issues/975 for the future reference. — , Oct 02 '13 at 05:55
@wvxvw I still see no reason. As Stefan outlined in his answer, they should better just try to call the function with all arguments first, and then gradually degrade to calls with fewer arguments, if `wrong-number-of-arguments` is signaled. — , Oct 02 '13 at 12:21
@lunaryorn well, that's their work, I'm not really responsible for how they manage it... But if it was up to me, I'd not do it like that, and especially not in this function: it has optional arguments, so it may eventually work with less arguments, then the new version, so you may be misguided to believe that you are using the old one. And it's deleting *directories*! Think of what would've happened if you believed that you moved a directory to trash, but in fact, you just deleted it forever... that's not the place where I'd like to try and see what happens :) — , Oct 02 '13 at 12:27
@wvxvw Uhm, I am not sure I can follow you. I can't see why optional arguments are relevant. Imho they don't change the game. You call `(delete-directory "foo" t)` first to attempt a direct recursive delete, and fall back to a manual recursive delete based on the single-argument variant of `delete-directory`, if `wrong-number-of-arguments` is signaled. What could possibly go wrong here?! In fact, that's much more obvious and predictable, than counting arguments, which is much more likely to be defeated by optional args… — , Oct 02 '13 at 12:39
@lunaryorn you absolutely don't want to first attempt a direct delete. Trust me... What if the error isn't the error you think it is? What if the error is due to a race condition? And so on. I mean, there could be tons of different issues with this, and especially those I can't think about at the moment, but their plausibility is enough to scare me :) What if by this trial and error you removed someoene's most precious memories from their PC? — , Oct 02 '13 at 13:48
@wvxvw I still fail to follow your dramatization. The whole point of `delete-directory` is, well, to delete a directory. Calling it twice would at worst cause an error to be signaled in the second invocation, because the directory does not exist anymore. But anyway, Emacs will signal the `wrong-number-of-arguments` if and only if a function was called with a wrong number of arguments. With a `condition-case` expression such as in Stefan's answer, only this error will be caught. All other errors propagate, and do not cause a repeated call of `delete-directory`. — , Oct 02 '13 at 18:07

score 1 · Answer 1 · answered Oct 02 '13 at 01:18

Indeed this "number in (aref bytecode 0)" was introduced for lexical-binding. The better fix is to throw away magit-max-args-internal and use (condition-case nil (delete-directory <args>) (wrong-number-of-arguments (delete-directory <fewerargs>)) instead.

Bytecode format changed for function signatures?

1 Answers1