[PATCH 1/2] Add new variable to force new-style backquote interpretation.

[PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Philipp Stephani]

* src/lread.c (syms_of_lread): Add new variable
'force-new-style-backquotes'.
(read_internal_start): Use it.

* test/src/lread-tests.el (lread-tests--force-new-style-backquotes):
New test.

* etc/NEWS: Document new variable.
---
 etc/NEWS                |  3 ++-
 src/lread.c             | 19 ++++++++++++++-----
 test/src/lread-tests.el |  8 ++++++++
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 64c74c0d56..33841c087d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -181,7 +181,8 @@ support, you should set 'eldoc-documentation-function' instead of
 calling 'eldoc-message' directly.
 
 ** Old-style backquotes now generate an error.  They have been
-generating warnings for a decade.
+generating warnings for a decade.  To interpret old-style backquotes
+as new-style, bind the new variable 'force-new-style-backquotes' to t.
 
 \f
 * Lisp Changes in Emacs 27.1
diff --git a/src/lread.c b/src/lread.c
index 52897b4fcc..57cb0a5e48 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -147,10 +147,10 @@ static ptrdiff_t prev_saved_doc_string_length;
 /* This is the file position that string came from.  */
 static file_offset prev_saved_doc_string_position;
 
-/* True means inside a new-style backquote
-   with no surrounding parentheses.
-   Fread initializes this to false, so we need not specbind it
-   or worry about what happens to it when there is an error.  */
+/* True means inside a new-style backquote with no surrounding
+   parentheses.  Fread initializes this to the value of
+   `force_new_style_backquotes', so we need not specbind it or worry
+   about what happens to it when there is an error.  */
 static bool new_backquote_flag;
 
 /* A list of file names for files being loaded in Fload.  Used to
@@ -2187,7 +2187,7 @@ read_internal_start (Lisp_Object stream, Lisp_Object start, Lisp_Object end)
   Lisp_Object retval;
 
   readchar_count = 0;
-  new_backquote_flag = 0;
+  new_backquote_flag = force_new_style_backquotes;
   /* We can get called from readevalloop which may have set these
      already.  */
   if (! HASH_TABLE_P (read_objects_map)
@@ -5006,6 +5006,15 @@ Note that if you customize this, obviously it will not affect files
 that are loaded before your customizations are read!  */);
   load_prefer_newer = 0;
 
+  DEFVAR_BOOL ("force-new-style-backquotes", force_new_style_backquotes,
+               doc: /* Non-nil means to always use the current syntax for backquotes.
+If nil, `load' and `read' raise errors when encountering some
+old-style variants of backquote and comma.  If non-nil, these
+constructs are always interpreted as described in the Info node
+`(elisp)Backquotes', even if that interpretation is incompatible with
+previous versions of Emacs.  */);
+  force_new_style_backquotes = false;
+
   /* Vsource_directory was initialized in init_lread.  */
 
   DEFSYM (Qcurrent_load_list, "current-load-list");
diff --git a/test/src/lread-tests.el b/test/src/lread-tests.el
index 3f41982eba..4dfed13be7 100644
--- a/test/src/lread-tests.el
+++ b/test/src/lread-tests.el
@@ -181,6 +181,14 @@ lread-tests--last-message
                      (list (concat (format-message "Loading `%s': " file-name)
                                    "old-style backquotes detected!")))))))
 
+(ert-deftest lread-tests--force-new-style-backquotes ()
+  (let ((data (should-error (read "(` (a b))"))))
+    (should (equal (cdr data)
+                   '("Loading `nil': old-style backquotes detected!"))))
+  (should (equal (let ((force-new-style-backquotes t))
+                   (read "(` (a b))"))
+                 '(`(a b)))))
+
 (ert-deftest lread-lread--substitute-object-in-subtree ()
   (let ((x (cons 0 1)))
     (setcar x x)
-- 
2.15.1

[PATCH 2/2] Improve error message for old-style backquotes

[Philipp Stephani]

* src/lread.c (load_error_old_style_backquotes): Improve error message
if no file is being loaded.

* test/src/lread-tests.el (lread-tests--force-new-style-backquotes):
Adapt test.
---
 src/lread.c             | 12 ++++++++++--
 test/src/lread-tests.el |  3 +--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 57cb0a5e48..5f8c85e769 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1006,8 +1006,16 @@ load_error_handler (Lisp_Object data)
 static _Noreturn void
 load_error_old_style_backquotes (void)
 {
-  AUTO_STRING (format, "Loading `%s': old-style backquotes detected!");
-  xsignal1 (Qerror, CALLN (Fformat_message, format, Vload_file_name));
+  if (NILP (Vload_file_name))
+    {
+      AUTO_STRING (message, "Old-style backquotes detected!");
+      xsignal1 (Qerror, message);
+    }
+  else
+    {
+      AUTO_STRING (format, "Loading `%s': old-style backquotes detected!");
+      xsignal1 (Qerror, CALLN (Fformat_message, format, Vload_file_name));
+    }
 }
 
 static void
diff --git a/test/src/lread-tests.el b/test/src/lread-tests.el
index 4dfed13be7..2ac3cdb2f0 100644
--- a/test/src/lread-tests.el
+++ b/test/src/lread-tests.el
@@ -183,8 +183,7 @@ lread-tests--last-message
 
 (ert-deftest lread-tests--force-new-style-backquotes ()
   (let ((data (should-error (read "(` (a b))"))))
-    (should (equal (cdr data)
-                   '("Loading `nil': old-style backquotes detected!"))))
+    (should (equal (cdr data) '("Old-style backquotes detected!"))))
   (should (equal (let ((force-new-style-backquotes t))
                    (read "(` (a b))"))
                  '(`(a b)))))
-- 
2.15.1

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Paul Eggert]

Why not simply take the new interpretation for backquotes? That would be simpler 
than adding a backward-compatibility variable that I suspect nobody will ever need.

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am Sa., 30. Dez. 2017 um 07:19 Uhr:

> Why not simply take the new interpretation for backquotes? That would be
> simpler
> than adding a backward-compatibility variable that I suspect nobody will
> ever need.
>

As discussed in
http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00051.html, we
(mostly) agreed to first turn them into errors because otherwise the
interpretation would suddenly silently change.

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

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Paul Eggert]

Philipp Stephani wrote:
> As discussed in
> http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00051.html, we
> (mostly) agreed to first turn them into errors

And we've done that in Emacs 26, right? So in Emacs 27 we can remove support for 
old-style backquotes; we don't need a new variable to enable old-style 
backquotes dynamically.

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am So., 31. Dez. 2017 um 00:07 Uhr:

> Philipp Stephani wrote:
> > As discussed in
> > http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00051.html, we
> > (mostly) agreed to first turn them into errors
>
> And we've done that in Emacs 26, right?


No, only in master (i.e. Emacs 27), see
http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00056.html


> So in Emacs 27 we can remove support for
> old-style backquotes;


Only in Emacs 28, because 26 will still have old-style backquotes only.


> we don't need a new variable to enable old-style
> backquotes dynamically.
>

My patch conditionally enables new-style backquotes; there's no way to
re-enable old-style backquotes once they generate errors. The assumption is
that everyone should migrate to new-style once they get the errors, and not
re-enable old-style.

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

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Paul Eggert]

Philipp Stephani wrote:
> 
> Only in Emacs 28, because 26 will still have old-style backquotes only.

Ah, OK, I was turned around then: this variable is for forward-compatibility and 
not backward. I suggest mentioning in the doc string that setting the variable 
makes Emacs compatible with planned-for-28 behavior, and that the variable is 
planned to go away in Emacs 28. (It's a temporary variable, so to speak....)

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am So., 31. Dez. 2017 um 01:40 Uhr:

> Philipp Stephani wrote:
> >
> > Only in Emacs 28, because 26 will still have old-style backquotes only.
>
> Ah, OK, I was turned around then: this variable is for
> forward-compatibility and
> not backward. I suggest mentioning in the doc string that setting the
> variable
> makes Emacs compatible with planned-for-28 behavior, and that the variable
> is
> planned to go away in Emacs 28. (It's a temporary variable, so to
> speak....)
>

Good point, I've amended the docstring and pushed the patch to master as
ddb74b2027.

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

Re: [PATCH 1/2] Add new variable to force new-style backquote interpretation.

[Stefan Monnier]

>> > As discussed in
>> > http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00051.html, we
>> > (mostly) agreed to first turn them into errors

Still looks like a complete waste of time to me.  I still haven't seen
a single example of code which works in Emacs-25 (say) and fails in
a byzantine manner if we silently drop support for old-style backquotes.

OTOH we have already seen at least one example of code which works
correctly in Emacs-25, works correctly if we drop support for old-style
backquotes, and yet fails with this "in-between" support.

So not only it's extra work for very little benefit (i.e. try and
protect us from a hypothetical case no-one has ever seen in the wild)
but it even costs us a known bug.

Sorry, but the tradeoff really doesn't look good from here.


        Stefan