* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
@ 2013-08-18 5:26 Drew Adams
2013-08-18 18:38 ` Richard Stallman
2013-08-19 2:59 ` Stefan Monnier
0 siblings, 2 replies; 11+ messages in thread
From: Drew Adams @ 2013-08-18 5:26 UTC (permalink / raw)
To: 15122
[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]
This StackOverflow entry suggested that the Emacs-Lisp byte compiler be
able to warn about the use of functions that are destructive, i.e., can
modify data structures in place:
http://stackoverflow.com/questions/17610046/elisp-destructive-operation-warning
Attached is a patch that provides this, at least a start.
It covers the basic functions that can perform in-place data structure
modification: lists, strings, vectors and other arrays, symbol properties.
These are generally the functions identified as destructive or modifying
in the Elisp manual.
A comment in the code identifies some other possible candidates that
occurred to me while browsing the manual. Modify the list of functions
as you see fit (variable `byte-compile-destructive-functions').
I'm no expert on some of the data structure implementations, and the
manual is sometimes not so clear about whether destructive modification
occurs. And perhaps there are cases where although the modification is
destructive at an implementation level the user need not be concerned
about that level, so such functions should not be included. Dunno.
Perhaps also something could be done in the future wrt recognition of
particular cases for `setf' modification.
In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
of 2013-08-07 on ODIEONE
Bzr revision: 113750 lekktu@gmail.com-20130808011911-0jzpc9xuncegg6x9
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
`configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
CFLAGS=-O0 -g3 LDFLAGS=-Lc:/Devel/emacs/lib
CPPFLAGS=-Ic:/Devel/emacs/include'
[-- Attachment #2: bytecomp-2013-08-17a.patch --]
[-- Type: application/octet-stream, Size: 3045 bytes --]
diff -c bytecomp.el bytecomp-patched.el
*** bytecomp.el Sat Aug 17 21:35:05 2013
--- bytecomp-patched.el Sat Aug 17 22:11:45 2013
***************
*** 274,281 ****
(defconst byte-compile-warning-types
'(redefine callargs free-vars unresolved
obsolete noruntime cl-functions interactive-only
! make-local mapcar constants suspicious lexical)
"The list of warning types used when `byte-compile-warnings' is t.")
(defcustom byte-compile-warnings t
"List of warnings that the byte-compiler should issue (t for all).
--- 274,282 ----
(defconst byte-compile-warning-types
'(redefine callargs free-vars unresolved
obsolete noruntime cl-functions interactive-only
! make-local mapcar constants suspicious lexical destructive)
"The list of warning types used when `byte-compile-warnings' is t.")
+
(defcustom byte-compile-warnings t
"List of warnings that the byte-compiler should issue (t for all).
***************
*** 298,303 ****
--- 299,305 ----
mapcar mapcar called for effect.
constants let-binding of, or assignment to, constants/nonvariables.
suspicious constructs that usually don't do what the coder wanted.
+ destructive functions that can modify data structure in place.
If the list begins with `not', then the remaining elements specify warnings to
suppress. For example, (not mapcar) will suppress warnings about mapcar."
***************
*** 353,358 ****
--- 355,370 ----
(t
(append byte-compile-warnings (list warning)))))))
+ ;; Possible candidates? `clrhash', `match-data' (with REUSE), `put', `puthash',
+ ;; `remhash', `set-char-table-extra-slot', `set-char-table-parent',
+ ;; `set-char-table-range', `setplist', `unintern', `write-region' (merging annotations)?
+ ;; What to do about `setf' with various generalized vars?
+ ;;
+ (defvar byte-compile-destructive-functions
+ '(aset clear-string delete delete-dups delq fillarray nbutlast nconc nreverse plist-put
+ ring-insert ring-insert-at-beginning ring-remove setcar setcdr sort store-substring)
+ "List of functions that can modify data structures in place.")
+
(defvar byte-compile-interactive-only-functions
'(beginning-of-buffer end-of-buffer replace-string replace-regexp
insert-file insert-buffer insert-file-literally previous-line next-line
***************
*** 2925,2930 ****
--- 2937,2946 ----
(handler (get fn 'byte-compile)))
(when (macroexp--const-symbol-p fn)
(byte-compile-warn "`%s' called as a function" fn))
+ (and (byte-compile-warning-enabled-p 'destructive)
+ (memq fn byte-compile-destructive-functions)
+ (byte-compile-warn "`%s' can modify data structure in place \
+ \(destructive)" fn))
(and (byte-compile-warning-enabled-p 'interactive-only)
(memq fn byte-compile-interactive-only-functions)
(byte-compile-warn "`%s' used from Lisp code\n\
Diff finished. Sat Aug 17 22:12:04 2013
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-18 5:26 bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions Drew Adams
@ 2013-08-18 18:38 ` Richard Stallman
2013-08-19 2:59 ` Stefan Monnier
1 sibling, 0 replies; 11+ messages in thread
From: Richard Stallman @ 2013-08-18 18:38 UTC (permalink / raw)
To: Drew Adams; +Cc: 15122
[ To any NSA and FBI agents reading my email: please consider
[ whether defending the US Constitution against all enemies,
[ foreign or domestic, requires you to follow Snowden's example.
It covers the basic functions that can perform in-place data structure
modification: lists, strings, vectors and other arrays, symbol properties.
Why cater to such a weird style preference? It would require
continuing work to maintain. Someone who wants to do this
can easily write a function to search for the function names he wants
to avoid using.
--
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
Use Ekiga or an ordinary phone call.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-18 5:26 bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions Drew Adams
2013-08-18 18:38 ` Richard Stallman
@ 2013-08-19 2:59 ` Stefan Monnier
2013-08-19 5:07 ` Drew Adams
1 sibling, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2013-08-19 2:59 UTC (permalink / raw)
To: Drew Adams; +Cc: 15122
> This StackOverflow entry suggested that the Emacs-Lisp byte compiler be
> able to warn about the use of functions that are destructive, i.e., can
> modify data structures in place:
> http://stackoverflow.com/questions/17610046/elisp-destructive-operation-warning
> Attached is a patch that provides this, at least a start.
All of those functions are used on a regular basis in perfectly
correct code. So just flagging every call is not going to fly. We need
to have some further analysis so that we don't flag all calls, but only
those that "could be dangerous".
As it stands, your code would just flood you with false positives,
so it wouldn't really help you find the problematic uses.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-19 2:59 ` Stefan Monnier
@ 2013-08-19 5:07 ` Drew Adams
2013-08-20 5:07 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Drew Adams @ 2013-08-19 5:07 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 15122
> > This StackOverflow entry suggested that the Emacs-Lisp byte compiler be
> > able to warn about the use of functions that are destructive, i.e., can
> > modify data structures in place:
> > http://stackoverflow.com/questions/17610046/elisp-destructive-operation-
> > warning Attached is a patch that provides this, at least a start.
>
> All of those functions are used on a regular basis in perfectly
> correct code. So just flagging every call is not going to fly. We need
> to have some further analysis so that we don't flag all calls, but only
> those that "could be dangerous".
>
> As it stands, your code would just flood you with false positives,
> so it wouldn't really help you find the problematic uses.
That was not the aim. The question was not whether particular code that
uses a destructive operation is correct. The question was whether
particular code uses a destructive operation at all. That's all.
See the OP, which spoke of an "elisp-newbie-mode". He requested a
feature that "adds warnings about destructive operations being used".
Not being used wrongly or problematically, just being used.
Anyway, I would agree that warning about use of a destructive function
should be turned off by default. It's also fine with me if you ignore
the patch altogether.
I can see a use for such a warning, as I expect there are a fair number
of people who want to avoid using destructive operations in general and
might not be aware of when they might be doing so. I mentioned Scheme's
explicit choice to name all such functions in a noticeable way. Drawing
attention to (all) use of destructive operations is not necessarily a
useless thing, depending on the user and context.
That was the point of the patch. It was not to detect uses that are
"problematic", such as where you forget to use `setq' after `delq' to
store the result back into the source variable, or similar. (Yes, it
is true that the OP example was such a case.)
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-19 5:07 ` Drew Adams
@ 2013-08-20 5:07 ` Stefan Monnier
2013-08-20 14:07 ` Drew Adams
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2013-08-20 5:07 UTC (permalink / raw)
To: Drew Adams; +Cc: 15122
> might not be aware of when they might be doing so. I mentioned Scheme's
> explicit choice to name all such functions in a noticeable way. Drawing
> attention to (all) use of destructive operations is not necessarily a
> useless thing, depending on the user and context.
Yes, I like Scheme's convention.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-20 5:07 ` Stefan Monnier
@ 2013-08-20 14:07 ` Drew Adams
2013-08-20 14:39 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Drew Adams @ 2013-08-20 14:07 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 15122
> > might not be aware of when they might be doing so. I mentioned Scheme's
> > explicit choice to name all such functions in a noticeable way. Drawing
> > attention to (all) use of destructive operations is not necessarily a
> > useless thing, depending on the user and context.
>
> Yes, I like Scheme's convention.
Then consider adding (subtle, optional) font-lock highlighting of such.
That would anyway be better than compiler warnings. The list of fns
I came up with is a start. You might even consider putting a recognizable
property on the function symbols, as well.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-20 14:07 ` Drew Adams
@ 2013-08-20 14:39 ` Stefan Monnier
2013-08-20 15:15 ` Drew Adams
2013-08-21 1:00 ` Leo Liu
0 siblings, 2 replies; 11+ messages in thread
From: Stefan Monnier @ 2013-08-20 14:39 UTC (permalink / raw)
To: Drew Adams; +Cc: 15122
>> > might not be aware of when they might be doing so. I mentioned Scheme's
>> > explicit choice to name all such functions in a noticeable way. Drawing
>> > attention to (all) use of destructive operations is not necessarily a
>> > useless thing, depending on the user and context.
>> Yes, I like Scheme's convention.
> Then consider adding (subtle, optional) font-lock highlighting of such.
> That would anyway be better than compiler warnings. The list of fns
> I came up with is a start. You might even consider putting a recognizable
> property on the function symbols, as well.
Font-lock might be a good idea, indeed.
We could also additionally start to introduce Scheme-style names, like
sort!, reverse!, butlast!, ...
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-20 14:39 ` Stefan Monnier
@ 2013-08-20 15:15 ` Drew Adams
2013-08-21 1:00 ` Leo Liu
1 sibling, 0 replies; 11+ messages in thread
From: Drew Adams @ 2013-08-20 15:15 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 15122
> >> Yes, I like Scheme's convention.
> > Then consider adding (subtle, optional) font-lock highlighting of such.
> > That would anyway be better than compiler warnings. The list of fns
> > I came up with is a start. You might even consider putting a recognizable
> > property on the function symbols, as well.
>
> Font-lock might be a good idea, indeed.
> We could also additionally start to introduce Scheme-style names, like
> sort!, reverse!, butlast!, ...
I would vote against that. We should aim to be closer to Common Lisp (in
behavior and in naming, wrt new or changed names), not closer to Scheme.
Optional font-locking is an appropriate Emacs response to this need.
Scheme naming is not, IMO. This indication should be optional and off by
default.
Just one opinion.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-20 14:39 ` Stefan Monnier
2013-08-20 15:15 ` Drew Adams
@ 2013-08-21 1:00 ` Leo Liu
2013-08-21 4:24 ` Stefan Monnier
1 sibling, 1 reply; 11+ messages in thread
From: Leo Liu @ 2013-08-21 1:00 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 15122
On 2013-08-20 22:39 +0800, Stefan Monnier wrote:
> Font-lock might be a good idea, indeed.
> We could also additionally start to introduce Scheme-style names, like
> sort!, reverse!, butlast!, ...
But one would desperately want lisp-1.
Leo
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-21 1:00 ` Leo Liu
@ 2013-08-21 4:24 ` Stefan Monnier
2013-08-21 16:54 ` Richard Stallman
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2013-08-21 4:24 UTC (permalink / raw)
To: Leo Liu; +Cc: 15122
>> Font-lock might be a good idea, indeed.
>> We could also additionally start to introduce Scheme-style names, like
>> sort!, reverse!, butlast!, ...
> But one would desperately want lisp-1.
We'll get that when we switch to Emacs-Guile.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions
2013-08-21 4:24 ` Stefan Monnier
@ 2013-08-21 16:54 ` Richard Stallman
0 siblings, 0 replies; 11+ messages in thread
From: Richard Stallman @ 2013-08-21 16:54 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 15122, sdl.web
[ To any NSA and FBI agents reading my email: please consider
[ whether defending the US Constitution against all enemies,
[ foreign or domestic, requires you to follow Snowden's example.
Changing text in a buffer is a very important part of Emacs.
Thus, I think applying a naming convention to all functions
with side effects is absurd in the context of Emacs.
--
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
Use Ekiga or an ordinary phone call.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-08-21 16:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-18 5:26 bug#15122: 24.3.50; [PATCH] byte-compiler warnings about destructive functions Drew Adams
2013-08-18 18:38 ` Richard Stallman
2013-08-19 2:59 ` Stefan Monnier
2013-08-19 5:07 ` Drew Adams
2013-08-20 5:07 ` Stefan Monnier
2013-08-20 14:07 ` Drew Adams
2013-08-20 14:39 ` Stefan Monnier
2013-08-20 15:15 ` Drew Adams
2013-08-21 1:00 ` Leo Liu
2013-08-21 4:24 ` Stefan Monnier
2013-08-21 16:54 ` Richard Stallman
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).