On Wed, May 23, 2012 at 08:13:20AM +0200, Vincent van Ravesteijn wrote: > Op 23-5-2012 2:28, Pavel Sanda schreef: > >Vincent van Ravesteijn wrote: > >>commit 0a33374c0d8125e27666fe513506fb6069df453c > >>Author: Vincent van Ravesteijn<v...@lyx.org> > >>Date: Wed May 23 01:31:43 2012 +0200 > >> > >> Fix bug #8166: Crash on clicking away from empty paragraph > >> > >> We rely on the 'or' operator to prevent us from calling > >> 'notifyCursorLeaves' if one of the two cursors is broken. This doesn't > >> work when using the '|' operator. The compiler 'optimizes' the code in > >> such a way that we always call notifyCursorLeaves anyway. Using the > >> '||' > >> operator fixes this. > >My memory is not particularly good device, but IIRC > >notifyCursorLeavesOrEnters > >call should _never_ be optimized-out when using single |. > > Yes, you are right. The intention was to call all functions, but the > erroneous part was that the compiler may change the order of > execution when using '|'. > > >So what makes difference between Win X Linux noted in the bug would be the > >order > >of function calls (which is undefined and thus nonportable by definition). > On Windows it only crashed when compiling in release mode. > > >Currently you rewrite it as: > >if old.fixIfBroken() > > if cur.fixIfBroken() > > notifyCursorLeavesOrEnters(old, cur); > > > >Maybe cur.fixIfBroken() should be called anyway --> (fix|fix)||notify ? > > Yes I fixed it in such a way that we always call all three > functions, but making sure that the order is fixed. > > I didn't notice that this was a recent change by Enrico, and I > changed the logic indeed without good reason. Apologies to Enrico > for that. The current fix doesn't change the logic.
Yes, I purposely used "|" instead of "||" in order to avoid its short-circuiting behavior. However, I didn't take into account the fact that the compiler is allowed to reorder execution in case of "|". I can confirm that your latest fix gets it right. BTW. it is also needed in branch. -- Enrico
