bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Drew Adams]

This REGRESSION was introduced after 24.3 and on or before 2013-05-15 (I
don't have an older 24.4 build than that, to test).

emacs -Q

C-h k C-g  ; Acts normally: shows help for `C-g'.

Evaluate this:

(custom-set-variables '(global-font-lock-mode t nil (font-lock)))

C-h k C-g  ; No help. Instead, a `quit' is performed.



In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-06-28 on ODIEONE
Bzr revision: 117431 rgm@gnu.org-20140628015517-eku6hj8mpgcvfnso
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Nicolas Richard]

Drew Adams <drew.adams@oracle.com> writes:
> emacs -Q
>
> C-h k C-g  ; Acts normally: shows help for `C-g'.
>
> Evaluate this:
>
> (custom-set-variables '(global-font-lock-mode t nil (font-lock)))
>
> C-h k C-g  ; No help. Instead, a `quit' is performed.

FWIW, I can't reproduce on GNU Emacs 24.4.50.3 (i686-pc-linux-gnu, GTK+
Version 3.10.8) of 2014-08-26.

-- 
Nico.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Drew Adams]

> FWIW, I can't reproduce on GNU Emacs 24.4.50.3 (i686-pc-linux-gnu, GTK+
> Version 3.10.8) of 2014-08-26.

Hopefully someone else will try with MS Windows or other platforms.

For me (on Windows) it is 100% reproducible on all of the builds
I mentioned.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Eli Zaretskii]

> Date: Tue, 26 Aug 2014 11:34:11 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 18331@debbugs.gnu.org
> 
> > FWIW, I can't reproduce on GNU Emacs 24.4.50.3 (i686-pc-linux-gnu, GTK+
> > Version 3.10.8) of 2014-08-26.
> 
> Hopefully someone else will try with MS Windows or other platforms.
> 
> For me (on Windows) it is 100% reproducible on all of the builds
> I mentioned.

I do see it, but only after evaluating that strange form.  If I don't
evaluate it, "C-h k" woks as expected.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Drew Adams]

> > For me (on Windows) it is 100% reproducible on all of the builds
> > I mentioned.
> 
> I do see it, but only after evaluating that strange form.  If I don't
> evaluate it, "C-h k" woks as expected.

Correct.

(That strange form is from my `custom-file', which is updated by Customize.
It corresponds to what `custom-set-variables' expects, AFAICT.)

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Eli Zaretskii]

> Date: Tue, 26 Aug 2014 12:01:23 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: theonewiththeevillook@yahoo.fr, 18331@debbugs.gnu.org
> 
> > > For me (on Windows) it is 100% reproducible on all of the builds
> > > I mentioned.
> > 
> > I do see it, but only after evaluating that strange form.  If I don't
> > evaluate it, "C-h k" woks as expected.
> 
> Correct.
> 
> (That strange form is from my `custom-file', which is updated by Customize.
> It corresponds to what `custom-set-variables' expects, AFAICT.)

It is strange because you don't need it: global-font-lock-mode is
already turned on by default, since long ago, and font-lock is
preloaded, so there's no need for it in the form, even if you want to
make sure global-font-lock-mode is on.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Eli Zaretskii]

> Date: Tue, 26 Aug 2014 21:51:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: theonewiththeevillook@yahoo.fr, 18331@debbugs.gnu.org
> 
> > Date: Tue, 26 Aug 2014 11:34:11 -0700 (PDT)
> > From: Drew Adams <drew.adams@oracle.com>
> > Cc: 18331@debbugs.gnu.org
> > 
> > > FWIW, I can't reproduce on GNU Emacs 24.4.50.3 (i686-pc-linux-gnu, GTK+
> > > Version 3.10.8) of 2014-08-26.
> > 
> > Hopefully someone else will try with MS Windows or other platforms.
> > 
> > For me (on Windows) it is 100% reproducible on all of the builds
> > I mentioned.
> 
> I do see it, but only after evaluating that strange form.  If I don't
> evaluate it, "C-h k" woks as expected.

This one was a bitch to debug.  It happens on the release branch as
well, so solving it is a somewhat urgent and delicate matter.

The root cause here is that we call specbind (inside message3_nolog)
with quit-flag non-nil.  What happens next is entirely unpredictable:
when specbind calls set_internal, the latter calls Fassq, which calls
QUIT after it examines every 4 members in the buffer's local
variables' list.  If you are unlucky, Emacs QUITs in the middle of
that, and the entire "C-h k" command gets silently aborted.

I have no idea why this works in Emacs 24.3 and how using the
custom-set-variables form causes it to fail in the current version.  I
did verify that in Emacs 24.3 we also call the same specbind with
quit-flag non-nil.  In the current sources, before I evaluate the
custom-set-variables form, the call to Fassq leaves the loop very
quickly, and thus avoids the QUIT.  This is all very strange, but I
think given the basic fact that Fassq is called when quit-flag is
non-nil, the rest is sheer luck (or lack thereof).

On GNU/Linux, I could only look into this in a -nw session, where we
never get to that specbind with quit-flag non-nil.  So the problem
doesn't happen.  Could someone please test this on GNU/Linux with a
GUI frame?  The place to put the breakpoint is on line 10574 of
xdisp.c (it's inside iwith_echo_area_buffer), and define the breakpoint
commands like this:

  p globals.f_Vquit_flag
  xsymbol
  continue
  end

Then run the recipe reported in the beginning of this bug discussion.

I can fix this problem with the simple patch below.  Does anyone see
any problems with it?

--- src/data.c~	2014-08-03 08:43:52 +0300
+++ src/data.c	2014-08-27 18:28:13 +0300
@@ -1311,10 +1311,10 @@ set_internal (Lisp_Object symbol, Lisp_O
 
 	    /* Find the new binding.  */
 	    XSETSYMBOL (symbol, sym); /* May have changed via aliasing.  */
-	    tem1 = Fassq (symbol,
-			  (blv->frame_local
-			   ? XFRAME (where)->param_alist
-			   : BVAR (XBUFFER (where), local_var_alist)));
+	    tem1 = assq_no_quit (symbol,
+				 (blv->frame_local
+				  ? XFRAME (where)->param_alist
+				  : BVAR (XBUFFER (where), local_var_alist)));
 	    set_blv_where (blv, where);
 	    blv->found = 1;
 

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Drew Adams]

> I can fix this problem with the simple patch below.  Does anyone see
> any problems with it?

Thanks for tracking this down and finding a fix so quickly.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Drew Adams]

> > (That strange form is from my `custom-file', which is updated by
> > Customize. It corresponds to what `custom-set-variables' expects,
> > AFAICT.)
> 
> It is strange because you don't need it: global-font-lock-mode is
> already turned on by default, since long ago, and font-lock is
> preloaded, so there's no need for it in the form, even if you want
> to make sure global-font-lock-mode is on.

Suffering a bit from Emacs Dev myopia? ;-)

I do need it.  I use my `custom-file' for multiple Emacs versions.

The fact that `global-font-lock-mode' is now enabled by default is
irrelevant when you use an Emacs release in which it is not enabled
by default.

(Besides which, a user can still create such a custom setting today,
using the latest and greatest Emacs version.)

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Eli Zaretskii]

> Date: Wed, 27 Aug 2014 08:44:59 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: theonewiththeevillook@yahoo.fr, 18331@debbugs.gnu.org
> 
> > > (That strange form is from my `custom-file', which is updated by
> > > Customize. It corresponds to what `custom-set-variables' expects,
> > > AFAICT.)
> > 
> > It is strange because you don't need it: global-font-lock-mode is
> > already turned on by default, since long ago, and font-lock is
> > preloaded, so there's no need for it in the form, even if you want
> > to make sure global-font-lock-mode is on.
> 
> Suffering a bit from Emacs Dev myopia? ;-)

Cannot do without some ad-hominem?

> I do need it.  I use my `custom-file' for multiple Emacs versions.

Code conditioned by Emacs version is no rocket science.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Drew Adams]

> > Suffering a bit from Emacs Dev myopia? ;-)
> 
> Cannot do without some ad-hominem?

Concerned about your well-being. ;-)
 
> > I do need it.  I use my `custom-file' for multiple Emacs versions.
> 
> Code conditioned by Emacs version is no rocket science.

I make it a point not to fiddle with code that Customize produces.

Yes, I could have multiple `custom-file's and conditionalize which
is used, but I prefer not to.  Not until/unless I have to.  This is
not a case where one should have to do that.

Emacs should be able to handle Customize-generated code, regardless
of version.  And I'm happy to see that you apparently agree enough to
be looking for a fix.  Thank you.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Eli Zaretskii]

> Date: Wed, 27 Aug 2014 10:19:58 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: theonewiththeevillook@yahoo.fr, 18331@debbugs.gnu.org
> 
> > > Suffering a bit from Emacs Dev myopia? ;-)
> > 
> > Cannot do without some ad-hominem?
> 
> Concerned about your well-being. ;-)

You could fool me.

> > > I do need it.  I use my `custom-file' for multiple Emacs versions.
> > 
> > Code conditioned by Emacs version is no rocket science.
> 
> I make it a point not to fiddle with code that Customize produces.

It's just Lisp, for heaven's sake.

> Yes, I could have multiple `custom-file's and conditionalize which
> is used, but I prefer not to.  Not until/unless I have to.  This is
> not a case where one should have to do that.

I just explained why I said "strange", that's all.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Glenn Morris]

Eli Zaretskii wrote:

> Could someone please test this on GNU/Linux with a GUI frame?

The reported issue does not occur.

bug#18331: 24.4.50; REGRESSION, no `C-h k' for `C-g'

[Eli Zaretskii]

> Date: Wed, 27 Aug 2014 18:30:15 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: theonewiththeevillook@yahoo.fr, 18331@debbugs.gnu.org
> 
> I can fix this problem with the simple patch below.  Does anyone see
> any problems with it?
> 
> --- src/data.c~	2014-08-03 08:43:52 +0300
> +++ src/data.c	2014-08-27 18:28:13 +0300
> @@ -1311,10 +1311,10 @@ set_internal (Lisp_Object symbol, Lisp_O
>  
>  	    /* Find the new binding.  */
>  	    XSETSYMBOL (symbol, sym); /* May have changed via aliasing.  */
> -	    tem1 = Fassq (symbol,
> -			  (blv->frame_local
> -			   ? XFRAME (where)->param_alist
> -			   : BVAR (XBUFFER (where), local_var_alist)));
> +	    tem1 = assq_no_quit (symbol,
> +				 (blv->frame_local
> +				  ? XFRAME (where)->param_alist
> +				  : BVAR (XBUFFER (where), local_var_alist)));
>  	    set_blv_where (blv, where);
>  	    blv->found = 1;

No comments, so I installed this on the emacs-24 branch (r117479) and
I'm marking this done.