"Warning: `save-excursion' defeated by `set-buffer'" madness

"Warning: `save-excursion' defeated by `set-buffer'" madness

[Štěpán Němec]

Is there any way I can get rid of these warnings globally?
It's really annoying to have *Compile-Log* pop up every now and then.

    Štěpán

Re: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Štěpán Němec]

On Sat, Feb 13, 2010 at 06:01:43PM -0500, Barry Fishman wrote:
> Hi Štěpán,
> 
> > Is there any way I can get rid of these warnings globally?
> > It's really annoying to have *Compile-Log* pop up every now and then.
> 
> I put the following in my .emacs file:
> 
> ;; Shut off compiler error pop-up warning about
> ;;    save-excursion defeated by set-buffer
> (setq byte-compile-warnings '(not suspicious))

Oh, great, thanks!
I knew there had to be a way...

Re: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Barry Fishman]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> > Is there any way I can get rid of these warnings globally?  It's
>>> > really annoying to have *Compile-Log* pop up every now and then.
>>> I put the following in my .emacs file:
>>> ;; Shut off compiler error pop-up warning about
>>> ;;    save-excursion defeated by set-buffer
>>> (setq byte-compile-warnings '(not suspicious))
>> Oh, great, thanks!
>> I knew there had to be a way...
>
> I really hope you did first report the warning to get it fixed.  I
> currently only know of one external package (BBDB) where this warnings
> pops up like this, so if you're using BBDB, try to install a newer
> version.

You think a new version would help?  The CVS head of BBDB still looks
like it would generate the compiler warning.  Not to mention w3m, w3,
and possibly other packages used with Gnus, that I don't feel like
scanning through at the moment.  They are easy enough to find just using
grep.

I do report problems that I find to the appropriate developers.  In this
case the issue is not necessarily a true bug but more a statement about
how the Emacs code should be written.  It is not the kind of thing I
feel I should be enforcing.  If you can't get these things fixed
upstream, why do you think I can?  I did fix all of my own code, and the
packages that I use that no longer had upstream support.

I send an off-list message to Štěpán because he did not seem to be
getting a reply from anyone on the list, and I thought it seemed cruel
to have him suffer because the Emacs developers want to send an annoying
message to the external package developers.  There already had been
discussion on the list about the issue, and I saw no value in going over
it again.  I just showed him what he wanted, which is how to shut off
the message, which nobody else seemed to have posted to the list.

I run repo versions of Emacs and the external Emacs packages that I use,
when they are avaliable.  In part it is in self defence.  The
development versions tend to be reliable, and when interfaces are
changed it usually happens one at time, rather that all at once.  This
gives me an opportunity to fix them in my code.  I do report bugs that I
find (and patches if I can) to the upstream developers.

If I just used releases, I would see multiple problems all at once, and
have to go back through the development mailing list to figure out what
to do, long after any discussion was over.  I have too much of my own
code for that approach to work.

-- 
Barry Fishman

Re: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Štěpán Němec]

On Sun, Feb 14, 2010 at 02:38:13PM -0500, Barry Fishman wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> >>> > Is there any way I can get rid of these warnings globally?  It's
> >>> > really annoying to have *Compile-Log* pop up every now and then.
> >>> I put the following in my .emacs file:
> >>> ;; Shut off compiler error pop-up warning about
> >>> ;;    save-excursion defeated by set-buffer
> >>> (setq byte-compile-warnings '(not suspicious))
> >> Oh, great, thanks!
> >> I knew there had to be a way...
> >
> > I really hope you did first report the warning to get it fixed.  I
> > currently only know of one external package (BBDB) where this warnings
> > pops up like this, so if you're using BBDB, try to install a newer
> > version.

Err, for some reason I didn't receive Stefan's mail Barry is quoting
here and replying to. It's also not in the online archives,
lists.gnu.org or gmane.

I'm using BBDB from CVS, but AFAIK the last commit occured in 2005.

Actually, it's not always obvious to me how to find the source of
the warnings (other than trying to grep all the sources) -- it appears
when invoking `ibuffer', for instance, but not with emacs -q; and I have
no customization of ibuffer whatsoever, so I have no idea what could be
the issue there.

In short, I believe it's a very annoying issue for the end users.

Regards,

    Štěpán

RE: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Drew Adams]

> Actually, it's not always obvious to me how to find the source of the warnings

FWIW (possibly amusing anecdote) -

When this (nifty feature | annoyance) first was (given to | foisted on) us, I
not only got the compiler warning when I compiled; I also got it in a popup
window when I started Emacs.

It took me a while to discover that the culprit was a defadvice that uses the
compile option. It was compiling on the fly at runtime, of course, but for some
reason that didn't occur to me while I searched for the cause.

Re: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Štěpán Němec]

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

On Mon, Feb 15, 2010 at 09:40:58AM -0500, Stefan Monnier wrote:
> > -- it appears when invoking `ibuffer', for instance, but not with
> > emacs -q; and I have no customization of ibuffer whatsoever, so I have
> > no idea what could be the issue there.
> 
> Can you reproduce it at will?
> If so, can you M-x debug-on-entry RET byte-compile-warn RET, then
> reproduce it and then show us the backtrace (and maybe also the result
> of "e form RET" since `form' may be truncated in the backtrace).

Yes, please see the attached file.
[`form' evaluates to
(save-excursion (set-buffer (car bbdb-complete-name-callback-data))
(apply (quote delete-region) (cdr bbdb-complete-name-callback-data))),
but it's visible from the backtrace just fine]

So it's BBDB-related again, but really Icicles-initiated in this case. I
didn't suspect Icicles originally, as I had it off when invoking
`ibuffer', but now that I tried not even requiring Icicles in my init file,
the problem goes away. Also, even with Icicles loaded, it only happens
the first time `ibuffer' is invoked; so it's probably related to all
those re-initializations Icicles is doing; I'm cc-ing Drew in case he
was interested (icicles.el is Update #: 22624).

Thanks,

    Štěpán

[-- Attachment #2: ibufferbacktrace --]
[-- Type: text/plain, Size: 10186 bytes --]

Debugger entered--entering a function:
* byte-compile-warn("`save-excursion' defeated by `set-buffer'")
  byte-compile-save-excursion((save-excursion (set-buffer (car bbdb-complete-name-callback-data)) (apply (quote delete-region) (cdr bbdb-complete-name-callback-data))))
  byte-compile-form((save-excursion (set-buffer (car bbdb-complete-name-callback-data)) (apply (quote delete-region) (cdr bbdb-complete-name-callback-data))) t)
  byte-compile-if((if bbdb-complete-name-callback-data (save-excursion (set-buffer ...) (apply ... ...))))
  byte-compile-form((if bbdb-complete-name-callback-data (save-excursion (set-buffer ...) (apply ... ...))) t)
  byte-compile-body(((if bbdb-complete-name-callback-data (save-excursion ... ...)) (setq ad-return-value (ad-Orig-choose-completion-string choice buffer base-size)) ad-return-value) nil)
  byte-compile-let((let (ad-return-value) (if bbdb-complete-name-callback-data (save-excursion ... ...)) (setq ad-return-value (ad-Orig-choose-completion-string choice buffer base-size)) ad-return-value))
  byte-compile-form((let (ad-return-value) (if bbdb-complete-name-callback-data (save-excursion ... ...)) (setq ad-return-value (ad-Orig-choose-completion-string choice buffer base-size)) ad-return-value) nil)
  byte-compile-top-level((progn (let (ad-return-value) (when bbdb-complete-name-callback-data ...) (setq ad-return-value ...) ad-return-value)) nil lambda)
  byte-compile-lambda((lambda (choice &optional buffer base-size) #("Advice doc string" 0 17 (ad-advice-info choose-completion-string)) (let (ad-return-value) (when bbdb-complete-name-callback-data ...) (setq ad-return-value ...) ad-return-value)))
  #[nil "\306\b!\307\211\211\211\211	\n\v\f\r\x1d\x1c^[\x1a\x19\x1e\r\x1e\x0e\x1e\x0f\x1e\x10\x1e\x11\x1e\x12\x0e\x139\2031\0\310\x0e\x13!\2053\0\x0e\x13K\2023\0\x0e\x13\211\x1e\x14\242\311=\211\x1e	\203D\0\x0e\x14A\x16\x14\x0e\x14\242\312=\205q\0\x0e	\203Z\0\311\313\x0e\x14!B\202^\0\313\x0e\x14!\x16\x14\x0e\x139\203o\0\314\x0e\x13\x0e\x14\"\202q\0\x0e\x14.\r\207" [byte-compile-initial-macro-environment byte-compile-verbose byte-optimize byte-compile-dynamic byte-compile-dynamic-docstrings byte-compile-warnings copy-alist nil fboundp macro lambda byte-compile-lambda defalias byte-compile-free-assignments byte-compile-free-references byte-compile-const-variables byte-compile-bound-variables byte-compile-function-environment byte-compile-macro-environment form fun] 12]()
  funcall(#[nil "\306\b!\307\211\211\211\211	\n\v\f\r\x1d\x1c^[\x1a\x19\x1e\r\x1e\x0e\x1e\x0f\x1e\x10\x1e\x11\x1e\x12\x0e\x139\2031\0\310\x0e\x13!\2053\0\x0e\x13K\2023\0\x0e\x13\211\x1e\x14\242\311=\211\x1e	\203D\0\x0e\x14A\x16\x14\x0e\x14\242\312=\205q\0\x0e	\203Z\0\311\313\x0e\x14!B\202^\0\313\x0e\x14!\x16\x14\x0e\x139\203o\0\314\x0e\x13\x0e\x14\"\202q\0\x0e\x14.\r\207" [byte-compile-initial-macro-environment byte-compile-verbose byte-optimize byte-compile-dynamic byte-compile-dynamic-docstrings byte-compile-warnings copy-alist nil fboundp macro lambda byte-compile-lambda defalias byte-compile-free-assignments byte-compile-free-references byte-compile-const-variables byte-compile-bound-variables byte-compile-function-environment byte-compile-macro-environment form fun] 12])
  byte-compile(advice-compilation)
  ad-compile-function(choose-completion-string)
  ad-activate-advised-definition(choose-completion-string nil)
  ad-activate-internal(choose-completion-string nil)
  defalias(choose-completion-string icicle-choose-completion-string)
  icicle-redefine-std-completion-fns()
  (cond (icicle-mode (icicle-define-icicle-maps) (icicle-bind-other-keymap-keys) (add-hook ... ...) (add-hook ... ...) (add-hook ... ...) (add-hook ... ...) (add-hook ... ... ...) (add-hook ... ... ...) (when icicle-customize-save-flag ...) (add-hook ... ...) (add-hook ... ...) (add-hook ... ...) (add-hook ... ...) (icicle-undo-std-completion-faces) (icicle-redefine-std-completion-fns) (icicle-redefine-standard-commands) (icicle-redefine-standard-options) (when ... ...) (when ... ...) (if icicle-menu-items-to-history-flag ... ...) (when ... ... ...)) (t (makunbound ...) (icicle-restore-other-keymap-keys) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ... t) (remove-hook ... ... t) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (remove-hook ... ...) (icicle-restore-std-completion-fns) (icicle-restore-standard-commands) (icicle-restore-standard-options) (when ... ...) (when ... ...) (remove-hook ... ...) (when ... ... ...)))
  (let ((last-message ...)) (setq icicle-mode (cond ... ... ...)) (cond (icicle-mode ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ...) (t ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ...)) (unless (eq icicle-guess-commands-in-path ...) (setq icicle-shell-command-candidates-cache nil)) (message "Turning %s Icicle mode..." (if icicle-mode "ON" "OFF")) (icicle-define-minibuffer-maps icicle-mode) (run-hooks (quote icicle-mode-hook)) (message "Turning %s Icicle mode...done" (if icicle-mode "ON" "OFF")) (run-hooks (quote icicle-mode-hook) (if icicle-mode ... ...)) (if (called-interactively-p ...) (progn ... ...)))
  icy-mode(1)
  icicle-toggle-icicle-mode-twice()
  eval((icicle-toggle-icicle-mode-twice))
  mapc(eval ((icicle-toggle-icicle-mode-twice)))
  #[(a-l-element) "\b@;\205\x1d\0\b@	\306\x1a^[\x1c\307\x1d\310\f\v\n#,\205\x1d\0\311\312\bA\"\207" [a-l-element abs-file start string regexp inhibit-changing-match-data nil t string-match mapc eval] 4](("\\(\\`\\|/\\)ibuffer\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)))
  mapc(#[(a-l-element) "\b@;\205\x1d\0\b@	\306\x1a^[\x1c\307\x1d\310\f\v\n#,\205\x1d\0\311\312\bA\"\207" [a-l-element abs-file start string regexp inhibit-changing-match-data nil t string-match mapc eval] 4] ((geiser-mode (progn ... ...)) ("\\(\\`\\|/\\)scheme\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (add-hook ... ...)) (scheme (progn ...)) (yaoddmuse (progn ... ...)) (w3m (progn ...)) ("\\(\\`\\|/\\)thingatpt\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (require ...)) (sunrise (progn ... ... ... ... ... ...)) ("\\(\\`\\|/\\)session\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (add-to-list ... ...)) ("\\(\\`\\|/\\)ecb\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (defadvice ecb-method-clicked ... "Make hierarchy visible when jumping into location from ECB tree buffer." ...)) ("\\(\\`\\|/\\)saveplace\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (defadvice save-place-find-file-hook ... "Make the position visible." ...)) ("\\(\\`\\|/\\)bookmark\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (if ... ... ...)) ("\\(\\`\\|/\\)speedbar\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ... ... ... ... ... ...)) ("\\(\\`\\|/\\)imenu\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ...)) ("\\(\\`\\|/\\)org-exp\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (add-hook ... ...)) ("\\(\\`\\|/\\)calendar\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ... ... ...)) ("\\(\\`\\|/\\)tcl\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)sh-script\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)shell\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)rlogin\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)net-utils\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)info\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)ielm\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)idlw-shell\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)ibuffer\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)recentf\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ...)) ("\\(\\`\\|/\\)dired-aux\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ...)) ("\\(\\`\\|/\\)dired-x\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ... ...)) ("\\(\\`\\|/\\)bbdb-com\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ...)) ("\\(\\`\\|/\\)gud\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ...) (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)ess-site\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ...) (icicle-toggle-icicle-mode-twice)) ("\\(\\`\\|/\\)comint\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (let ... ... ... ... ...)) (filesets (defun filesets-get-filelist ... "Get all files for fileset ENTRY.\nAssume MODE (see `filesets-entry-mode'), if provided." ...)) ("\\(\\`\\|/\\)crm\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ... ... ... ... ... ... ... ... ...)) ("\\(\\`\\|/\\)eev-all\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ...)) (diff-mode (progn ... ...)) ("\\(\\`\\|/\\)sendmail\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ... ...)) ("\\(\\`\\|/\\)tar-mode\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ... ...)) (eldoc (let ... ...)) (help-mode (when vimpulse-want-vi-keys-in-help ... ... ...)) (info (when vimpulse-want-vi-keys-in-Info ... ... ... ... ... ... ...)) (dired (when vimpulse-want-vi-keys-in-dired ... ... ...)) ("\\(\\`\\|/\\)buff-menu\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (when vimpulse-want-vi-keys-in-buffmenu ... ... ...)) ("\\(\\`\\|/\\)mule-cmds\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (progn ... ...) (defadvice toggle-input-method ... "Adjust input-method toggling in vi-state." ...)) ("\\(\\`\\|/\\)iso-acc\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (defadvice iso-accents-mode ... "Set viper-automatic-iso-accents to iso-accents-mode." ...)) ("\\(\\`\\|/\\)rmailedit\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (defadvice rmail-cease-edit ... "Switch to Emacs state when done editing message." ...)) ("\\(\\`\\|/\\)passwd\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (defadvice read-passwd-1 ... "Switch to Emacs state while reading password." ...)) ("\\(\\`\\|/\\)emerge\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (defadvice emerge-quit ... "Run `viper-change-state-to-vi' after quitting emerge." ...)) ("\\(\\`\\|/\\)flyspell\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (viper-normalize-minor-mode-map-alist)) ("\\(\\`\\|/\\)reftex\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (viper-normalize-minor-mode-map-alist)) ("\\(\\`\\|/\\)view\\(\\.elc\\|\\.el\\)?\\(\\.gz\\)?\\'" (viper-normalize-minor-mode-map-alist)) ...))
  do-after-load-evaluation("/usr/local/share/emacs/23.1.91/lisp/ibuffer.elc")

RE: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Drew Adams]

> So it's BBDB-related again, but really Icicles-initiated in 
> this case. I didn't suspect Icicles originally, as I had it off when invoking
> `ibuffer', but now that I tried not even requiring Icicles in my init file,
> the problem goes away. Also, even with Icicles loaded, it only happens
> the first time `ibuffer' is invoked; so it's probably related to all
> those re-initializations Icicles is doing; I'm cc-ing Drew in case he
> was interested (icicles.el is Update #: 22624).

Thanks. Feel free to follow up with me off list. Sorry for the inconvenience.

Yes, Icicles toggles Icicle mode twice after you load ibuffer, in order to bind
`M-s i' in `ibuffer-mode-map' to command `icicle-search-ibuffer-marked' (only
when Icicle mode is on).

Your backtrace indicates that `choose-completion-string' is advised. Icicles
does not advise it (instead, it uses `defalias' to redefine it when in Icicle
mode). BBDB advises it, in file `bbdb-com.el'. That explains why the backtrace
for byte-compiling function `choose-completion-string' shows `(if
bbdb-complete-name-callback-data (save-excursion ... ...))'.

In the latest version of Icicles, advice is automatically deactivated (when in
Icicle mode) for any of the standard functions that Icicles redefines (see
variable `icicle-redefined-functions'). (The advice is re-activated when you
quit Icicle mode.) So this problem should not arise in the latest Icicles
version. 

But using BBDB in Icicle mode will no longer use the BBDB advice, which might
pose a problem (the advice deletes the completed BBDB name before replacing it).
You can modify the value of `icicle-redefined-functions' to allow the BBDB
advice (hence to also get back the byte-compiler warning).

RE: "Warning: `save-excursion' defeated by `set-buffer'" madness

[Drew Adams]

> Good, so the code that triggers the warning is in bbdb-com.el.
> But the code that runs the byte-compiler (and hence causes those
> harmless/informative warnings to turn into annoying windows 
> popping up) is in advice.el.
> 
> I've just installed the patch below which should fix your problem.

I would have thought that you would consider this annoyance to be a feature,
drawing attention as it does so well to code that you consider worth warning
about. ;-)

So now, to suppress this one annoyance, you've removed all byte-compiler
warnings for byte-compiling advice. I don't necessarily disagree, but are you
sure that's TRT? Should this perhaps be configurable by users of advice (e.g.
defadvice)? (Dunno.)