bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Philipp Stephani]

Create an Elisp file such as

   (defvar foo ?()

and try to run `update-file-autoloads' on it.  This will fail with an
unbalanced parenthesis error.  However, this is valid (albeit
discouraged) Elisp code.


In GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.10.7)
 of 2014-03-07 on lamiak, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Configured using:
 `configure '--build' 'x86_64-linux-gnu' '--build' 'x86_64-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var/lib' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3/site-lisp:/usr/share/emacs/site-lisp'
 '--with-crt-dir=/usr/lib/x86_64-linux-gnu' '--with-x=yes'
 '--with-x-toolkit=gtk3' '--with-toolkit-scroll-bars'
 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fstack-protector
 --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall'
 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro'
 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x C-f / t m p / a u t o l o a d t e s t . e l <return> 
( d e f v a r SPC f o o SPC ? ( ) <return> C-x C-s 
M-x b y t e - c o m <tab> <return> a u t o <tab> <return> 
M-x u p d a t e - a u <tab> <tab> f <tab> <return> 
a u <tab> <return> a u <backspace> <backspace> l o 
a <tab> d d e f s . e l <return> M-x r e p o r t <
return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
(New file)
Saving file /tmp/autoloadtest.el...
Wrote /tmp/autoloadtest.el
Compiling /tmp/autoloadtest.el...done
Wrote /tmp/autoloadtest.elc
Making completion list...
Generating autoloads for /tmp/autoloadtest.el...
forward-sexp: Scan error: "Unbalanced parentheses", 1, 17

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message byte-opt format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils autoload help-fns help-mode easymenu
warnings compile comint ansi-color ring bytecomp byte-compile cconv
time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)

-- 
Google Germany GmbH
Dienerstraße 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

Diese E-Mail ist vertraulich.  Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen
Sie die E-Mail und alle Anhänge.  Vielen Dank.
       
This e-mail is confidential.  If you are not the right addressee please do not
forward it, please inform the sender, and please erase this e-mail including
any attachments.  Thanks.

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Stefan Monnier]

> Create an Elisp file such as
>    (defvar foo ?()
> and try to run `update-file-autoloads' on it.  This will fail with an
> unbalanced parenthesis error.  However, this is valid (albeit
> discouraged) Elisp code.

Yes: don't do that.  Use (defvar foo ?\() instead.
This same error will appear during interactive editing of that code.


        Stefan

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Philipp Stephani]

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

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Fr., 19. Juni 2015 um
23:24 Uhr:

> > Create an Elisp file such as
> >    (defvar foo ?()
> > and try to run `update-file-autoloads' on it.  This will fail with an
> > unbalanced parenthesis error.  However, this is valid (albeit
> > discouraged) Elisp code.
>
> Yes: don't do that.  Use (defvar foo ?\() instead.
> This same error will appear during interactive editing of that code.
>
>
>
If this is generally unsupported, can we then remove it from the doc and
have the reader print a warning (or reject it altogether)?

[-- Attachment #2: Type: text/html, Size: 913 bytes --]

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Stefan Monnier]

> If this is generally unsupported, can we then remove it from the doc and

Yes.  Do you happen to know where it's documented?

> have the reader print a warning (or reject it altogether)?

We can't reject it, because I'm pretty sure there's code out there which
uses it.  I'd accept a patch which adds a warning for it,


        Stefan

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Philipp Stephani]

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

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mo., 22. Juni 2015 um
03:18 Uhr:

> > If this is generally unsupported, can we then remove it from the doc and
>
> Yes.  Do you happen to know where it's documented?
>

https://www.gnu.org/software/emacs/manual/html_node/elisp/Basic-Char-Syntax.html

"You can use the same syntax for punctuation characters, but it is often a
good idea to add a ‘\’ so that the Emacs commands for editing Lisp code
don’t get confused. For example, ‘?\(’ is the way to write the open-paren
character. If the character is ‘\’, you must use a second ‘\’ to quote it:
‘?\\’."

Here I would put ( and ) into the same category as \, so that it's not only
"a good idea" to escape it.

"However, you should add a backslash before any of the characters
‘()\|;'`"#.,’ to avoid confusing the Emacs commands for editing Lisp code."

Here I would make escaping mandatory for the first three characters (or
even for all of them).


>
> > have the reader print a warning (or reject it altogether)?
>
> We can't reject it, because I'm pretty sure there's code out there which
> uses it.  I'd accept a patch which adds a warning for it,
>
>
>
OK, I'll try to come up with a patch.

[-- Attachment #2: Type: text/html, Size: 1955 bytes --]

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 219 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am So., 28. Juni 2015 um
15:12 Uhr:

> OK, I'll try to come up with a patch.
>

I've attached a patch that uses the same mechanism as the check for
old-style backquotes.

[-- Attachment #1.2: Type: text/html, Size: 577 bytes --]

[-- Attachment #2: 0001-lread.c-Fload-Warn-about-missing-backslashes.patch --]
[-- Type: application/octet-stream, Size: 2653 bytes --]

From 570b3fa697a9597169a27ac47b1a56f5440203b9 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Tue, 30 Jun 2015 22:38:35 +0200
Subject: [PATCH] * lread.c (Fload): Warn about missing backslashes

* lread.c (load_warn_unescaped_character_literals, Fload, read1)
(syms_of_lread): Warn if unescaped parenthesis character literals
are found (Bug#20152).
---
 src/lread.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/lread.c b/src/lread.c
index 11c8d00..c2df2bd 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -952,6 +952,15 @@ load_warn_old_style_backquotes (Lisp_Object file)
     }
 }
 
+static void
+load_warn_unescaped_character_literals (Lisp_Object file) {
+  if (!NILP (Vunescaped_character_literals))
+    {
+      AUTO_STRING (format, "Loading `%s': unescaped parenthesis character literals detected!");
+      CALLN (Fmessage, format, file);
+    }
+}
+
 DEFUN ("get-load-suffixes", Fget_load_suffixes, Sget_load_suffixes, 0, 0, 0,
        doc: /* Return the suffixes that `load' should try if a suffix is \
 required.
@@ -1194,6 +1203,11 @@ Return t if the file exists and loads successfully.  */)
   specbind (Qold_style_backquotes, Qnil);
   record_unwind_protect (load_warn_old_style_backquotes, file);
 
+  /* Check for the presence of unescaped parenthesis character literals
+     and warn about them. */
+  specbind (Qunescaped_character_literals, Qnil);
+  record_unwind_protect(load_warn_unescaped_character_literals, file);
+
   if (!memcmp (SDATA (found) + SBYTES (found) - 4, ".elc", 4)
       || (fd >= 0 && (version = safe_to_load_version (fd)) > 0))
     /* Load .elc files directly, but not when they are
@@ -2971,6 +2985,9 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 	if (c == ' ' || c == '\t')
 	  return make_number (c);
 
+	if (c == '(' || c == ')')
+	  Vunescaped_character_literals = Qt;
+
 	if (c == '\\')
 	  c = read_escape (readcharfun, 0);
 	modifiers = c & CHAR_MODIFIER_MASK;
@@ -4678,6 +4695,10 @@ variables, this must be set in the first line of a file.  */);
   Vold_style_backquotes = Qnil;
   DEFSYM (Qold_style_backquotes, "old-style-backquotes");
 
+  DEFVAR_LISP ("unescaped-character-literals", Vunescaped_character_literals,
+               doc: /* Set to non-nil when `read' encounters a deprecated unescaped character literal.  */);
+  DEFSYM (Qunescaped_character_literals, "unescaped-character-literals");
+
   DEFVAR_BOOL ("load-prefer-newer", load_prefer_newer,
                doc: /* Non-nil means `load' prefers the newest version of a file.
 This applies when a filename suffix is not explicitly specified and
-- 
2.4.4

bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal

[Stefan Monnier]

> I've attached a patch that uses the same mechanism as the check for
> old-style backquotes.

Looks pretty good, thank you.
A few comments and nitpicks:

> +static void
> +load_warn_unescaped_character_literals (Lisp_Object file) {
> +  if (!NILP (Vunescaped_character_literals))
> +    {
> +      AUTO_STRING (format, "Loading `%s': unescaped parenthesis character literals detected!");
> +      CALLN (Fmessage, format, file);
> +    }
> +}

It would be even better to show the actual unescaped character literal(s).
Either just displaying one of the chars, or displaying them all.

>  	if (c == ' ' || c == '\t')
>  	  return make_number (c);
> 
> +	if (c == '(' || c == ')')
> +	  Vunescaped_character_literals = Qt;

I think we want to emit the same warning for several other chars, at
least ';', '"', '[', and ']' (we could also include ' ' and '\t', tho
I'm not sure it's worth the trouble.  If/when we want to do that, we'd
want to do it not just for ?<SPC> and ?<TAB> but also for ?\<SPC>
and ?\<TAB> and recommend the use of ?\s and ?\t instead).

>    DEFSYM (Qold_style_backquotes, "old-style-backquotes");
> +  DEFSYM (Qunescaped_character_literals, "unescaped-character-literals");

I think "old-style-backquotes" was a bad choice.  It should have been
"read--found-old-style-backquotes" or something along these lines.
AFAIK this variable is only used internally, so we can rename it without
any extra precautions.  If you could do that in your patch, that'd
be great.

One more thing: old-style-backquotes is not only used by `load' but also
by the byte-compiler.  So take a look at bytecomp.el where you'll
probably want to add similar code for your unescaped-character-literals.


        Stefan

bug#20852: [PATCH] Warn about missing backslashes during load

[Stefan Monnier]

Looks great, thank you very much.  See comments below.

> +	if (c == '(' || c == ')' || c == '[' || c == ']'
> +            || c == '"' || c == ';')

These are indeed the most important ones w.r.t elisp-mode's
syntax-table, but I think we should put #, ?, and ' in there as well.
This said, maybe we should add chars in there "little by little" to try
and not overwhelm people with warnings.
So the above set might actually be a good set with which to start.

> +               doc: /* List of deprecated unescaped character literals encountered by `read'.  For internal use only.  */);

Please keep the first line of docstrings with 80 columns.  I.e. move the
"For internal use only." to a second line.


        Stefan

bug#20852: [PATCH] Warn about missing backslashes during load

[Philipp Stephani]

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

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mo., 1. Mai 2017 um
19:59 Uhr:

> Looks great, thank you very much.  See comments below.
>
> > +     if (c == '(' || c == ')' || c == '[' || c == ']'
> > +            || c == '"' || c == ';')
>
> These are indeed the most important ones w.r.t elisp-mode's
> syntax-table, but I think we should put #, ?, and ' in there as well.
> This said, maybe we should add chars in there "little by little" to try
> and not overwhelm people with warnings.
> So the above set might actually be a good set with which to start.
>

Yeah, I've left it as-is for now, the current set are those characters that
cause problems in practice (wrong paren highlighting, confusion with
paredit).


>
> > +               doc: /* List of deprecated unescaped character literals
> encountered by `read'.  For internal use only.  */);
>
> Please keep the first line of docstrings with 80 columns.  I.e. move the
> "For internal use only." to a second line.
>
>
Done and pushed as c2bbdc3316.

[-- Attachment #2: Type: text/html, Size: 1622 bytes --]

[PATCH] Make `old-style-backquotes' variable internal

[Philipp]

* src/lread.c (load_warn_old_style_backquotes, Fload, read1)
(syms_of_lread): Rename `old-style-backquotes' to
`lread--old-style-backquotes', and clarify that it's for internal
use only.
* lisp/emacs-lisp/bytecomp.el (byte-compile-from-buffer): Rename
variable.
* test/src/lread-tests.el (lread-tests--old-style-backquotes): Add
unit test.
* emacs-lisp/bytecomp-tests.el
(bytecomp-tests--old-style-backquotes): Add unit test.
---
 lisp/emacs-lisp/bytecomp.el            |  4 ++--
 src/lread.c                            | 17 +++++++++--------
 test/lisp/emacs-lisp/bytecomp-tests.el | 15 +++++++++++++++
 test/src/lread-tests.el                |  9 +++++++++
 4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 25102548a9..3fd3f5a1b8 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2026,11 +2026,11 @@ byte-compile-from-buffer
 		 (not (eobp)))
 	  (setq byte-compile-read-position (point)
 		byte-compile-last-position byte-compile-read-position)
-	  (let* ((old-style-backquotes nil)
+	  (let* ((lread--old-style-backquotes nil)
                  (lread--unescaped-character-literals nil)
                  (form (read inbuffer)))
             ;; Warn about the use of old-style backquotes.
-            (when old-style-backquotes
+            (when lread--old-style-backquotes
               (byte-compile-warn "!! The file uses old-style backquotes !!
 This functionality has been obsolete for more than 10 years already
 and will be removed soon.  See (elisp)Backquote in the manual."))
diff --git a/src/lread.c b/src/lread.c
index 6467043b1d..3cf8ef5a4e 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -948,7 +948,7 @@ load_error_handler (Lisp_Object data)
 static void
 load_warn_old_style_backquotes (Lisp_Object file)
 {
-  if (!NILP (Vold_style_backquotes))
+  if (!NILP (Vlread_old_style_backquotes))
     {
       AUTO_STRING (format, "Loading `%s': old-style backquotes detected!");
       CALLN (Fmessage, format, file);
@@ -1214,7 +1214,7 @@ Return t if the file exists and loads successfully.  */)
   version = -1;
 
   /* Check for the presence of old-style quotes and warn about them.  */
-  specbind (Qold_style_backquotes, Qnil);
+  specbind (Qlread_old_style_backquotes, Qnil);
   record_unwind_protect (load_warn_old_style_backquotes, file);
 
   /* Check for the presence of unescaped character literals and warn
@@ -3037,7 +3037,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 	   "(\`" anyway).  */
 	if (!new_backquote_flag && first_in_list && next_char == ' ')
 	  {
-	    Vold_style_backquotes = Qt;
+	    Vlread_old_style_backquotes = Qt;
 	    goto default_label;
 	  }
 	else
@@ -3091,7 +3091,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 	  }
 	else
 	  {
-	    Vold_style_backquotes = Qt;
+	    Vlread_old_style_backquotes = Qt;
 	    goto default_label;
 	  }
       }
@@ -4840,10 +4840,11 @@ variables, this must be set in the first line of a file.  */);
 	       doc: /* List of buffers being read from by calls to `eval-buffer' and `eval-region'.  */);
   Veval_buffer_list = Qnil;
 
-  DEFVAR_LISP ("old-style-backquotes", Vold_style_backquotes,
-	       doc: /* Set to non-nil when `read' encounters an old-style backquote.  */);
-  Vold_style_backquotes = Qnil;
-  DEFSYM (Qold_style_backquotes, "old-style-backquotes");
+  DEFVAR_LISP ("lread--old-style-backquotes", Vlread_old_style_backquotes,
+	       doc: /* Set to non-nil when `read' encounters an old-style backquote.
+For internal use only.  */);
+  Vlread_old_style_backquotes = Qnil;
+  DEFSYM (Qlread_old_style_backquotes, "lread--old-style-backquotes");
 
   DEFVAR_LISP ("lread--unescaped-character-literals",
                Vlread_unescaped_character_literals,
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 3624904753..a83c998a59 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -529,6 +529,21 @@ bytecomp-tests--with-temp-file
                        (list (concat "unescaped character literals "
                                      "\", (, ), ;, [, ] detected!"))))))))
 
+(ert-deftest bytecomp-tests--old-style-backquotes ()
+  "Check that byte compiling warns about old-style backquotes."
+  (should (boundp 'lread--old-style-backquotes))
+  (bytecomp-tests--with-temp-file source
+    (write-region "(` (a b))" nil source)
+    (bytecomp-tests--with-temp-file destination
+      (let* ((byte-compile-dest-file-function (lambda (_) destination))
+            (byte-compile-error-on-warn t)
+            (byte-compile-debug t)
+            (err (should-error (byte-compile-file source))))
+        (should (equal (cdr err)
+                       (list "!! The file uses old-style backquotes !!
+This functionality has been obsolete for more than 10 years already
+and will be removed soon.  See (elisp)Backquote in the manual.")))))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
diff --git a/test/src/lread-tests.el b/test/src/lread-tests.el
index 84342348d4..e45d184cad 100644
--- a/test/src/lread-tests.el
+++ b/test/src/lread-tests.el
@@ -142,4 +142,13 @@ lread-tests--last-message
                            "unescaped character literals "
                            "\", (, ), ;, [, ] detected!")))))
 
+(ert-deftest lread-tests--old-style-backquotes ()
+  "Check that loading warns about old-style backquotes."
+  (lread-tests--with-temp-file file-name
+    (write-region "(` (a b))" nil file-name)
+    (should (equal (load file-name nil :nomessage :nosuffix) t))
+    (should (equal (lread-tests--last-message)
+                   (concat (format-message "Loading `%s': " file-name)
+                           "old-style backquotes detected!")))))
+
 ;;; lread-tests.el ends here
-- 
2.12.2

Re: [PATCH] Make `old-style-backquotes' variable internal

[Eli Zaretskii]

> From: Philipp <phil07c1@gmail.com>
> Date: Sat,  6 May 2017 23:14:20 +0200
> Cc: Philipp <phst@google.com>
> 
> * src/lread.c (load_warn_old_style_backquotes, Fload, read1)
> (syms_of_lread): Rename `old-style-backquotes' to
> `lread--old-style-backquotes', and clarify that it's for internal
> use only.

Should this be in NEWS?

[PATCH] Make `old-style-backquotes' variable internal

[Philipp]

* src/lread.c (load_warn_old_style_backquotes, Fload, read1)
(syms_of_lread): Rename `old-style-backquotes' to
`lread--old-style-backquotes', and clarify that it's for internal
use only.
* lisp/emacs-lisp/bytecomp.el (byte-compile-from-buffer): Rename
variable.
* test/src/lread-tests.el (lread-tests--old-style-backquotes): Add
unit test.
* emacs-lisp/bytecomp-tests.el
(bytecomp-tests--old-style-backquotes): Add unit test.
---
 etc/NEWS                               |  5 +++++
 lisp/emacs-lisp/bytecomp.el            |  4 ++--
 src/lread.c                            | 17 +++++++++--------
 test/lisp/emacs-lisp/bytecomp-tests.el | 15 +++++++++++++++
 test/src/lread-tests.el                |  9 +++++++++
 5 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1f1f4b4b4b..166744b848 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -896,6 +896,11 @@ which was sometimes numerically incorrect.  For example, on a 64-bit
 host (max 1e16 10000000000000001) now returns its second argument
 instead of its first.
 
++++
+** The variable 'old-style-backquotes' has been made internal and
+renamed to 'lread--old-style-backquotes'.  No user code should use
+this variable.
+
 \f
 * Lisp Changes in Emacs 26.1
 
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 201733ff03..04335ffbfe 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2021,11 +2021,11 @@ byte-compile-from-buffer
 		 (not (eobp)))
 	  (setq byte-compile-read-position (point)
 		byte-compile-last-position byte-compile-read-position)
-	  (let* ((old-style-backquotes nil)
+	  (let* ((lread--old-style-backquotes nil)
                  (lread--unescaped-character-literals nil)
                  (form (read inbuffer)))
             ;; Warn about the use of old-style backquotes.
-            (when old-style-backquotes
+            (when lread--old-style-backquotes
               (byte-compile-warn "!! The file uses old-style backquotes !!
 This functionality has been obsolete for more than 10 years already
 and will be removed soon.  See (elisp)Backquote in the manual."))
diff --git a/src/lread.c b/src/lread.c
index 6467043b1d..3cf8ef5a4e 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -948,7 +948,7 @@ load_error_handler (Lisp_Object data)
 static void
 load_warn_old_style_backquotes (Lisp_Object file)
 {
-  if (!NILP (Vold_style_backquotes))
+  if (!NILP (Vlread_old_style_backquotes))
     {
       AUTO_STRING (format, "Loading `%s': old-style backquotes detected!");
       CALLN (Fmessage, format, file);
@@ -1214,7 +1214,7 @@ Return t if the file exists and loads successfully.  */)
   version = -1;
 
   /* Check for the presence of old-style quotes and warn about them.  */
-  specbind (Qold_style_backquotes, Qnil);
+  specbind (Qlread_old_style_backquotes, Qnil);
   record_unwind_protect (load_warn_old_style_backquotes, file);
 
   /* Check for the presence of unescaped character literals and warn
@@ -3037,7 +3037,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 	   "(\`" anyway).  */
 	if (!new_backquote_flag && first_in_list && next_char == ' ')
 	  {
-	    Vold_style_backquotes = Qt;
+	    Vlread_old_style_backquotes = Qt;
 	    goto default_label;
 	  }
 	else
@@ -3091,7 +3091,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 	  }
 	else
 	  {
-	    Vold_style_backquotes = Qt;
+	    Vlread_old_style_backquotes = Qt;
 	    goto default_label;
 	  }
       }
@@ -4840,10 +4840,11 @@ variables, this must be set in the first line of a file.  */);
 	       doc: /* List of buffers being read from by calls to `eval-buffer' and `eval-region'.  */);
   Veval_buffer_list = Qnil;
 
-  DEFVAR_LISP ("old-style-backquotes", Vold_style_backquotes,
-	       doc: /* Set to non-nil when `read' encounters an old-style backquote.  */);
-  Vold_style_backquotes = Qnil;
-  DEFSYM (Qold_style_backquotes, "old-style-backquotes");
+  DEFVAR_LISP ("lread--old-style-backquotes", Vlread_old_style_backquotes,
+	       doc: /* Set to non-nil when `read' encounters an old-style backquote.
+For internal use only.  */);
+  Vlread_old_style_backquotes = Qnil;
+  DEFSYM (Qlread_old_style_backquotes, "lread--old-style-backquotes");
 
   DEFVAR_LISP ("lread--unescaped-character-literals",
                Vlread_unescaped_character_literals,
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 3624904753..a83c998a59 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -529,6 +529,21 @@ bytecomp-tests--with-temp-file
                        (list (concat "unescaped character literals "
                                      "\", (, ), ;, [, ] detected!"))))))))
 
+(ert-deftest bytecomp-tests--old-style-backquotes ()
+  "Check that byte compiling warns about old-style backquotes."
+  (should (boundp 'lread--old-style-backquotes))
+  (bytecomp-tests--with-temp-file source
+    (write-region "(` (a b))" nil source)
+    (bytecomp-tests--with-temp-file destination
+      (let* ((byte-compile-dest-file-function (lambda (_) destination))
+            (byte-compile-error-on-warn t)
+            (byte-compile-debug t)
+            (err (should-error (byte-compile-file source))))
+        (should (equal (cdr err)
+                       (list "!! The file uses old-style backquotes !!
+This functionality has been obsolete for more than 10 years already
+and will be removed soon.  See (elisp)Backquote in the manual.")))))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
diff --git a/test/src/lread-tests.el b/test/src/lread-tests.el
index 84342348d4..e45d184cad 100644
--- a/test/src/lread-tests.el
+++ b/test/src/lread-tests.el
@@ -142,4 +142,13 @@ lread-tests--last-message
                            "unescaped character literals "
                            "\", (, ), ;, [, ] detected!")))))
 
+(ert-deftest lread-tests--old-style-backquotes ()
+  "Check that loading warns about old-style backquotes."
+  (lread-tests--with-temp-file file-name
+    (write-region "(` (a b))" nil file-name)
+    (should (equal (load file-name nil :nomessage :nosuffix) t))
+    (should (equal (lread-tests--last-message)
+                   (concat (format-message "Loading `%s': " file-name)
+                           "old-style backquotes detected!")))))
+
 ;;; lread-tests.el ends here
-- 
2.12.2

Re: [PATCH] Make `old-style-backquotes' variable internal

[Philipp Stephani]

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

Philipp <phil07c1@gmail.com> schrieb am So., 7. Mai 2017 um 13:49 Uhr:

> * src/lread.c (load_warn_old_style_backquotes, Fload, read1)
> (syms_of_lread): Rename `old-style-backquotes' to
> `lread--old-style-backquotes', and clarify that it's for internal
> use only.
> * lisp/emacs-lisp/bytecomp.el (byte-compile-from-buffer): Rename
> variable.
> * test/src/lread-tests.el (lread-tests--old-style-backquotes): Add
> unit test.
> * emacs-lisp/bytecomp-tests.el
> (bytecomp-tests--old-style-backquotes): Add unit test.
> ---
>  etc/NEWS                               |  5 +++++
>  lisp/emacs-lisp/bytecomp.el            |  4 ++--
>  src/lread.c                            | 17 +++++++++--------
>  test/lisp/emacs-lisp/bytecomp-tests.el | 15 +++++++++++++++
>  test/src/lread-tests.el                |  9 +++++++++
>  5 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index 1f1f4b4b4b..166744b848 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -896,6 +896,11 @@ which was sometimes numerically incorrect.  For
> example, on a 64-bit
>  host (max 1e16 10000000000000001) now returns its second argument
>  instead of its first.
>
> ++++
> +** The variable 'old-style-backquotes' has been made internal and
> +renamed to 'lread--old-style-backquotes'.  No user code should use
> +this variable.
> +
>
>  * Lisp Changes in Emacs 26.1
>
> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> index 201733ff03..04335ffbfe 100644
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -2021,11 +2021,11 @@ byte-compile-from-buffer
>                  (not (eobp)))
>           (setq byte-compile-read-position (point)
>                 byte-compile-last-position byte-compile-read-position)
> -         (let* ((old-style-backquotes nil)
> +         (let* ((lread--old-style-backquotes nil)
>                   (lread--unescaped-character-literals nil)
>                   (form (read inbuffer)))
>              ;; Warn about the use of old-style backquotes.
> -            (when old-style-backquotes
> +            (when lread--old-style-backquotes
>                (byte-compile-warn "!! The file uses old-style backquotes !!
>  This functionality has been obsolete for more than 10 years already
>  and will be removed soon.  See (elisp)Backquote in the manual."))
> diff --git a/src/lread.c b/src/lread.c
> index 6467043b1d..3cf8ef5a4e 100644
> --- a/src/lread.c
> +++ b/src/lread.c
> @@ -948,7 +948,7 @@ load_error_handler (Lisp_Object data)
>  static void
>  load_warn_old_style_backquotes (Lisp_Object file)
>  {
> -  if (!NILP (Vold_style_backquotes))
> +  if (!NILP (Vlread_old_style_backquotes))
>      {
>        AUTO_STRING (format, "Loading `%s': old-style backquotes
> detected!");
>        CALLN (Fmessage, format, file);
> @@ -1214,7 +1214,7 @@ Return t if the file exists and loads successfully.
> */)
>    version = -1;
>
>    /* Check for the presence of old-style quotes and warn about them.  */
> -  specbind (Qold_style_backquotes, Qnil);
> +  specbind (Qlread_old_style_backquotes, Qnil);
>    record_unwind_protect (load_warn_old_style_backquotes, file);
>
>    /* Check for the presence of unescaped character literals and warn
> @@ -3037,7 +3037,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool
> first_in_list)
>            "(\`" anyway).  */
>         if (!new_backquote_flag && first_in_list && next_char == ' ')
>           {
> -           Vold_style_backquotes = Qt;
> +           Vlread_old_style_backquotes = Qt;
>             goto default_label;
>           }
>         else
> @@ -3091,7 +3091,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool
> first_in_list)
>           }
>         else
>           {
> -           Vold_style_backquotes = Qt;
> +           Vlread_old_style_backquotes = Qt;
>             goto default_label;
>           }
>        }
> @@ -4840,10 +4840,11 @@ variables, this must be set in the first line of a
> file.  */);
>                doc: /* List of buffers being read from by calls to
> `eval-buffer' and `eval-region'.  */);
>    Veval_buffer_list = Qnil;
>
> -  DEFVAR_LISP ("old-style-backquotes", Vold_style_backquotes,
> -              doc: /* Set to non-nil when `read' encounters an old-style
> backquote.  */);
> -  Vold_style_backquotes = Qnil;
> -  DEFSYM (Qold_style_backquotes, "old-style-backquotes");
> +  DEFVAR_LISP ("lread--old-style-backquotes", Vlread_old_style_backquotes,
> +              doc: /* Set to non-nil when `read' encounters an old-style
> backquote.
> +For internal use only.  */);
> +  Vlread_old_style_backquotes = Qnil;
> +  DEFSYM (Qlread_old_style_backquotes, "lread--old-style-backquotes");
>
>    DEFVAR_LISP ("lread--unescaped-character-literals",
>                 Vlread_unescaped_character_literals,
> diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el
> b/test/lisp/emacs-lisp/bytecomp-tests.el
> index 3624904753..a83c998a59 100644
> --- a/test/lisp/emacs-lisp/bytecomp-tests.el
> +++ b/test/lisp/emacs-lisp/bytecomp-tests.el
> @@ -529,6 +529,21 @@ bytecomp-tests--with-temp-file
>                         (list (concat "unescaped character literals "
>                                       "\", (, ), ;, [, ] detected!"))))))))
>
> +(ert-deftest bytecomp-tests--old-style-backquotes ()
> +  "Check that byte compiling warns about old-style backquotes."
> +  (should (boundp 'lread--old-style-backquotes))
> +  (bytecomp-tests--with-temp-file source
> +    (write-region "(` (a b))" nil source)
> +    (bytecomp-tests--with-temp-file destination
> +      (let* ((byte-compile-dest-file-function (lambda (_) destination))
> +            (byte-compile-error-on-warn t)
> +            (byte-compile-debug t)
> +            (err (should-error (byte-compile-file source))))
> +        (should (equal (cdr err)
> +                       (list "!! The file uses old-style backquotes !!
> +This functionality has been obsolete for more than 10 years already
> +and will be removed soon.  See (elisp)Backquote in the manual.")))))))
> +
>  ;; Local Variables:
>  ;; no-byte-compile: t
>  ;; End:
> diff --git a/test/src/lread-tests.el b/test/src/lread-tests.el
> index 84342348d4..e45d184cad 100644
> --- a/test/src/lread-tests.el
> +++ b/test/src/lread-tests.el
> @@ -142,4 +142,13 @@ lread-tests--last-message
>                             "unescaped character literals "
>                             "\", (, ), ;, [, ] detected!")))))
>
> +(ert-deftest lread-tests--old-style-backquotes ()
> +  "Check that loading warns about old-style backquotes."
> +  (lread-tests--with-temp-file file-name
> +    (write-region "(` (a b))" nil file-name)
> +    (should (equal (load file-name nil :nomessage :nosuffix) t))
> +    (should (equal (lread-tests--last-message)
> +                   (concat (format-message "Loading `%s': " file-name)
> +                           "old-style backquotes detected!")))))
> +
>  ;;; lread-tests.el ends here
> --
> 2.12.2
>
>
Pushed as a1d4615921.

[-- Attachment #2: Type: text/html, Size: 8412 bytes --]