GNU bug report logs - #62761
ruby-add-log-current-method drops some segments when singleton definition references outer module

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 62761 <at> debbugs.gnu.org
Subject: bug#62761: ruby-add-log-current-method drops some segments when singleton definition references outer module
Date: Tue, 11 Apr 2023 08:51:18 +0300

> Date: Tue, 11 Apr 2023 00:02:03 +0300
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
>    module M
>      module N
>        module C
>          class D
>            def C.foo
>              _
>            end
>          end
>        end
>      end
>    end
> 
> (ruby-add-log-current-method) currently returns "M::C.foo"
> 
> While it should return "M::N::C.foo". Patch attached.
> 
> This discovery stems from Mattias Engdegård's report (in private) about 
> an ignored return value from `nreverse`.
> 
> Is this good for emacs-29?

I guess so.  But I wonder: this code was there since ruby-mode.el was
added to Emacs in 2008, so are you saying that (cdr ml) can never be
nil at this place, or if it is, it's okay to leave ml at the nil
value?

IOW, the return value of nreverse has been ignored, but what about the
nreverse call before that:

> --- a/lisp/progmodes/ruby-mode.el
> +++ b/lisp/progmodes/ruby-mode.el
> @@ -1911,7 +1911,7 @@ ruby-add-log-current-method
>                          (while ml
>                            (if (string-equal (car ml) (car mn))
>                                (setq mlist (nreverse (cdr ml)) ml nil))
> -                          (or (setq ml (cdr ml)) (nreverse mlist))))
> +                          (setq ml (cdr ml))))
>                        (if mlist
>                            (setcdr (last mlist) (butlast mn))
>                          (setq mlist (butlast mn))))

Isn't the second nreverse there to undo the first?

I guess I'm asking for slightly more detailed rationale for the change
you are proposing, to include the analysis of the code involved and
where that code is mistaken.  For example, why not say

   (setq mlist (nreverse mlist))
or
   (setq ml (nreverse mlist))

instead of just removing the 2nd nreverse call?

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.