* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change
[not found] ` <20191001144137.1032C20927@vcs0.savannah.gnu.org>
@ 2019-10-01 15:08 ` Stefan Monnier
2019-10-01 15:22 ` Lars Ingebrigtsen
2019-10-02 16:57 ` Sven Joachim
0 siblings, 2 replies; 5+ messages in thread
From: Stefan Monnier @ 2019-10-01 15:08 UTC (permalink / raw)
To: emacs-devel; +Cc: Lars Ingebrigtsen
Hi Lars,
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -124,7 +124,7 @@
> (require 'backquote)
> (require 'macroexp)
> (require 'cconv)
> -(eval-when-compile (require 'compile))
> +(require 'compile)
This is a bad change.
It brings in many more packages, and every additional package brought
in reduces the quality of our warnings (e.g. if any of the packages
(transitively) loaded load cl-lib, then we fail to signal when
a package uses cl-lib functionality without requiring cl-lib).
We should try and find some other solution.
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change
2019-10-01 15:08 ` [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change Stefan Monnier
@ 2019-10-01 15:22 ` Lars Ingebrigtsen
2019-10-01 21:30 ` Stefan Monnier
2019-10-02 16:57 ` Sven Joachim
1 sibling, 1 reply; 5+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-01 15:22 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> -(eval-when-compile (require 'compile))
>> +(require 'compile)
>
> This is a bad change.
>
> It brings in many more packages, and every additional package brought
> in reduces the quality of our warnings (e.g. if any of the packages
> (transitively) loaded load cl-lib, then we fail to signal when
> a package uses cl-lib functionality without requiring cl-lib).
I'm not quite sure I followed the last bit -- if something transitively
required cl-lib, then there's no point for the "top" file to require
cl-lib, surely? Or is this a style issue.
But I agree that requiring compile.el runtime is probably not good, but:
> We should try and find some other solution.
No other solution I thought of at the top of my head seemed good.
Moving either of those keymaps to a different place seemed logical,
either...
The problem is that the mode now inherits from the parent defined in
compile.el:
(defvar emacs-lisp-compilation-mode-map
(let ((map (make-sparse-keymap)))
(set-keymap-parent map compilation-minor-mode-map)
(define-key map "g" 'emacs-lisp-compilation-recompile)
map))
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change
2019-10-01 15:22 ` Lars Ingebrigtsen
@ 2019-10-01 21:30 ` Stefan Monnier
2019-10-03 15:18 ` Lars Ingebrigtsen
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2019-10-01 21:30 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: emacs-devel
> I'm not quite sure I followed the last bit -- if something transitively
> required cl-lib, then there's no point for the "top" file to require
> cl-lib, surely? Or is this a style issue.
If a file (transitively) required by bytecomp.el requires cl-lib,
then when compiling *another* file any missing (require 'cl-lib) in that
other file won't be noticed.
> (defvar emacs-lisp-compilation-mode-map
> (let ((map (make-sparse-keymap)))
> (set-keymap-parent map compilation-minor-mode-map)
> (define-key map "g" 'emacs-lisp-compilation-recompile)
> map))
Do we need this set-keymap-parent? define-derived-mode should do it for
us (when the mode is called).
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change
2019-10-01 15:08 ` [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change Stefan Monnier
2019-10-01 15:22 ` Lars Ingebrigtsen
@ 2019-10-02 16:57 ` Sven Joachim
1 sibling, 0 replies; 5+ messages in thread
From: Sven Joachim @ 2019-10-02 16:57 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel
On 2019-10-01 11:08 -0400, Stefan Monnier wrote:
> Hi Lars,
>
>> --- a/lisp/emacs-lisp/bytecomp.el
>> +++ b/lisp/emacs-lisp/bytecomp.el
>> @@ -124,7 +124,7 @@
>> (require 'backquote)
>> (require 'macroexp)
>> (require 'cconv)
>> -(eval-when-compile (require 'compile))
>> +(require 'compile)
>
> This is a bad change.
>
> It brings in many more packages, and every additional package brought
> in reduces the quality of our warnings (e.g. if any of the packages
> (transitively) loaded load cl-lib, then we fail to signal when
> a package uses cl-lib functionality without requiring cl-lib).
Indeed, this is bug #30635, mentioned in the comments you had written
just around Lars' changes:
,----
| ;; Refrain from using cl-lib at run-time here, since it otherwise prevents
| ;; us from emitting warnings when compiling files which use cl-lib without
| ;; requiring it! (bug#30635)
`----
> We should try and find some other solution.
Cheers,
Sven
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change
2019-10-01 21:30 ` Stefan Monnier
@ 2019-10-03 15:18 ` Lars Ingebrigtsen
0 siblings, 0 replies; 5+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-03 15:18 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I'm not quite sure I followed the last bit -- if something transitively
>> required cl-lib, then there's no point for the "top" file to require
>> cl-lib, surely? Or is this a style issue.
>
> If a file (transitively) required by bytecomp.el requires cl-lib,
> then when compiling *another* file any missing (require 'cl-lib) in that
> other file won't be noticed.
Oh, this is just special for bytecomp.el; I get it.
>> (defvar emacs-lisp-compilation-mode-map
>> (let ((map (make-sparse-keymap)))
>> (set-keymap-parent map compilation-minor-mode-map)
>> (define-key map "g" 'emacs-lisp-compilation-recompile)
>> map))
>
> Do we need this set-keymap-parent? define-derived-mode should do it for
> us (when the mode is called).
I didn't know that. I've now made the change (and reverted the
`require'), and this do indeed work as they should.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-03 15:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20191001144135.32717.19241@vcs0.savannah.gnu.org>
[not found] ` <20191001144137.1032C20927@vcs0.savannah.gnu.org>
2019-10-01 15:08 ` [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change Stefan Monnier
2019-10-01 15:22 ` Lars Ingebrigtsen
2019-10-01 21:30 ` Stefan Monnier
2019-10-03 15:18 ` Lars Ingebrigtsen
2019-10-02 16:57 ` Sven Joachim
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.