bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't res


From: Philipp Stephani
Subject: bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag
Date: Sun, 11 Feb 2018 17:54:02 +0000



Michael Heerdegen <address@hidden> schrieb am So., 4. Feb. 2018 um 22:02 Uhr:
Philipp Stephani <address@hidden> writes:

>  #+begin_src emacs-lisp
>  (setq my-alist '((x . 1)))
>  (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist))
>  my-alist
>  ==> ((y) (x . 1))
>  #+end_src

> I think we should spend significant efforts to avoid surprises. In
> this case, if it means we should remove `alist-get' as well from the
> forms supported by `cl-letf', then I think that's what we should
> do. The documentation for `cl-letf' clearly states: "On exit, either
> normally or because of a ‘throw’ or error, the PLACEs are set back to
> their original values." If it can't do that for some place form, it
> shouldn't be allowed.

But

  (alist-get value my-alist)

doesn't change for any value (especially for y), so the alist, or the
`alist-get' place expressions, aren't effectively changed.  The object
that represents the alist changes, however.  Is that a problem or an
internal implementation detail?



Since it affects user-visible behavior, I wouldn't classify it as internal implementation detail.
It seems to me that the approach that `cl-letf` takes is too naive: binding a generalized variable is never the same as setting it and later resetting it to the previous value, not even for simple dynamic symbols (consider unbound variables). Rather than having `(cl-letf ((place val)) body)` expand to

(let ((oldval place))
  (setf place val)
  (unwind-protect body
    (setf place oldval)))

it should rather expand to

(let ((old-state (internal-get-state place)))
  (setf place val)
  (unwind-protect body
    (internal-reset-state place old-state)))

with suitably defined `internal-get-state` and `internal-reset-state`. For most use cases `internal-get-state` and `internal-reset-state` could just be `identity` and `setf`, but for the cases discussed here they would contain additional information.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]