unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
@ 2018-01-04  9:40 Robert Cochran
  2018-01-04 13:22 ` Noam Postavsky
  0 siblings, 1 reply; 22+ messages in thread
From: Robert Cochran @ 2018-01-04  9:40 UTC (permalink / raw)
  To: emacs-devel

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

Greetings emacs-devel,

Attached is a patch that adds the variable `byte-compile-error-on-warn'
to the safe variable list temporarily during calls to
`byte-compile-file`, so that individual files can easily and simply
include "byte-compile-error-on-warn: t" in their file local variables.

This allows programmers to be able to ensure thoroughly that there are
no warnings in their code, which theoretically leads to better
correctness.

It is my eventual hope that this is accepted so that
`byte-compile-error-on-warn' can be enable on files in the core Emacs
Git repository, extending the aforementioned correctness potential to
files in the core.

For example, something that is bothering me right now
is that there are quite a few 'foo is obsolete' warnings during the byte
compilation portion of the build phase. Enabling error on warning means
that trying to byte compile files that reference stale
functions/variables/what have you will fail, prompting patches that
obsolete Lisp objects to also fix usage of it in the core with the
appropriate recommended replacement.

I ran the full test suite (`make check-expensive`), and got the same
results pre- and post-patch. Unless there are already files that have
`byte-compile-error-on-warn' set in their local variables (which
wouldn't have worked prior to this), nothing should have suddenly
broken as a result of this patch.

Suggestions, comments, concerns, and all those other things are welcomed
and requested.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2

-----


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch to make byte-compile-error-on-warn a safe file local variable --]
[-- Type: text/x-patch, Size: 1549 bytes --]

From d36eccbc8fe6c1737c02a11dce1d845b3e3e5211 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Thu, 4 Jan 2018 01:09:50 -0800
Subject: [PATCH] Make byte-compile-error-on-warn a safe variable for file
 compilation

This allows individual files to easily specify whether or not byte
compilation should error on encountering warnings, ala -Werror from
GCC.

* lisp/emacs-lisp/bytecomp.el (byte-compile-file): Make
  `byte-compiler-error-on-warn' a safe local variable for file
  compliation.
---
 lisp/emacs-lisp/bytecomp.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 2f5f9f8c05..3c43ab21a5 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1874,6 +1874,11 @@ byte-compile-file
       ;; If they change the file name, then change it for the output also.
       (let ((buffer-file-name filename)
             (dmm (default-value 'major-mode))
+            ;; Ensure that byte-compile-error-on-warn is recognized as
+            ;; safe, even if it's not in safe-local-variable-values.
+            (safe-local-variable-values (append (list '(byte-compile-error-on-warn . t)
+                                                      '(byte-compile-error-on-warn . nil))
+                                                safe-local-variable-values))
             ;; Ignore unsafe local variables.
             ;; We only care about a few of them for our purposes.
             (enable-local-variables :safe)
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-04  9:40 [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation Robert Cochran
@ 2018-01-04 13:22 ` Noam Postavsky
  2018-01-05  3:51   ` Stefan Monnier
  2018-01-05  6:17   ` Robert Cochran
  0 siblings, 2 replies; 22+ messages in thread
From: Noam Postavsky @ 2018-01-04 13:22 UTC (permalink / raw)
  To: Robert Cochran; +Cc: Emacs developers

On Thu, Jan 4, 2018 at 4:40 AM, Robert Cochran
<robert+Emacs@cochranmail.com> wrote:

> Attached is a patch that adds the variable `byte-compile-error-on-warn'
> to the safe variable list temporarily during calls to
> `byte-compile-file`, so that individual files can easily and simply
> include "byte-compile-error-on-warn: t" in their file local variables.

Wouldn't it be simpler and clearer to make it permanently safe? (PS
you can use #'booleanp instead of listing t and nil).

> For example, something that is bothering me right now
> is that there are quite a few 'foo is obsolete' warnings during the byte
> compilation portion of the build phase. Enabling error on warning means
> that trying to byte compile files that reference stale
> functions/variables/what have you will fail, prompting patches that
> obsolete Lisp objects to also fix usage of it in the core with the
> appropriate recommended replacement.

Solving Bug#4837 would be helpful for this, as currently there is no
practical way to fix/suppress the obsolete warnings in the code that
remains to support the obsolete option.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=4837



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-04 13:22 ` Noam Postavsky
@ 2018-01-05  3:51   ` Stefan Monnier
  2018-01-05  6:17   ` Robert Cochran
  1 sibling, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2018-01-05  3:51 UTC (permalink / raw)
  To: emacs-devel

>> Attached is a patch that adds the variable `byte-compile-error-on-warn'
>> to the safe variable list temporarily during calls to
>> `byte-compile-file`, so that individual files can easily and simply
>> include "byte-compile-error-on-warn: t" in their file local variables.
> Wouldn't it be simpler and clearer to make it permanently safe? (PS
> you can use #'booleanp instead of listing t and nil).

Indeed.


        Stefan




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-04 13:22 ` Noam Postavsky
  2018-01-05  3:51   ` Stefan Monnier
@ 2018-01-05  6:17   ` Robert Cochran
  2018-01-05 12:05     ` Noam Postavsky
  1 sibling, 1 reply; 22+ messages in thread
From: Robert Cochran @ 2018-01-05  6:17 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> On Thu, Jan 4, 2018 at 4:40 AM, Robert Cochran
> <robert+Emacs@cochranmail.com> wrote:
>
>> Attached is a patch that adds the variable `byte-compile-error-on-warn'
>> to the safe variable list temporarily during calls to
>> `byte-compile-file`, so that individual files can easily and simply
>> include "byte-compile-error-on-warn: t" in their file local variables.
>
> Wouldn't it be simpler and clearer to make it permanently safe? (PS
> you can use #'booleanp instead of listing t and nil).

I somehow came to the conclusion that the way I did it was the simplest
and cleanest to me. I noticed that `safe-local-variable-values' is nil
by default and I took that as a hint (rightly or wrongly) to not touch
the value directly. I was also aiming to prevent easily tampering with
the variable, again, however right or wrong that particular decision
seems to you.

On that topic, since `safe-local-variable-values' is nil by default,
where is it exactly that you would recommend I add this variable? A few
quick greps didn't give me any clear ideas.

Thanks for your suggestion about using #'booleanp, btw.

>
>> For example, something that is bothering me right now
>> is that there are quite a few 'foo is obsolete' warnings during the byte
>> compilation portion of the build phase. Enabling error on warning means
>> that trying to byte compile files that reference stale
>> functions/variables/what have you will fail, prompting patches that
>> obsolete Lisp objects to also fix usage of it in the core with the
>> appropriate recommended replacement.
>
> Solving Bug#4837 would be helpful for this, as currently there is no
> practical way to fix/suppress the obsolete warnings in the code that
> remains to support the obsolete option.
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=4837

Yeah, I was thinking about that. In private discussion with friends, it
was mentioned that it could be a `declare' option - something like
(declare no-warn-obsolete) - but that's not very satifying for anything
other than defuns.

This is why I was wanting the file local variables. It can be turned on
slowly as warnings are fixed, in a similar vein to
lexical-bindings. Even if we find that there are a few files where it
can't be enabled, it can at least be enabled on the vast majority of
them.

My $0.02 of course.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-05  6:17   ` Robert Cochran
@ 2018-01-05 12:05     ` Noam Postavsky
  2018-01-06 22:42       ` Robert Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Noam Postavsky @ 2018-01-05 12:05 UTC (permalink / raw)
  To: Robert Cochran; +Cc: Emacs developers

On Fri, Jan 5, 2018 at 1:17 AM, Robert Cochran
<robert+Emacs@cochranmail.com> wrote:

> I somehow came to the conclusion that the way I did it was the simplest
> and cleanest to me. I noticed that `safe-local-variable-values' is nil
> by default and I took that as a hint (rightly or wrongly) to not touch
> the value directly. I was also aiming to prevent easily tampering with
> the variable, again, however right or wrong that particular decision
> seems to you.

Okay, I think preventing easy tampering (by the user) is not a useful goal.

> On that topic, since `safe-local-variable-values' is nil by default,
> where is it exactly that you would recommend I add this variable? A few
> quick greps didn't give me any clear ideas.

See `(elisp) File Local Variables':

   You can specify safe values for a variable with a
`safe-local-variable' property.  The property has to be a function of
one argument; any value is safe if the function returns non-`nil' given
that value.  Many commonly-encountered file variables have
`safe-local-variable' properties; these include `fill-column',
`fill-prefix', and `indent-tabs-mode'.  For boolean-valued variables
that are safe, use `booleanp' as the property value.

>> Solving Bug#4837 would be helpful for this, as currently there is no
>> practical way to fix/suppress the obsolete warnings in the code that
>> remains to support the obsolete option.
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=4837
>
> Yeah, I was thinking about that. In private discussion with friends, it
> was mentioned that it could be a `declare' option - something like
> (declare no-warn-obsolete) - but that's not very satifying for anything
> other than defuns.

That would cover too much code as well.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-05 12:05     ` Noam Postavsky
@ 2018-01-06 22:42       ` Robert Cochran
  2018-01-08 12:25         ` Noam Postavsky
  0 siblings, 1 reply; 22+ messages in thread
From: Robert Cochran @ 2018-01-06 22:42 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

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

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Okay, I think preventing easy tampering (by the user) is not a useful goal.

Yeah, I'll agree with that now that I've set the property the right way.

> See `(elisp) File Local Variables':
>
>    You can specify safe values for a variable with a
> `safe-local-variable' property.  The property has to be a function of
> one argument; any value is safe if the function returns non-`nil' given
> that value.  Many commonly-encountered file variables have
> `safe-local-variable' properties; these include `fill-column',
> `fill-prefix', and `indent-tabs-mode'.  For boolean-valued variables
> that are safe, use `booleanp' as the property value.

Ah, symbol properties! I don't ever use them myself in my Lisp, so I
have a tendency to forget that's something I can do. New patch attached
that does it the right way.

>>> Solving Bug#4837 would be helpful for this, as currently there is no
>>> practical way to fix/suppress the obsolete warnings in the code that
>>> remains to support the obsolete option.
>>>
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=4837
>>
>> Yeah, I was thinking about that. In private discussion with friends, it
>> was mentioned that it could be a `declare' option - something like
>> (declare no-warn-obsolete) - but that's not very satifying for anything
>> other than defuns.
>
> That would cover too much code as well.

I still hold that we can transition to having everything compile
error-on-warn in the same manner that lexical-binding was enabled slowly
as the code was changed in individual files to support it. I definitely
agree that looking into some way to fix/suppress the warning in
supporting code is someting worthwhile, but I'm just as unsure of a
proper solution as anyone else is.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Make byte-compile-error-on-warn a safe file local variable --]
[-- Type: text/x-patch, Size: 1036 bytes --]

From 6e8573da34a9a76e6114449b30c2354d87da6039 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Sat, 6 Jan 2018 13:19:55 -0800
Subject: [PATCH] Make byte-compile-error-on-warn a safe variable for file
 compilation

This allows individual files to easily specify whether or not byte
compilation should error on encountering warnings, ala -Werror from
GCC.

* lisp/emacs-lisp/bytecomp.el (byte-compile-error-on-warn): Mark as a
  safe local variable.
---
 lisp/emacs-lisp/bytecomp.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 2f5f9f8c05..a1fe524390 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -296,6 +296,8 @@ byte-compile-error-on-warn
   :group 'bytecomp
   :type 'boolean)
 
+(put 'byte-compile-error-on-warn 'safe-local-variable #'booleanp)
+
 (defconst byte-compile-warning-types
   '(redefine callargs free-vars unresolved
 	     obsolete noruntime cl-functions interactive-only
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-06 22:42       ` Robert Cochran
@ 2018-01-08 12:25         ` Noam Postavsky
  2018-01-08 18:50           ` Eli Zaretskii
  2018-01-09  3:42           ` Robert Cochran
  0 siblings, 2 replies; 22+ messages in thread
From: Noam Postavsky @ 2018-01-08 12:25 UTC (permalink / raw)
  To: Robert Cochran; +Cc: Emacs developers

On Sat, Jan 6, 2018 at 5:42 PM, Robert Cochran
<robert+Emacs@cochranmail.com> wrote:

>> See `(elisp) File Local Variables':

Oh, actually I should have posted the next paragraph from the manual:

   When defining a user option using `defcustom', you can set its
`safe-local-variable' property by adding the arguments `:safe FUNCTION'
to `defcustom' (*note Variable Definitions::).

> New patch attached that does it the right way.

This can go to emacs-26, right?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-08 12:25         ` Noam Postavsky
@ 2018-01-08 18:50           ` Eli Zaretskii
  2018-01-09  3:42           ` Robert Cochran
  1 sibling, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-08 18:50 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: emacs-devel

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Mon, 8 Jan 2018 07:25:37 -0500
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> > New patch attached that does it the right way.
> 
> This can go to emacs-26, right?

Yes, I think so.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-08 12:25         ` Noam Postavsky
  2018-01-08 18:50           ` Eli Zaretskii
@ 2018-01-09  3:42           ` Robert Cochran
  2018-01-09 13:24             ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Robert Cochran @ 2018-01-09  3:42 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

Noam Postavsky <npostavs@users.sourceforge.net> writes:

>    When defining a user option using `defcustom', you can set its
> `safe-local-variable' property by adding the arguments `:safe FUNCTION'
> to `defcustom' (*note Variable Definitions::).

Uhm, when I did that (removing the put, and adding ":safe #'booleanp" to
the defcustom), Emacs prompts that the variable list in the file may not
be safe, despite the fact that describe variable clearly states "This
variable is safe as a file local variable if its value satisfies the
predicate ‘booleanp’".

This is the whole contents of the file:

#+BEGIN_SRC emacs-lisp
;;; -*- byte-compile-error-on-warn: t -*-

(defun test/foobar ()
  ;; string-to-unibyte being something I know for fact is marked
  ;; obsolete, causing a warn
  (string-to-unibyte "foo"))
#+END_SRC

Have I done something wrong?

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-09  3:42           ` Robert Cochran
@ 2018-01-09 13:24             ` Stefan Monnier
  2018-01-10  6:28               ` Robert Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2018-01-09 13:24 UTC (permalink / raw)
  To: emacs-devel

>>    When defining a user option using `defcustom', you can set its
>> `safe-local-variable' property by adding the arguments `:safe FUNCTION'
>> to `defcustom' (*note Variable Definitions::).
> Uhm, when I did that (removing the put, and adding ":safe #'booleanp" to
> the defcustom), Emacs prompts that the variable list in the file may not
> be safe, despite the fact that describe variable clearly states "This
> variable is safe as a file local variable if its value satisfies the
> predicate ‘booleanp’".

That's because the var and its safety predicate are only known once the
bytecompiler is loaded, so if you iopen the file in a session where the
bytecompiler has not yet been loaded Emacs won't know about the
safety predicate.

For this reason the `:safe` arg of defcustom is often not useful.
Instead, you want to use the `(put ...)` form with a `;;;###autoload`
cookie in front of it, so that the safety predicate is preloaded.


        Stefan




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-09 13:24             ` Stefan Monnier
@ 2018-01-10  6:28               ` Robert Cochran
  2018-01-10 15:26                 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Robert Cochran @ 2018-01-10  6:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

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

> That's because the var and its safety predicate are only known once the
> bytecompiler is loaded, so if you iopen the file in a session where the
> bytecompiler has not yet been loaded Emacs won't know about the
> safety predicate.

Huh. I would have guessed that the byte compiler would have been loaded
automatically myself, but I suppose I can see why it wouldn't be.

> For this reason the `:safe` arg of defcustom is often not useful.
> Instead, you want to use the `(put ...)` form with a `;;;###autoload`
> cookie in front of it, so that the safety predicate is preloaded.
>

Yeah, I noticed I forgot the autoload cookie, so I knew I had to do it
over again anyways.

Thus, new (and hopefully) final version of the patch.

Please let me know if I need to do anything else.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch to mark byte-compile-error-on-warn as a safe file local variable --]
[-- Type: text/x-patch, Size: 1052 bytes --]

From f7e5bc29caafba09766ec17853f6c6561d96434f Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Sat, 6 Jan 2018 13:19:55 -0800
Subject: [PATCH] Make byte-compile-error-on-warn a safe variable for file
 compilation

This allows individual files to easily specify whether or not byte
compilation should error on encountering warnings, ala -Werror from
GCC.

* lisp/emacs-lisp/bytecomp.el (byte-compile-error-on-warn): Mark as a
  safe local variable.
---
 lisp/emacs-lisp/bytecomp.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 2f5f9f8c05..a481b41aa5 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -295,6 +295,7 @@ byte-compile-error-on-warn
   "If true, the byte-compiler reports warnings with `error'."
   :group 'bytecomp
   :type 'boolean)
+;;;###autoload(put 'byte-compile-error-on-warn 'safe-local-variable 'booleanp)
 
 (defconst byte-compile-warning-types
   '(redefine callargs free-vars unresolved
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10  6:28               ` Robert Cochran
@ 2018-01-10 15:26                 ` Eli Zaretskii
  2018-01-10 15:57                   ` Noam Postavsky
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-10 15:26 UTC (permalink / raw)
  To: Robert Cochran; +Cc: monnier, emacs-devel

> From: Robert Cochran <robert+Emacs@cochranmail.com>
> Date: Tue, 09 Jan 2018 22:28:18 -0800
> Cc: emacs-devel@gnu.org
> 
> Yeah, I noticed I forgot the autoload cookie, so I knew I had to do it
> over again anyways.
> 
> Thus, new (and hopefully) final version of the patch.
> 
> Please let me know if I need to do anything else.

I think we'd like a comment before the autoload cookie explaining why
we do things this way.

Thanks.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 15:26                 ` Eli Zaretskii
@ 2018-01-10 15:57                   ` Noam Postavsky
  2018-01-10 16:17                     ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Noam Postavsky @ 2018-01-10 15:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stefan Monnier, Emacs developers

On Wed, Jan 10, 2018 at 10:26 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Robert Cochran <robert+Emacs@cochranmail.com>
>> Date: Tue, 09 Jan 2018 22:28:18 -0800
>> Cc: emacs-devel@gnu.org
>>
>> Yeah, I noticed I forgot the autoload cookie, so I knew I had to do it
>> over again anyways.
>>
>> Thus, new (and hopefully) final version of the patch.
>>
>> Please let me know if I need to do anything else.
>
> I think we'd like a comment before the autoload cookie explaining why
> we do things this way.

Since this autoload vs :safe property consideration applies more
generally, shouldn't we rather explain in it in the manual?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 15:57                   ` Noam Postavsky
@ 2018-01-10 16:17                     ` Eli Zaretskii
  2018-01-10 16:29                       ` Noam Postavsky
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-10 16:17 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: monnier, emacs-devel

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Wed, 10 Jan 2018 10:57:34 -0500
> Cc: Robert Cochran <robert+Emacs@cochranmail.com>, Stefan Monnier <monnier@iro.umontreal.ca>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> > I think we'd like a comment before the autoload cookie explaining why
> > we do things this way.
> 
> Since this autoload vs :safe property consideration applies more
> generally, shouldn't we rather explain in it in the manual?

I think we should do both.  The fact that some unusual technique is in
the manual doesn't yet mean commentary about it will be redundant.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 16:17                     ` Eli Zaretskii
@ 2018-01-10 16:29                       ` Noam Postavsky
  2018-01-10 19:36                         ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Noam Postavsky @ 2018-01-10 16:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stefan Monnier, Emacs developers

On Wed, Jan 10, 2018 at 11:17 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Wed, 10 Jan 2018 10:57:34 -0500
>> Cc: Robert Cochran <robert+Emacs@cochranmail.com>, Stefan Monnier <monnier@iro.umontreal.ca>,
>>       Emacs developers <emacs-devel@gnu.org>
>>
>> > I think we'd like a comment before the autoload cookie explaining why
>> > we do things this way.
>>
>> Since this autoload vs :safe property consideration applies more
>> generally, shouldn't we rather explain in it in the manual?
>
> I think we should do both.  The fact that some unusual technique is in
> the manual doesn't yet mean commentary about it will be redundant.

But there are many other cases of this autoloaded put, should we
comment on all of them?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 16:29                       ` Noam Postavsky
@ 2018-01-10 19:36                         ` Eli Zaretskii
  2018-01-10 19:44                           ` Noam Postavsky
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-10 19:36 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: monnier, emacs-devel

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Wed, 10 Jan 2018 11:29:45 -0500
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Emacs developers <emacs-devel@gnu.org>
> 
> >> Since this autoload vs :safe property consideration applies more
> >> generally, shouldn't we rather explain in it in the manual?
> >
> > I think we should do both.  The fact that some unusual technique is in
> > the manual doesn't yet mean commentary about it will be redundant.
> 
> But there are many other cases of this autoloaded put, should we
> comment on all of them?

Maybe we should do that on more than just this one.  But even a
thousand-mile journey begins with a single step.

Did you never have this moment of staring at a piece of code that
doesn't explain itself and has no comments, and wondering why the heck
did they do it that way?  It is not easy to find answers to such
questions, even if they are in some manual.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 19:36                         ` Eli Zaretskii
@ 2018-01-10 19:44                           ` Noam Postavsky
  2018-01-10 20:22                             ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Noam Postavsky @ 2018-01-10 19:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stefan Monnier, Emacs developers

On Wed, Jan 10, 2018 at 2:36 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Wed, 10 Jan 2018 11:29:45 -0500
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Emacs developers <emacs-devel@gnu.org>
>>
>> >> Since this autoload vs :safe property consideration applies more
>> >> generally, shouldn't we rather explain in it in the manual?
>> >
>> > I think we should do both.  The fact that some unusual technique is in
>> > the manual doesn't yet mean commentary about it will be redundant.
>>
>> But there are many other cases of this autoloaded put, should we
>> comment on all of them?
>
> Maybe we should do that on more than just this one.  But even a
> thousand-mile journey begins with a single step.
>
> Did you never have this moment of staring at a piece of code that
> doesn't explain itself and has no comments, and wondering why the heck
> did they do it that way?  It is not easy to find answers to such
> questions, even if they are in some manual.

To me, this looks like it should be considered a standard technique,
that we shouldn't need to explain over and over again (but currently
it's not even explained once in the manual, which is not good).



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 19:44                           ` Noam Postavsky
@ 2018-01-10 20:22                             ` Eli Zaretskii
  2018-02-12  8:21                               ` Robert Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-10 20:22 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: monnier, emacs-devel

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Wed, 10 Jan 2018 14:44:59 -0500
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Emacs developers <emacs-devel@gnu.org>
> 
> To me, this looks like it should be considered a standard technique,
> that we shouldn't need to explain over and over again

It would be good to get there at some future point, yes.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-01-10 20:22                             ` Eli Zaretskii
@ 2018-02-12  8:21                               ` Robert Cochran
  2018-02-12 17:57                                 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Robert Cochran @ 2018-02-12  8:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, monnier, Noam Postavsky

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Wed, 10 Jan 2018 14:44:59 -0500
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Emacs developers <emacs-devel@gnu.org>
>> 
>> To me, this looks like it should be considered a standard technique,
>> that we shouldn't need to explain over and over again
>
> It would be good to get there at some future point, yes.
>

Whoops. Didn't mean to leave this set as long as I did. IMO, the
discussion about whether or not the autoload should be documented in the
manual is orthogonal to this particular patch, but if you want me to
document it somehow to get this patch applied, I will.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-02-12  8:21                               ` Robert Cochran
@ 2018-02-12 17:57                                 ` Eli Zaretskii
  2018-02-13  4:49                                   ` Robert Cochran
       [not found]                                   ` <87tvulmrkd.fsf@cochranmail.com>
  0 siblings, 2 replies; 22+ messages in thread
From: Eli Zaretskii @ 2018-02-12 17:57 UTC (permalink / raw)
  To: Robert Cochran; +Cc: emacs-devel, monnier, npostavs

> From: Robert Cochran <robert+Emacs@cochranmail.com>
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 12 Feb 2018 00:21:08 -0800
> 
> Whoops. Didn't mean to leave this set as long as I did. IMO, the
> discussion about whether or not the autoload should be documented in the
> manual is orthogonal to this particular patch, but if you want me to
> document it somehow to get this patch applied, I will.

I asked for a comment near the autoload explaining why we use the
autoload cookie instead of the usual way of marking variables safe.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
  2018-02-12 17:57                                 ` Eli Zaretskii
@ 2018-02-13  4:49                                   ` Robert Cochran
       [not found]                                   ` <87tvulmrkd.fsf@cochranmail.com>
  1 sibling, 0 replies; 22+ messages in thread
From: Robert Cochran @ 2018-02-13  4:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, monnier, npostavs

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

[ Sending this again... Internet was acting up at the time, and it seems
like Gnus didn't do the resend properly... ]

Eli Zaretskii <eliz@gnu.org> writes:

> I asked for a comment near the autoload explaining why we use the
> autoload cookie instead of the usual way of marking variables safe.

Right. Misremembered where exactly you wanted the note.

At any rate, I did add a comment note above the autoload. New patch
attached.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Mark byte-compile-error-on-warn as a safe file-local variable --]
[-- Type: text/x-patch, Size: 1345 bytes --]

From af8f27a1d393847136a5b2932d86f28c525321a8 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Sat, 6 Jan 2018 13:19:55 -0800
Subject: [PATCH] Make byte-compile-error-on-warn a safe variable for file
 compilation

This allows individual files to easily specify whether or not byte
compilation should error on encountering warnings, ala -Werror from
GCC.

* lisp/emacs-lisp/bytecomp.el (byte-compile-error-on-warn): Mark as a
  safe local variable.
---
 lisp/emacs-lisp/bytecomp.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 2f5f9f8c05..6db3617c04 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -295,6 +295,11 @@ byte-compile-error-on-warn
   "If true, the byte-compiler reports warnings with `error'."
   :group 'bytecomp
   :type 'boolean)
+;; This needs to be autoloaded because it needs to be available to
+;; Emacs before the byte compiler is loaded, otherwise Emacs will not
+;; know that this variable is marked as safe until it is too late.
+;; (See https://lists.gnu.org/archive/html/emacs-devel/2018-01/msg00261.html )
+;;;###autoload(put 'byte-compile-error-on-warn 'safe-local-variable 'booleanp)
 
 (defconst byte-compile-warning-types
   '(redefine callargs free-vars unresolved
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation
       [not found]                                   ` <87tvulmrkd.fsf@cochranmail.com>
@ 2018-02-16 15:53                                     ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2018-02-16 15:53 UTC (permalink / raw)
  To: Robert Cochran; +Cc: rnpostavs, monnier, emacs-devel

> From: Robert Cochran <robert+Emacs@cochranmail.com>
> Cc: Robert Cochran <robert+Emacs@cochranmail.com>,  npostavs@users.sourceforge.net,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 12 Feb 2018 11:35:06 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I asked for a comment near the autoload explaining why we use the
> > autoload cookie instead of the usual way of marking variables safe.
> 
> Right. I misremembered that specific piece.
> 
> Anyways, I went ahead and left a note there.

Thanks, pushed to emacs-26.



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-02-16 15:53 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-04  9:40 [PATCH] Make byte-compile-error-on-warn a safe variable for file compilation Robert Cochran
2018-01-04 13:22 ` Noam Postavsky
2018-01-05  3:51   ` Stefan Monnier
2018-01-05  6:17   ` Robert Cochran
2018-01-05 12:05     ` Noam Postavsky
2018-01-06 22:42       ` Robert Cochran
2018-01-08 12:25         ` Noam Postavsky
2018-01-08 18:50           ` Eli Zaretskii
2018-01-09  3:42           ` Robert Cochran
2018-01-09 13:24             ` Stefan Monnier
2018-01-10  6:28               ` Robert Cochran
2018-01-10 15:26                 ` Eli Zaretskii
2018-01-10 15:57                   ` Noam Postavsky
2018-01-10 16:17                     ` Eli Zaretskii
2018-01-10 16:29                       ` Noam Postavsky
2018-01-10 19:36                         ` Eli Zaretskii
2018-01-10 19:44                           ` Noam Postavsky
2018-01-10 20:22                             ` Eli Zaretskii
2018-02-12  8:21                               ` Robert Cochran
2018-02-12 17:57                                 ` Eli Zaretskii
2018-02-13  4:49                                   ` Robert Cochran
     [not found]                                   ` <87tvulmrkd.fsf@cochranmail.com>
2018-02-16 15:53                                     ` Eli Zaretskii

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).