Re: master e971ce6: Make autoloads populate a new definition-prefixes table

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Glenn Morris]

It could be an unrelated failure (eg due to a change in the build deps),
but this change may be causing problems on 32-bit systems, see
http://hydra.nixos.org/build/36189760

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Date: Thu, 26 May 2016 11:52:26 -0400
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
> 
> 
> It could be an unrelated failure (eg due to a change in the build deps),
> but this change may be causing problems on 32-bit systems, see
> http://hydra.nixos.org/build/36189760

Is the below related?

    ELC      ../lisp/w32-fns.elc

  In toplevel form:
  w32-fns.el:163:16:Warning: w32-get-locale-info called with 1 argument, but
      accepts only 0

  In w32-list-locales:
  w32-fns.el:179:23:Warning: w32-get-locale-info called with 1 argument, but
      accepts only 0
  w32-fns.el:179:23:Warning: w32-get-locale-info called with 2 arguments, but
      accepts only 0
  w32-fns.el:245:2:Warning: set-message-beep called with 1 argument, but accepts
      only 0

(All of these warnings are bogus, of course.)

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Glenn Morris]

Actually, from the fact that emacs-25 still builds fine on 32-bit hydra,
I conclude this change probably is the cause.

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Stefan Monnier]

> Actually, from the fact that emacs-25 still builds fine on 32-bit hydra,
> I conclude this change probably is the cause.

I think so too.  Looking into it...


        Stefan

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Stefan Monnier]

>   w32-fns.el:245:2:Warning: set-message-beep called with 1 argument, but accepts
>       only 0
> (All of these warnings are bogus, of course.)

These are indeed bogus, but I can't imagine how they could be the result
of my commit.


        Stefan

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Stefan Monnier]

>> Actually, from the fact that emacs-25 still builds fine on 32-bit hydra,
>> I conclude this change probably is the cause.
> I think so too.  Looking into it...

Hmm... can't find the magic ingredient to reproduce it.
master bootstraps fine for me both on 64bit and 32bit systems.


        Stefan

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Glenn Morris <rgm@gnu.org>, emacs-devel@gnu.org
> Date: Thu, 26 May 2016 14:11:01 -0400
> 
> >   w32-fns.el:245:2:Warning: set-message-beep called with 1 argument, but accepts
> >       only 0
> > (All of these warnings are bogus, of course.)
> 
> These are indeed bogus, but I can't imagine how they could be the result
> of my commit.

You are right.  The reason is that on master 'declare-function' must
specify the full arglist of the announced function, otherwise the
byte-compiler emits this warning (I get them in quite a few files).
Is that intentional?  If so, we should fix all the 'declare-function'
forms that don't already state the argument lists.

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Stefan Monnier]

> You are right.  The reason is that on master 'declare-function' must
> specify the full arglist of the announced function, otherwise the
> byte-compiler emits this warning (I get them in quite a few files).
> Is that intentional?  If so, we should fix all the 'declare-function'
> forms that don't already state the argument lists.

No, that's not intentional (at least it was originally designed so as
to distinguish the "no arg" case from the "nil arg" case).


        Stefan

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > You are right.  The reason is that on master 'declare-function' must
> > specify the full arglist of the announced function, otherwise the
> > byte-compiler emits this warning (I get them in quite a few files).
> > Is that intentional?  If so, we should fix all the 'declare-function'
> > forms that don't already state the argument lists.
>
> No, that's not intentional (at least it was originally designed so as
> to distinguish the "no arg" case from the "nil arg" case).

The reason seems to be

b4d1cd: Pacify byte-compiler for
byte-compile-macroexpand-declare-function

by Paul (CC'd).  AFAIU the patch indeed doesn't distinguish "arglist ()"
and "no arglist specified" - it treats both as "arglist ()".

Paul, could you please have a look?


Thanks,

Michael.

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Paul Eggert]

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

Michael Heerdegen wrote:
> AFAIU the patch indeed doesn't distinguish "arglist ()"
> and "no arglist specified" - it treats both as "arglist ()".

Clearly I missed the distinction between the two. Sorry about that. I installed 
the attached patch to master, which should fix things. Thanks for reporting it.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-byte-compiler-pacification-for-declare-function.patch --]
[-- Type: text/x-diff; name="0001-Fix-byte-compiler-pacification-for-declare-function.patch", Size: 3871 bytes --]

From 91e4cad36ab1cc557ab7676cc93c8e05591a53dd Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 26 May 2016 19:10:26 -0700
Subject: [PATCH] Fix byte-compiler pacification for declare-function

Problem reported by Michael Heerdegen in:
http://lists.gnu.org/archive/html/emacs-devel/2016-05/msg00590.html
* lisp/emacs-lisp/bytecomp.el:
(byte-compile-macroexpand-declare-function):
Revert signature to previous value.
* lisp/subr.el (declare-function): Change signature to
match the reverted signature used in the byte compiler.
---
 lisp/emacs-lisp/bytecomp.el | 11 +++++------
 lisp/subr.el                |  8 +++++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index aa13210..11eb44c 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2958,24 +2958,23 @@ byte-compile-top-level-body
 	 (list body))))
 
 ;; Special macro-expander used during byte-compilation.
-(defun byte-compile-macroexpand-declare-function (fn file &optional arglist
-                                                     fileonly)
-  (let ((gotargs (listp arglist))
+(defun byte-compile-macroexpand-declare-function (fn file &rest args)
+  (let ((gotargs (and (consp args) (listp (car args))))
 	(unresolved (assq fn byte-compile-unresolved-functions)))
     (when unresolved	      ; function was called before declaration
       (if (and gotargs (byte-compile-warning-enabled-p 'callargs))
-	  (byte-compile-arglist-warn fn arglist nil)
+	  (byte-compile-arglist-warn fn (car args) nil)
 	(setq byte-compile-unresolved-functions
 	      (delq unresolved byte-compile-unresolved-functions))))
     (push (cons fn (if gotargs
-		       (list 'declared arglist)
+		       (list 'declared (car args))
 		     t))                     ; Arglist not specified.
 	  byte-compile-function-environment))
   ;; We are stating that it _will_ be defined at runtime.
   (setq byte-compile-noruntime-functions
         (delq fn byte-compile-noruntime-functions))
   ;; Delegate the rest to the normal macro definition.
-  (macroexpand `(declare-function ,fn ,file ,arglist ,fileonly)))
+  (macroexpand `(declare-function ,fn ,file ,@args)))
 
 \f
 ;; This is the recursive entry point for compiling each subform of an
diff --git a/lisp/subr.el b/lisp/subr.el
index b5d6f6f..7cbf006 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -29,13 +29,13 @@
 ;; Beware: while this file has tag `utf-8', before it's compiled, it gets
 ;; loaded as "raw-text", so non-ASCII chars won't work right during bootstrap.
 
-(defmacro declare-function (_fn _file &optional _arglist _fileonly)
+(defmacro declare-function (_fn _file &rest _args)
   "Tell the byte-compiler that function FN is defined, in FILE.
 Optional ARGLIST is the argument list used by the function.
 The FILE argument is not used by the byte-compiler, but by the
 `check-declare' package, which checks that FILE contains a
-definition for FN.  ARGLIST is used by both the byte-compiler
-and `check-declare' to check for consistency.
+definition for FN.  Remaining ARGS are used by both the
+byte-compiler and `check-declare' to check for consistency.
 
 FILE can be either a Lisp file (in which case the \".el\"
 extension is optional), or a C file.  C files are expanded
@@ -46,6 +46,8 @@ declare-function
 `check-declare' will check such files if they are found, and skip
 them without error if they are not.
 
+ARGS can contain one or two optional args.  First optional arg
+ARGLIST specifies the function arguments.  Second optional arg
 FILEONLY non-nil means that `check-declare' will only check that
 FILE exists, not that it defines FN.  This is intended for
 function-definitions that `check-declare' does not recognize, e.g.
-- 
2.5.5

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 26 May 2016 19:11:37 -0700
> Cc: Emacs Development <Emacs-devel@gnu.org>
> 
> Michael Heerdegen wrote:
> > AFAIU the patch indeed doesn't distinguish "arglist ()"
> > and "no arglist specified" - it treats both as "arglist ()".
> 
> Clearly I missed the distinction between the two. Sorry about that. I installed 
> the attached patch to master, which should fix things. Thanks for reporting it.

Thanks.  Could we please have a more prominent documentation of this
feature?  The doc string doesn't actually explain the special
significance of omitting the arglist, except by an indirect hint, and
the ELisp manual says even less.  We shouldn't leave these subtleties
unsaid.

TIA

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  Could we please have a more prominent documentation of this
> feature?  The doc string doesn't actually explain the special
> significance of omitting the arglist, except by an indirect hint, and
> the ELisp manual says even less.  We shouldn't leave these subtleties
> unsaid.

What do you mean?  AFAIU after Paul's fix, omitting the arglist will
give you no warnings about the arglist when compiling - as I think is
absolutely expected.


Michael.

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Michael Heerdegen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Michael Heerdegen wrote:
> > AFAIU the patch indeed doesn't distinguish "arglist ()"
> > and "no arglist specified" - it treats both as "arglist ()".
>
> Clearly I missed the distinction between the two. Sorry about that. I
> installed the attached patch to master, which should fix
> things. Thanks for reporting it.

Thanks, Paul.  I've rebuilt master, and all unwarranted warnings are
gone (checked by compiling my init file).


Regards,

Michael.

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Paul Eggert]

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

On 05/26/2016 11:39 PM, Eli Zaretskii wrote:
> Could we please have a more prominent documentation of this
> feature?

I gave that a shot by installing the attached patch.


[-- Attachment #2: 0001-Improve-define-function-omitted-arg-documentation.patch --]
[-- Type: application/x-patch, Size: 5897 bytes --]

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Eli Zaretskii]

> Cc: michael_heerdegen@web.de, Emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 27 May 2016 09:48:41 -0700
> 
> On 05/26/2016 11:39 PM, Eli Zaretskii wrote:
> > Could we please have a more prominent documentation of this
> > feature?
> 
> I gave that a shot by installing the attached patch.

Thanks!

Re: master e971ce6: Make autoloads populate a new definition-prefixes table

[Michael Heerdegen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> I gave that a shot by installing the attached patch.

Now I see what you were talking about.  It was good to explain that,
thanks.


Michael.