* bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn
@ 2011-05-10 22:26 Glenn Morris
2011-05-11 4:33 ` Glenn Morris
0 siblings, 1 reply; 7+ messages in thread
From: Glenn Morris @ 2011-05-10 22:26 UTC (permalink / raw)
To: 8646
Package: emacs
Version: 24.0.50
Severity: minor
Since the lexical merge, compiling subr.el warns:
In declare-function:
subr.el:39:11:Warning: macro declare-function used to take 0+ arguments,
now takes 2-4
This is caused by the element in byte-compile-initial-macro-environment:
(declare-function . byte-compile-macroexpand-declare-function)
This confuses byte-compile-arglist-warn, because
(byte-compile-fdefinition 'declare-function t)
returns `byte-compile-macroexpand-declare-function', which leads to a
(bogus) arglist signature of 0+.
^ permalink raw reply [flat|nested] 7+ messages in thread* bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn
2011-05-10 22:26 bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn Glenn Morris
@ 2011-05-11 4:33 ` Glenn Morris
2011-05-11 13:54 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Glenn Morris @ 2011-05-11 4:33 UTC (permalink / raw)
To: 8646
Here's a patch that I think might be right in general, but still leaves
a warning in this specific case:
In declare-function:
subr.el:39:11:Warning: macro declare-function used to take 2+ arguments,
now takes 2-4
Strictly speaking, that warning is correct.
Actually, I guess this patch is not fully correct, because something
that is in byte-compile-initial-macro-environment could be redefined
more than once, in theory. But it's better than the current version. (?)
*** lisp/emacs-lisp/bytecomp.el 2011-05-07 04:03:49 +0000
--- lisp/emacs-lisp/bytecomp.el 2011-05-11 04:22:58 +0000
***************
*** 1314,1320 ****
;; number of arguments.
(defun byte-compile-arglist-warn (form macrop)
(let* ((name (nth 1 form))
! (old (byte-compile-fdefinition name macrop)))
(if (and old (not (eq old t)))
(progn
(and (eq 'macro (car-safe old))
--- 1314,1327 ----
;; number of arguments.
(defun byte-compile-arglist-warn (form macrop)
(let* ((name (nth 1 form))
! (old (byte-compile-fdefinition name macrop))
! (initial (and macrop
! (cdr (assq name
! byte-compile-initial-macro-environment)))))
! ;; Assumes an element of b-c-i-macro-env that is a symbol points
! ;; to a defined function.
! (and initial (symbolp initial)
! (setq old (byte-compile-fdefinition initial nil)))
(if (and old (not (eq old t)))
(progn
(and (eq 'macro (car-safe old))
^ permalink raw reply [flat|nested] 7+ messages in thread* bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn
2011-05-11 4:33 ` Glenn Morris
@ 2011-05-11 13:54 ` Stefan Monnier
2011-05-11 17:39 ` Glenn Morris
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2011-05-11 13:54 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8646
> Here's a patch that I think might be right in general, but still leaves
> a warning in this specific case:
The patch looks right, thanks. The "2+" vs "2-4" can be fixed by
changing either the macro definition of its override so that their
arglist matches.
> Actually, I guess this patch is not fully correct, because something
> that is in byte-compile-initial-macro-environment could be redefined
> more than once, in theory. But it's better than the current version. (?)
The intention of byte-compile-initial-macro-environment is to override
"the" actual macro definition. Obviously, this presumes that the macro is
not redefined elsewhere in a different way, otherwise the intended
behavior is not defined.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread* bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn
2011-05-11 13:54 ` Stefan Monnier
@ 2011-05-11 17:39 ` Glenn Morris
2011-05-12 1:00 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Glenn Morris @ 2011-05-11 17:39 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8646
Stefan Monnier wrote:
> The "2+" vs "2-4" can be fixed by changing either the macro definition
> of its override so that their arglist matches.
No, really? ;)
I don't see how to change the override definition to not use &rest,
since it is a function that needs to distinguish "no argument" from
"argument nil". And changing the macro to match the override seems like
putting the cart before the horse.
How about the following, based on the idea that it is the override that
should be congruent with the real macro definition, even though the
former gets defined first? It's the reason that
byte-compile-macroexpand-declare-function can work.
*** lisp/emacs-lisp/bytecomp.el 2011-05-11 17:32:38 +0000
--- lisp/emacs-lisp/bytecomp.el 2011-05-11 17:35:51 +0000
***************
*** 1321,1327 ****
;; Assumes an element of b-c-i-macro-env that is a symbol points
;; to a defined function. (Bug#8646)
(and initial (symbolp initial)
! (setq old (byte-compile-fdefinition initial nil)))
(if (and old (not (eq old t)))
(progn
(and (eq 'macro (car-safe old))
--- 1321,1328 ----
;; Assumes an element of b-c-i-macro-env that is a symbol points
;; to a defined function. (Bug#8646)
(and initial (symbolp initial)
! (setq old (byte-compile-fdefinition initial nil)
! initial 'yes))
(if (and old (not (eq old t)))
(progn
(and (eq 'macro (car-safe old))
***************
*** 1334,1340 ****
((pred byte-code-function-p) (aref old 0))
(t '(&rest def)))))
(sig2 (byte-compile-arglist-signature (nth 2 form))))
! (unless (byte-compile-arglist-signatures-congruent-p sig1 sig2)
(byte-compile-set-symbol-position name)
(byte-compile-warn
"%s %s used to take %s %s, now takes %s"
--- 1335,1345 ----
((pred byte-code-function-p) (aref old 0))
(t '(&rest def)))))
(sig2 (byte-compile-arglist-signature (nth 2 form))))
! ;; If something is on b-c-initial-m-e, *that* should be congruent
! ;; with the normal (new) definition, not vice versa.
! (unless (if (eq initial 'yes)
! (byte-compile-arglist-signatures-congruent-p sig2 sig1)
! (byte-compile-arglist-signatures-congruent-p sig1 sig2))
(byte-compile-set-symbol-position name)
(byte-compile-warn
"%s %s used to take %s %s, now takes %s"
^ permalink raw reply [flat|nested] 7+ messages in thread* bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn
2011-05-11 17:39 ` Glenn Morris
@ 2011-05-12 1:00 ` Stefan Monnier
2011-05-19 19:06 ` Glenn Morris
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2011-05-12 1:00 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8646
>> The "2+" vs "2-4" can be fixed by changing either the macro definition
>> of its override so that their arglist matches.
> No, really? ;)
> I don't see how to change the override definition to not use &rest,
> since it is a function that needs to distinguish "no argument" from
> "argument nil". And changing the macro to match the override seems like
> putting the cart before the horse.
> How about the following, based on the idea that it is the override that
> should be congruent with the real macro definition, even though the
> former gets defined first? It's the reason that
> byte-compile-macroexpand-declare-function can work.
It makes some sense, indeed. But I'm not thrilled about this solution.
I'd rather change the defmacro to match the override.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-05-20 2:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-10 22:26 bug#8646: byte-compile-initial-macro-environment confuses byte-compile-arglist-warn Glenn Morris
2011-05-11 4:33 ` Glenn Morris
2011-05-11 13:54 ` Stefan Monnier
2011-05-11 17:39 ` Glenn Morris
2011-05-12 1:00 ` Stefan Monnier
2011-05-19 19:06 ` Glenn Morris
2011-05-20 2:18 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).