bug#15045: Point jumps inappropriately around time of Semantic lexing

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

See this thread for the original description:

   http://lists.gnu.org/archive/html/emacs-devel/2013-07/msg00328.html

The short of it is that I occasionally witness point jump to elsewhere
in the buffer while I'm editing and then back again. The change of
point is visually apparent in the display. This has consequences such
as causing undesired scrolling while editing.

To debug it, I used a change like this:

diff --git a/src/editfns.c b/src/editfns.c
index 50bde90..039e13f 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -234,6 +234,8 @@ The return value is POSITION.  */)
   (register Lisp_Object position)
 {
   ptrdiff_t pos;
+  bool noninteractive_old = noninteractive;
+  noninteractive = true;

   if (MARKERP (position)
       && current_buffer == XMARKER (position)->buffer)
@@ -245,7 +247,19 @@ The return value is POSITION.  */)
        SET_PT_BOTH (ZV, ZV_BYTE);
       else
        SET_PT_BOTH (pos, marker_byte_position (position));
-
+      int btI = 0;
+      Lisp_Object theBacktrace;
+      do
+      {
+         theBacktrace = Fbacktrace_frame( make_number(btI) );
+         ++btI;
+         Fprin1(theBacktrace, Qnil);
+         printf("\017\n");
+      } while( ! EQ(theBacktrace, Qnil) );
+      { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm); }
+           printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: goto-char
marker pos=%ld\n", // TODO: debugging
+                             debug_dateStr, debug_ts.tv_nsec,
getpid(), pthread_self(), __FILE__, __LINE__, pos ); fflush(stdout); }
+      noninteractive = noninteractive_old;
       return position;
     }

@@ -253,6 +267,19 @@ The return value is POSITION.  */)

   pos = clip_to_bounds (BEGV, XINT (position), ZV);
   SET_PT (pos);
+   int btI = 0;
+   Lisp_Object theBacktrace;
+   do
+   {
+      theBacktrace = Fbacktrace_frame( make_number(btI) );
+      ++btI;
+      Fprin1(theBacktrace, Qnil);
+      printf("\017\n");
+   } while( ! EQ(theBacktrace, Qnil) );
+   { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm); }
+        printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: goto-char
position pos=%ld\n", // TODO: debugging
+                          debug_dateStr, debug_ts.tv_nsec, getpid(),
pthread_self(), __FILE__, __LINE__, pos ); fflush(stdout); }
+   noninteractive = noninteractive_old;
   return position;
 }


I got a reproduction with these debug statements. I was editing a
Makefile in comments at about buffer position 3396 and observed point
temporarily jump to the beginning of the comment block at position
3084. I filtered the output for calls to go to position 3084 around
the time I witnessed this:

   [backtrace A]
   2013-08-01T10:37:28.119778000|pid:10485|tid:2342111488|editfns.c|281|
DEBUG: goto-char position pos=3084
   [backtrace B]
   2013-08-01T10:37:28.412962000|pid:10485|tid:2342111488|editfns.c|261|
DEBUG: goto-char marker pos=3084
   [backtrace C]
   2013-08-01T10:37:29.715413000|pid:10485|tid:2342111488|editfns.c|281|
DEBUG: goto-char position pos=3084

Strangly, the backtrace A and B are 35 and 45 empty (except my Ctrl-O
chars) stack frames respectively. Backtrace C is:

(t semantic-make-lexer 3083 3257 nil nil)^O
(t semantic-lex 3083 3257 nil)^O
(t semantic-parse-region-default 3083 3257 nil nil nil)^O
(t semantic-parse-region 3083 3257 nil)^O
(t semantic-edits-incremental-parser-1)^O
parser error: %S" error-message-string t] 4)))] 3)^O
(t semantic-parse-changes-default)^O
(t semantic-parse-changes)^O
(t semantic-fetch-tags)^O
(t byte-code "<U+008A><U+008A>À <U+0088>*Á<U+0087>"
[semantic-fetch-tags nil] 1)^O
(t semantic-idle-scheduler-refresh-tags)^O
(t byte-code "Æ^XÇpÇÆÈÉÊ
\"\"\"^YÆ<U+0089>^ZESCÆ^\^M;<U+0085>^\^@Ë^M!^^#^N$<U+0085>^^@ÌÍ!<U+0085>+^@^N^M?<U+0085>^^@^N%?<U+0085>^^@^N#<U+0084>E^@^M;<U+0083>E^@Î^M!<U+0084>R^@^N#<U+0085>^^@Ë^MÆÏ#<U+0085>^^@^N&ÐX<U+0086>^
^M;<U+0085>­^@Ë^M!^^#^N$<U+0085>ï^@ÌÍ!<U+0085>¼^@^N^M?<U+0085>ï^@^N%?<U+0085>ï^@^N#<U+0084>Ö^@^M;<U+0083>Ö^@Î^M!<U+0084>ã^@^N#<U+0085>ï^@Ë^MÆÏ#<U+0085>ï^@^N&ÐX<U+0086>ï^@Ñ
^N&W)<U+0083>
^A^N(<U+0083>þ^@Ò <U+0088><U+0082>
^AÓÚÛ<U+008F><U+0088><U+0082>
^Ap^KB^S)^N*A<U+0089>^V*<U+0084>^?^@*^K^Q)^N,Æ^^-<U+0089>^^*<U+0083>o^A^N*@^V-<U+008A>^N+<U+0083>>^AÖ
<U+0084>8^A× <U+0083>>^AØ^N+Ü\"<U+0088>^N.<U+0083>I^AÝÞ^N-\"<U+0088>^N(<U+0083>U^A^N-
<U+0088><U+0082>Z^AÓßà<U+008F>
e-scheduler-queue service semantic-idle-scheduler-verbose-flag] 8)^O
(t semantic-idle-core-handler)^O
(t semantic-idle-scheduler-function)^O
(t apply semantic-idle-scheduler-function nil)^O
(t byte-code "rÂÃH\"<U+0088>)Á<U+0087>" [timer apply 5 6] 4)^O
(t timer-event-handler [t 0 1 0 t semantic-idle-scheduler-function nil
idle 0])^O
nil^O

I can see the call to goto-char in define-lex (the macro which creates
semantic-make-lexer). However, there is a save-excursion in effect at
the semantic-idle-core-handler frame, so this goto-char wouldn't seem
to be the same goto-char I observe in the display.

I'm not sure about the empty backtraces. Is the code I used to print
backtraces valid? In my many runs, empty backtraces are very rare.

I have since started using Fbacktrace() instead of the more long
winded code above. Unfortunately I'm having a reproduction drought in
the past week.

One additional observation not previously noted is that I see this bug
much more often when editing comments or strings. However, I'm fairly
sure I've seen it when editing straight code too.

In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.4)
 of 2013-06-18 on psd15
Windowing system distributor `The X.Org Foundation', version 11.0.70101000
System Description:	Red Hat Enterprise Linux Client release 5.4 (Tikanga)

Configured using:
 `configure
 --prefix=/redacted/user/boreilly/sw/emacs-install-trunk-20899d085afe62520113b5acbfe3dbba57823dc9
 --with-gif=no'

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  outline-minor-mode: t
  global-whitespace-mode: t
  global-ede-mode: t
  global-semanticdb-minor-mode: t
  global-semantic-idle-scheduler-mode: t
  semantic-mode: t
  evil-mode: t
  evil-local-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  show-paren-mode: t
  delete-selection-mode: t
  global-auto-revert-mode: t
  tooltip-mode: t
  mouse-wheel-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r e p o r t - <tab> <return>

Recent messages:
Loading redacted
Loading redacted
Loading whitespace...done
2013-08-07T13:47:52.565183 Loading tags file: redacted
Starting a new list of tags tables
2013-08-07T13:47:52.716876 Finished loading init file.
2013-08-07T13:47:52.723671 Inside my-prog-mode-hook
2013-08-07T13:47:52.732424 Inside my-emacs-lisp-mode-hook for buffer *scratch*
For information about GNU Emacs and the GNU system, type C-h C-a.
2013-08-07T13:47:52.748012 ---------------- Finished with
my-emacs-startup-hook. ----------------

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils noutline outline easy-mmode my-config warnings
semantic/lex-spp etags package cl-macs whitespace cus-start cus-load
my-proj ede/cpp-root ede/speedbar ede/files ede ede/base ede/auto
ede/source eieio-speedbar speedbar sb-image dframe eieio-custom wid-edit
semantic/db-mode semantic/idle semantic/bovine/gcc semantic/dep
semantic/ia semantic/analyze/refs semantic/db-find semantic/db-ref
semantic/senator semantic/decorate pulse semantic/analyze semantic/sort
semantic/scope semantic/analyze/fcn semantic/db gv eieio-base
semantic/ctxt semantic/format ezimage semantic/tag-ls semantic/find
semantic/util-modes easymenu semantic/util semantic semantic/tag
semantic/lex semantic/fw eieio byte-opt bytecomp byte-compile cconv
eieio-core mode-local cedet evil evil-integration evil-maps
evil-commands evil-types evil-search evil-ex evil-macros evil-repeat
evil-states evil-core evil-common windmove rect evil-digraphs evil-vars
ring edmacro kmacro undo-tree diff goto-chg rainbow-delimiters my-util
advice help-fns electric paren delsel autorevert cl nadvice cl-lib
time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> The short of it is that I occasionally witness point jump to elsewhere
> in the buffer while I'm editing and then back again.

So it's "displayed at A" then "displayed at B" then "displayed at
A again"?  what happens between each one of those 3 displays?

If "nothing", then I suspect there's something like a `sit-for'
somewhere that causes a redisplay in the middle of the command (i.e. in
the middle of a save-excursion).


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Stefan Monnier writes:
>> The short of it is that I occasionally witness point jump to elsewhere
>> in the buffer while I'm editing and then back again.
>
> So it's "displayed at A" then "displayed at B" then "displayed at
> A again"?  what happens between each one of those 3 displays?
>
> If "nothing", then I suspect there's something like a `sit-for'
> somewhere that causes a redisplay in the middle of the command (i.e. in
> the middle of a save-excursion).

I sometimes see this as well, and yes, "nothing" happens between those
three displays. I also think there's a redisplay triggered by another
background task while Semantic does its idle parsing stuff. I surely
won't rule out that we're missing a save-excursion somewhere in
Semantic, but I think that if this were the case, it should happen much
more often.

I wonder which background tasks could trigger such redisplays. Maybe an
update of the mode-line? How could I debug this? The problem is that
this happens maybe every 15 minutes or so...

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

> So it's "displayed at A" then "displayed at B" then "displayed at A
> again"? what happens between each one of those 3 displays?

No. A, B, and C are backtraces. Specifically, the backtraces printed
when goto-char went to the offending position at the beginning of the
comment block. I don't know which of the three is the one I see
actually displayed. The timestamps are too close together for me to be
certain.

I'm interested to see if I get more empty backtraces at the next
reproduction. I don't know what they could mean.

> If "nothing", then I suspect there's something like a `sit-for'
> somewhere that causes a redisplay in the middle of the command (i.e.
> in the middle of a save-excursion).

Ahh, I didn't know about sit-for. It might explain what I'm seeing.

I added a debug statement to Fredisplay. In a simple run (no
reproduction of the bug yet), I get a few of these:

2013-08-07T15:18:09.576922000|pid:11494|tid:47776720943360|dispnew.c|5822|
DEBUG: redisplay
  redisplay()
  sit-for(0)
  jit-lock-deferred-fontify()
  apply(jit-lock-deferred-fontify nil)
  byte-code("rÁ\bÂH\bÃH\"ˆ)Á‡" [timer apply 5 6] 4)
  timer-event-handler([t 0 0 10000 t jit-lock-deferred-fontify nil idle 0])

I don't suppose there are ways deferred jit lock would execute during
Semantic lexing?

Anyway, next time I see the bug, I expect I'll get better data.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Wed, 7 Aug 2013 15:31:38 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: 15045@debbugs.gnu.org
> 
> > So it's "displayed at A" then "displayed at B" then "displayed at A
> > again"? what happens between each one of those 3 displays?
> 
> No. A, B, and C are backtraces. Specifically, the backtraces printed
> when goto-char went to the offending position at the beginning of the
> comment block. I don't know which of the three is the one I see
> actually displayed. The timestamps are too close together for me to be
> certain.
> 
> I'm interested to see if I get more empty backtraces at the next
> reproduction. I don't know what they could mean.
> 
> > If "nothing", then I suspect there's something like a `sit-for'
> > somewhere that causes a redisplay in the middle of the command (i.e.
> > in the middle of a save-excursion).
> 
> Ahh, I didn't know about sit-for. It might explain what I'm seeing.
> 
> I added a debug statement to Fredisplay. In a simple run (no
> reproduction of the bug yet), I get a few of these:
> 
> 2013-08-07T15:18:09.576922000|pid:11494|tid:47776720943360|dispnew.c|5822|
> DEBUG: redisplay
>   redisplay()
>   sit-for(0)
>   jit-lock-deferred-fontify()
>   apply(jit-lock-deferred-fontify nil)
>   byte-code("rÁ\bÂH\bÃH\"ˆ)Á‡" [timer apply 5 6] 4)
>   timer-event-handler([t 0 0 10000 t jit-lock-deferred-fontify nil idle 0])
> 
> I don't suppose there are ways deferred jit lock would execute during
> Semantic lexing?

I think the jit-lock you see is the result of the scrolling, not its
reason.

> Anyway, next time I see the bug, I expect I'll get better data.

In the discussion that you cite at the beginning of this bug report, I
suggested a different approach to finding out the culprit, by using
the redisplay tracing facility.  Can you try that and post the
results?  It might give better clues.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> In the discussion that you cite at the beginning of this bug report,
> I suggested a different approach to finding out the culprit, by
> using the redisplay tracing facility. Can you try that and post the
> results? It might give better clues.

Though I can't reproduce it at will, I do have a trace-redisplay from
when I saw the bug in a Java file. Specifically, I saw point move
inappropriately, but redisplay didn't need to scroll. It's attached in
case you find it useful.

[-- Attachment #2: trace-redisplay-pt-move.txt --]
[-- Type: text/plain, Size: 14970 bytes --]

0x1108f40 (*Buffer List*): same window start
0x1108f40 (*Buffer List*): 1
redisplay_preserve_echo_area (7)
redisplay_internal 0
0x1108f40 (*Buffer List*): same window start
0x1108f40 (*Buffer List*): 1
expose_frame (1538, 0, 16, 14)
expose_window (1538, 0, 16, 14)
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (*Buffer List*): cursor movement
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (11)
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): cursor movement
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): cursor movement
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): cursor movement
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (9)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
expose_frame  garbaged
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): cursor movement
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): cursor movement
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): cursor movement
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): A
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
expose_frame  garbaged
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_internal 0
0x1108f40 (Redacted.java): same window start
0x1108f40 (Redacted.java): 1
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
expose_frame (211, 331, 1017, 239)
expose_window (211, 331, 1017, 239)
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
expose_frame (0, 0, 5, 975)
expose_window (0, 0, 5, 952)
expose_window (0, 952, 5, 14)
expose_frame (1555, 0, 5, 975)
expose_frame (211, 331, 1017, 239)
expose_window (211, 331, 1017, 239)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
0x1108f40 (Redacted.java): try_window_id 3
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (20)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_preserve_echo_area (8)
redisplay_internal 0
redisplay_internal 0
redisplay_preserve_echo_area (8)

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Wed, 7 Aug 2013 16:39:29 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: deng@randomsample.de, 15045@debbugs.gnu.org
> 
> > In the discussion that you cite at the beginning of this bug report,
> > I suggested a different approach to finding out the culprit, by
> > using the redisplay tracing facility. Can you try that and post the
> > results? It might give better clues.
> 
> Though I can't reproduce it at will, I do have a trace-redisplay from
> when I saw the bug in a Java file. Specifically, I saw point move
> inappropriately, but redisplay didn't need to scroll. It's attached in
> case you find it useful.

Can you identify the area in this trace where the unwarranted scroll
was visible?

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

Eli:
> Can you identify the area in this trace where the unwarranted scroll
> was visible?

Barry:
> but redisplay didn't need to scroll

Undesired scrolling is a downstream symptom. Upstream from it is point
visibly moving around inappropriately. I have scroll-margin set to 4,
so when point briefly moves into the top scroll-margin, I get
undesired scrolling. I don't know that I've seen point move to outside
the visible part of the buffer. If so, then users with scroll-margin 0
won't see undesired scrolling, but would still see point moving
around.

Thus, I'm debugging the symptom of point moving.

> A, B, and C are local variables whose meaning depends on the email
> in which they're used.

Ah, so it's not in "thread local storage".

> in Fredisplay, walk the specpdl stack looking for a
> save_excursion_restore where the saved position is different from
> the current value of point in that buffer.

Thanks for the tip. I tested this change and it seems to implement
what you described.

diff --git a/src/dispnew.c b/src/dispnew.c
index 522a0e6..cf0103e 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5815,6 +5815,32 @@ immediately by pending input.  */)
   (Lisp_Object force)
 {
   ptrdiff_t count;
+  bool noninteractive_old = noninteractive;
+  noninteractive = true;
+  Lisp_Object curPtMarker = Fpoint_marker();
+  union specbinding *pdl = specpdl_ptr;
+  while (pdl > specpdl)
+     {
+        --pdl;
+        if (pdl->kind == SPECPDL_UNWIND
+            && pdl->unwind.func == save_excursion_restore
+            && ! EQ (Fequal (XSAVE_OBJECT (pdl->unwind.arg, 0),
curPtMarker), Qt))
+           {
+              { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm)
+                 printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: Found
save_excursion_restore with mismatched point markers ", // TODO:
debugging
+                         debug_dateStr, debug_ts.tv_nsec, getpid(),
pthread_self(), __FILE__, __LINE__ ); }
+              Fprin1(XSAVE_OBJECT (pdl->unwind.arg, 0), Qnil);
+              Fprin1(curPtMarker, Qnil);
+              printf("\n");
+              Fbacktrace();
+              fflush(stdout);
+           }
+     }
+  /* { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm); } */
+  /*      printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: redisplay
\n", // TODO: debugging */
+  /*                        debug_dateStr, debug_ts.tv_nsec,
getpid(), pthread_self(), __FILE__, __LINE__ ); fflush(stdout); } */
+  /* Fbacktrace(); */
+  noninteractive = noninteractive_old;

   swallow_events (1);
   if ((detect_input_pending_run_timers (1)

We'll see what I get next time it comes up.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Thu, 8 Aug 2013 13:07:01 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: deng@randomsample.de, 15045@debbugs.gnu.org
> 
> Eli:
> > Can you identify the area in this trace where the unwarranted scroll
> > was visible?
> 
> Barry:
> > but redisplay didn't need to scroll
> 
> Undesired scrolling is a downstream symptom. Upstream from it is point
> visibly moving around inappropriately.

I think you are wrong.  Emacs moves point in its Lisp code all over
the place, and that never causes any unwarranted scrolling, nor should
it ever display point in anything but the final position -- unless the
Lisp code itself forces redisplay.

But even if you are right, knowing which parts of the display engine
are involved in this will allow us to put breakpoints in a few
strategic places, and produce backtraces, both in C and in Lisp, which
will show which Lisp code triggers the problem.  This is IMO better
than trying to guess which Lisp primitives are involved in point
movement, because you are likely to guess wrong.  By contrast,
redisplay is certainly involved, so using it will most probably show
us the light much sooner and easier.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> So it's "displayed at A" then "displayed at B" then "displayed at A
>> again"? what happens between each one of those 3 displays?
> No. A, B, and C are backtraces.

A, B, and C are local variables whose meaning depends on the email in
which they're used.  Yours have nothing to do with mine.  Sorry I reused
the same identifiers in the same thread.

> 2013-08-07T15:18:09.576922000|pid:11494|tid:47776720943360|dispnew.c|5822|
> DEBUG: redisplay
>   redisplay()
>   sit-for(0)
>   jit-lock-deferred-fontify()
>   apply(jit-lock-deferred-fontify nil)
>   byte-code("rÁ\bÂH\bÃH\"ˆ)Á‡" [timer apply 5 6] 4)
>   timer-event-handler([t 0 0 10000 t jit-lock-deferred-fontify nil idle 0])

Running timers is pretty much the same as running redisplay: both are
things that can happen either between commands or in things like
`sit-for', so no, jit-lock is unlikely to be the culprit.

Maybe a way to check that is:

in Fredisplay, walk the specpdl stack looking for
a save_excursion_restore where the saved position is different from the
current value of point in that buffer.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

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

[Adding Eric to CC]

David Engster writes:
> Stefan Monnier writes:
>>> The short of it is that I occasionally witness point jump to elsewhere
>>> in the buffer while I'm editing and then back again.
>>
>> So it's "displayed at A" then "displayed at B" then "displayed at
>> A again"?  what happens between each one of those 3 displays?
>>
>> If "nothing", then I suspect there's something like a `sit-for'
>> somewhere that causes a redisplay in the middle of the command (i.e. in
>> the middle of a save-excursion).
>
> I sometimes see this as well, and yes, "nothing" happens between those
> three displays. I also think there's a redisplay triggered by another
> background task while Semantic does its idle parsing stuff.

I think I tracked this down. I first instrumented `sit-for' to see which
background tasks trigger it. On my machine, this happens by two things:
`jit-lock-deferred-fontify', and `display-time-event-handler'.

The display-time handler is called twice per minute: I have
(display-time) in my .emacs, and every full minute the time gets updated
in the mode-line. Also, it is called in the
gnus-after-get-new-news-hook, and since I check for new mails in the
background, it gets called through this, too.

It's kinda hard to trigger this problem through jit-lock, since its idle
time is much smaller than the one from Semantic. So I disabled it and
tried to trigger the jump by stopping typing at roughly XX:XX:59. The
semantic idle function kicks in after 1 second, and lo and behold, I saw
a jump. It's still difficult to reproduce, but I managed to get two
backtraces in the past hour, which are attached.

As you can see, the display-time-event-handler does indeed interrupt the
lexing phase. It does a `sit-for', the display jumps. Not sure what
happens after that. Does the semantic-idle function resume? Anyway,
somehow point gets back to its original position, and through
`trace-redisplay', I saw the following on stderr:

0x7f35178 (test.cpp): same window start
0x7f35178 (test.cpp): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x7f35178 (test.cpp): try_scrolling
redisplay_preserve_echo_area (8)
redisplay_internal 0
0x7f35178 (test.cpp): same window start
0x7f35178 (test.cpp): 1
0x7f35178 (test.cpp): try_scrolling
redisplay_preserve_echo_area (8)

I guess this just says that redisplay made point visible by scrolling,
right?

You might wonder how the display-time-event-handler can interrupt the
Semantic lexer. In the two backtraces, you see that it calls
`accept-process-output' and `input-pending-p'. This is hidden inside the
macro `semantic-throw-on-input', which can be called in code wrapped
inside `semantic-exit-on-input'; it's our poor-man's 'yield'. It's used
extensively in the idle function code, and it's just there to do a
non-local exit in case the user does something. However, now I know that
it also allows other timers to run.

If you look in the `define-lex' macro, you see that it calls
`semantic-throw-on-input' after each identified token. The problem is
that it does not restore the cursor position before that, so I guess the
fix is simply to change this call to

	   (save-excursion
	     (goto-char starting-position)
	     (semantic-throw-on-input 'lex))

Obviously, we will have to check all other calls to
`semantic-throw-on-input' for this as well. However, I also wonder if
display-time-event-handler couldn't just call `force-mode-line-update';
or does this repaint the whole display as well?

-David


[-- Attachment #2: backtr1.txt --]
[-- Type: text/plain, Size: 991 bytes --]

  sit-for(0)
  display-time-event-handler()
  apply(display-time-event-handler nil)
  byte-code("[snipped]" [timer apply 5 6] 4)
  timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
  accept-process-output()
  semantic-c-lexer(2886 7946 nil nil)
  semantic-lex(2886 7946 nil)
  semantic-parse-region-default(2886 7946 nil nil nil)
  semantic-parse-region-c-mode(2886 7946 nil nil nil)
  semantic-parse-region(2886 7946 nil)
  semantic-edits-incremental-parser-1()
  byte-code("[snipped]" [err message "incremental parser error: %S" error-message-string t] 4)))] 3)
  semantic-parse-changes-default()
  semantic-parse-changes()
  semantic-fetch-tags()
  byte-code("[snipped]" [semantic-fetch-tags nil] 1)
  semantic-idle-scheduler-refresh-tags()
  byte-code("[snipped]")
  semantic-idle-core-handler()
  semantic-idle-scheduler-function()
  apply(semantic-idle-scheduler-function nil)
  timer-event-handler([t 0 1 0 t semantic-idle-scheduler-function nil idle 0])

[-- Attachment #3: backtr2.txt --]
[-- Type: text/plain, Size: 1266 bytes --]

  sit-for(0)
  display-time-event-handler()
  apply(display-time-event-handler nil)
  byte-code("[snipped]" [timer apply 5 6] 4)
  timer-event-handler([t 20995 36800 0 60 display-time-event-handler nil nil 0])
  input-pending-p()
  semantic-c-lexer(2939 7944 nil nil)
  semantic-lex(2939 7944 nil)
  semantic-parse-region-default(2939 7944 bovine-inner-scope nil t)
  semantic-parse-region-c-mode(2939 7944 bovine-inner-scope nil t)
  semantic-parse-region(2939 7944 bovine-inner-scope nil t)
  semantic-get-local-variables-default()
  semantic-get-local-variables-c++-mode()
  semantic-get-local-variables()
  semantic-get-all-local-variables-default(nil)
  semantic-get-all-local-variables()
  byte-code("[snipped]" [scopecache eieio-oset localvar semantic-get-all-local-variables] 4)
  semantic-calculate-scope(7797)
  semantic-analyze-current-context-default(7797)
  semantic-analyze-current-context(7797)
  byte-code("[snipped]" [semantic-analyze-current-context] 2)
  semantic-idle-summary-current-symbol-info-context()
  semantic-idle-summary-current-symbol-info-default()
  semantic-idle-summary-current-symbol-info-c-mode()
  semantic-idle-summary-current-symbol-info()
  semantic-idle-summary-idle-function()
  funcall(semantic-idle-summary-idle-function)

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>   sit-for(0)
>   display-time-event-handler()
>   apply(display-time-event-handler nil)
>   byte-code("[snipped]" [timer apply 5 6] 4)
>   timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
>   accept-process-output()
>   semantic-c-lexer(2886 7946 nil nil)

Right, that would do it.
What happens if you remove the calls to sit-for from time.el?


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Stefan Monnier writes:
>>   sit-for(0)
>>   display-time-event-handler()
>>   apply(display-time-event-handler nil)
>>   byte-code("[snipped]" [timer apply 5 6] 4)
>>   timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
>>   accept-process-output()
>>   semantic-c-lexer(2886 7946 nil nil)
>
> Right, that would do it.
> What happens if you remove the calls to sit-for from time.el?

I would have thought that the time in the mode-line does not get updated
when Emacs is idle, but that does still work. So I've no idea why the
sit-for is there.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Barry OReilly <gundaetiapo@gmail.com>, 15045@debbugs.gnu.org,
>         "Eric M. Ludlam" <eric@siege-engine.com>, Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 08 Aug 2013 14:06:28 -0400
> 
> >   sit-for(0)
> >   display-time-event-handler()
> >   apply(display-time-event-handler nil)
> >   byte-code("[snipped]" [timer apply 5 6] 4)
> >   timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
> >   accept-process-output()
> >   semantic-c-lexer(2886 7946 nil nil)
> 
> Right, that would do it.
> What happens if you remove the calls to sit-for from time.el?

You cannot ensure redisplay without that.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> Right, that would do it.
>> What happens if you remove the calls to sit-for from time.el?
> You cannot ensure redisplay without that.

I don't know what scenario you have in mind.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: deng@randomsample.de,  gundaetiapo@gmail.com,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Thu, 08 Aug 2013 18:51:17 -0400
> 
> >> Right, that would do it.
> >> What happens if you remove the calls to sit-for from time.el?
> > You cannot ensure redisplay without that.
> 
> I don't know what scenario you have in mind.

Any one.  Emacs enters redisplay for any number of reasons, but you
can never be sure it will do so at any specific point unless you force
redisplay at that point.  As you well know, in general, while Lisp
code runs, Emacs does not redisplay.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> >> Right, that would do it.
>> >> What happens if you remove the calls to sit-for from time.el?
>> > You cannot ensure redisplay without that.
>> I don't know what scenario you have in mind.
> Any one.  Emacs enters redisplay for any number of reasons, but you
> can never be sure it will do so at any specific point unless you force
> redisplay at that point.  As you well know, in general, while Lisp
> code runs, Emacs does not redisplay.

Of course, but that's true in general.  What makes it more true in
display-time-event-handler?  Remember that display-time-update (called
just before the sit-for) ends with a call to force-mode-line-update.

In practice, is there any important scenario where
display-time-event-handler's sit-for is useful?


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: deng@randomsample.de,  gundaetiapo@gmail.com,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Fri, 09 Aug 2013 10:03:05 -0400
> 
> >> >> Right, that would do it.
> >> >> What happens if you remove the calls to sit-for from time.el?
> >> > You cannot ensure redisplay without that.
> >> I don't know what scenario you have in mind.
> > Any one.  Emacs enters redisplay for any number of reasons, but you
> > can never be sure it will do so at any specific point unless you force
> > redisplay at that point.  As you well know, in general, while Lisp
> > code runs, Emacs does not redisplay.
> 
> Of course, but that's true in general.  What makes it more true in
> display-time-event-handler?

Why should we care?  Good engineering does not build things on what
"currently happens to work", because that will eventually break,
given enough development.

> Remember that display-time-update (called just before the sit-for)
> ends with a call to force-mode-line-update.

Whose effect no one really understands.

> In practice, is there any important scenario where
> display-time-event-handler's sit-for is useful?

I never analyzed this to tell.

And anyway, display-time is just one case of a timer that needs to
force redisplay.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> Why should we care?  Good engineering does not build things on what
> "currently happens to work", because that will eventually break,
> given enough development.

I care because if I don't have a positive reason to keep this call to
sit-for I'm going to remove it.

>> In practice, is there any important scenario where
>> display-time-event-handler's sit-for is useful?

Thanks.  So as far as you know it's not any more useful than adding
sit-for at the end of any other function.

> And anyway, display-time is just one case of a timer that needs to
> force redisplay.

No disagreement here.  This bug report just made me bump into this
tangentially related call to sit-for which I'll be happy to remove.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: deng@randomsample.de,  gundaetiapo@gmail.com,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Fri, 09 Aug 2013 13:34:25 -0400
> 
> > Why should we care?  Good engineering does not build things on what
> > "currently happens to work", because that will eventually break,
> > given enough development.
> 
> I care because if I don't have a positive reason to keep this call to
> sit-for I'm going to remove it.

If you don't care about a bug report that will have you re-add it,
feel free.

> Thanks.  So as far as you know it's not any more useful than adding
> sit-for at the end of any other function.

Only those that need to make sure their effect is displayed.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

>> Undesired scrolling is a downstream symptom. Upstream from it is
>> point visibly moving around inappropriately.

> I think you are wrong. Emacs moves point in its Lisp code all over
> the place, and that never causes any unwarranted scrolling, nor
> should it ever display point in anything but the final position --
> unless the Lisp code itself forces redisplay.

My use of the adverb "visibly" was meant to indicate redisplay of
point at other positions. Sorry to have confused you.

>   timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
>   accept-process-output()
>   semantic-c-lexer(2886 7946 nil nil)

and

>   timer-event-handler([t 20995 36800 0 60 display-time-event-handler nil nil 0])
>   input-pending-p()
>   semantic-c-lexer(2939 7944 nil nil)

This is indeed a key finding.

If arbitrary timers can execute during the lexer's call to
accept-process-output or input-pending-p, then doesn't that mean
jit-lock-deferred-fontify can run too? If removing timers' sit-for
calls is the solution, then what's to become of
jit-lock-deferred-fontify's call to sit-for? Doesn't deferred jit
locking necessarily have to call redisplay?

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Barry OReilly writes:
>>   timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
>>   accept-process-output()
>>   semantic-c-lexer(2886 7946 nil nil)
>
> and
>
>>   timer-event-handler([t 20995 36800 0 60 display-time-event-handler nil nil 0])
>>   input-pending-p()
>>   semantic-c-lexer(2939 7944 nil nil)
>
> This is indeed a key finding.
>
> If arbitrary timers can execute during the lexer's call to
> accept-process-output or input-pending-p, then doesn't that mean
> jit-lock-deferred-fontify can run too?

Under certain circumstances I guess it could, but usually, deferred
jit-lock will happen after a keypress, and that would already make the
Semantic idle function quit.

> If removing timers' sit-for calls is the solution, then what's to
> become of jit-lock-deferred-fontify's call to sit-for?

That is not the solution, at least not in general. As I've written, I
think the correct fix is for Semantic to make sure we restore point
before calling `semantic-throw-on-input'. Or maybe we should do this in
`semantic-exit-on-input'; I'm not sure.

However, doing redisplay in timers is not nice. If it is not necessary,
like maybe in the time-display handler, then it should be removed.

> Doesn't deferred jit locking necessarily have to call redisplay?

I would think so, too.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  15045@debbugs.gnu.org,  "Eric M. Ludlam" <eric@siege-engine.com>,  Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 08 Aug 2013 22:30:26 +0200
> 
> However, doing redisplay in timers is not nice.

Why not?

> > Doesn't deferred jit locking necessarily have to call redisplay?
> 
> I would think so, too.

But since jit-lock-deferred-fontify only happens when Emacs is idle,
there's no problem with that, since Emacs enters redisplay also when
it is idle.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>> 15045@debbugs.gnu.org, "Eric M. Ludlam" <eric@siege-engine.com>, Eli
>> Zaretskii <eliz@gnu.org>
>> Date: Thu, 08 Aug 2013 22:30:26 +0200
>> 
>> However, doing redisplay in timers is not nice.
>
> Why not?

Because doing redisplay is a user-visible side effect, and in general,
timers shouldn't have those. AFAICS, any function that calls things like
`accept-process-output', `input-pending-p' or `sit-for' inside a
`save-excursion' might get an unwanted scrolling effect if point is
temporarily moved to some invisible location.

In fact, I just understood another bug in speck-mode (which is similar
to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
that those also happen at every full minute while typing.

>> > Doesn't deferred jit locking necessarily have to call redisplay?
>> 
>> I would think so, too.
>
> But since jit-lock-deferred-fontify only happens when Emacs is idle,
> there's no problem with that, since Emacs enters redisplay also when
> it is idle.

I thought that the jit-lock timer is a non-idle timer, but you are
right.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: gundaetiapo@gmail.com,  monnier@iro.umontreal.ca,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Fri, 09 Aug 2013 07:36:07 +0200
> 
> >> However, doing redisplay in timers is not nice.
> >
> > Why not?
> 
> Because doing redisplay is a user-visible side effect, and in general,
> timers shouldn't have those.

How else can a timer such as that of display-time do its thing?  IOW,
when the purpose of a timer is to cause something to be displayed,
they have no alternative but to force redisplay.

> AFAICS, any function that calls things like
> `accept-process-output', `input-pending-p' or `sit-for' inside a
> `save-excursion' might get an unwanted scrolling effect if point is
> temporarily moved to some invisible location.

You mean, because of timers that might get run and cause redisplay?
Yes, that's possible.  But the way to avoid this is to never call
these when point is in a place the user won't expect.  Forcing the
Emacs community not to trigger redisplay inside timers is IMO _not_
the right way, because this is unnecessarily restrictive, and would
disallow a whole bunch of useful features for no good reason.

IOW, it's the caller of input-pending-p etc. that is the fault here,
not the timers that it inadvertently lets run.

> In fact, I just understood another bug in speck-mode (which is similar
> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
> that those also happen at every full minute while typing.

Then there's the same bug there, that's all.

Doing things in the background is hard in Emacs, so it's a small
wonder some modes get it wrong.

> >> > Doesn't deferred jit locking necessarily have to call redisplay?
> >> 
> >> I would think so, too.
> >
> > But since jit-lock-deferred-fontify only happens when Emacs is idle,
> > there's no problem with that, since Emacs enters redisplay also when
> > it is idle.
> 
> I thought that the jit-lock timer is a non-idle timer, but you are
> right.

There's no jit-lock timer.  There are timers for jit-lock-stealth and
for jit-lock-deferred-fontify (both optional features), and they are
all idle timers.  JIT Lock itself is not triggered by any timer, it is
invoked by redisplay when a portion of text that is not fontified yet
comes into view.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eric M. Ludlam]

On 08/09/2013 03:53 AM, Eli Zaretskii wrote:
>> AFAICS, any function that calls things like
>> `accept-process-output', `input-pending-p' or `sit-for' inside a
>> `save-excursion' might get an unwanted scrolling effect if point is
>> temporarily moved to some invisible location.
>
> You mean, because of timers that might get run and cause redisplay?
> Yes, that's possible.  But the way to avoid this is to never call
> these when point is in a place the user won't expect.  Forcing the
> Emacs community not to trigger redisplay inside timers is IMO _not_
> the right way, because this is unnecessarily restrictive, and would
> disallow a whole bunch of useful features for no good reason.
>
> IOW, it's the caller of input-pending-p etc. that is the fault here,
> not the timers that it inadvertently lets run.

In my mind, to anyone using input-pending-p to deal with responsiveness 
in long running code, a timer that causes a redisplay is the source of 
the problem.  In addition, it is clear that anyone with a sit-for in 
their timer will think that moving the point and calling input-pending-p 
is a bug.

Even if we resolve which is the real bug, or at least the proposed 
solution here, people will still cause redisplays in timeres, and call 
input-pending-p in long running code, and those problems will need to be 
debugged again in the future.

It seems like it would be more developer friendly for Emacs to enforce 
some kind of behavior with timers so as to remove this class of 
collision.  Here are some random options:

* In a timer, disallow dispatch of other timers until done.
* Record redisplay requests in timers, and force redisplay when the
   timer exists.
* In input-pending-p, dont' allow new timers to run,
   doc only talks about command input, not timers.
* In timers, redisplay always assumes you don't want to move point and
   puts it back before the draw.

It is clear to me that there are valid use cases for both redisplay in a 
timer, and input-pending-p being used in long-running programs.  Adding 
funky restrictions to either seems untenable with the size of the Emacs 
dev community.  It would be better to tackle the problem at its source.

Eric

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Fri, 09 Aug 2013 07:50:19 -0400
> From: "Eric M. Ludlam" <eric@siege-engine.com>
> CC: David Engster <deng@randomsample.de>, gundaetiapo@gmail.com, 
>  monnier@iro.umontreal.ca, 15045@debbugs.gnu.org
> 
> > IOW, it's the caller of input-pending-p etc. that is the fault here,
> > not the timers that it inadvertently lets run.
> 
> In my mind, to anyone using input-pending-p to deal with responsiveness 
> in long running code, a timer that causes a redisplay is the source of 
> the problem.

Emacs is a complex piece of software.  We cannot remove that
complexity no matter how hard we try.  And yes, this requires
developers to be aware of what can happen when they call certain
APIs.  There's nothing new here.

> Even if we resolve which is the real bug, or at least the proposed 
> solution here, people will still cause redisplays in timeres, and call 
> input-pending-p in long running code, and those problems will need to be 
> debugged again in the future.

Again, nothing new.  It's okay to propose solutions to these issues,
but a "solution" which tells timers not to force redisplay is a
non-starter, IMO, because some of them cannot do their thing without
displaying something at specific times.

> * In a timer, disallow dispatch of other timers until done.

Sounds not a good idea to me, as too many useful features use timers,
and users will be surprised to see them sometimes unable to run.

> * Record redisplay requests in timers, and force redisplay when the
>    timer exists.

Not good for timers that must display at certain times, with minimal
delays.

> * In input-pending-p, dont' allow new timers to run,
>    doc only talks about command input, not timers.

What if input arrives because of a timer?

> * In timers, redisplay always assumes you don't want to move point and
>    puts it back before the draw.

Redisplay doesn't have any idea where is "back".  When redisplay is
entered, it finds point at a certain location.  It has no idea where
it was before, all it knows (in some cases, not all of them) is where
point was in each window during the last redisplay cycle, which could
be 5 msec ago or 5 min ago.

> It is clear to me that there are valid use cases for both redisplay in a 
> timer, and input-pending-p being used in long-running programs.  Adding 
> funky restrictions to either seems untenable with the size of the Emacs 
> dev community.  It would be better to tackle the problem at its source.

I'm sorry, but I don't see "the source" here.  What is the source of
the problem, in your opinion?

In my opinion, the source of the problem is that application code
moves point to a place it didn't intend the user to see, and then
calls some APIs that trigger redisplay and reveal that location of
point (and potentially even cause a scroll).  The solution could be
(a) avoid triggering redisplay, or (b) restore point before calling
something that could cause redisplay.  What other solutions do you
envision that don't limit the basic features involved in this?

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> In my mind, to anyone using input-pending-p to deal with responsiveness in

Actually, now that I think about it.  Why would input-pending-p
run timers?  That sounds wrong.

Of course, fixing it won't change anything to the OP's problem since he
also has backtraces where the problem is triggered via
accept-process-output.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  David Engster <deng@randomsample.de>,  gundaetiapo@gmail.com,  15045@debbugs.gnu.org
> Date: Fri, 09 Aug 2013 10:04:41 -0400
> 
> > In my mind, to anyone using input-pending-p to deal with responsiveness in
> 
> Actually, now that I think about it.  Why would input-pending-p
> run timers?

The code is very explicit:

  DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 0, 0,
	 doc: /* Return t if command input is currently available with no wait.
  Actually, the value is nil only if we can be sure that no input is available;
  if there is a doubt, the value is t.  */)
    (void)
  {
    if (!NILP (Vunread_command_events)
	|| !NILP (Vunread_post_input_method_events)
	|| !NILP (Vunread_input_method_events))
      return (Qt);

    /* Process non-user-visible events (Bug#10195).  */
    process_special_events ();

    return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
			       | READABLE_EVENTS_FILTER_EVENTS)
	    ? Qt : Qnil);


> That sounds wrong.

Are you saying that using the READABLE_EVENTS_FILTER_EVENTS flag above
is wrong?

> Of course, fixing it won't change anything to the OP's problem since he
> also has backtraces where the problem is triggered via
> accept-process-output.

Right.  Emacs generally always runs timers when it waits.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> That sounds wrong.
> Are you saying that using the READABLE_EVENTS_FILTER_EVENTS flag above
> is wrong?

I think so: input-pending-p is not expected to wait, so I don't see any
reason to run timers.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: eric@siege-engine.com, deng@randomsample.de, gundaetiapo@gmail.com,
>         15045@debbugs.gnu.org
> Date: Fri, 09 Aug 2013 14:08:13 -0400
> 
> >> That sounds wrong.
> > Are you saying that using the READABLE_EVENTS_FILTER_EVENTS flag above
> > is wrong?
> 
> I think so: input-pending-p is not expected to wait, so I don't see any
> reason to run timers.

I think the only reason, as with all the other places where we run
timers, is to make Emacs look appear more responsive, notwithstanding
the on-going processing.

(And I meant the READABLE_EVENTS_DO_TIMERS_NOW flag, of course.)

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Eli Zaretskii writes:
> I think the only reason, as with all the other places where we run
> timers, is to make Emacs look appear more responsive, notwithstanding
> the on-going processing.

Is there some easy way to get a list of functions which may cause timers
to run?  IMHO all those functions would need a word of warning in their
doc-string.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>,  eric@siege-engine.com,  gundaetiapo@gmail.com,  15045@debbugs.gnu.org
> Date: Fri, 09 Aug 2013 20:41:44 +0200
> 
> Eli Zaretskii writes:
> > I think the only reason, as with all the other places where we run
> > timers, is to make Emacs look appear more responsive, notwithstanding
> > the on-going processing.
> 
> Is there some easy way to get a list of functions which may cause timers
> to run?

Scanning the C sources is what I'd do.  Not sure if that can go by the
"easy way" handle, though.

> IMHO all those functions would need a word of warning in their
> doc-string.

Yep.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> I think the only reason, as with all the other places where we run
>> timers, is to make Emacs look appear more responsive, notwithstanding
>> the on-going processing.
> Is there some easy way to get a list of functions which may cause timers
> to run?  IMHO all those functions would need a word of warning in their
> doc-string.

Basically, read-event, read-char, read-key-sequence (and friends), and
accept-process-output and input-pending-p, plus any function that calls
one of them (e.g. sit-for).


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Stefan Monnier writes:
>>> I think the only reason, as with all the other places where we run
>>> timers, is to make Emacs look appear more responsive, notwithstanding
>>> the on-going processing.
>> Is there some easy way to get a list of functions which may cause timers
>> to run?  IMHO all those functions would need a word of warning in their
>> doc-string.
>
> Basically, read-event, read-char, read-key-sequence (and friends), and
> accept-process-output and input-pending-p, 

I think we should mention this issue in their doc-strings. Should they
all get same blurb like "This might run timers that trigger redisplay,
so you should make sure that etc." or should they just get a short
notice like "This function runs timers (see 'some-doc-string or info
page').".

> plus any function that calls one of them (e.g. sit-for).

Those are at least a few hundred, so we cannot possibly alter all the
doc-strings for those.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> >> That sounds wrong.
>> > Are you saying that using the READABLE_EVENTS_DO_TIMERS_NOW flag above
>> > is wrong?
>> I think so: input-pending-p is not expected to wait, so I don't see any
>> reason to run timers.
> I think the only reason, as with all the other places where we run
> timers, is to make Emacs look appear more responsive, notwithstanding
> the on-going processing.

I suspect that the users of input-pending-p which care about running
timers would be better served by (sit-for 0 t).  Currently, it's 100%
equivalent, but the intention is a lot more clear.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Cc: gundaetiapo@gmail.com, monnier@iro.umontreal.ca,
>> 15045@debbugs.gnu.org, eric@siege-engine.com
>
>> Date: Fri, 09 Aug 2013 07:36:07 +0200
>> 
>> >> However, doing redisplay in timers is not nice.
>> >
>> > Why not?
>> 
>> Because doing redisplay is a user-visible side effect, and in general,
>> timers shouldn't have those.
>
> How else can a timer such as that of display-time do its thing?  IOW,
> when the purpose of a timer is to cause something to be displayed,
> they have no alternative but to force redisplay.

As you know, Emacs does not have strict timers. If you say to Emacs: one
minute from now, say 'foo', and Emacs happens to be busy one minute from
now, nothing will happen until it waits again. So I wonder: with such a
lenient definition of "timer", does the wait for the next redisplay
really matter? After which idle time does redisplay kick in anyway?

>> AFAICS, any function that calls things like
>> `accept-process-output', `input-pending-p' or `sit-for' inside a
>> `save-excursion' might get an unwanted scrolling effect if point is
>> temporarily moved to some invisible location.
>
> You mean, because of timers that might get run and cause redisplay?

Yes.

> Yes, that's possible.  But the way to avoid this is to never call
> these when point is in a place the user won't expect. Forcing the
> Emacs community not to trigger redisplay inside timers is IMO _not_
> the right way, because this is unnecessarily restrictive, and would
> disallow a whole bunch of useful features for no good reason.

I said: "doing redisplay in timers is not nice". I did not say that it
should be forbidden (if that's even possible).

However, I do propose the following:

- Deleting the 'sit-for' in the display-time-event-handler, since I
  would think that it is the most likely suspect for a non-idle timer
  that does redisplay in a usual Emacs session. The
  force-mode-line-update should be enough, and even if it isn't: if the
  minute is updated half a second too late, we can live with that (we
  can already live with a displayed 'load' value that's up to a minute
  old).

- In (info "(elisp) Timers"), the manual already discourages certain
  things in timers, like changing buffer content or waiting with
  `sit-for'. I think we should add something like this:

  "Also, enforcing a redisplay in non-idle timers is problematic, since
  they may run at times when other functions have moved point or
  narrowed the buffer temporarily.  The redisplay would make these
  visible, leading to unwanted scrolling or sudden narrowing and
  subsequent widening of the buffer.  You should rather try if waiting
  for the next redisplay after the timer has run is sufficient for your
  needs.  If you just change the mode-line, use `force-mode-line-update'
  instead of a full redisplay."

- We should add information to the documentation of
  `access-process-output' and `input-pending-p', like

  "Note that timers may run during this function, which may trigger a
  redisplay.  So before calling this function, you should make sure that
  any temporary changes you made to point position or the buffer (like
  narrowing) are undone, as to not make these changes visible."

As a sidenote: What "bunch of useful features" do you have in mind for
non-idle timers that do redisplay? They shouldn't alter buffer content,
so what can they do that would need immediate redisplay? Displaying some
kind of progress/time indicator is pretty much the only thing I can come
up with.

> IOW, it's the caller of input-pending-p etc. that is the fault here,
> not the timers that it inadvertently lets run.

Until yesterday, I was completely unaware that `accept-process-output'
or `input-pending-p' could run timers that do redisplay, and I think I'm
in good company. The doc-strings do not mention it, so there must be
lots of third-party code which does not pay attention to this, so we
should try that this bug triggers as little as possible.

>> In fact, I just understood another bug in speck-mode (which is similar
>> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
>> that those also happen at every full minute while typing.
>
> Then there's the same bug there, that's all.

Yes. But it shows that this bug is easy to make, and it's hard to track
down. Also, the bug is slightly different in that speck does not only
alter point, but also narrows the buffer, which is even more scary than
unwanted scrolling.

>> I thought that the jit-lock timer is a non-idle timer, but you are
>> right.
>
> There's no jit-lock timer.

Yes. Sorry for being imprecise. I meant the jit-lock defer timer.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: gundaetiapo@gmail.com,  monnier@iro.umontreal.ca,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Fri, 09 Aug 2013 18:10:04 +0200
> 
> >> >> However, doing redisplay in timers is not nice.
> >> >
> >> > Why not?
> >> 
> >> Because doing redisplay is a user-visible side effect, and in general,
> >> timers shouldn't have those.
> >
> > How else can a timer such as that of display-time do its thing?  IOW,
> > when the purpose of a timer is to cause something to be displayed,
> > they have no alternative but to force redisplay.
> 
> As you know, Emacs does not have strict timers. If you say to Emacs: one
> minute from now, say 'foo', and Emacs happens to be busy one minute from
> now, nothing will happen until it waits again. So I wonder: with such a
> lenient definition of "timer", does the wait for the next redisplay
> really matter?

Emacs very seldom is busy for a long time, so in practice you will see
the 1-min timer do its thing pretty much on time.

> After which idle time does redisplay kick in anyway?

Redisplay doesn't count idle time to kick in, so there's no answer to
your question.  Redisplay is triggered by several reasons, one of them
being waiting for keyboard input, which you might call "being idle",
although that's not exactly accurate.

> As a sidenote: What "bunch of useful features" do you have in mind for
> non-idle timers that do redisplay? They shouldn't alter buffer content,
> so what can they do that would need immediate redisplay? Displaying some
> kind of progress/time indicator is pretty much the only thing I can come
> up with.

Any display-related feature, such as blinking something, displaying
something ion the mode line or header line, etc.  Altering a buffer
can be one of the effects, btw, e.g. in a feature that shows the list
of processes on a system and refreshes that list every second.

> > IOW, it's the caller of input-pending-p etc. that is the fault here,
> > not the timers that it inadvertently lets run.
> 
> Until yesterday, I was completely unaware that `accept-process-output'
> or `input-pending-p' could run timers that do redisplay, and I think I'm
> in good company. The doc-strings do not mention it, so there must be
> lots of third-party code which does not pay attention to this, so we
> should try that this bug triggers as little as possible.

I agree that the doc strings should make a prominent reference to this
issue.

> >> In fact, I just understood another bug in speck-mode (which is similar
> >> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
> >> that those also happen at every full minute while typing.
> >
> > Then there's the same bug there, that's all.
> 
> Yes. But it shows that this bug is easy to make, and it's hard to track
> down.

It is also very rare.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Yes. But it shows that this bug is easy to make, and it's hard to track
>> down.
>
> It is also very rare.

I don't think the bug is rare. I do think that this bug being triggered
is rare, since we don't have many non-idle timers in Emacs that trigger
redisplay (are there others in Emacs core besides the display-time event
handler?). It might be fun to work for a while with a dummy timer which
triggers redisplay once a second or so.

I also think this bug is seldomly reported because it's impossible to
give a recipe if you don't know that it is a timer that triggers it
(which is why I never bothered to report the bug in speck-mode).

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: gundaetiapo@gmail.com,  monnier@iro.umontreal.ca,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Sat, 10 Aug 2013 11:54:32 +0200
> 
> Eli Zaretskii writes:
> >> From: David Engster <deng@randomsample.de>
> >> Yes. But it shows that this bug is easy to make, and it's hard to track
> >> down.
> >
> > It is also very rare.
> 
> I don't think the bug is rare. I do think that this bug being triggered
> is rare, since we don't have many non-idle timers in Emacs that trigger
> redisplay

The last sentence exactly means that the bug is rare.

> (are there others in Emacs core besides the display-time event
> handler?).

And so does this.

> It might be fun to work for a while with a dummy timer which
> triggers redisplay once a second or so.

Customize display-time-interval to 1 (after restoring the sit-for that
got deleted), and you've got that.  I'm running for years with that
variable customized to 5 sec, and never saw any unwarranted movement
of point or scrolling.  That's "rare" in my book.

> I also think this bug is seldomly reported because it's impossible to
> give a recipe if you don't know that it is a timer that triggers it
> (which is why I never bothered to report the bug in speck-mode).

The bug database is replete with problems that are not reproducible,
so I'm quite sure people would report them regardless.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

>>> It is also very rare.

>> I don't think the bug is rare. I do think that this bug being
>> triggered is rare, since we don't have many non-idle timers in
>> Emacs that trigger redisplay

> That's "rare" in my book.

Three people in this thread independently witnessed the bug. Given the
proportion of Emacs users who follow the mailing lists, calling this
rare is questionable.

> are there others in Emacs core besides the display-time event
> handler?

I have never used display-time-mode, so another timer is involved,
possibly non core. I don't see this bug nearly as often as David's
"every 15 minutes". I would guess 10 times in the past several
months. I'm running Emacs now with the debugging changes Stefan
suggested:

> in Fredisplay, walk the specpdl stack looking for a
> save_excursion_restore where the saved position is different from
> the current value of point in that buffer.

as the change I posted:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15045#29

I did not witness the bug yesterday, but I did hit the debug
statements a few times. In those cases the Fprin1 and Fbacktrace calls
printed empty strings, and coincided with Emacs pausing for a few
seconds. I don't know why I'd get empty strings. Maybe there's a flaw
in that debugging code?

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> >> >> That sounds wrong.
> >> > Are you saying that using the READABLE_EVENTS_DO_TIMERS_NOW flag
above
> >> > is wrong?
> >> I think so: input-pending-p is not expected to wait, so I don't see any
> >> reason to run timers.
> > I think the only reason, as with all the other places where we run
> > timers, is to make Emacs look appear more responsive, notwithstanding
> > the on-going processing.
>
> I suspect that the users of input-pending-p which care about running
> timers would be better served by (sit-for 0 t).  Currently, it's 100%
> equivalent, but the intention is a lot more clear.

Given this has come up again separately in bug 15567, may I make the
change to input-pending-p?

diff --git a/src/keyboard.c b/src/keyboard.c
index e0cd4d4..fdb7c7d 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9962,8 +9962,7 @@ if there is a doubt, the value is t.  */)
   /* Process non-user-visible events (Bug#10195).  */
   process_special_events ();

-  return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
-                            | READABLE_EVENTS_FILTER_EVENTS)
+  return (get_input_pending (READABLE_EVENTS_FILTER_EVENTS)
          ? Qt : Qnil);
 }

'make check' passes.

Eli Zaretskii:
> What if input arrives because of a timer?

If this is an issue, I propose input-pending-p return t if a timer is
ready to run. Then the long running task would exit, unwind its
save-excursion, then allow the timer to run. Perhaps this contorts the
meaning of "input", but OTOH the documentation for input-pending-p
already states that a false positive return can happen:

  Return t if command input is currently available with no wait.
  Actually, the value is nil only if we can be sure that no input is
  available; if there is a doubt, the value is t.

Would you want this in the same changeset as the patch above? Or not
worry about it until "someone screams"?

[-- Attachment #2: Type: text/html, Size: 2191 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Mon, 14 Oct 2013 15:32:40 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: David Engster <deng@randomsample.de>, Stefan Monnier <monnier@iro.umontreal.ca>, 
> 	15045@debbugs.gnu.org, Eric Ludlam <eric@siege-engine.com>
> 
> Given this has come up again separately in bug 15567, may I make the
> change to input-pending-p?
> 
> diff --git a/src/keyboard.c b/src/keyboard.c
> index e0cd4d4..fdb7c7d 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -9962,8 +9962,7 @@ if there is a doubt, the value is t.  */)
>    /* Process non-user-visible events (Bug#10195).  */
>    process_special_events ();
> 
> -  return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
> -                            | READABLE_EVENTS_FILTER_EVENTS)
> +  return (get_input_pending (READABLE_EVENTS_FILTER_EVENTS)
>           ? Qt : Qnil);
>  }
> 
> 'make check' passes.

FWIW, I'm against such changes.  You can never know how many places
will be subtly broken by changes like that in such a popular
interface.

The original problem is rare enough, and should IMO be fixed in the
code where it happens.  Making changes like above on behalf of such
marginal use cases is a tail wagging the dog.

> Eli Zaretskii:
> > What if input arrives because of a timer?
> 
> If this is an issue, I propose input-pending-p return t if a timer is
> ready to run.

Likewise: too serious change in behavior for too obscure a reason.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> FWIW, I'm against such changes.  You can never know how many places
> will be subtly broken by changes like that in such a popular
> interface.

Hmm... I kind of agree with both:
- this is a subtle change affecting behavior in a corner case and it's
  difficult to know which code might care about it.
- nothing in input-pending-p indicates this is some kind of
  blocking/waiting operation which might hence correspond to a "yield
  point".  On the contrary, the intuition behind input-pending-p would
  make you think that the whole point of it is to let you find out if
  there's input *without waiting*.

So I tend to agree with Barry that input-pending-p should not run
timers.  Not just based on his particular problem case, but on the basis
of what ideally input-pending-p should do.

Arguably it shouldn't process_special_events either, tho obviously,
removing this call would introduce a known breakage, so we should keep
this for now (until we have a better fix for it).
  
> The original problem is rare enough, and should IMO be fixed in the
> code where it happens.

It can be difficult to "fix" it there, because there's simply no other
operation that lets you detect if there's input in the queue without
running arbitrary code, so it can require non-trivial refactoring.

>> > What if input arrives because of a timer?
>> If this is an issue, I propose input-pending-p return t if a timer is
>> ready to run.
> Likewise: too serious change in behavior for too obscure a reason.

100% agreement.  `input-pending-p' is meant to detect *user input*, and
timers do not count as such (and input generated by timers doesn't
count as "user input" either).


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> The original problem is rare enough

You're wrong to call it rare. Dave Milter, David Engster, and I have
seen it separately. It has occurred because of tasks in Semantic,
Speck, and now NXML. That input-pending-p behaves this way is
apparently surprising to the Elisp developer. It is even more
surprising to the user upon witnessing such a jarring symptom.

> So I tend to agree with Barry that input-pending-p should not run
> timers. Not just based on his particular problem case, but on the
> basis of what ideally input-pending-p should do.

I don't claim the change solves the issue completely. This bug report
would have to stay open because Semantic also calls
accept-process-output while point is on an excursion.

Since Stefan had said input-pending-p behavior seems wrong, I followed
up in the hopes we can both correct a wrong and reduce how common this
user-visible symptom is.

[-- Attachment #2: Type: text/html, Size: 1004 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Tue, 15 Oct 2013 10:12:32 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, David Engster <deng@randomsample.de>, 15045@debbugs.gnu.org, 
> 	Eric Ludlam <eric@siege-engine.com>
> 
> > The original problem is rare enough
> 
> You're wrong to call it rare. Dave Milter, David Engster, and I have
> seen it separately.

For functionality so widely and freely used in every corner of Emacs,
anything is "rare" in my book, except if it totally screws up Emacs
for almost everyone.  input-pending-p is all over the place, so if it
were not "rare", we would be facing an outcry long ago.

> That input-pending-p behaves this way is apparently surprising to
> the Elisp developer.

Yes, it _is_ surprising.  But if every surprising behavior will
necessarily lead to change in long entrenched behavior, Emacs will
never be stable.

> It is even more surprising to the user upon witnessing such a
> jarring symptom.

I didn't say that problem shouldn't be solved.  I didn't say we should
let users deal with this.  So I find this remark unfair.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

I'm thinking I should augment the patch so as the observable behavior
of (sit-for 0) doesn't change. It would no longer run timers if only
input-pending-p changes as described. Currently:

  (defun sit-for (seconds &optional nodisp obsolete)
    [...]
    (cond
     [...]
     ((input-pending-p)
      nil)
     ((<= seconds 0)
      (or nodisp (redisplay)))
     [...]

I considered whether (<= seconds 0) could fall through to the t case
instead. The read_char function has a complex implementation, so it's
not clear doing so wouldn't change behavior.

Is there another Lisp function that does timer_check and little else,
which the (<= seconds 0) case could call? Should I write a new subr
for it?

Looking ahead to the concurrency branch, should (sit-for 0 t) and
(thread-yield) have different behaviors? Or perhaps the former calls
the latter? I don't see thread-yield checking for timers to run,
should it? (And it didn't look like timers were split out into their
own threads on the concurrency branch.)

[-- Attachment #2: Type: text/html, Size: 1166 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Tue, 15 Oct 2013 13:08:58 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, David Engster <deng@randomsample.de>, 
> 	15045@debbugs.gnu.org, Eric Ludlam <eric@siege-engine.com>
> 
>   (defun sit-for (seconds &optional nodisp obsolete)
>     [...]
>     (cond
>      [...]
>      ((input-pending-p)
>       nil)
>      ((<= seconds 0)
>       (or nodisp (redisplay)))
>      [...]
> 
> I considered whether (<= seconds 0) could fall through to the t case
> instead.

Maybe I'm missing something, but wouldn't that bypass redisplay?
Currently, (sit-for 0) is a very popular method to trigger redisplay;
changing that _will_ bite a lot of code out there.

> The read_char function has a complex implementation, so it's not
> clear doing so wouldn't change behavior.

Are you assuming that read_char always redisplays?  That's not true,
AFAIK.

> Is there another Lisp function that does timer_check and little else,
> which the (<= seconds 0) case could call? Should I write a new subr
> for it?

Not sure I understand the question, sorry.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

What I'm saying is that

  [...]
  ((input-pending-p)
   nil)
  ((<= seconds 0)
   (or nodisp (redisplay)))
  [...]

would become

  [...]
  ((input-pending-p)
   nil)
  ((<= seconds 0)
   (to-be-determined-call-to-timer-check)
   (or nodisp (redisplay)))
  [...]

The idea is to compensate for input-pending-p no longer calling
timer_check so as sit-for will behave the same.

[-- Attachment #2: Type: text/html, Size: 481 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> of (sit-for 0) doesn't change. It would no longer run timers if only
> input-pending-p changes as described. Currently:

>   (defun sit-for (seconds &optional nodisp obsolete)
>     [...]
>     (cond
>      [...]
>      ((input-pending-p)
>       nil)
>      ((<= seconds 0)
>       (or nodisp (redisplay)))
>      [...]

Indeed, the Elisp version of sit-for is a mess.

> I considered whether (<= seconds 0) could fall through to the t case
> instead.

The easiest solution is to add an optional argument `check-timers' to
input-pending-p, so you can preserve the exact same behavior.
In the long run, we should try and clean up sit-for, but you may not
have the courage to dig into this right now.

> Looking ahead to the concurrency branch, should (sit-for 0 t) and
> (thread-yield) have different behaviors?

Not really.

> I don't see thread-yield checking for timers to run, should it?

Yes, it should.

> (And it didn't look like timers were split out into their own threads
> on the concurrency branch.)

The current state of the concurrency branch is not always
representative of what it should ideally look like ;-)
But even if timers are made to belong to a thread, thread-yield should
probably check timers.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> The easiest solution is to add an optional argument `check-timers'
> to input-pending-p, so you can preserve the exact same behavior.

I assume you mean no-check-timers, nil being default.

> In the long run, we should try and clean up sit-for, but you may not
> have the courage to dig into this right now.

I'm arguing I don't need to dig into sit-for's complexity to maintain
its current behavior. If (sit-for 0 ...) calls something like
thread-yield which in turn calls timer_check, then all ways of calling
sit-for would have unchanged behavior.

Even so, because of other calls to input-pending-p not through
sit-for, there could conceivably be regressions in the latencies of
timers, due to background tasks not yielding when they used to. If we
discovered such cases, the solution is straight forward: replace
(input-pending-p) with (sit-for 0 t) in the offending background task.
But I understand the objection.

If the no-check-timers arg solution is what you want, I'll prepare a
patch to add it and have Semantic and NXML use it.

Also, what do you think are the merits of putting in the
specpdl-walking check discussed earlier, enabled with
--enable-checking? I can't see a legit case where check_timers is
called when point is on an excursion. Such code should not make any
assumptions about the other timers the user is running which might
call redisplay.

> The current state of the concurrency branch is not always
> representative of what it should ideally look like ;-) But even if
> timers are made to belong to a thread, thread-yield should probably
> check timers.

The timer behavior on the concurrency branch seems reasonable. Moving
them to their own threads might be complex, not least because of
concerns that the global lock will be yielded often enough to not
regress timer latencies.

[-- Attachment #2: Type: text/html, Size: 2024 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> The easiest solution is to add an optional argument `check-timers'
>> to input-pending-p, so you can preserve the exact same behavior.
> I assume you mean no-check-timers, nil being default.

No, I meant `check-timers', so the default behavior is modified, but
you can recover the previous behavior by passing an extra argument.

>> In the long run, we should try and clean up sit-for, but you may not
>> have the courage to dig into this right now.
> I'm arguing I don't need to dig into sit-for's complexity to maintain
> its current behavior. If (sit-for 0 ...) calls something like
> thread-yield which in turn calls timer_check, then all ways of calling
> sit-for would have unchanged behavior.

That's theory: there is no thread-yield right now, nor any timer-check.

> Also, what do you think are the merits of putting in the
> specpdl-walking check discussed earlier, enabled with
> --enable-checking?

I like the idea, tho I'd have to see the code first to know
whether it should depend on --enable-checking or something else.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> That's theory: there is no thread-yield right now, nor any
> timer-check.

Why not add either? We could implement thread-yield on trunk to call
the timer_check C function and nothing else for now. Or call it
"yield" and rename "thread-yield" to "yield" on the concurrency
branch.

[-- Attachment #2: Type: text/html, Size: 370 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

Here's the solution you requested. Let me know that it's good to
install.

diff --git a/ChangeLog b/ChangeLog
index a755b5c..8b58ebc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2013-10-17  Barry O'Reilly  <gundaetiapo@gmail.com>
+
+       Change how input-pending-p checks for timers to run. Its default
+       changes from checking to not checking. Its new check-timers param
+       allows for prior behavior. (Bug#15045).
+       * src/keyboard.c (Finput_pending_p): Accept optional check-timers
+       param.
+       * lisp/subr.el (sit-for): Call (input-pending-p t) so as to behave
+       as before.
+       * test/automated/timer-tests.el: New file. Tests that (sit-for 0)
+       allows another timer to run.
+
 2013-10-13  Glenn Morris  <rgm@gnu.org>

        * configure.ac [alpha]: Explicit error in non-ELF case.  (Bug#15601)
diff --git a/etc/NEWS b/etc/NEWS
index ddb9a9f..06372f9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -611,6 +611,9 @@ low-level libraries gfilenotify.c, inotify.c or
w32notify.c.
 ^L
 * Incompatible Lisp Changes in Emacs 24.4

+** `(input-pending-p)' no longer runs other timers which are ready to
+run. The new optional CHECK-TIMERS param allows for the prior behavior.
+
 ** `defvar' and `defcustom' in a let-binding affect the "external" default.

 ** The syntax of ?» and ?« is now punctuation instead of matched parens.
diff --git a/lisp/subr.el b/lisp/subr.el
index 0d03e9a..952b9b6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2222,7 +2222,7 @@ floating point support."
    (noninteractive
     (sleep-for seconds)
     t)
-   ((input-pending-p)
+   ((input-pending-p t)
     nil)
    ((<= seconds 0)
     (or nodisp (redisplay)))
diff --git a/src/keyboard.c b/src/keyboard.c
index e0cd4d4..7f8692a 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9947,12 +9947,13 @@ requeued_events_pending_p (void)
   return (!NILP (Vunread_command_events));
 }

-
-DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 0, 0,
+DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
        doc: /* Return t if command input is currently available with no
wait.
 Actually, the value is nil only if we can be sure that no input is
available;
-if there is a doubt, the value is t.  */)
-  (void)
+if there is a doubt, the value is t.
+
+If CHECK-TIMERS is non-nil, timers that are ready to run will do so.  */)
+  (Lisp_Object check_timers)
 {
   if (!NILP (Vunread_command_events)
       || !NILP (Vunread_post_input_method_events)
@@ -9962,8 +9963,9 @@ if there is a doubt, the value is t.  */)
   /* Process non-user-visible events (Bug#10195).  */
   process_special_events ();

-  return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
-                 | READABLE_EVENTS_FILTER_EVENTS)
+  return (get_input_pending ((EQ (check_timers, Qnil)
+                              ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
+                             | READABLE_EVENTS_FILTER_EVENTS)
       ? Qt : Qnil);
 }

diff --git a/test/automated/timer-tests.el b/test/automated/timer-tests.el
new file mode 100644
index 0000000..b4cb1e6
--- /dev/null
+++ b/test/automated/timer-tests.el
@@ -0,0 +1,42 @@
+;;; timer-tests.el --- tests for timers -*- coding: utf-8;
lexical-binding:t -*-
+
+;; Copyright (C) 2013 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see `http://www.gnu.org/licenses/'.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest timer-tests-sit-for ()
+  (let ((timer-ran nil)
+        (timeout (time-add (current-time)
+                           '(0 10 0 0)))
+        ;; Want sit-for behavior when interactive
+        (noninteractive nil))
+    (run-at-time '(0 1 0 0)
+                 nil
+                 (lambda ()
+                   (setq timer-ran t)))
+    (while (not timer-ran)
+        (should (time-less-p (current-time)
+                             timeout))
+        (sit-for 0 t))))
+
+;;; timer-tests.el ends here
+

[-- Attachment #2: Type: text/html, Size: 5290 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> Here's the solution you requested. Let me know that it's good to
> install.

Looks fine.  See comments below.

> +       Change how input-pending-p checks for timers to run. Its default
> +       changes from checking to not checking. Its new check-timers param
> +       allows for prior behavior.

Good.
"Don't run timers in input-pending-p" would have sufficed.

> +** `(input-pending-p)' no longer runs other timers which are ready to
> +run. The new optional CHECK-TIMERS param allows for the prior behavior.

Please use 2 spaces after a full-stop (see sentence-end-double-space).

> +  return (get_input_pending ((EQ (check_timers, Qnil)

EQ (check_timers, Qnil)  =>  NILP (check_timers)

> +;;; timer-tests.el --- tests for timers -*- coding: utf-8;
> lexical-binding:t -*-

The "coding:utf-8" is redundant nowadays.

> +(require 'ert)

IIUC this `require' is also redundant.

> +(ert-deftest timer-tests-sit-for ()
> +  (let ((timer-ran nil)
> +        (timeout (time-add (current-time)
> +                           '(0 10 0 0)))
> +        ;; Want sit-for behavior when interactive
> +        (noninteractive nil))
> +    (run-at-time '(0 1 0 0)
> +                 nil
> +                 (lambda ()
> +                   (setq timer-ran t)))
> +    (while (not timer-ran)
> +        (should (time-less-p (current-time)
> +                             timeout))
> +        (sit-for 0 t))))

I think there's a race-condition, here:
- let's say we're at time < timeout.
- we run sit-for, which does not run the timer since we're still <timeout.
- time advances to > timeout.
- we check (should (time-less-p (current-time) timeout))
- we complain unjustly.

I.e. we should test, after running sit-for, that the "current time before
running sit-for" was < timeout.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> I think there's a race-condition, here:
> - let's say we're at time < timeout.
> - we run sit-for, which does not run the timer since we're still <timeout.
> - time advances to > timeout.
> - we check (should (time-less-p (current-time) timeout))
> - we complain unjustly.

This statement isn't right:

> - we run sit-for, which does not run the timer since we're still <timeout.

If sit-for doesn't run the timer at any time in T+1s to T+10s, then
it's supposed to fail.

However, reconsidering the test, I don't think the timing is
necessary, so I've simplified it.

diff --git a/ChangeLog b/ChangeLog
index a755b5c..553abe2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2013-10-17  Barry O'Reilly  <gundaetiapo@gmail.com>
+
+    Don't run timers in input-pending-p.  Its new check-timers param
+    provides the prior behavior. (Bug#15045).
+    * src/keyboard.c (Finput_pending_p): Accept optional check-timers
+    param.
+    * lisp/subr.el (sit-for): Call (input-pending-p t) so as to behave
+    as before.
+    * test/automated/timer-tests.el: New file.  Tests that (sit-for 0)
+    allows another timer to run.
+
 2013-10-13  Glenn Morris  <rgm@gnu.org>

     * configure.ac [alpha]: Explicit error in non-ELF case.  (Bug#15601)
diff --git a/etc/NEWS b/etc/NEWS
index ddb9a9f..963c372 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -611,6 +611,9 @@ low-level libraries gfilenotify.c, inotify.c or
w32notify.c.

 * Incompatible Lisp Changes in Emacs 24.4

+** `(input-pending-p)' no longer runs other timers which are ready to
+run.  The new optional CHECK-TIMERS param allows for the prior behavior.
+
 ** `defvar' and `defcustom' in a let-binding affect the "external" default.

 ** The syntax of ?» and ?« is now punctuation instead of matched parens.
diff --git a/lisp/subr.el b/lisp/subr.el
index 0d03e9a..952b9b6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2222,7 +2222,7 @@ floating point support."
    (noninteractive
     (sleep-for seconds)
     t)
-   ((input-pending-p)
+   ((input-pending-p t)
     nil)
    ((<= seconds 0)
     (or nodisp (redisplay)))
diff --git a/src/keyboard.c b/src/keyboard.c
index bb8fefa..85a1ce7 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9947,12 +9947,13 @@ requeued_events_pending_p (void)
   return (!NILP (Vunread_command_events));
 }

-
-DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 0, 0,
+DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
        doc: /* Return t if command input is currently available with no
wait.
 Actually, the value is nil only if we can be sure that no input is
available;
-if there is a doubt, the value is t.  */)
-  (void)
+if there is a doubt, the value is t.
+
+If CHECK-TIMERS is non-nil, timers that are ready to run will do so.  */)
+  (Lisp_Object check_timers)
 {
   if (!NILP (Vunread_command_events)
       || !NILP (Vunread_post_input_method_events)
@@ -9962,8 +9963,9 @@ if there is a doubt, the value is t.  */)
   /* Process non-user-visible events (Bug#10195).  */
   process_special_events ();

-  return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
-                 | READABLE_EVENTS_FILTER_EVENTS)
+  return (get_input_pending ((NILP (check_timers)
+                              ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
+                             | READABLE_EVENTS_FILTER_EVENTS)
       ? Qt : Qnil);
 }

diff --git a/test/automated/timer-tests.el b/test/automated/timer-tests.el
new file mode 100644
index 0000000..71a9b96
--- /dev/null
+++ b/test/automated/timer-tests.el
@@ -0,0 +1,38 @@
+;;; timer-tests.el --- tests for timers -*- lexical-binding:t -*-
+
+;; Copyright (C) 2013 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see `http://www.gnu.org/licenses/'.
+
+;;; Commentary:
+
+;;; Code:
+
+(ert-deftest timer-tests-sit-for ()
+  (let ((timer-ran nil)
+        ;; Want sit-for behavior when interactive
+        (noninteractive nil))
+    (run-at-time '(0 0 0 0)
+                 nil
+                 (lambda () (setq timer-ran t)))
+    ;; The test assumes run-at-time didn't take the liberty of firing
+    ;; the timer, so assert the test's assumption
+    (should (not timer-ran))
+    (sit-for 0 t)
+    (should timer-ran)))
+
+;;; timer-tests.el ends here
+

[-- Attachment #2: Type: text/html, Size: 5702 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> However, reconsidering the test, I don't think the timing is
> necessary, so I've simplified it.

Looks good, please install,


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

Rev 114704. Leaving this open because of Semantic use of
accept-process-output during an excursion.

[-- Attachment #2: Type: text/html, Size: 132 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

In terms of solving the issue of how Semantic calls
accept-process-output, here are two proposals:

1: Per David Engster:

> If you look in the `define-lex' macro, you see that it calls
> `semantic-throw-on-input' after each identified token. The problem is
> that it does not restore the cursor position before that, so I guess the
> fix is simply to change this call to
>
>        (save-excursion
>          (goto-char starting-position)
>          (semantic-throw-on-input 'lex))

Here's a patch to implement the basic idea. My debugging indicates
accept-process-output can be called when lexing other buffers, so I
saved off the marker rather than just position.

That said, I have never personally witnessed point unexpectedly
redisplayed in another buffer -- only within the same buffer. That
could be a consequence of how it interacts with the
jit-lock-deferred-fontify idle timer. (I have confirmed with my debug
statements that Semantic is the outer timer and
jit-lock-deferred-fontify is the inner timer which redisplays.)

Please let me know if this fix is suitable.

diff --git a/lisp/cedet/semantic/fw.el b/lisp/cedet/semantic/fw.el
index 825cdc9..869d183 100644
--- a/lisp/cedet/semantic/fw.el
+++ b/lisp/cedet/semantic/fw.el
@@ -369,6 +369,8 @@ later installation should be done in MODE hook."
 ;;
 (defvar semantic-current-input-throw-symbol nil
   "The current throw symbol for `semantic-exit-on-input'.")
+(defvar semantic--on-input-start-marker nil
+  "The marker when starting a semantic-exit-on-input form.")

 (defmacro semantic-exit-on-input (symbol &rest forms)
   "Using SYMBOL as an argument to `throw', execute FORMS.
@@ -376,7 +378,8 @@ If FORMS includes a call to `semantic-throw-on-input',
then
 if a user presses any key during execution, this form macro
 will exit with the value passed to `semantic-throw-on-input'.
 If FORMS completes, then the return value is the same as `progn'."
-  `(let ((semantic-current-input-throw-symbol ,symbol))
+  `(let ((semantic-current-input-throw-symbol ,symbol)
+         (semantic--on-input-start-marker (point-marker)))
      (catch ,symbol
        ,@forms)))
 (put 'semantic-exit-on-input 'lisp-indent-function 1)
@@ -387,7 +390,16 @@ FROM is an indication of where this function is called
from as a value
 to pass to `throw'.  It is recommended to use the name of the function
 calling this one."
   `(when (and semantic-current-input-throw-symbol
-              (or (input-pending-p) (accept-process-output)))
+              (or (input-pending-p)
+                  (save-excursion
+                    ;; Timers might run during accept-process-output.
+                    ;; If they redisplay, point must be where the user
+                    ;; expects. (Bug#15045)
+                    (set-buffer (marker-buffer
+                                 semantic--on-input-start-marker))
+                    (goto-char (marker-position
+                                semantic--on-input-start-marker))
+                    (accept-process-output))))
      (throw semantic-current-input-throw-symbol ,from)))

 ^L

2: Extend accept-process-output to allow suppressing timer_check.
Actually, there's an input arg already for that, but only applicable
if the PROCESS arg is non nil.

[-- Attachment #2: Type: text/html, Size: 3646 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

Eric, David,

I would very much like to use Semantic without unwanted scrolling, so I'd
appreciate your feedback. How is the patch I submitted? What else do I need
to do or investigate?

Patch: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15045#212

[-- Attachment #2: Type: text/html, Size: 366 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> I would very much like to use Semantic without unwanted scrolling, so I'd
> appreciate your feedback. How is the patch I submitted? What else do I need
> to do or investigate?
> Patch: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15045#212

The patch is a bit "ugly" (in the sense that it's a bit too ad-hoc), but
it's probably what it takes (tho maybe semantic-exit-on-input could be
replaced by while-no-input, which does not suffer from this problem,
but it's not a simple replacement).

But the main problem I see is that you already have a few "trivial"
patches installed, so this is summing up to something non-trivial which
requires copyright paperwork.

If that's OK with you, then, please fill the form below and send it to
the FSF as instructed, so they can send you the relevant paperwork.


        Stefan


Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]


[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]


[For the copyright registration, what country are you a citizen of?]


[What year were you born?]


[Please write your email address here.]


[Please write your postal address here.]





[Which files have you changed so far, and which new files have you written
so far?]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> The patch is a bit "ugly" (in the sense that it's a bit too ad-hoc)

Yes, I feel the same way. What seems more appropriate is if there was
something like process-output-p which would indicate to Semantic that
process output is ready to accept, and thus that Semantic needs to
unwind out of its save-excursion form before allowing
accept-process-output. Does the while-no-input you mentioned exit in
that circumstance?

> But the main problem I see is that you already have a few "trivial"
> patches installed, so this is summing up to something non-trivial
> which requires copyright paperwork.

That's already done and you granted me commit privs to Emacs and
ELPA. Let me know if you want me to forward the docs again confirming
the processing is complete.

[-- Attachment #2: Type: text/html, Size: 882 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> accept-process-output. Does the while-no-input you mentioned exit in
> that circumstance?

Inside a while-no-input, if the user hits a key (or clicks), then
a `throw' is executed that exits the while-no-input.

>> But the main problem I see is that you already have a few "trivial"
>> patches installed, so this is summing up to something non-trivial
>> which requires copyright paperwork.
> That's already done and you granted me commit privs to Emacs and
> ELPA. Let me know if you want me to forward the docs again confirming
> the processing is complete.

Oh, sorry, I didn't look up the copyright.list properly.  Then please
go ahead and install your change.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Barry OReilly]

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

> Inside a while-no-input, if the user hits a key (or clicks), then a
> `throw' is executed that exits the while-no-input.

I thought input-pending-p covers that base. If we use while-no-input,
don't we still need to use accept-process-output, thus not solving the
problem?

> Then please go ahead and install your change.

Thanks, rev 115123. Closing.

[-- Attachment #2: Type: text/html, Size: 427 bytes --]

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> Inside a while-no-input, if the user hits a key (or clicks), then a
>> `throw' is executed that exits the while-no-input.
> I thought input-pending-p covers that base.

The difference is that input-pending-p does polling (you have to keep
calling input-pending-p), whereas while-no-input is interrupt based (it
will stop whatever you're doing when the key comes in).

> If we use while-no-input, don't we still need to use
> accept-process-output, thus not solving the problem?

Might very well be: while-no-input does not by-itself let processes run.
I don't know what the CEDET code does, so I just mentioned
while-no-input as something related, but indeed it might not solve
our problem.

>> Then please go ahead and install your change.
> Thanks, rev 115123. Closing.

Great, thank you,


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

I just removed the two calls to sit-for from time.el.
We'll see if someone screams showing a scenario where they were useful.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[martin rudalics]

 > In fact, I just understood another bug in speck-mode (which is similar
 > to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
 > that those also happen at every full minute while typing.

What precisely is the bug in speck-mode?  Is it that it doesn't restore
`point' when input arrives?

martin

bug#15045: Point jumps inappropriately around time of Semantic lexing

[David Engster]

martin rudalics writes:
>> In fact, I just understood another bug in speck-mode (which is similar
>> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
>> that those also happen at every full minute while typing.
>
> What precisely is the bug in speck-mode?  Is it that it doesn't restore
> `point' when input arrives?

The main problem is that speck narrows the buffer before calling
`accept-process-output'. If that happens while the
time-display-event-handler runs, you suddenly see a narrowed buffer,
which is soon widened again. Pretty scary.

It's quite easy to reproduce, since speck runs much longer than the
Semantic idle function. Here's a recipe:

- Put a bunch of text in the kill ring
- Create a new text buffer and activate speck
- Activate `display-time'
- At roughly XX:58, yank the text into the buffer and wait

Here's the backtrace from a `debug' call in `sit-for':

  sit-for(0)
  display-time-event-handler()
  apply(display-time-event-handler nil)
  byte-code("" [timer apply 5 6] 4)
  timer-event-handler([t 20997 5988 0 60 display-time-event-handler nil nil 0])
  accept-process-output(nil 0.01)
  speck-chunk()
  speck-multi-chunk()
  speck-chunks()
  speck-line()
  speck-window(t)
  speck-windows(t)
  apply(speck-windows t)
  byte-code("" [timer apply 5 6] 4)
  timer-event-handler([t 0 1 262453 nil speck-windows (t) idle 785000])

The fix would be to undo any narrowing and restore point before calling
`accept-process-output'.

-David

bug#15045: Point jumps inappropriately around time of Semantic lexing

[martin rudalics]

 > The main problem is that speck narrows the buffer before calling
 > `accept-process-output'. If that happens while the
 > time-display-event-handler runs, you suddenly see a narrowed buffer,
 > which is soon widened again. Pretty scary.

Very annoying, at least.

 > The fix would be to undo any narrowing and restore point before calling
 > `accept-process-output'.

Very inconvenient, at least.

Many thanks for the explanation and the investigative work, martin

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> What precisely is the bug in speck-mode?  Is it that it doesn't restore
>> `point' when input arrives?
> The main problem is that speck narrows the buffer before calling
> `accept-process-output'.

Yes, that's a bug in speck-mode.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

> If arbitrary timers can execute during the lexer's call to
> accept-process-output or input-pending-p, then doesn't that mean
> jit-lock-deferred-fontify can run too?

Yes.

> If removing timers' sit-for calls is the solution,

It's a workaround.

> then what's to become of jit-lock-deferred-fontify's call to sit-for?
> Doesn't deferred jit locking necessarily have to call redisplay?

Yes, tho I guess if absolutely needed, we could probably arrange for
jit-lock-deferred-fontify not to call sit-for.  I'm not completely sure
how that could work, but it doesn't sound impossible.


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: David Engster <deng@randomsample.de>, 15045@debbugs.gnu.org,
>         "Eric M. Ludlam" <eric@siege-engine.com>, Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 08 Aug 2013 17:26:50 -0400
> 
> > If arbitrary timers can execute during the lexer's call to
> > accept-process-output or input-pending-p, then doesn't that mean
> > jit-lock-deferred-fontify can run too?
> 
> Yes.

No, I don't think so, because idle timers don't run.

> > If removing timers' sit-for calls is the solution,
> 
> It's a workaround.

I think the right solution is to not call input-pending-p etc. during
lexer run.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Stefan Monnier]

>> > If arbitrary timers can execute during the lexer's call to
>> > accept-process-output or input-pending-p, then doesn't that mean
>> > jit-lock-deferred-fontify can run too?
>> Yes.
> No, I don't think so, because idle timers don't run.

You might be right, indeed.

>> > If removing timers' sit-for calls is the solution,
>> It's a workaround.
> I think the right solution is to not call input-pending-p etc. during
> lexer run.

Not calling accept-process-output might be tricky.
And not calling input-pending-p can also require very significant
code changes (I think support for concurrency would help a lot here).


        Stefan

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: gundaetiapo@gmail.com,  deng@randomsample.de,  15045@debbugs.gnu.org,  eric@siege-engine.com
> Date: Thu, 08 Aug 2013 18:50:42 -0400
> 
> > I think the right solution is to not call input-pending-p etc. during
> > lexer run.
> 
> Not calling accept-process-output might be tricky.
> And not calling input-pending-p can also require very significant
> code changes (I think support for concurrency would help a lot here).

Then perhaps the code which calls them should make sure to move point
to where the user expects it, before it does call those APIs.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eli Zaretskii]

> Date: Thu, 8 Aug 2013 16:03:30 -0400
> From: Barry OReilly <gundaetiapo@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 15045@debbugs.gnu.org, 
> 	"Eric M. Ludlam" <eric@siege-engine.com>, Eli Zaretskii <eliz@gnu.org>
> 
> If arbitrary timers can execute during the lexer's call to
> accept-process-output or input-pending-p, then doesn't that mean
> jit-lock-deferred-fontify can run too?

Not normally, because jit-lock-deferred-fontify runs off an idle
timer, and Emacs is not idle in the situation you describe.

bug#15045: Point jumps inappropriately around time of Semantic lexing

[Eric M. Ludlam]

On 08/08/2013 01:21 PM, David Engster wrote:
> [Adding Eric to CC]
>
> David Engster writes:
>
> It's kinda hard to trigger this problem through jit-lock, since its idle
> time is much smaller than the one from Semantic. So I disabled it and
> tried to trigger the jump by stopping typing at roughly XX:XX:59. The
> semantic idle function kicks in after 1 second, and lo and behold, I saw
> a jump. It's still difficult to reproduce, but I managed to get two
> backtraces in the past hour, which are attached.
>
> As you can see, the display-time-event-handler does indeed interrupt the
> lexing phase. It does a `sit-for', the display jumps. Not sure what
> happens after that. Does the semantic-idle function resume? Anyway,
> somehow point gets back to its original position, and through
> `trace-redisplay', I saw the following on stderr:

David, this is some impressing debugging.  Thanks for investigating so 
thoroughly.

> You might wonder how the display-time-event-handler can interrupt the
> Semantic lexer. In the two backtraces, you see that it calls
> `accept-process-output' and `input-pending-p'. This is hidden inside the
> macro `semantic-throw-on-input', which can be called in code wrapped
> inside `semantic-exit-on-input'; it's our poor-man's 'yield'. It's used
> extensively in the idle function code, and it's just there to do a
> non-local exit in case the user does something. However, now I know that
> it also allows other timers to run.
>
> If you look in the `define-lex' macro, you see that it calls
> `semantic-throw-on-input' after each identified token. The problem is
> that it does not restore the cursor position before that, so I guess the
> fix is simply to change this call to
>
> 	   (save-excursion
> 	     (goto-char starting-position)
> 	     (semantic-throw-on-input 'lex))

I pulled up some of the bigger C files in Emacs, ran the lexer, and they 
all took less than .3 second to lex.  It may also be safe to remove this 
interactive optimization and just depend on input checking that happens 
during parsing.

The detriment is that if someone had a really big file, they might 
notice that Emacs stops responding if lexing takes a long time.

Eric