bug#34318: 26.1.90; Strange behavior of two line message with running shell

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

The following behavior might be considered a minor bug.  It is,
however, disconcerting because I have not been able to reveal its
cause.

With emacs -Q evaluate

(message "2\n3")

This gets me a two line message

"1
2"

in the minibuffer.  Now do M-x: shell and re-evaluate the form above.
This gets me a one line message

"1\n2"

Is this on purpose?  Emacs 26 is currently the only version here to
exhibit such behavior.

martin

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Andreas Schwab]

On Feb 04 2019, martin rudalics <rudalics@gmx.at> wrote:

> With emacs -Q evaluate
>
> (message "2\n3")

How do you evaluate it?  With M-:?  With C-x C-e?  With C-M-x?

> This gets me a two line message
>
> "1
> 2"
>
> in the minibuffer.

I don't see that here (with 26.1.91).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Glenn Morris]

For me, with no shell involved, in emacs-26 but not master:

emacs -Q

Type in scratch:

(message "1\n2")

Press C-x C-e

-> two line message

Now type:

M-: (message "1\n2")

-> one line message "1\n2".

Now repeating C-x C-e on the original expression in scratch also
produces a one line message, forever more.

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

 > For me, with no shell involved, in emacs-26 but not master:
 >
 > emacs -Q
 >
 > Type in scratch:
 >
 > (message "1\n2")
 >
 > Press C-x C-e
 >
 > -> two line message
 >
 > Now type:
 >
 > M-: (message "1\n2")
 >
 > -> one line message "1\n2".
 >
 > Now repeating C-x C-e on the original expression in scratch also
 > produces a one line message, forever more.

Ahh...  This is a correct and considerably simpler scenario than the
one I provided.  Hence anyone investigating this issue, please stick
to Glenn's scenario.

Many thanks, martin

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Eli Zaretskii]

On February 5, 2019 10:36:35 AM GMT+02:00, martin rudalics <rudalics@gmx.at> wrote:
>  > For me, with no shell involved, in emacs-26 but not master:
>  >
>  > emacs -Q
>  >
>  > Type in scratch:
>  >
>  > (message "1\n2")
>  >
>  > Press C-x C-e
>  >
>  > -> two line message
>  >
>  > Now type:
>  >
>  > M-: (message "1\n2")
>  >
>  > -> one line message "1\n2".
>  >
>  > Now repeating C-x C-e on the original expression in scratch also
>  > produces a one line message, forever more.
> 
> Ahh...  This is a correct and considerably simpler scenario than the
> one I provided.  Hence anyone investigating this issue, please stick
> to Glenn's scenario.
> 
> Many thanks, martin

Repeat the same recipe in "emacs -Q", after typing "C-x 4 C-o RET", which should show "*Messages*" in another window without selecting that window.  Look at what "*Messages*" shows after each evaluation.  Does the behavior still look strange?  Any questions left?

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

 > Repeat the same recipe in "emacs -Q", after typing "C-x 4 C-o RET",
 > which should show "*Messages*" in another window without selecting
 > that window.  Look at what "*Messages*" shows after each evaluation.
 > Does the behavior still look strange?  Any questions left?

Yes.  Why does only Emacs 26 show this behavior and not all other
Emacs versions I have around here?

martin

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Andreas Schwab]

On Feb 04 2019, Glenn Morris <rgm@gnu.org> wrote:

> Now type:
>
> M-: (message "1\n2")
>
> -> one line message "1\n2".

That's not the message, it's the echo of the return value.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

 >> M-: (message "1\n2")
 >>
 >> -> one line message "1\n2".
 >
 > That's not the message, it's the echo of the return value.

Sure.  I used this recipe because I found it in the context of
Bug#34179 (and reported it there already) but could not reproduce it
otherwise.

martin

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

 >> With emacs -Q evaluate
 >>
 >> (message "2\n3")
 >
 > How do you evaluate it?  With M-:?  With C-x C-e?  With C-M-x?

Any of them.  No difference.

 >> This gets me a two line message
 >>
 >> "1
 >> 2"
 >>
 >> in the minibuffer.
 >
 > I don't see that here (with 26.1.91).

Sorry - I messed up the the numbers from two different experiments so
the report makes no sense.  Note: What matters is the _two lines vs
one line_ behavior.  I'll rephrase the report now, hopefully correct
this time:


With emacs -Q evaluate

(message "2\n3")

This gets me a two line message

"2
3"

in the minibuffer window.  Now do M-x: shell and reevaluate the form
above.  This gets me a one line message

"2\n3"

in the minibuffer window.


Reliably reproducible with today's build of Emacs 26.1.91 on GNU/Linux
with the GTK, Lucid and Motif toolkits and on Windows XP.

Two further observations:

Once the shell has been entered, exiting it and killing the *shell*
buffer does not change the behavior.

The *Messages* buffer reliably reflects the behavior.  Below see the
entire buffer with the first 5 lines after evaluating the form before
invoking shell and the final three lines after evaluating the form
after invoking shell.

-----------------------------------------------------------------
For information about GNU Emacs and the GNU system, type C-h C-a.
2
3
"2
3"
2
3
"2\n3"
-----------------------------------------------------------------

martin

PS: The scenario provided by Glenn is much simpler so let's stick to that.

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Andreas Schwab]

What is the value of print-escape-newlines?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

 > What is the value of print-escape-newlines?

So it's

commit a92e7b4ef6915e079a97e4e33e45b11508170cb1
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Wed Apr 25 12:20:04 2018 -0700

     Don’t set print-escape-newlines in the minibuffer

     This appears to be an unnecessary and possibly-confusing
     revenant from ancient code (Bug#31251).  See thread containing:
     https://lists.gnu.org/r/emacs-devel/2018-04/msg00654.html
     * src/minibuf.c (read_minibuf): Do not set print-escape-newlines.
     * src/print.c (syms_of_print): Do not defsym print-escape-newlines
     or print-escape-control-characters, as these symbols are not used
     in C code.

that fixed the behavior for Emacs 27.  Since it apparently worked
until Emacs 25 we have a regression in Emacs 26 though.

Thanks for spotting it, martin

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Eli Zaretskii]

On February 5, 2019 12:21:50 PM GMT+02:00, martin rudalics <rudalics@gmx.at> wrote:
>  > What is the value of print-escape-newlines?
> 
> So it's
> 
> commit a92e7b4ef6915e079a97e4e33e45b11508170cb1
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Date:   Wed Apr 25 12:20:04 2018 -0700
> 
>      Don’t set print-escape-newlines in the minibuffer
> 
>      This appears to be an unnecessary and possibly-confusing
>      revenant from ancient code (Bug#31251).  See thread containing:
>      https://lists.gnu.org/r/emacs-devel/2018-04/msg00654.html
>      * src/minibuf.c (read_minibuf): Do not set print-escape-newlines.
>     * src/print.c (syms_of_print): Do not defsym print-escape-newlines
>      or print-escape-control-characters, as these symbols are not used
>      in C code.
> 
> that fixed the behavior for Emacs 27.  Since it apparently worked
> until Emacs 25 we have a regression in Emacs 26 though.
> 
> Thanks for spotting it, martin

What regression is that?  Maybe I'm missing something, but I don't see any problem with the latest pretest of Emacs 26.2.

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Andreas Schwab]

On Feb 05 2019, martin rudalics <rudalics@gmx.at> wrote:

> Since it apparently worked
> until Emacs 25 we have a regression in Emacs 26 though.

The regression appears to be that the synchronisation of buffer local
and global values doesn't work.  When I type M-x, then C-h v
print-escape-newlines, I get this:

print-escape-newlines is a variable defined in ‘C source code’.
Its value is nil
Local in buffer  *Minibuf-1*; global value is t

Which doesn't make sense (the values are swapped).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Eli Zaretskii]

On February 5, 2019 1:39:16 PM GMT+02:00, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Feb 05 2019, martin rudalics <rudalics@gmx.at> wrote:
> 
> > Since it apparently worked
> > until Emacs 25 we have a regression in Emacs 26 though.
> 
> The regression appears to be that the synchronisation of buffer local
> and global values doesn't work.  When I type M-x, then C-h v
> print-escape-newlines, I get this:
> 
> print-escape-newlines is a variable defined in ‘C source code’.
> Its value is nil
> Local in buffer  *Minibuf-1*; global value is t
> 
> Which doesn't make sense (the values are swapped).
> 
> Andreas.

I'm not sure I agree with the conclusion.  It could simply be that the code which distinguishes between local and global values  is confused because M-x puts you in *Minibuf-1*, whereas C-h v switches to *Minibuf-2*.

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Andreas Schwab]

On Feb 05 2019, Eli Zaretskii <eliz@gnu.org> wrote:

> On February 5, 2019 1:39:16 PM GMT+02:00, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> On Feb 05 2019, martin rudalics <rudalics@gmx.at> wrote:
>> 
>> > Since it apparently worked
>> > until Emacs 25 we have a regression in Emacs 26 though.
>> 
>> The regression appears to be that the synchronisation of buffer local
>> and global values doesn't work.  When I type M-x, then C-h v
>> print-escape-newlines, I get this:
>> 
>> print-escape-newlines is a variable defined in ‘C source code’.
>> Its value is nil
>> Local in buffer  *Minibuf-1*; global value is t
>> 
>> Which doesn't make sense (the values are swapped).
>> 
>> Andreas.
>
> I'm not sure I agree with the conclusion.  It could simply be that the code which distinguishes between local and global values  is confused because M-x puts you in *Minibuf-1*, whereas C-h v switches to *Minibuf-2*.

The latter no longer exists when describe-variable is executed.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

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

 > The regression appears to be that the synchronisation of buffer local
 > and global values doesn't work.  When I type M-x, then C-h v
 > print-escape-newlines, I get this:
 >
 > print-escape-newlines is a variable defined in ‘C source code’.
 > Its value is nil
 > Local in buffer  *Minibuf-1*; global value is t
 >
 > Which doesn't make sense (the values are swapped).

The problem is with

2018-06-03  Stefan Monnier  <monnier@iro.umontreal.ca>

	Fix bug#30846, along with misc cleanups found along the way

and in particular

	Don't call swap_in_symval_forwarding since the currently swapped
	binding is never one we've modified.

I don't care how it gets fixed but it should get fixed, for example,
as in the attached patch.  Bugs like this can make people spend hours
of debugging all sorts of completely unrelated areas which is no fun.

So please, pretty please review that entire patch again - maybe it
contains additional sorts of problems.

Thank you in advance, martin

[-- Attachment #2: data.c.diff --]
[-- Type: text/plain, Size: 544 bytes --]

diff --git a/src/data.c b/src/data.c
index 5711148..c8193db 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1956,6 +1956,13 @@ struct Lisp_Symbol *
 		BVAR (current_buffer, local_var_alist)));
     }
 
+  /* If the symbol forwards into a C variable, then load the binding
+     for this buffer now.  If C code modifies the variable before we
+     load the binding in, then that new value will clobber the default
+     binding the next time we unload it.  */
+  if (blv->fwd)
+    swap_in_symval_forwarding (sym, blv);
+
   return variable;
 }
 

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Stefan Monnier]

> The problem is with
>
> 2018-06-03  Stefan Monnier  <monnier@iro.umontreal.ca>
>
> 	Fix bug#30846, along with misc cleanups found along the way
>
> and in particular
>
> 	Don't call swap_in_symval_forwarding since the currently swapped
> 	binding is never one we've modified.

Indeed, good spotting.  I installed the patch below which mostly reverts
this part of the commit.

> So please, pretty please review that entire patch again - maybe it
> contains additional sorts of problems.

This is a rather tricky part of the code, indeed.  I reviewed it
thoroughly before installing it and yet here we are.
I re-reviewed it now and couldn't spot any further mistakes, but you
know what this means.


        Stefan


diff --git a/src/data.c b/src/data.c
index 571114802a..ed6dedbe24 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1954,6 +1954,16 @@ Instead, use `add-hook' and specify t for the LOCAL argument.  */)
 	(current_buffer,
 	 Fcons (Fcons (variable, XCDR (blv->defcell)),
 		BVAR (current_buffer, local_var_alist)));
+
+      /* If the symbol forwards into a C variable, then load the binding
+         for this buffer now, to preserve the invariant that forwarded
+         variables must always hold the value corresponding to the
+         current buffer (they are swapped eagerly).
+         Otherwise, if C code modifies the variable before we load the
+         binding in, then that new value would clobber the default binding
+         the next time we unload it.  See bug#34318.  */
+      if (blv->fwd)
+        swap_in_symval_forwarding (sym, blv);
     }
 
   return variable;
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 0069ee84fe..f3b4262de4 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -508,4 +508,22 @@ binding-test-some-local
                        (bound-and-true-p data-tests-foo2)
                        (bound-and-true-p data-tests-foo3)))))))
 
+(ert-deftest data-tests-make-local-forwarded-var () ;bug#34318
+  ;; Boy, this bug is tricky to trigger.  You need to:
+  ;; - call make-local-variable on a forwarded var (i.e. one that
+  ;;   has a corresponding C var linked via DEFVAR_(LISP|INT|BOOL))
+  ;; - cause the C code to modify this variable from the C side of the
+  ;;   forwarding, but this needs to happen before the var is accessed
+  ;;   from the Lisp side and before we switch to another buffer.
+  ;; The trigger in bug#34318 doesn't exist any more because the C code has
+  ;; changes.  Instead I found the trigger below.
+  (with-temp-buffer
+    (setq last-coding-system-used 'bug34318)
+    (make-local-variable 'last-coding-system-used)
+    ;; This should set last-coding-system-used to `no-conversion'.
+    (decode-coding-string "hello" nil)
+    (should (equal (list last-coding-system-used
+                         (default-value 'last-coding-system-used))
+                   '(no-conversion bug34318)))))
+
 ;;; data-tests.el ends here

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[martin rudalics]

fixed 34318 26.2
quit

 >> 	Don't call swap_in_symval_forwarding since the currently swapped
 >> 	binding is never one we've modified.
 >
 > Indeed, good spotting.  I installed the patch below which mostly reverts
 > this part of the commit.

Thanks for the fix.

 > +(ert-deftest data-tests-make-local-forwarded-var () ;bug#34318
 > +  ;; Boy, this bug is tricky to trigger.  You need to:
 > +  ;; - call make-local-variable on a forwarded var (i.e. one that
 > +  ;;   has a corresponding C var linked via DEFVAR_(LISP|INT|BOOL))
 > +  ;; - cause the C code to modify this variable from the C side of the
 > +  ;;   forwarding, but this needs to happen before the var is accessed
 > +  ;;   from the Lisp side and before we switch to another buffer.
 > +  ;; The trigger in bug#34318 doesn't exist any more because the C code has
 > +  ;; changes.

I suppose you refer to Paul's "Don’t set print-escape-newlines in the
minibuffer" here.  Right?

 > Instead I found the trigger below.
 > +  (with-temp-buffer
 > +    (setq last-coding-system-used 'bug34318)
 > +    (make-local-variable 'last-coding-system-used)
 > +    ;; This should set last-coding-system-used to `no-conversion'.
 > +    (decode-coding-string "hello" nil)
 > +    (should (equal (list last-coding-system-used
 > +                         (default-value 'last-coding-system-used))
 > +                   '(no-conversion bug34318)))))
 > +
 >   ;;; data-tests.el ends here

martin, closing this bug

bug#34318: 26.1.90; Strange behavior of two line message with running shell

[Stefan Monnier]

>> +  ;; The trigger in bug#34318 doesn't exist any more because the C code has
>> +  ;; changes.
> I suppose you refer to Paul's "Don’t set print-escape-newlines in the
> minibuffer" here.  Right?

Yes (and the last word should be "change*d*").


        Stefan