* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal @ 2015-06-19 19:22 Philipp Stephani 2015-06-19 21:24 ` Stefan Monnier 0 siblings, 1 reply; 9+ messages in thread From: Philipp Stephani @ 2015-06-19 19:22 UTC (permalink / raw) To: 20852 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal 2015-06-19 19:22 bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal Philipp Stephani @ 2015-06-19 21:24 ` Stefan Monnier 2015-06-21 12:13 ` Philipp Stephani 0 siblings, 1 reply; 9+ messages in thread From: Stefan Monnier @ 2015-06-19 21:24 UTC (permalink / raw) To: Philipp Stephani; +Cc: 20852 > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal 2015-06-19 21:24 ` Stefan Monnier @ 2015-06-21 12:13 ` Philipp Stephani 2015-06-22 1:18 ` Stefan Monnier 0 siblings, 1 reply; 9+ messages in thread From: Philipp Stephani @ 2015-06-21 12:13 UTC (permalink / raw) To: Stefan Monnier; +Cc: 20852 [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal 2015-06-21 12:13 ` Philipp Stephani @ 2015-06-22 1:18 ` Stefan Monnier 2015-06-28 13:12 ` Philipp Stephani 0 siblings, 1 reply; 9+ messages in thread From: Stefan Monnier @ 2015-06-22 1:18 UTC (permalink / raw) To: Philipp Stephani; +Cc: 20852 > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal 2015-06-22 1:18 ` Stefan Monnier @ 2015-06-28 13:12 ` Philipp Stephani 2015-06-30 20:43 ` Philipp Stephani 0 siblings, 1 reply; 9+ messages in thread From: Philipp Stephani @ 2015-06-28 13:12 UTC (permalink / raw) To: Stefan Monnier; +Cc: 20852 [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal 2015-06-28 13:12 ` Philipp Stephani @ 2015-06-30 20:43 ` Philipp Stephani 2015-07-02 15:50 ` Stefan Monnier 0 siblings, 1 reply; 9+ messages in thread From: Philipp Stephani @ 2015-06-30 20:43 UTC (permalink / raw) To: Stefan Monnier; +Cc: 20852 [-- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal 2015-06-30 20:43 ` Philipp Stephani @ 2015-07-02 15:50 ` Stefan Monnier [not found] ` <20170501142718.3510-1-phst@google.com> 0 siblings, 1 reply; 9+ messages in thread From: Stefan Monnier @ 2015-07-02 15:50 UTC (permalink / raw) To: Philipp Stephani; +Cc: 20852 > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20170501142718.3510-1-phst@google.com>]
* bug#20852: [PATCH] Warn about missing backslashes during load [not found] ` <20170501142718.3510-1-phst@google.com> @ 2017-05-01 17:59 ` Stefan Monnier 2017-05-01 18:41 ` Philipp Stephani 0 siblings, 1 reply; 9+ messages in thread From: Stefan Monnier @ 2017-05-01 17:59 UTC (permalink / raw) To: Philipp Stephani; +Cc: Philipp Stephani, 20852 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#20852: [PATCH] Warn about missing backslashes during load 2017-05-01 17:59 ` bug#20852: [PATCH] Warn about missing backslashes during load Stefan Monnier @ 2017-05-01 18:41 ` Philipp Stephani 0 siblings, 0 replies; 9+ messages in thread From: Philipp Stephani @ 2017-05-01 18:41 UTC (permalink / raw) To: Stefan Monnier, 20852-done; +Cc: 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-05-01 18:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-19 19:22 bug#20852: 24.3; update-file-autoloads doesn't accept unescaped parenthesis character literal Philipp Stephani 2015-06-19 21:24 ` Stefan Monnier 2015-06-21 12:13 ` Philipp Stephani 2015-06-22 1:18 ` Stefan Monnier 2015-06-28 13:12 ` Philipp Stephani 2015-06-30 20:43 ` Philipp Stephani 2015-07-02 15:50 ` Stefan Monnier [not found] ` <20170501142718.3510-1-phst@google.com> 2017-05-01 17:59 ` bug#20852: [PATCH] Warn about missing backslashes during load Stefan Monnier 2017-05-01 18:41 ` Philipp Stephani
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).