Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Lars Ingebrigtsen]

Thanks, Mattias.

Just a note that this adds a couple of warnings which perhaps Michael
and Alan can have a look at:

In c-get-lang-constant:
progmodes/cc-defs.el:2685:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
progmodes/cc-defs.el:2685:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

In toplevel form:
net/tramp-gvfs.el:2511:32: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
net/tramp-gvfs.el:2511:10: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
In tramp-archive-file-name-handler:
net/tramp-archive.el:351:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
net/tramp-archive.el:351:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> Just a note that this adds a couple of warnings which perhaps Michael
> and Alan can have a look at:
>
> In toplevel form:
> net/tramp-gvfs.el:2511:32: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> net/tramp-gvfs.el:2511:10: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> In tramp-archive-file-name-handler:
> net/tramp-archive.el:351:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> net/tramp-archive.el:351:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

Tramp needs this for Emacs 26. I've wrapped it with `with-no-warnings'.

Best regards, Michael.

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> Tramp needs this for Emacs 26. I've wrapped it with `with-no-warnings'.

Thanks.

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Mattias Engdegård]

20 sep. 2022 kl. 14.15 skrev Michael Albinus <michael.albinus@gmx.de>:

> Tramp needs this for Emacs 26. I've wrapped it with `with-no-warnings'.

Thank you, but maybe silencing all warnings in the affected code was a cudgel heavier than the situation called for?
Perhaps something like this would do:

> (defmacro tramp--with-max-specpdl-size (new-limit &rest body)
>   (declare (indent 1))
>   (if (< emacs-major-version 29)
>       `(let ((max-specpdl-size ,new-limit)) . ,body)
>     `(progn . ,body))))

Or we could postpone the obsoletion for a while. The change was made partly for user convenience, but the typically package maintainer may not agree that this has been achieved: there is no way to be avoid warnings in code that has to work on previous Emacs versions without making it uglier.

Would it be a bad idea to, say, make a note to obsolete max-specpdl-size in Emacs 31? 

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> Would it be a bad idea to, say, make a note to obsolete
> max-specpdl-size in Emacs 31?

Obsoleting it is nice, because that's a convenient way to let people
know that altering it doesn't do anything any more.  But as you point
out, it makes it difficult to write portable code without disabling
warnings for more code than you really want to disable those warnings
for.

So postponing the obsoletion might make sense.

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Michael Albinus]

Mattias Engdegård <mattiase@acm.org> writes:

Hi Mattias,

>> Tramp needs this for Emacs 26. I've wrapped it with `with-no-warnings'.
>
> Thank you, but maybe silencing all warnings in the affected code was a cudgel heavier than the situation called for?

Well, I wanted to use `with-suppressed-warnings', but this exists in
Emacs 27+ only.

> Perhaps something like this would do:
>
>> (defmacro tramp--with-max-specpdl-size (new-limit &rest body)
>>   (declare (indent 1))
>>   (if (< emacs-major-version 29)
>>       `(let ((max-specpdl-size ,new-limit)) . ,body)
>>     `(progn . ,body))))

It isn't Tramp code which changes frequently, so I guess we could keep
it as it is. And with Tramp 2.7 (Emacs 30), Emacs 26 won't be supported
any longer, and Tramp can get rid of it.

Let's work on a soonish Emacs 29 release ! :-)

Best regards, Michael.

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Alan Mackenzie]

Hello, Mattias.

On Tue, Sep 20, 2022 at 14:49:40 +0200, Mattias Engdegård wrote:
> 20 sep. 2022 kl. 14.15 skrev Michael Albinus <michael.albinus@gmx.de>:

> > Tramp needs this for Emacs 26. I've wrapped it with `with-no-warnings'.

> Thank you, but maybe silencing all warnings in the affected code was a
> cudgel heavier than the situation called for?  Perhaps something like
> this would do:

> > (defmacro tramp--with-max-specpdl-size (new-limit &rest body)
> >   (declare (indent 1))
> >   (if (< emacs-major-version 29)
> >       `(let ((max-specpdl-size ,new-limit)) . ,body)
> >     `(progn . ,body))))

> Or we could postpone the obsoletion for a while. The change was made
> partly for user convenience, but the typically package maintainer may
> not agree that this has been achieved: there is no way to be avoid
> warnings in code that has to work on previous Emacs versions without
> making it uglier.

> Would it be a bad idea to, say, make a note to obsolete
> max-specpdl-size in Emacs 31? 

I suspect it would go the same way as font-lock-reference-face did.  That
face was renamed to font-lock-constant-face in the early 1990s, with an
alias.  People just continued to use the alias.

If you postpone the obsoletion of m-s-s to Emacs 31, the code using it
probably won't change until then.  :-(

I suspect I am going to implement a macro something like:

(defmacro c-maybe-max-specpdl-size-let (varlist &rest body)
  (let ((-varlist- varlist) msp-binding)
    (if (get 'max-specpdl-size 'byte-obsolete-variable)
        (cond
         ((memq 'max-specpdl-size -varlist-)
          (setq -varlist- (delq 'max-spec-pdl-size -varlist-)))
         ((setq msp-binding (assq 'max-spec-pdl-size -varlist-))
          (setq -varlist- (delq msp-binding -varlist-)))))
    `(let ,varlist ,@body)))

..  Which points out a deficit in Emacs, namely the lack of functions
obsolete-variable-p and obsolete-function-p.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Mattias Engdegård]

20 sep. 2022 kl. 16.56 skrev Alan Mackenzie <acm@muc.de>:

> If you postpone the obsoletion of m-s-s to Emacs 31, the code using it
> probably won't change until then.

Very likely, but at least we then have the old NEWS entries to point at when people act surprised...
But you are right in that we shouldn't invent a problem that doesn't necessarily exist. If legitimate-sounding concerns arise before 29.1 is released, including the prerelease, then we can take mitigating action.

bug#57911: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Alan Mackenzie]

Hello, Lars.

On Mon, Sep 19, 2022 at 20:50:15 +0200, Lars Ingebrigtsen wrote:
> Thanks, Mattias.

> Just a note that this adds a couple of warnings which perhaps Michael
> and Alan can have a look at:

> In c-get-lang-constant:
> progmodes/cc-defs.el:2685:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> progmodes/cc-defs.el:2685:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

I've now fixed these warnings.

> In toplevel form:
> net/tramp-gvfs.el:2511:32: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> net/tramp-gvfs.el:2511:10: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> In tramp-archive-file-name-handler:
> net/tramp-archive.el:351:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> net/tramp-archive.el:351:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Alan Mackenzie]

Hello, Lars.

On Mon, Sep 19, 2022 at 20:50:15 +0200, Lars Ingebrigtsen wrote:
> Thanks, Mattias.

> Just a note that this adds a couple of warnings which perhaps Michael
> and Alan can have a look at:

> In c-get-lang-constant:
> progmodes/cc-defs.el:2685:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> progmodes/cc-defs.el:2685:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

I've now fixed these warnings.

> In toplevel form:
> net/tramp-gvfs.el:2511:32: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> net/tramp-gvfs.el:2511:10: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> In tramp-archive-file-name-handler:
> net/tramp-archive.el:351:37: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).
> net/tramp-archive.el:351:15: Warning: ‘max-specpdl-size’ is an obsolete variable (as of 29.1).

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#57911: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Mattias Engdegård]

27 sep. 2022 kl. 10.47 skrev Alan Mackenzie <acm@muc.de>:

> I've now fixed these warnings.

Thank you Alan! One detail though:

>   (let ((-varlist- varlist) msp-binding)

Here `-varlist-` aliases `varlist`...

>          (setq -varlist- (delq 'max-specpdl-size -varlist-)))

... the original macro argument is mutated, which is directly from the source code -- maybe don't do that in a macro?
Then we set `-varlist-`, but that's a dead store because...

>     `(let* ,varlist ,@body)))

... we use `varlist`.

bug#57911: master 60102016e4: Abolish max-specpdl-size (bug#57911)

[Alan Mackenzie]

Hello, Mattias.

On Tue, Sep 27, 2022 at 11:01:40 +0200, Mattias Engdegård wrote:
> 27 sep. 2022 kl. 10.47 skrev Alan Mackenzie <acm@muc.de>:

> > I've now fixed these warnings.

> Thank you Alan! One detail though:

> >   (let ((-varlist- varlist) msp-binding)

> Here `-varlist-` aliases `varlist`...

> >          (setq -varlist- (delq 'max-specpdl-size -varlist-)))

> ... the original macro argument is mutated, which is directly from the source code -- maybe don't do that in a macro?
> Then we set `-varlist-`, but that's a dead store because...

> >     `(let* ,varlist ,@body)))

> ... we use `varlist`.

Thanks for spotting those mistakes.  I think I've now fixed them with a
second commit.

-- 
Alan Mackenzie (Nuremberg, Germany).