bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Andrea Greselin]

[-- Attachment #1: Type: text/plain, Size: 5294 bytes --]

Hi all,
I have this function (the part at issue here should be the first
line):

    (defun my-recenter-after-jump (window _window-start-after)
      "Recenter the point after a non-scroll command brings it out of view.
    This function is meant to be called from the hook
    ‘window-scroll-functions’."
      (with-selected-window window
        (unless (or (> 0.001 (float-time (time-subtract (current-time)
buffer-display-time))) ; Don’t run this function after a change of buffer.
Non-nil here means the selected window displayed the current buffer less
than a thousandth of a second ago, so it was most likely a change of buffer
that triggered ‘window-scroll-functions’, not a scroll. See also
https://emacs.stackexchange.com/questions/63558/how-to-distinguish-a-scroll-from-a-change-of-buffer-in-a-function-called-from-w
                    ;; Don’t recenter for these
                    ;;   - major modes
                    (memq major-mode '(help-mode ;
           ‘my-recenter-after-jump’ breaks scrolling with SPC/<backspace>
in Help mode and Info mode because they don’t obey
‘scroll-preserve-screen-position’ and ‘scroll-error-top-bottom’.
                                       Info-mode))
                    ;;   - minor modes
                    isearch-mode ;
           For Isearch, change the value of ‘scroll-conservatively’ and
‘hscroll-step’ in ‘isearch-update-post-hook’ and ‘isearch-mode-end-hook’.
Doing so allows recentering the point even after a purely horizontal scroll.
                    (bound-and-true-p view-mode) ;
           ‘my-recenter-after-jump’ breaks scrolling with SPC/<backspace>
in View mode because View mode doesn’t obey
‘scroll-preserve-screen-position’ and ‘scroll-error-top-bottom’.
                    (bound-and-true-p follow-mode)
                    ;;   - commands
                    (and (symbolp last-command) (get last-command
'scroll-command)) ; Don’t check the value of the ‘scroll-command’ property
for anonymous commands (it can’t be t anyway).
                    (memq last-command '(recenter-top-bottom
                                         query-replace query-replace-regexp
;         Query replacement is taken care of by the ‘recentering-on-jump’
advice to ‘perform-replace’, which allows recentering the point after a
purely horizontal scroll.
                                         previous-line next-line ;
           Don’t let these commands trigger ‘my-recenter-after-jump’. It
happens when Emacs has a moment of lag during which the point slips beyond
the scroll margin.
                                         previous-logical-line
next-logical-line)))
          (let* ((bottom-line-before (line-number-at-pos (window-end)))
                 (bottom-line-after (line-number-at-pos (window-end nil
'update))) ; NB: ‘window-start’ doesn’t take the ‘update’ argument.
                 (vertical-displacement (- bottom-line-after
bottom-line-before)))
            (unless (> 2 (abs vertical-displacement)) ; The purpose of
having the first ‘unless’ separate from this one is to call
‘line-number-at-pos’ only when it’s needed, because it can be slow. See (2
links)
https://emacs.stackexchange.com/questions/51648/how-to-detect-the-number-of-lines-scrolled-from-scroll-up-down/51664#51664,
https://emacs.stackexchange.com/questions/3821/a-faster-method-to-obtain-line-number-at-pos-in-large-buffers
              (recenter)
              (when truncate-lines
                (my-horizontal-recenter))))))) ; REVIEW: Isn’t there a
built-in function for recentering the point horizontally?

which I call from `window-scroll-functions` like this

    (add-hook 'window-scroll-functions #'my-recenter-after-jump)


It's worked until I upgraded from v. 28.1 (from Fedora’s repos) to
29.0.50 (commit 6de88b6b02). Now I get

    Error in window-scroll-functions (my-recenter-after-jump):
(wrong-number-of-arguments (lambda (window _window-start-after) "Recenter
the point after a non-scroll command brings it out of view.
    This function is meant to be called from the hook
    ‘window-scroll-functions’." (let ((save-selected-window--state
(internal--before-with-selected-window window))) (save-current-buffer
(unwind-protect (progn (select-window (car save-selected-window--state)
'norecord) (if (or (> 0.001 (float-time (time-subtract (current-time)
buffer-display-time))) (memq major-mode '(help-mode Info-mode))
isearch-mode (and (boundp 'view-mode) view-mode) (and (boundp 'follow-mode)
follow-mode) (and (symbolp last-command) (get last-command
'scroll-command)) (memq last-command '(recenter-top-bottom query-replace
query-replace-regexp previous-line next-line previous-logical-line
next-logical-line))) nil (let* ((bottom-line-before (line-number-at-pos
(window-end))) (bottom-line-after (line-number-at-pos (window-end nil
'update))) (vertical-displacement (- bottom-line-after
bottom-line-before))) (if (> 2 (abs vertical-displacement)) nil (recenter)
(if truncate-lines (progn (my-horizontal-recenter)))))))
(internal--after-with-selected-window save-selected-window--state))))) 0)

All the best,
Andrea

[-- Attachment #2: Type: text/html, Size: 6326 bytes --]

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Eli Zaretskii]

> From: Andrea Greselin <greselin.andrea@gmail.com>
> Date: Sat, 13 Aug 2022 18:17:54 +0200
> 
> It's worked until I upgraded from v. 28.1 (from Fedora’s repos) to
> 29.0.50 (commit 6de88b6b02). Now I get
> 
>     Error in window-scroll-functions (my-recenter-after-jump):
> (wrong-number-of-arguments (lambda (window _window-start-after) "Recenter
> the point after a non-scroll command brings it out of view.
>     This function is meant to be called from the hook
>     ‘window-scroll-functions’." (let ((save-selected-window--state
> (internal--before-with-selected-window window))) (save-current-buffer
> (unwind-protect (progn (select-window (car save-selected-window--state)

The backtrace seems to show that a function that is called is
different from the one you show as your hook function.  So I think you
should look closer into how you set up window-scroll-functions.

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> The backtrace seems to show that a function that is called is
> different from the one you show as your hook function.  So I think you
> should look closer into how you set up window-scroll-functions.

I too have seen this problem just yesterday.  There must be something
going on, it is unlikely that this is a coincidence.

In the backtrace I saw that the function had been called by name with
zero arguments.  But on top of the backtrace the symbol-function had
been shown (a closure).  I also wondered about that.

I verified that the function was only accessible via
`window-scroll-functions'.  My code did not use the function in any
other place.

Some entries below in the backtrace was `redisplay' - so... somehow
`redisplay' is sometimes calling functions in `window-scroll-functions'
with zero arguments.  Maybe the buffer local binding of
`window-scroll-functions'.

Please tell whether this information suffices and you already have some
idea or if I should try to create a recipe.  I guess this must be
related to a recent change from within the last few days.

Michael.

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Andrea Greselin]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

On Sat, 13 Aug 2022 at 19:30, Eli Zaretskii <eliz@gnu.org> wrote:

>
> > The backtrace seems to show that a function that is called is
> > different from the one you show as your hook function.  So I think you
> > should look closer into how you set up window-scroll-functions.
>

‘my-recenter-after-jump’ is the only thing I add to
‘window-scroll-functions’.

Andrea

[-- Attachment #2: Type: text/html, Size: 708 bytes --]

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Please tell whether this information suffices and you already have some
> idea or if I should try to create a recipe.  I guess this must be
> related to a recent change from within the last few days.

Reverting a part of

48215c41d1 New debugging facility: backtraces from errors in Lisp called
from redisplay (Alan Mackenzie <acm@muc.de> 2022-08-11)

like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Test-for-57179.patch --]
[-- Type: text/x-diff, Size: 944 bytes --]

From ffb7c3d558b599bcc84f1bf4fb4388569892d927 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Sun, 14 Aug 2022 02:14:58 +0200
Subject: [PATCH] Test for #57179

---
 src/xdisp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 719b131baa..47eaddabce 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18221,8 +18221,8 @@ run_window_scroll_functions (Lisp_Object window, struct text_pos startp)
     {
       specpdl_ref count = SPECPDL_INDEX ();
       specbind (Qinhibit_quit, Qt);
-      safe_run_hooks_2
-	(Qwindow_scroll_functions, window, make_fixnum (CHARPOS (startp)));
+      run_hook_with_args_2 (Qwindow_scroll_functions, window,
+			    make_fixnum (CHARPOS (startp)));
       unbind_to (count, Qnil);
       SET_TEXT_POS_FROM_MARKER (startp, w->start);
       /* In case the hook functions switch buffers.  */
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 106 bytes --]


seems to fix my incarnation of this problem.  Can it be that?  CC'ing
Alan (the author).

TIA,

Michael.

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Andrea Greselin <greselin.andrea@gmail.com>,  57179@debbugs.gnu.org,
>  Alan Mackenzie <acm@muc.de>
> Date: Sun, 14 Aug 2022 02:19:31 +0200
> 
> Michael Heerdegen <michael_heerdegen@web.de> writes:
> 
> > Please tell whether this information suffices and you already have some
> > idea or if I should try to create a recipe.  I guess this must be
> > related to a recent change from within the last few days.
> 
> Reverting a part of
> 
> 48215c41d1 New debugging facility: backtraces from errors in Lisp called
> from redisplay (Alan Mackenzie <acm@muc.de> 2022-08-11)

Indeed, the new function safe_run_hooks_2 has a bug.  In a build with
enable-checking I get an assertion violation in safe_run_hook_funcall,
because nargs is not 2 but 4.

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Alan Mackenzie]

Hello, Michael and Andrea.

On Sun, Aug 14, 2022 at 02:19:31 +0200, Michael Heerdegen wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:

> > Please tell whether this information suffices and you already have some
> > idea or if I should try to create a recipe.  I guess this must be
> > related to a recent change from within the last few days.

> Reverting a part of

> 48215c41d1 New debugging facility: backtraces from errors in Lisp called
> from redisplay (Alan Mackenzie <acm@muc.de> 2022-08-11)

> like this:


> From ffb7c3d558b599bcc84f1bf4fb4388569892d927 Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Sun, 14 Aug 2022 02:14:58 +0200
> Subject: [PATCH] Test for #57179
> 
> ---
>  src/xdisp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/xdisp.c b/src/xdisp.c
> index 719b131baa..47eaddabce 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -18221,8 +18221,8 @@ run_window_scroll_functions (Lisp_Object window, struct text_pos startp)
>      {
>        specpdl_ref count = SPECPDL_INDEX ();
>        specbind (Qinhibit_quit, Qt);
> -      safe_run_hooks_2
> -	(Qwindow_scroll_functions, window, make_fixnum (CHARPOS (startp)));
> +      run_hook_with_args_2 (Qwindow_scroll_functions, window,
> +			    make_fixnum (CHARPOS (startp)));
>        unbind_to (count, Qnil);
>        SET_TEXT_POS_FROM_MARKER (startp, w->start);
>        /* In case the hook functions switch buffers.  */
> --
> 2.30.2
> 


> seems to fix my incarnation of this problem.  Can it be that?  CC'ing
> Alan (the author).

Yes, this was indeed the problem.  Can I ask you please, instead of
applying your patch (above) to try out the following patch, which works
for me.

Thanks!



diff --git a/src/keyboard.c b/src/keyboard.c
index 8a2b7d58c4..1d7125a0a3 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1832,8 +1832,16 @@ adjust_point_for_property (ptrdiff_t last_pt, bool modified)
 static Lisp_Object
 safe_run_hooks_1 (ptrdiff_t nargs, Lisp_Object *args)
 {
-  eassert (nargs == 2);
-  return call0 (args[1]);
+  eassert (nargs >= 2 && nargs <= 4);
+  switch (nargs)
+    {
+    case 2:
+      return call0 (args[1]);
+    case 3:
+      return call1 (args[1], args[2]);
+    default:
+      return call2 (args[1], args[2], args[3]);
+    }
 }
 
 /* Subroutine for safe_run_hooks: handle an error by clearing out the function
@@ -1878,11 +1886,27 @@ safe_run_hooks_error (Lisp_Object error, ptrdiff_t nargs, Lisp_Object *args)
 static Lisp_Object
 safe_run_hook_funcall (ptrdiff_t nargs, Lisp_Object *args)
 {
-  eassert (nargs == 2);
+  eassert (nargs >= 2 && nargs <= 4);
   /* Yes, run_hook_with_args works with args in the other order.  */
-  internal_condition_case_n (safe_run_hooks_1,
-			     2, ((Lisp_Object []) {args[1], args[0]}),
-			     Qt, safe_run_hooks_error);
+  switch (nargs)
+    {
+    case 2:
+      internal_condition_case_n (safe_run_hooks_1,
+				 2, ((Lisp_Object []) {args[1], args[0]}),
+				 Qt, safe_run_hooks_error);
+      break;
+    case 3:
+      internal_condition_case_n (safe_run_hooks_1,
+				 3, ((Lisp_Object []) {args[1], args[0], args[2]}),
+				 Qt, safe_run_hooks_error);
+      break;
+    default:
+      internal_condition_case_n (safe_run_hooks_1,
+				 4, ((Lisp_Object [])
+				     {args[1], args[0], args[2], args[3]}),
+				 Qt, safe_run_hooks_error);
+      break;
+    }
   return Qnil;
 }
 


> TIA,

> Michael.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Michael Heerdegen]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Michael and Andrea.

> [patch...]

Thanks.  Yes, seems my case is fixed, and I didn't see any new problems
so far.

Michael.

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Andrea Greselin]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

On Sun, 14 Aug 2022 at 15:22, Alan Mackenzie <acm@muc.de> wrote:

> > Can I ask you please, instead of applying your patch (above) to try
> > out the following patch, which works for me.
>

It fixes my bug too.

Andrea

[-- Attachment #2: Type: text/html, Size: 567 bytes --]

bug#57179: 29.0.50: 'wrong-number-of-arguments' for function of two arguments called from 'window-scroll-functions'

[Alan Mackenzie]

Hello, Michael and Andrea.

Thanks for testing my patch.  I have just committed the patch to master,
and I am closing the bug with this post.

On Sat, Aug 13, 2022 at 22:50:47 +0200, Michael Heerdegen wrote:
> Eli Zaretskii <eliz@gnu.org> writes:

> > The backtrace seems to show that a function that is called is
> > different from the one you show as your hook function.  So I think you
> > should look closer into how you set up window-scroll-functions.

> I too have seen this problem just yesterday.  There must be something
> going on, it is unlikely that this is a coincidence.

> In the backtrace I saw that the function had been called by name with
> zero arguments.  But on top of the backtrace the symbol-function had
> been shown (a closure).  I also wondered about that.

> I verified that the function was only accessible via
> `window-scroll-functions'.  My code did not use the function in any
> other place.

> Some entries below in the backtrace was `redisplay' - so... somehow
> `redisplay' is sometimes calling functions in `window-scroll-functions'
> with zero arguments.  Maybe the buffer local binding of
> `window-scroll-functions'.

> Please tell whether this information suffices and you already have some
> idea or if I should try to create a recipe.  I guess this must be
> related to a recent change from within the last few days.

> Michael.

-- 
Alan Mackenzie (Nuremberg, Germany).