bug#45619: 28.0.50; pcase-let on MacOS doesn't work

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[jixiuf]

(pcase-let ((`(,default-directory) '( "/tmp/")))
    (call-interactively 'find-file))

Expected behavior: Find file in "/tmp/"
Observed behavior: Find file in "~"

Original Bug: https://github.com/minad/consult/issues/108



In GNU Emacs 28.0.50 (build 3, x86_64-apple-darwin19.6.0, NS appkit-1894.60 Version 10.15.7 (Build 19H114))
of 2021-01-02 built on jxfhome
Repository revision: 0f561ee55348ff451600cc6027db5940ee14606f
Repository branch: master
Windowing system distributor 'Apple', version 10.3.1894
System Description:  Mac OS X 10.15.7

Configured using:
'configure --disable-silent-rules
--enable-locallisppath=/usr/local/share/emacs/site-lisp
--prefix=/Users/jixiuf/gccemacs --with-nativecomp --with-ns
--disable-ns-self-contained CC=clang LDFLAGS=-L/usr/local/lib/gcc/10
'CFLAGS=-I/usr/local/opt/openssl@1.1/include
-I/usr/local/opt/texinfo/include -I/usr/local/opt/gnu-sed/include
-I/usr/local/opt/gcc/include -I/usr/local/opt/libxml2/include
-I/usr/local/opt/jpeg/include -I/usr/local/opt/libtiff/include
-I/usr/local/opt/gnutls/include -I/usr/local/opt/nettle/include
-I/usr/local/opt/libtasn1/include -I/usr/local/opt/p11-kit/include '
'CPPFLAGS=-I/usr/local/opt/openssl@1.1/include
-I/usr/local/opt/texinfo/include -I/usr/local/opt/gnu-sed/include
-I/usr/local/opt/gcc/include -I/usr/local/opt/libxml2/include
-I/usr/local/opt/jpeg/include -I/usr/local/opt/libtiff/include
-I/usr/local/opt/gnutls/include -I/usr/local/opt/nettle/include
-I/usr/local/opt/libtasn1/include -I/usr/local/opt/p11-kit/include ''

Configured features:
JPEG TIFF PNG RSVG GLIB NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS NS MODULES THREADS JSON PDUMPER LCMS2

Important settings:
  value of $LC_ALL: zh_CN.UTF-8
  value of $LANG: zh_CN.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-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

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
pcase china-util iso-transl tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/ns-win ns-win ucs-normalize
mule-util term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads kqueue cocoa ns lcms2
multi-tty make-network-process emacs)

Memory information:
((conses 16 52619 7203)
(symbols 48 6722 1)
(strings 32 18551 2095)
(string-bytes 1 611982)
(vectors 16 12909)
(vector-slots 8 230707 8768)
(floats 8 21 43)
(intervals 56 296 4)
(buffers 984 10))

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Michael Heerdegen]

jixiuf <jixiuf@qq.com> writes:

> (pcase-let ((`(,default-directory) '( "/tmp/")))
>     (call-interactively 'find-file))
>
> Expected behavior: Find file in "/tmp/"
> Observed behavior: Find file in "~"

AFAICT the issue here is that `pcase-let' always creates lexically
scoped bindings, even for special variables.  That can be surprising.

@Stefan, what can we do?  Document better?  Add compiler warnings?  Or
is it possible to "fix" this?

Regards,

Michael.

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Drew Adams]

> > (pcase-let ((`(,default-directory) '( "/tmp/")))
> >     (call-interactively 'find-file))
> >
> > Expected behavior: Find file in "/tmp/"
> > Observed behavior: Find file in "~"
> 
> AFAICT the issue here is that `pcase-let' always creates lexically
> scoped bindings, even for special variables.  That can be surprising.
> 
> @Stefan, what can we do?  Document better?  Add compiler warnings?  Or
> is it possible to "fix" this?

Please do at least document it (along with the reason for it perhaps?).

What is the reason for it, BTW?

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier]

> AFAICT the issue here is that `pcase-let' always creates lexically
> scoped bindings, even for special variables.  That can be surprising.

It's messier than that: the issue is that it doesn't bind variables
which aren't used lexically (this is needed to avoid spurious warnings
about unused vars when the var is used in on place but not in another).

> @Stefan, what can we do?  Document better?  Add compiler warnings?  Or
> is it possible to "fix" this?

We could try and make `pcase` more aware of dyn-scoped vars and have it
refrain from optimizing away "unused" dyn-scoped vars, but maybe it's
better to document it (and maybe even add a warning when a dyn-scoped
var is bound by `pcase`, like we already have for dyn-scoped vars bound
as function arguments).


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> It's messier than that: the issue is that it doesn't bind variables
> which aren't used lexically (this is needed to avoid spurious warnings
> about unused vars when the var is used in on place but not in
> another).

Ok, so this "works":

(pcase-let ((`(,default-directory) '("/tmp/")))
  (ignore default-directory)
  (call-interactively 'find-file))

and also this:

(pcase-let ((default-directory "/tmp/"))
  (call-interactively 'find-file))

I wonder what is messier: the warnings or the semantics resulting from
avoiding the warnings.  I guess people would more often complain about
the warnings...

Michael.

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier]

> (pcase-let ((default-directory "/tmp/"))
>   (call-interactively 'find-file))
>
> I wonder what is messier: the warnings or the semantics resulting from
> avoiding the warnings.  I guess people would more often complain about
> the warnings...

I think the cleaner semantics is to say that if variables bound by pcase
are dynamically scoped the behavior is "undefined", and to add
a check&warning about it in pcase (the problem with the old "unused
var" warning is that it was cumbersome to avoid it, whereas this new
warning would be easy to avoid by moving the binding to a separate
`let`).


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> It's messier than that: the issue is that it doesn't bind variables
> which aren't used lexically (this is needed to avoid spurious warnings
> about unused vars when the var is used in on place but not in another).

BTW, a similar issue (pitfall) I faced was that I sometimes expect that
when a variable is already bound, already the first appearance of the
symbol would be turned into an equality test.

I wonder if pcase variable bindings could be hygienic in the sense that
they would use fresh symbols internally.  That would maybe make the
semantics clearer.

Michael.

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier]

>> It's messier than that: the issue is that it doesn't bind variables
>> which aren't used lexically (this is needed to avoid spurious warnings
>> about unused vars when the var is used in on place but not in another).
> BTW, a similar issue (pitfall) I faced was that I sometimes expect that
> when a variable is already bound, already the first appearance of the
> symbol would be turned into an equality test.

Hmm... so-called "non-linear patterns".  We should emit a warning when the
same var is used twice in a pattern, indeed, to avoid surprises.

We could also do as you describe but I'm not completely sure it's a good
idea (it begs questions about which equality test to use and when it
should be tested).

> I wonder if pcase variable bindings could be hygienic in the sense that
> they would use fresh symbols internally.  That would maybe make the
> semantics clearer.

I don't know what you mean by that.  Can you clarify?


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hmm... so-called "non-linear patterns".  We should emit a warning when the
> same var is used twice in a pattern, indeed, to avoid surprises.

Not sure if we speak about the same case.  I spoke about this: I caught
myself trying something like that:

(defvar thing-tag 'a-thing)

(let ((my-thing (cons thing-tag '(thing-contents...))))
  (pcase my-thing
    (`(,thing-tag . ,contents) (do-something-with contents))))

> I don't know what you mean by that.  Can you clarify?

Something diametral to my previous suggestion, I don't know if it would
be appropriate: You know scheme syntax rules and it's concept of
hygiene?  Similarly in `pcase' we could silently transform any
appearance of a SYMBOL with a fresh uninterned symbol.  While that would
not change the behavior for the common use cases, it would be clear that
bindings created by pcase would never interfere in any way with already
existing bindings of any kind.

The advantage would be clearer semantics.  The disadvantage would
be that we would limit the binding capabilities of pcase.

Being able to change the binding of a special variable sounds nice, but
I think that could also happen by accident, right?

Regards,

Michael.

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier]

>> Hmm... so-called "non-linear patterns".  We should emit a warning when the
>> same var is used twice in a pattern, indeed, to avoid surprises.
>
> Not sure if we speak about the same case.  I spoke about this: I caught
> myself trying something like that:
>
> (defvar thing-tag 'a-thing)
>
> (let ((my-thing (cons thing-tag '(thing-contents...))))
>   (pcase my-thing
>     (`(,thing-tag . ,contents) (do-something-with contents))))

Ah, indeed it's not the same thing as non-linear patterns.

An example of a non-linear pattern could be:

    (pcase foo
      (`(,a . ,a)
       (message "foo is a pair with car equal to cdr"))
      ...)

For your use case, you can write:

      (pcase my-thing
        (`(,(pred (equal thing-tag)) . ,contents) (do-something-with contents))))

> Something diametral to my previous suggestion, I don't know if it would
> be appropriate: You know scheme syntax rules and it's concept of
> hygiene?  Similarly in `pcase' we could silently transform any
> appearance of a SYMBOL with a fresh uninterned symbol.  While that would
> not change the behavior for the common use cases, it would be clear that
> bindings created by pcase would never interfere in any way with already
> existing bindings of any kind.

I know about macro hygiene, but what you describe is a mechanism
to get a particular semantics (a form a hygiene, typically), but I don't
understand what semantics you're trying to get.

Could you give some examples of problems you'd like to avoid this way?

> Being able to change the binding of a special variable sounds nice, but
> I think that could also happen by accident, right?

As I said, I think we should declare this is unsupported and try to emit
a warning what we detect such a thing (tho we should still support
corner cases like

    (pcase-let
        (((,foo . ,bar) (blabla))
         (default-directory (blibli)))
      ...)


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> An example of a non-linear pattern could be:
>
>     (pcase foo
>       (`(,a . ,a)
>        (message "foo is a pair with car equal to cdr"))
>       ...)

Yes, I sometimes need this for el-searches.

> Could you give some examples of problems you'd like to avoid this way?

I wondered what happens when a pcase form binds a symbol S that is not
defined at compile time (normal case) but then a user loads a package
that declares S as (globally) special.  Then that pcase binding gets
dynamical scope at runtime (as it would happen with any `let' binding),
right?  Sorry if this is silly or trivial, I was just wondering...


Michael.

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier]

> I wondered what happens when a pcase form binds a symbol S that is not
> defined at compile time (normal case) but then a user loads a package
> that declares S as (globally) special.  Then that pcase binding gets
> dynamical scope at runtime (as it would happen with any `let' binding),
> right?  Sorry if this is silly or trivial, I was just wondering...

This is a much more general problem which affects all variable bindings
rather than only those of `pcase`.
Note that the choice between dynamic and lexical scoping is made at
compile-time (when you compile the code), so if the code is compiled you
should be somewhat immune to the problem.


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> (pcase-let ((`(,default-directory) '( "/tmp/")))
>>     (call-interactively 'find-file))
>>
>> Expected behavior: Find file in "/tmp/"
>> Observed behavior: Find file in "~"
>
> AFAICT the issue here is that `pcase-let' always creates lexically
> scoped bindings, even for special variables.  That can be surprising.
>
> @Stefan, what can we do?  Document better?  Add compiler warnings?  Or
> is it possible to "fix" this?

I tried the form in Emacs 29, and I got the expected behaviour here.  So
was this fixed since this bug was reported?  (But I only tested on Debian.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-02-12 08:46:02] wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>>> (pcase-let ((`(,default-directory) '( "/tmp/")))
>>>     (call-interactively 'find-file))
>>>
>>> Expected behavior: Find file in "/tmp/"
>>> Observed behavior: Find file in "~"
>>
>> AFAICT the issue here is that `pcase-let' always creates lexically
>> scoped bindings, even for special variables.  That can be surprising.
>>
>> @Stefan, what can we do?  Document better?  Add compiler warnings?  Or
>> is it possible to "fix" this?
> I tried the form in Emacs 29, and I got the expected behaviour here.  So
> was this fixed since this bug was reported?  (But I only tested on Debian.)

Hmm... I doubt it's fixed in all cases.
I still think pcase should emit a warning when asked to bind
a dynamically scoped variable.


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hmm... I doubt it's fixed in all cases.

Here's a more self-contained example:

;;;  -*- lexical-binding: t -*-

(defvar foo 1)

(defun zot ()
  (pcase-let ((`(,foo) '(2)))
    (bar)))

(defun bar ()
  (message "%s" foo))

M-: (zot) messages "2".

> I still think pcase should emit a warning when asked to bind
> a dynamically scoped variable.

If pcase-let currently does work fine for dynamic variables then it's
likely that people are depending on it, and it's too late to change...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-02-12 17:30:54] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Hmm... I doubt it's fixed in all cases.
> Here's a more self-contained example:
>
> ;;;  -*- lexical-binding: t -*-
>
> (defvar foo 1)
>
> (defun zot ()
>   (pcase-let ((`(,foo) '(2)))
>     (bar)))
>
> (defun bar ()
>   (message "%s" foo))
>
> M-: (zot) messages "2".

I don't think a single example can represent all cases.  Try:

    (defun zot ()
      (pcase-let (((or `(,foo) foo) '(2)))
        (progn (bar))))

>> I still think pcase should emit a warning when asked to bind
>> a dynamically scoped variable.
> If pcase-let currently does work fine for dynamic variables then it's
> likely that people are depending on it, and it's too late to change...

I don't mean to change the generated code, but to discourage such uses
since they may break when the code is modified in apparently-minor ways.
Hence a warning.


        Stefan

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I don't think a single example can represent all cases.  Try:
>
>     (defun zot ()
>       (pcase-let (((or `(,foo) foo) '(2)))
>         (progn (bar))))

Hm, yes.

>>> I still think pcase should emit a warning when asked to bind
>>> a dynamically scoped variable.
>> If pcase-let currently does work fine for dynamic variables then it's
>> likely that people are depending on it, and it's too late to change...
>
> I don't mean to change the generated code, but to discourage such uses
> since they may break when the code is modified in apparently-minor ways.
> Hence a warning.

Byte-compiling this does yield a warning in Emacs 29:

pcase.el:6:28: Warning: Lexical argument shadows the dynamic variable foo


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45619: 28.0.50; pcase-let on MacOS doesn't work

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-02-12 18:21:31] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I don't think a single example can represent all cases.  Try:
>>
>>     (defun zot ()
>>       (pcase-let (((or `(,foo) foo) '(2)))
>>         (progn (bar))))
>
> Hm, yes.
>
>>>> I still think pcase should emit a warning when asked to bind
>>>> a dynamically scoped variable.
>>> If pcase-let currently does work fine for dynamic variables then it's
>>> likely that people are depending on it, and it's too late to change...
>>
>> I don't mean to change the generated code, but to discourage such uses
>> since they may break when the code is modified in apparently-minor ways.
>> Hence a warning.
>
> Byte-compiling this does yield a warning in Emacs 29:
>
> pcase.el:6:28: Warning: Lexical argument shadows the dynamic variable foo

Two problems with this:
- The warning talks about the generated code rather than the source
  code, so it's hard for the user to understand what's going on
  (there's no *argument* by that name in their code).
- The warning only shows up when the problem actually bites, whereas we
  should warn about all uses of dynamically scoped vars, since they all
  *may* bite at some point.
- You only get the warning if you byte-compile the code.


        Stefan "off-by-one 4ever (in short, 5ever)"