Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Hi, Richard, Chong, Martin, and Steffan!

On Sun, Dec 10, 2006 at 03:30:49PM -0500, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> The slowdown [scrolling in very big files.c, e.g. xdisp.c] has been a
> big distraction for me.

> As I understand, in CC mode the o-p-i-c-0-i-d-s variable is only
> relevant for Emacs hackers.  Now:

> - If o-p-i-c-0-i-d-s is t, some Emacs source files may get
>   misfontified.  But as Martin has pointed out, we can tweak these
>   files to follow the o-p-i-c-0-i-d-s standard.  Thus no problem.

> - If o-p-i-c-0-i-d-s is nil, there's just no way to avoid a noticeable
>   slowdown in the bigger Emacs source file.

> So let's make o-p-i-c-0-i-d-s default to t for the release.

The right default for opic0ids in CC Mode is nil.  Please believe me on
this point.

> If someone later comes up with a clever c-beginning-of-defun function,
> we can switch it back.

There is already an exceptionally clever c-beginning-of-defun function.
Martin Stjernholm put a _lot_ of work into it.  It depends essentially on
beginning-of-defun working "correctly" (i.e. syntactically) when opic0ids
is set to nil.

> In any case, could someone check in Martin's optimization to
> c-beginning-of-defun-raw, or send me the patch with a changelog so
> that I can check it in?

Chong, especially: Please load xdisp.c with the following patch, and tell
me whether or not its slowness is still a problem.

Everybody: this patch is quite intricate, so could you please review it
and tell me if you find any bugs.


2006-12-13  Alan Mackenzie  <acm@muc.de>

	* emacs-lisp/lisp.el (beginning-of-defun-raw): Optimise (for
	speed) the case when open-paren-in-column-0-is-defun-start is nil.
	Based on code by Martin Rudalics.


Index: lisp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/lisp.el,v
retrieving revision 1.76
diff -c -r1.76 lisp.el
*** lisp.el	7 Dec 2006 04:47:47 -0000	1.76
--- lisp.el	13 Dec 2006 21:08:25 -0000
***************
*** 185,197 ****
  means move forward to Nth following beginning of defun.
  Returns t unless search stops due to beginning or end of buffer.
  
- Normally a defun starts when there is a char with open-parenthesis
- syntax at the beginning of a line.  If `defun-prompt-regexp' is
- non-nil, then a string which matches that regexp may precede the
- open-parenthesis, and point ends up at the beginning of the line.
- 
  If variable `beginning-of-defun-function' is non-nil, its value
! is called as a function to find the defun's beginning."
    (interactive "p")
    (or (not (eq this-command 'beginning-of-defun))
        (eq last-command 'beginning-of-defun)
--- 185,202 ----
  means move forward to Nth following beginning of defun.
  Returns t unless search stops due to beginning or end of buffer.
  
  If variable `beginning-of-defun-function' is non-nil, its value
! is called as a function to find the defun's beginning.
! 
! Normally a defun is assumed to start where there is a char with
! open-parenthesis syntax at the beginning of a line.  If
! `defun-prompt-regexp' is non-nil, then a string which matches
! that regexp may precede the open-parenthesis, and point ends up
! at the beginning of the line.
! 
! If `defun-prompt-regexp' and `open-paren-in-column-0-is-defun-start'
! are both nil, the function instead finds an open-paren at the
! outermost level."
    (interactive "p")
    (or (not (eq this-command 'beginning-of-defun))
        (eq last-command 'beginning-of-defun)
***************
*** 208,215 ****
  
  If variable `beginning-of-defun-function' is non-nil, its value
  is called as a function to find the defun's beginning."
!   (interactive "p") ; change this to "P", maybe, if we ever come to pass ARG
! 		    ; to beginning-of-defun-function.
    (unless arg (setq arg 1))		; The call might not be interactive.
    (cond
     (beginning-of-defun-function
--- 213,220 ----
  
  If variable `beginning-of-defun-function' is non-nil, its value
  is called as a function to find the defun's beginning."
!   (interactive "p")   ; change this to "P", maybe, if we ever come to pass ARG
! 					; to beginning-of-defun-function.
    (unless arg (setq arg 1))		; The call might not be interactive.
    (cond
     (beginning-of-defun-function
***************
*** 230,271 ****
  			     nil 'move arg)
  	 (progn (goto-char (1- (match-end 0)))) t))
  
-    (t
      ;; Column 0 has no significance - so scan forward from BOB to see how
      ;; nested point is, then carry on from there.
!     (let* ((floor (point-min))
! 	   (ceiling (point-max))
! 	   (pps-state (let (syntax-begin-function
! 			    font-lock-beginning-of-syntax-function)
! 			(syntax-ppss)))
! 	   (nesting-depth (nth 0 pps-state)))
        (save-restriction
  	(widen)
! 	;; Get outside of any string or comment.
! 	(if (nth 8 pps-state)
! 	    (goto-char (nth 8 pps-state)))
! 
! 	(cond
! 	 ((> arg 0)
! 	  (when (> nesting-depth 0)
! 	    (up-list (- nesting-depth))
! 	    (setq arg (1- arg)))
! 	  ;; We're now outside of any defun.
! 	  (backward-list arg)
! 	  (if (< (point) floor) (goto-char floor)))
! 
! 	 ((< arg 0)
! 	  (cond
! 	   ((> nesting-depth 0)
! 	    (up-list nesting-depth)
! 	    (setq arg (1+ arg)))
! 	   ((not (looking-at "\\s("))
! 	    ;; We're between defuns, and not at the start of one.
! 	    (setq arg (1+ arg))))
! 	  (forward-list (- arg))
! 	  (down-list)
! 	  (backward-char)
! 	  (if (> (point) ceiling) (goto-char ceiling)))))))))
  
  (defvar end-of-defun-function nil
    "If non-nil, function for function `end-of-defun' to call.
--- 235,284 ----
  			     nil 'move arg)
  	 (progn (goto-char (1- (match-end 0)))) t))
  
      ;; Column 0 has no significance - so scan forward from BOB to see how
      ;; nested point is, then carry on from there.
!    ((eq arg 0))
!    (t
!     (let ((floor (point-min))
! 	  (ceiling (point-max))
! 	  (arg-+ve (> arg 0)))
        (save-restriction
  	(widen)
! 	(let ((ppss (let (syntax-begin-function
! 			  font-lock-beginning-of-syntax-function)
! 		      (syntax-ppss)))
! 	      encl-pos)		  ; position of least enclosing paren, or nil.
! 	  ;; Back out of any comment/string, so that encl-pos will always
! 	  ;; become nil if we're at top-level.
! 	  (when (nth 8 ppss)
! 	    (goto-char (nth 8 ppss))
! 	    (setq ppss (syntax-ppss)))	; should be fast, due to cache.
! 	  (setq encl-pos (syntax-ppss-toplevel-pos ppss))
! 	  (if encl-pos (goto-char encl-pos))
! 
! 	  (if (and encl-pos arg-+ve)
! 	      (setq arg (1- arg)))
! 	  (if (and (not encl-pos) (not arg-+ve) (not (looking-at "\\s(")))
! 	      (setq arg (1+ arg)))
! 	    
! 	  (condition-case nil		; to catch there being crazy parens.
! 	      (progn
! 		(goto-char (scan-lists (point) (- arg) 0)) ; will work or trigger
! 					                   ; the condition-case.
! 		(if arg-+ve
! 		    (if (>= (point) floor)
! 			t
! 		      (goto-char floor)
! 		      nil)
! 		  (goto-char (1- (scan-lists (point) 1 -1))) ; forward to next (,
! 		  	                                     ; or trigger the c-c
! 		  (if (<= (point) ceiling)
! 		      t
! 		    (goto-char ceiling)
! 		    nil)))
! 	    (error
! 	     (goto-char (if arg-+ve floor ceiling))
! 	     nil))))))))
  
  (defvar end-of-defun-function nil
    "If non-nil, function for function `end-of-defun' to call.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

>> In any case, could someone check in Martin's optimization to
>> c-beginning-of-defun-raw, or send me the patch with a changelog so
>> that I can check it in?
>
> Chong, especially: Please load xdisp.c with the following patch, and tell
> me whether or not its slowness is still a problem.

Before the changes beginning-to-defun-raw, doing M-> to move to the
end of xdisp.c is instantaneous.

With the patch applied, M-> takes four seconds.  This is on a Pentium 4
1700MHz computer.

Is it really so bad to revert the changes for now?  If the only files
misfontified are Emacs source files, I don't think this "fix" is
necessary for Emacs 22---especially if the files in question can be
tweaked to avoid the bug.  (AFAIK, the issue of how to see this
problem was never discussed on this mailing list, but I've certainly
never come across it in my frequent perusal of the Emacs sources.)

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Good morning/evening, Chong!

On Wed, Dec 13, 2006 at 08:02:37PM -0500, Chong Yidong wrote:
> >> In any case, could someone check in Martin's optimization to
> >> c-beginning-of-defun-raw, or send me the patch with a changelog so
> >> that I can check it in?

> > Chong, especially: Please load xdisp.c with the following patch, and tell
> > me whether or not its slowness is still a problem.

> Before the changes beginning-to-defun-raw, doing M-> to move to the
> end of xdisp.c is instantaneous.

Hey, that's not fair!  When you raised this problem, you were
complaining about how slow a M-v was, after having got to EOF.  Is this
M-v now acceptably fast for you on your 1.7 GHz machine?

> With the patch applied, M-> takes four seconds.  This is on a Pentium 4
> 1700MHz computer.

It's worth pointing out that this is only the first time.  If you then
do M-< and another M->, these are then instantaneous.  If (some) cache
later gets disrupted, then the M-> would be a bit slow again.

Do you have any idea why M->, which is surely nothing more than
(goto-char (point-max)) is so slow?  I have an idea it is the filling of
CC Mode's cache.

> Is it really so bad to revert the changes for now?

Yes.

By "for now", you mean for Emacs 22.  If the changes are reverted "for
now", they'll stay reverted until the release of Emacs 23, sometime
around 2011.

> If the only files misfontified are Emacs source files, I don't think
> this "fix" is necessary for Emacs 22---especially if the files in
> question can be tweaked to avoid the bug.

It isn't only Emacs source files.  It happens a lot in normal users'
files.c.  There's a FAQ about it in the CC Mode manual.  After all,
having parentheses inside strings and comments in C is perfectly valid
and acceptable syntax, and it looks like a bug (which indeed it is), if
Emacs can't fontify such things properly.

xdisp.c, at ~750k, is an unusually large source file.  A 1.7 GHz machine
will become unusually old and slow during the lifetime of Emacs 22.

> (AFAIK, the issue of how to see this problem was never discussed on
> this mailing list, but I've certainly never come across it in my
> frequent perusal of the Emacs sources.)

The problem here is that CC Mode sets opic0ids to nil in a hard coded
fashion.  The solution must be that opic0ids in CC Mode should be nil by
default, yet give the user a way to set it to t.  How about the
following:

open-paren-in-column-0-is-defun-start should become a buffer-local
variable, with default value 'mode.  This symbol will mean "use the
major mode's default for the variable".  This will give the user the
flexibility she needs.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

beginning-of-defun's

   (and (beginning-of-defun-raw arg)
        (progn (beginning-of-line) t)))

might still spoil everything with a construct like

(foo
...) (bar ...)

and point after "bar".  Maybe you should try

   (and (prog1 (beginning-of-defun-raw arg)
	 (when open-paren-in-column-0-is-defun-start
	   (beginning-of-line)))))

instead.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 >>Before the changes beginning-to-defun-raw, doing M-> to move to the
 >>end of xdisp.c is instantaneous.
 >
 >
 > Hey, that's not fair!  When you raised this problem, you were
 > complaining about how slow a M-v was, after having got to EOF.  Is this
 > M-v now acceptably fast for you on your 1.7 GHz machine?
 >
 >
 >>With the patch applied, M-> takes four seconds.  This is on a Pentium 4
 >>1700MHz computer.
 >
 >
 > It's worth pointing out that this is only the first time.  If you then
 > do M-< and another M->, these are then instantaneous.  If (some) cache
 > later gets disrupted, then the M-> would be a bit slow again.
 >
 > Do you have any idea why M->, which is surely nothing more than
 > (goto-char (point-max)) is so slow?  I have an idea it is the filling of
 > CC Mode's cache.

M-> ran almost instantaneously before `beginning-of-defun-raw' changed.
And you did fill CC Mode's cache already before the change.  I could
imagine it's because font-locking runs `beginning-of-defun' repeatedly
for decreasing buffer positions and the `syntax-ppss' cache doesn't
handle these cases optimally.

 > It isn't only Emacs source files.  It happens a lot in normal users'
 > files.c.  There's a FAQ about it in the CC Mode manual.  After all,
 > having parentheses inside strings and comments in C is perfectly valid
 > and acceptable syntax, and it looks like a bug (which indeed it is), if
 > Emacs can't fontify such things properly.

That statement is not fair either: Nobody doubted the validity of
parentheses inside strings and comments.  The problem is due to
non-escaped parens in the leftmost column that do not start a defun.
Such parens do not follow GNU coding standards (Section 5.5.1, as of
November 15, 2006):

     It is important to put the open-brace that starts the body of a C
     function in column one, and avoid putting any other open-brace or
     open-parenthesis or open-bracket in column one. Several tools look
     for open-braces in column one to find the beginnings of C
     functions. These tools will not work on code not formatted that way.

If C mode were to weaken that rule we'd end up in a situation where

- more and more programmers put parens that do not start defuns in the
   leftmost column (after all the GNU editor doesn't complain and they
   already pay with their CPU time for that extra service), thus

- implicitly forcing the authors of other tools to abandon the rule as
   well.

If that's the intention, GNU should change the coding standard first.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

> xdisp.c, at ~750k, is an unusually large source file.  A 1.7 GHz machine
> will become unusually old and slow during the lifetime of Emacs 22.

There are plenty of people running Emacs on very old Unix boxes,
especially in academic environments.

>> If the only files misfontified are Emacs source files, I don't think
>> this "fix" is necessary for Emacs 22---especially if the files in
>> question can be tweaked to avoid the bug.
>
> It isn't only Emacs source files.  It happens a lot in normal users'
> files.c.  There's a FAQ about it in the CC Mode manual.  After all,
> having parentheses inside strings and comments in C is perfectly valid
> and acceptable syntax, and it looks like a bug (which indeed it is), if
> Emacs can't fontify such things properly.

If the current problems are already addressed in the FAQ, that should
be enough---especially if adhering to a de facto coding standard
prevents the bug.

With the fix, this FAQ entry would have to be replaced with another
FAQ entry: "Why is CC mode intolerably slow for big C files?"

>> Is it really so bad to revert the changes for now?
>
> Yes.
>
> By "for now", you mean for Emacs 22.  If the changes are reverted "for
> now", they'll stay reverted until the release of Emacs 23, sometime
> around 2011.

If someone can come up with a non-slow way to handle this, it could go
into emacs 22.2.  By the way, one of the reasons the Emacs release
process takes so long is the addition of big problematic changes like
this during pretest.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Grüßle Göttle, Martin!

On Thu, Dec 14, 2006 at 11:47:59AM +0100, martin rudalics wrote:
> >>Before the changes beginning-to-defun-raw, doing M-> to move to the
> >>end of xdisp.c is instantaneous.

[ .... ]

> > Do you have any idea why M->, which is surely nothing more than
> > (goto-char (point-max)) is so slow?  I have an idea it is the
> > filling of CC Mode's cache.
 
> M-> ran almost instantaneously before `beginning-of-defun-raw'
> changed.  And you did fill CC Mode's cache already before the change.
> I could imagine it's because font-locking runs `beginning-of-defun'
> repeatedly for decreasing buffer positions and the `syntax-ppss' cache
> doesn't handle these cases optimally.

You're probably right, there.

> > It isn't only Emacs source files.  It happens a lot in normal users'
> > files.c.  There's a FAQ about it in the CC Mode manual.  After all,
> > having parentheses inside strings and comments in C is perfectly
> > valid and acceptable syntax, and it looks like a bug (which indeed
> > it is), if Emacs can't fontify such things properly.

> That statement is not fair either: Nobody doubted the validity of
> parentheses inside strings and comments.  The problem is due to
> non-escaped parens in the leftmost column that do not start a defun.
> Such parens do not follow GNU coding standards (Section 5.5.1, as of
> November 15, 2006):

>     It is important to put the open-brace that starts the body of a C
>     function in column one, and avoid putting any other open-brace or
>     open-parenthesis or open-bracket in column one. Several tools look
>     for open-braces in column one to find the beginnings of C
>     functions. These tools will not work on code not formatted that way.

We're down to philosophy/politics, here.  Lots of CC Mode users,
probably most, are not working on GNU projects - Linux hackers, for
example, to say nothing of thousands (?hundreds of thousands) of
programmers working in their day jobs.  We shouldn't be ramming GNU
standards down their throats - they should have the freedom to accept or
ignore these.

In normal, carefree use, an open paren/brace in column 0 doesn't happen
that often.  A typical user won't notice when a M-q in a string puts a (
at column 0, so when the fontification goes awry, it's a big jolt, and
an indefinite time sink to sort out (or ignore) the problem.

I say we should GIVE THE USER THE CHOICE.  I have proposed a way of
doing this to which nobody's commented yet:  Have three values for the
variable with the furlong long name, namely (t nil mode).  t and nil
will carry on meaning what they mean, and the symbol 'mode, the default,
will mean "Major mode may set its own default here".

Comments, please!

> If C mode were to weaken that rule we'd end up in a situation where

> - more and more programmers put parens that do not start defuns in the
>   leftmost column (after all the GNU editor doesn't complain and they
>   already pay with their CPU time for that extra service), thus

They do that already.  Most of them neither know nor care about the GNU
rule.  I don't know of any program (aside from Obfuscated C entries)
where programmers knowingly put { or ( in C0 (apart from defun openers).
It's something that just happens, much as it did in syntax.c.

Now that we've got 1 to 4 GHz processors, (eq opic0ids nil) is a
reasonable default for C.  If we can just agree on a mechanism, I will
amend cc-mode.el to avoid forcing the sluggishness down people's
throats.  I actually feel quite guilty about that, believe it or not.

> - implicitly forcing the authors of other tools to abandon the rule as
>   well.

They will be experiencing the same hiccups as Emacs.  By the way, what
other tools?  ;-)

> If that's the intention, GNU should change the coding standard first.

I think that's a separate issue.  Maybe these other tools could now be
adapted to recognise parens inside strings and comments, now that
processors are fast.  Maybe not.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

On Thu, Dec 14, 2006 at 11:45:22AM +0100, martin rudalics wrote:
> beginning-of-defun's
> 
>   (and (beginning-of-defun-raw arg)
>        (progn (beginning-of-line) t)))
> 
> might still spoil everything with a construct like
> 
> (foo
> ...) (bar ...)
> 
> and point after "bar".  Maybe you should try
> 
>   (and (prog1 (beginning-of-defun-raw arg)
> 	 (when open-paren-in-column-0-is-defun-start
> 	   (beginning-of-line)))))
 
Or only move to BOL when there's only whitespace between BOL and (bar.
 
> instead.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> Now that we've got 1 to 4 GHz processors, (eq opic0ids nil) is a
> reasonable default for C.

No, it isn't.  We have clearly established that the current code is
_at_ _least_ quadratic in computation time requirements.  If some
medium size Emacs source file already causes editing to become painful
on 1 GHz processors, a file with corresponding features four times the
size of xdisp.c (and Emacs is a small application by today's
standards) will be too painful to edit on the hottest machines for
quite a few years to come.

I don't want to have Emacs reduced to be recommended only for small
files and small projects.  If we don't have an approach that scales,
it is not fit for a default setting.

And there are lot of hackers who are not using Windows at all, and
thus are not involved in the vicious circle of forced hardware
upgrades.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Chong Yidong <cyd@stupidchicken.com> writes:

>> xdisp.c, at ~750k, is an unusually large source file.  A 1.7 GHz machine
>> will become unusually old and slow during the lifetime of Emacs 22.
>
> There are plenty of people running Emacs on very old Unix boxes,
> especially in academic environments.

And a 750k source file will become unusually uncomplicated and small
during the lifetime of Emacs 22.

>> Yes.
>>
>> By "for now", you mean for Emacs 22.  If the changes are reverted "for
>> now", they'll stay reverted until the release of Emacs 23, sometime
>> around 2011.

If we assume that the code is to stay around until 2011, then we
really should not install a performance hog with at least quadratic
time behavior.

> If someone can come up with a non-slow way to handle this, it could
> go into emacs 22.2.  By the way, one of the reasons the Emacs
> release process takes so long is the addition of big problematic
> changes like this during pretest.

Very much so, yes.  This is not something that should ever have been
pushed in at pretest time.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> I say we should GIVE THE USER THE CHOICE.  I have proposed a way of
> doing this to which nobody's commented yet:  Have three values for the
> variable with the furlong long name, namely (t nil mode).  t and nil
> will carry on meaning what they mean, and the symbol 'mode, the default,
> will mean "Major mode may set its own default here".
>
> Comments, please!

The default should give the behavior before the beginning-of-defun-raw
change.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> I say we should GIVE THE USER THE CHOICE.  I have proposed a way of
> doing this to which nobody's commented yet:  Have three values for the
> variable with the furlong long name, namely (t nil mode).  t and nil
> will carry on meaning what they mean, and the symbol 'mode, the default,
> will mean "Major mode may set its own default here".

We should also introduce a new variable, call it
c-open-paren-in-column-0-is-defun-start, defaulting to t.

Furthermore, C-mode should define its own beginning-of-defun-function,
and the syntax-pps code that was inserted into beginning-of-defun-raw
should be relocated to that function.  Furthermore, the syntax-pps
code should not be the default behavior, taking effect only if
c-o-p-i-c-0-i-d-s is nil.

(As I and other people have pointed out, defaulting to nil leads to
unacceptable slowness for moderately big C files on even fast
machines.  People can set it to t if they are running on 1000000 Ghz
machines and want to put there parens where they please.)

OK?

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Hi, Chong!

On Thu, Dec 14, 2006 at 03:35:45PM -0500, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > I say we should GIVE THE USER THE CHOICE.  I have proposed a way of
> > doing this to which nobody's commented yet:  Have three values for
> > the variable with the furlong long name, namely (t nil mode).  t and
> > nil will carry on meaning what they mean, and the symbol 'mode, the
> > default, will mean "Major mode may set its own default here".

> We should also introduce a new variable, call it
> c-open-paren-in-column-0-is-defun-start, defaulting to t.

This approach was tried with c-require-final-newline.  It lead to a lot
of confusion (with require-final-newline) and even resentment.  A
consensus was reached that c-require-final-newline was a bad solution to
a problem.

What problems do you see with open-paren-in-column-0-is-defun-start
having the default 'mode?

> Furthermore, C-mode should define its own beginning-of-defun-function,
> and the syntax-pps code that was inserted into beginning-of-defun-raw
> should be relocated to that function.  Furthermore, the syntax-pps
> code should not be the default behavior, taking effect only if
> c-o-p-i-c-0-i-d-s is nil.

CC Mode has its own b-o-d-f, c-beginning-of-defun.  However, it relies
on a beginning-of-defun before it has filled its own structure cache.
The ppss version of b-o-d expresses the original intent of that
function, namely to find an outermost open-paren.  The Emacs core is
surely the right place for this, since it is a generally useful
functionality.

> (As I and other people have pointed out, defaulting to nil leads to
> unacceptable slowness for moderately big C files on even fast
> machines.  People can set it to t if they are running on 1000000 Ghz
> machines and want to put there parens where they please.)

Well, xdisp.c is a massive file, not a moderately big one, and I
wouldn't find the slowness unacceptable, though that's obviously a
personal thing.  If it were 15 seconds rather than 4, I would freak out
like you're doing now.  I would find massive misfontification (as
happened in syntax.c, the bug for which the pertinent patch is a repair)
totally unacceptable.

This is why I have suggested making the thing configurable.

Another possibility would be a variable like this in place of opic0ids:

(defvar threshold-ppss-bod 200000
  "The buffer size above which a paren in column0 will signal a defun start.
Below this size, a paren at the top level signals a defun start.
If this variable is nil, a paren at top level is always used.  If t, a
paren in column 0 is always used.")

What do you think?

With the patch I suggested last night, you still find an initial M-> in
xdisp.c too slow (~4 seconds on your 1.7 GHz PC).  That's a massively
large file.  How fast must this operation be before you'd find it
acceptable?  In emacs/src, there are files.c of all sizes, ranging from
pre-crt0.c (484 bytes) to xdisp.c (722032 bytes).  If David Kastrup's
conjectured "quadratic dependency" were true (I don't think it is), then
a "typical" sized file.c (say, eval.c at just over 100k) would take ~0.1
seconds for this.  If the dependency is linear, it would take ~0.5
seconds.

Please satisfy my curiosity, experiment with decreasing file sizes, and
tell me at what file size the reaction becomes too slow for you.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> What problems do you see with open-paren-in-column-0-is-defun-start
> having the default 'mode?

None---if the default behavior from setting this to 'mode is the
behavior before the beginning-of-defun-raw changes, i.e. M-> is
instantaneous for xdisp.c.  If the default behavior is the current
slowness, then this change alone doesn't solve anything.

> Well, xdisp.c is a massive file, not a moderately big one, and I
> wouldn't find the slowness unacceptable, though that's obviously a
> personal thing.  If it were 15 seconds rather than 4, I would freak out
> like you're doing now.  I would find massive misfontification (as
> happened in syntax.c, the bug for which the pertinent patch is a repair)
> totally unacceptable.

As has been pointed out, the font-lock limitations have existed for
years.  If you want to introduce a change that has been known to cause
problems for people (and please don't try to downplay these
problems---4 seconds is a long time for an interactive command), when
Emacs is in *pretest*, it makes sense to leave that change turned off
by default.

> Another possibility would be a variable like this in place of opic0ids:
>
> (defvar threshold-ppss-bod 200000
>   "The buffer size above which a paren in column0 will signal a defun start.
> Below this size, a paren at the top level signals a defun start.
> If this variable is nil, a paren at top level is always used.  If t, a
> paren in column 0 is always used.")
>
> What do you think?

Given the fact that Emacs is in pretest, changes to the Emacs core
should be discouraged if a more self-contained change can be used in
its place.  Since no one has described this bug as occurring outside
C-mode, we should postphone such changes till after the release.

That is why I think C-mode should introduce its own
beginning-of-defun-function, which can employ the buffer-size
threshold you described.  That change I can agree with.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Hi, Chong!

On Thu, Dec 14, 2006 at 12:29:18PM -0500, Chong Yidong wrote:
> > xdisp.c, at ~750k, is an unusually large source file.  A 1.7 GHz
> > machine will become unusually old and slow during the lifetime of
> > Emacs 22.

> There are plenty of people running Emacs on very old Unix boxes,
> especially in academic environments.

OK.  We need options here.

> >> If the only files misfontified are Emacs source files, I don't think
> >> this "fix" is necessary for Emacs 22---especially if the files in
> >> question can be tweaked to avoid the bug.

> > It isn't only Emacs source files.  It happens a lot in normal users'
> > files.c.  There's a FAQ about it in the CC Mode manual.  After all,
> > having parentheses inside strings and comments in C is perfectly
> > valid and acceptable syntax, and it looks like a bug (which indeed
> > it is), if Emacs can't fontify such things properly.

> If the current problems are already addressed in the FAQ, that should
> be enough---especially if adhering to a de facto coding standard
> prevents the bug.

It's not enough.  The misfontification is perceived by users as a bug.
And sorry, I made a mistake there - it's not in the FAQ.

> With the fix, this FAQ entry would have to be replaced with another
> FAQ entry: "Why is CC mode intolerably slow for big C files?"

Well, more like "CC mode is intolerably slow for big files, what can I
do about it?".

> >> Is it really so bad to revert the changes for now?

> > Yes.

> > By "for now", you mean for Emacs 22.  If the changes are reverted
> > "for now", they'll stay reverted until the release of Emacs 23,
> > sometime around 2011.
 
> If someone can come up with a non-slow way to handle this, it could go
> into emacs 22.2.  By the way, one of the reasons the Emacs release
> process takes so long is the addition of big problematic changes like
> this during pretest.

This change was made to fix a bug.  It has uncovered vagueness,
misunderstanding and missing functionality.  It is better that such
things are resolved before the release.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> With the patch I suggested last night, you still find an initial M->
> in xdisp.c too slow (~4 seconds on your 1.7 GHz PC).  That's a
> massively large file.  How fast must this operation be before you'd
> find it acceptable?  In emacs/src, there are files.c of all sizes,
> ranging from pre-crt0.c (484 bytes) to xdisp.c (722032 bytes).  If
> David Kastrup's conjectured "quadratic dependency" were true (I
> don't think it is), then a "typical" sized file.c (say, eval.c at
> just over 100k) would take ~0.1 seconds for this.  If the dependency
> is linear, it would take ~0.5 seconds.
>
> Please satisfy my curiosity, experiment with decreasing file sizes,
> and tell me at what file size the reaction becomes too slow for you.

The problem appears not quadratic on file size to me, but rather
quadratic on function size, or average fontification search size.  So
large automatically generated code passages would be candidates for
performance hits.

The large slowdown most likely would not be necessary if information
was reused intelligently and not scanned repeatedly.  That requires a
good analysis and careful implementation/reimplementation.

And the end of the pretest is the wrong time to start something like
this.  And it is also the wrong time to check in a half-baked solution
that makes for somewhat more pleasing results while causing large
performance hits.

The feature is not ready for Emacs 22.  And there is no useful plan
that would make it become so.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Richard Stallman]

I have now read the messages about this which arrived in the past week
or two, and checked the code again.

Basically, we have three different things in Emacs that
need to find, more or less, "the start of this defun".

* beginning-of-defun-raw.

* syntax.el

* find_defun_start in syntax.c

There are different features for customizing each of these three.
This is a very confusing situation.

The change I asked for, to make open-paren-in-column-0-is-defun-start
affect beginning-of-defun-raw, causes a major slowness.  4 seconds for
M-> is too painful.  So now I think that change should be undone.  But
just undoing it won't make everything ok.

The name open-paren-in-column-0-is-defun-start implies to me
that this variable ought to affect beginning-of-defun-raw.
After all, that command finds a defun start.

So if we make that variable apply only to syntax.c, as it used to,
perhaps we should rename it to open-paren-in-column-0-is-outer-level.

However, that name suggests that syntax.el ought to obey it too.

That makes me wonder whether find_defun_start should use
syntax-begin-function.  Does that work right?  If so, we could
eliminate open-paren-in-column-0-is-defun-start.  Stefan suggested
something similar: making back_comment use syntax-ppss.

How hard is that to implement?

Stefan also suggested we arrange another better way to
customize beginning-of-defun (and the parts of Font Lock that
highlight things that look like beginnings of defuns).

With those two changes, open-paren-in-column-0-is-defun-start
could be eliminated.

As for fixing the misfontifications of C mode, David Kastrup wrote:

    The large slowdown most likely would not be necessary if information
    was reused intelligently and not scanned repeatedly.  That requires a
    good analysis and careful implementation/reimplementation.

I agree with him that that is worth trying, but only after the
release.


A different but related question:

Should C mode do (setq beginning-of-defun-function 'c-beginning-of-defun)
Is there any reason that would be bad?

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Sean O'Rourke]

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

Richard Stallman <rms@gnu.org> writes:
> Should C mode do (setq beginning-of-defun-function
> 'c-beginning-of-defun) Is there any reason that would be bad?

As the code currently stands, this goes into infinite recursion
when c-beginning-of-defun calls beginning-of-defun (likewise for
end-of-defun{,-function}).  I've fixed this locally by
let-binding {beginning,end}-of-defun-function, which seems like a
reasonable solution (attached).

/s


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2127 bytes --]

Index: lisp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/lisp.el,v
retrieving revision 1.76
diff -u -r1.76 lisp.el
--- lisp.el	7 Dec 2006 04:47:47 -0000	1.76
+++ lisp.el	15 Dec 2006 06:22:10 -0000
@@ -208,17 +208,18 @@
 
 If variable `beginning-of-defun-function' is non-nil, its value
 is called as a function to find the defun's beginning."
-  (interactive "p") ; change this to "P", maybe, if we ever come to pass ARG
-		    ; to beginning-of-defun-function.
-  (unless arg (setq arg 1))		; The call might not be interactive.
+  (interactive "p")
+  (setq arg (or arg 1))
   (cond
-   (beginning-of-defun-function
-    (if (> arg 0)
-	(dotimes (i arg)
-	  (funcall beginning-of-defun-function))
-      ;; Better not call end-of-defun-function directly, in case
-      ;; it's not defined.
-      (end-of-defun (- arg))))
+    (beginning-of-defun-function
+      (let ((bodf beginning-of-defun-function)
+            beginning-of-defun-function)
+        (if (> (setq arg (or arg 1)) 0)
+            (dotimes (i arg)
+              (funcall bodf))
+            ;; Better not call end-of-defun-function directly, in case
+            ;; it's not defined.
+            (end-of-defun (- arg)))))
 
    ((or defun-prompt-regexp open-paren-in-column-0-is-defun-start)
     (and (< arg 0) (not (eobp)) (forward-char 1))
@@ -297,12 +298,14 @@
       (push-mark))
   (if (or (null arg) (= arg 0)) (setq arg 1))
   (if end-of-defun-function
-      (if (> arg 0)
-	  (dotimes (i arg)
-	    (funcall end-of-defun-function))
-	;; Better not call beginning-of-defun-function
-	;; directly, in case it's not defined.
-	(beginning-of-defun (- arg)))
+      (let ((eodf end-of-defun-function)
+            end-of-defun-function)
+        (if (> arg 0)
+            (dotimes (i arg)
+              (funcall eodf))
+            ;; Better not call beginning-of-defun-function
+            ;; directly, in case it's not defined.
+            (beginning-of-defun (- arg))))
     (let ((first t))
       (while (and (> arg 0) (< (point) (point-max)))
 	(let ((pos (point)))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

Good morning, Alan

 > In normal, carefree use, an open paren/brace in column 0 doesn't happen
 > that often.  A typical user won't notice when a M-q in a string puts a (
 > at column 0, so when the fontification goes awry, it's a big jolt, and
 > an indefinite time sink to sort out (or ignore) the problem.

At the time I noticed the bug it indeed took me some time to find its
cause.  That's why I convinced Richard to write a tiny patch which now
allows me to highlight offending parens in C mode appropriately.

 > I say we should GIVE THE USER THE CHOICE.  I have proposed a way of
 > doing this to which nobody's commented yet:  Have three values for the
 > variable with the furlong long name, namely (t nil mode).  t and nil
 > will carry on meaning what they mean, and the symbol 'mode, the default,
 > will mean "Major mode may set its own default here".

I don't mind if C mode sets this to nil by default.  I do mind, however,
if things break when I set this to t in my c-mode-hook.  Your earlier
remark that c-beginning-of-defun function "depends essentially on
beginning-of-defun working "correctly" (i.e. syntactically) when
opic0ids is set to nil" is not entirely reassuring in this respect.

Hence please tell me: Does `c-beginning-of-defun' work correctly when I
set `open-paren-in-column-0-is-defun-start' to t?  If you say it does, I
can (1) speed-up fontification of C buffers _and_ get information about
potential mis-fontification by setting

(set (make-local-variable 'open-paren-in-column-0-is-defun-start) t)
(put font-lock-beginning-of-syntax-function 'font-lock-syntax-paren-check t)

in my `c-mode-hook' and (2) get correct albeit slow fontification by not
doing so.

In general, we could make (1) the standard for users leaving alone the
default value of `open-paren-in-column-0-is-defun-start' and (2) the
standard for users who customized that to nil.  Until, eventually,
`open-paren-in-column-0-is-defun-start' is made obsolete.

 >>- more and more programmers put parens that do not start defuns in the
 >>  leftmost column (after all the GNU editor doesn't complain and they
 >>  already pay with their CPU time for that extra service), thus
 >
 >
 > They do that already.  Most of them neither know nor care about the GNU
 > rule.  I don't know of any program (aside from Obfuscated C entries)
 > where programmers knowingly put { or ( in C0 (apart from defun openers).
 > It's something that just happens, much as it did in syntax.c.

I should have said "less and less programmers care about not putting
parens that do not start defuns in the leftmost column" (the paren in
syntax.c is still there).

 > Now that we've got 1 to 4 GHz processors, (eq opic0ids nil) is a
 > reasonable default for C.  If we can just agree on a mechanism, I will
 > amend cc-mode.el to avoid forcing the sluggishness down people's
 > throats.  I actually feel quite guilty about that, believe it or not.

I already feel quite guilty for you feeling guilty.

 >>- implicitly forcing the authors of other tools to abandon the rule as
 >>  well.
 >
 >
 > They will be experiencing the same hiccups as Emacs.  By the way, what
 > other tools?  ;-)

That's what I wanted you to ask.  If the purpose of that entry in GNU
standards is to enforce a rule for Emacs users only and C mode does not
care about it ...

 >>If that's the intention, GNU should change the coding standard first.
 >
 >
 > I think that's a separate issue.  Maybe these other tools could now be
 > adapted to recognise parens inside strings and comments, now that
 > processors are fast.  Maybe not.

But you agree that, if tools are not able to do so easily, we should not
encourage people to write programs that do not follow the standard?

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Johan Bockgård]

Alan Mackenzie <acm@muc.de> writes:

> Please satisfy my curiosity, experiment with decreasing file sizes,
> and tell me at what file size the reaction becomes too slow for you.

Try scrolling around in lisp.h (119K).

-- 
Johan Bockgård

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

"Sean O'Rourke" <sorourke@cs.ucsd.edu> writes:

> Richard Stallman <rms@gnu.org> writes:
>> Should C mode do (setq beginning-of-defun-function
>> 'c-beginning-of-defun) Is there any reason that would be bad?
>
> As the code currently stands, this goes into infinite recursion
> when c-beginning-of-defun calls beginning-of-defun (likewise for
> end-of-defun{,-function}).  I've fixed this locally by
> let-binding {beginning,end}-of-defun-function, which seems like a
> reasonable solution (attached).

This binding should take place in CC-mode code; there's no reason to
expect all beginning-of-defun functions to run with
beginning-of-defun-function rebound to nil.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Sean O'Rourke]

Chong Yidong <cyd@stupidchicken.com> writes:
> This binding should take place in CC-mode code; there's no
> reason to expect all beginning-of-defun functions to run with
> beginning-of-defun-function rebound to nil.

I think this is a reasonable expectation: if the custom b-o-d-f
wants to call itself, it can do so directly, and b-o-d-f needs to
be bound to nil at some point anyways to avoid recursion.  It's
probably less error-prone to do it in just one place, rather than
letting each new foo-mode's foo-beginning-of-defun discover this
problem for itself.

/s

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

"Sean O'Rourke" <sorourke@cs.ucsd.edu> writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>> This binding should take place in CC-mode code; there's no
>> reason to expect all beginning-of-defun functions to run with
>> beginning-of-defun-function rebound to nil.
>
> I think this is a reasonable expectation: if the custom b-o-d-f
> wants to call itself, it can do so directly, and b-o-d-f needs to
> be bound to nil at some point anyways to avoid recursion.  It's
> probably less error-prone to do it in just one place, rather than
> letting each new foo-mode's foo-beginning-of-defun discover this
> problem for itself.

OTOH, there's no other place in Emacs that assumes that you should
bind foo-function to nil before calling foo-function.  That would be
considered surprising behavior.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> The name open-paren-in-column-0-is-defun-start implies to me
> that this variable ought to affect beginning-of-defun-raw.
> After all, that command finds a defun start.
>
> So if we make that variable apply only to syntax.c, as it used to,
> perhaps we should rename it to open-paren-in-column-0-is-outer-level.

It also applied to beginning-to-defun-raw when there is a non-nil
defun-prompt-regexp.  That's been the behavior for the last 6 years.

> That makes me wonder whether find_defun_start should use
> syntax-begin-function.  Does that work right?  If so, we could
> eliminate open-paren-in-column-0-is-defun-start.  Stefan suggested
> something similar: making back_comment use syntax-ppss.
>
> How hard is that to implement?
>
> Stefan also suggested we arrange another better way to
> customize beginning-of-defun (and the parts of Font Lock that
> highlight things that look like beginnings of defuns).
>
> With those two changes, open-paren-in-column-0-is-defun-start
> could be eliminated.

I don't see the immediate practical benefit of eliminating this
variable; we could simply alter its docstring to explicitly state what
effects it has, and maybe that making use of it is not encouraged.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Hi, Martin!

On Fri, Dec 15, 2006 at 08:32:53AM +0100, martin rudalics wrote:
> Good morning, Alan

> > In normal, carefree use, an open paren/brace in column 0 doesn't
> > happen that often.  A typical user won't notice when a M-q in a
> > string puts a ( at column 0, so when the fontification goes awry,
> > it's a big jolt, and an indefinite time sink to sort out (or ignore)
> > the problem.

> At the time I noticed the bug it indeed took me some time to find its
> cause.  That's why I convinced Richard to write a tiny patch which now
> allows me to highlight offending parens in C mode appropriately.
 
Yet we're Emacs experts.  I think we're placing too little importance on
the shock and emotional distress that such a bug [totally hosed
fontification over an entire screen] will cause an ordinary Emacs user.
Even highlighting these parens might well puzzle, rather than help, this
user.

[ .... ]

> I don't mind if C mode sets this to nil by default.  I do mind,
> however, if things break when I set this to t in my c-mode-hook.  Your
> earlier remark that c-beginning-of-defun function "depends essentially
> on beginning-of-defun working "correctly" (i.e. syntactically) when
> opic0ids is set to nil" is not entirely reassuring in this respect.

It wasn't a very bright thing for me to say.  I think I meant for a
fully general source file, including syntaxes.c.  For a file without a (
in C0, it shouldn't be important.  Sorry, and thanks for calling me on
that one.

> Hence please tell me: Does `c-beginning-of-defun' work correctly when
> I set `open-paren-in-column-0-is-defun-start' to t?  If you say it
> does, I can (1) speed-up fontification of C buffers _and_ get
> information about potential mis-fontification by setting

Yes, as long as its not syntax.c.

> (set (make-local-variable 'open-paren-in-column-0-is-defun-start) t)
> (put font-lock-beginning-of-syntax-function
> 'font-lock-syntax-paren-check t)

I'm a bit confused at the moment.  I'll pass on that one for now.

> in my `c-mode-hook' and (2) get correct albeit slow fontification by
> not doing so.

> In general, we could make (1) the standard for users leaving alone the
> default value of `open-paren-in-column-0-is-defun-start' and (2) the
> standard for users who customized that to nil.  Until, eventually,
> `open-paren-in-column-0-is-defun-start' is made obsolete.

"Until" ??

[ .... ]

> > Most of them neither know nor care about the GNU rule.  I don't know
> > of any program (aside from Obfuscated C entries) where programmers
> > knowingly put { or ( in C0 (apart from defun openers).  It's
> > something that just happens, much as it did in syntax.c.

> I should have said "less and less programmers care about not putting
> parens that do not start defuns in the leftmost column" (the paren in
> syntax.c is still there).

It's gone, now.  Still, that mere fact that it was there at all
demonstrates how difficult it is to avoid these things entirely.

[ .... ]

> If the purpose of that entry in GNU standards is to enforce a rule for
> Emacs users only and C mode does not care about it ...

I think the rule was pure optimisation - to allow Emacs to work at a
tolerable speed when processor speeds were measured in single or double
digits, the units being MHz.

[ .... ]

> But you agree that, if tools are not able to do so easily, we should
> not encourage people to write programs that do not follow the
> standard?

For some value of "encouragement".  ;-)  I would say, rather, we should
attempt not to penalize people into whose files a columnd 0 ( strays.

Maybe, as Chong has said, the time is not yet ripe.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stuart D. Herring]

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

>> Chong Yidong <cyd@stupidchicken.com> writes:
>>> This binding should take place in CC-mode code; there's no
>>> reason to expect all beginning-of-defun functions to run with
>>> beginning-of-defun-function rebound to nil.
>>
>> I think this is a reasonable expectation: if the custom b-o-d-f
>> wants to call itself, it can do so directly, and b-o-d-f needs to
>> be bound to nil at some point anyways to avoid recursion.  It's
>> probably less error-prone to do it in just one place, rather than
>> letting each new foo-mode's foo-beginning-of-defun discover this
>> problem for itself.
>
> OTOH, there's no other place in Emacs that assumes that you should
> bind foo-function to nil before calling foo-function.  That would be
> considered surprising behavior.

Most other places in Emacs aren't delegating a function's entire job, so
recursion isn't an issue.  If it's thought better, I've attached a patch
that sets up b-o-d-f, e-o-d-f, and f-s-f (forward-sexp-function) to be
bound to nil while their values are being called, and updates the
docstrings accordingly.  I've provided a ChangeLog below, although perhaps
a NEWS entry would be needed as well.  (There might also be other files
that could apply this technique, but I don't want to do too much if it
turns out to be undesired.)

Davis

2006-12-15  Stuart D. Herring  <herring@lanl.gov>

	* emacs-lisp/lisp.el (forward-sexp-function,
	beginning-of-defun-function, end-of-defun-function): Document
	rebinding during use.
	(forward-sexp): Bind `forward-sexp-function' to nil when calling
	it, and document this.
	(beginning-of-defun): Document rebinding of
	`beginning-of-defun-function' during use.
	(beginning-of-defun-raw): Bind `beginning-of-defun-function' to
	nil when calling it, and document this.
	(end-of-defun): Bind `end-of-defun-function' to nil when calling
	it, and document this.

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

[-- Attachment #2: lisp-function.patch --]
[-- Type: application/octet-stream, Size: 6741 bytes --]

*** lisp.el.~1.75.~	2006-11-15 10:08:27.000000000 -0700
--- lisp.el	2006-12-15 13:41:40.000000000 -0700
***************
*** 47,53 ****
  
  (defvar forward-sexp-function nil
    "If non-nil, `forward-sexp' delegates to this function.
! Should take the same arguments and behave similarly to `forward-sexp'.")
  
  (defun forward-sexp (&optional arg)
    "Move forward across one balanced expression (sexp).
--- 47,54 ----
  
  (defvar forward-sexp-function nil
    "If non-nil, `forward-sexp' delegates to this function.
! Should take the same arguments as and behave similarly to `forward-sexp'.
! This variable will be bound to nil when the function is called.")
  
  (defun forward-sexp (&optional arg)
    "Move forward across one balanced expression (sexp).
***************
*** 56,62 ****
    (interactive "p")
    (or arg (setq arg 1))
    (if forward-sexp-function
!       (funcall forward-sexp-function arg)
      (goto-char (or (scan-sexps (point) arg) (buffer-end arg)))
      (if (< arg 0) (backward-prefix-chars))))
  
--- 57,66 ----
    (interactive "p")
    (or arg (setq arg 1))
    (if forward-sexp-function
!       ;; Remember the value and then bind it to nil so that this function is
!       ;; usable from the function we call.
!       (let ((fsf forward-sexp-function) forward-sexp-function)
! 	(funcall fsf arg))
      (goto-char (or (scan-sexps (point) arg) (buffer-end arg)))
      (if (< arg 0) (backward-prefix-chars))))
  
***************
*** 177,183 ****
  
  The function (of no args) should go to the line on which the current
  defun starts, and return non-nil, or should return nil if it can't
! find the beginning.")
  
  (defun beginning-of-defun (&optional arg)
    "Move backward to the beginning of a defun.
--- 181,188 ----
  
  The function (of no args) should go to the line on which the current
  defun starts, and return non-nil, or should return nil if it can't
! find the beginning.  This variable will be bound to nil when the
! function is called.")
  
  (defun beginning-of-defun (&optional arg)
    "Move backward to the beginning of a defun.
***************
*** 190,197 ****
  non-nil, then a string which matches that regexp may precede the
  open-parenthesis, and point ends up at the beginning of the line.
  
! If variable `beginning-of-defun-function' is non-nil, its value
! is called as a function to find the defun's beginning."
    (interactive "p")
    (or (not (eq this-command 'beginning-of-defun))
        (eq last-command 'beginning-of-defun)
--- 195,203 ----
  non-nil, then a string which matches that regexp may precede the
  open-parenthesis, and point ends up at the beginning of the line.
  
! If variable `beginning-of-defun-function' is non-nil, its value is
! called as a function to find the defun's beginning.  The variable is
! bound to nil when the function is called."
    (interactive "p")
    (or (not (eq this-command 'beginning-of-defun))
        (eq last-command 'beginning-of-defun)
***************
*** 206,221 ****
  does not move to the beginning of the line when `defun-prompt-regexp'
  is non-nil.
  
! If variable `beginning-of-defun-function' is non-nil, its value
! is called as a function to find the defun's beginning."
    (interactive "p") ; change this to "P", maybe, if we ever come to pass ARG
  		    ; to beginning-of-defun-function.
    (unless arg (setq arg 1))		; The call might not be interactive.
    (cond
     (beginning-of-defun-function
      (if (> arg 0)
! 	(dotimes (i arg)
! 	  (funcall beginning-of-defun-function))
        ;; Better not call end-of-defun-function directly, in case
        ;; it's not defined.
        (end-of-defun (- arg))))
--- 212,230 ----
  does not move to the beginning of the line when `defun-prompt-regexp'
  is non-nil.
  
! If variable `beginning-of-defun-function' is non-nil, its value is
! called as a function to find the defun's beginning.  The variable is
! bound to nil when the function is called."
    (interactive "p") ; change this to "P", maybe, if we ever come to pass ARG
  		    ; to beginning-of-defun-function.
    (unless arg (setq arg 1))		; The call might not be interactive.
    (cond
     (beginning-of-defun-function
      (if (> arg 0)
! 	;; Remember the value and then bind it to nil so that this function is
! 	;; usable from the function we call.
! 	(let ((bodf beginning-of-defun-function) beginning-of-defun-function)
! 	  (dotimes (i arg) (funcall bodf))))
        ;; Better not call end-of-defun-function directly, in case
        ;; it's not defined.
        (end-of-defun (- arg))))
***************
*** 271,277 ****
    "If non-nil, function for function `end-of-defun' to call.
  This is used to find the end of the defun instead of using the normal
  recipe (see `end-of-defun').  Major modes can define this if the
! normal method is not appropriate.")
  
  (defun buffer-end (arg)
    "Return the \"far end\" position of the buffer, in direction ARG.
--- 280,287 ----
    "If non-nil, function for function `end-of-defun' to call.
  This is used to find the end of the defun instead of using the normal
  recipe (see `end-of-defun').  Major modes can define this if the
! normal method is not appropriate.
! This variable will be bound to nil when the function is called.")
  
  (defun buffer-end (arg)
    "Return the \"far end\" position of the buffer, in direction ARG.
***************
*** 289,295 ****
  `beginning-of-defun'.
  
  If variable `end-of-defun-function' is non-nil, its value
! is called as a function to find the defun's end."
    (interactive "p")
    (or (not (eq this-command 'end-of-defun))
        (eq last-command 'end-of-defun)
--- 299,306 ----
  `beginning-of-defun'.
  
  If variable `end-of-defun-function' is non-nil, its value
! is called as a function to find the defun's end.  The variable is
! bound to nil when the function is called."
    (interactive "p")
    (or (not (eq this-command 'end-of-defun))
        (eq last-command 'end-of-defun)
***************
*** 298,305 ****
    (if (or (null arg) (= arg 0)) (setq arg 1))
    (if end-of-defun-function
        (if (> arg 0)
! 	  (dotimes (i arg)
! 	    (funcall end-of-defun-function))
  	;; Better not call beginning-of-defun-function
  	;; directly, in case it's not defined.
  	(beginning-of-defun (- arg)))
--- 309,318 ----
    (if (or (null arg) (= arg 0)) (setq arg 1))
    (if end-of-defun-function
        (if (> arg 0)
! 	  ;; Remember the value and then bind it to nil so that this function
! 	  ;; is usable from the function we call.
! 	  (let ((eodf end-of-defun-function) end-of-defun-function)
! 	    (dotimes (i arg) (funcall eodf)))
  	;; Better not call beginning-of-defun-function
  	;; directly, in case it's not defined.
  	(beginning-of-defun (- arg)))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Richard Stallman]

    > Should C mode do (setq beginning-of-defun-function
    > 'c-beginning-of-defun) Is there any reason that would be bad?

    As the code currently stands, this goes into infinite recursion
    when c-beginning-of-defun calls beginning-of-defun (likewise for
    end-of-defun{,-function}).  I've fixed this locally by
    let-binding {beginning,end}-of-defun-function, which seems like a
    reasonable solution (attached).

Of course, this change requires implementing c-beginning-of-defun
appropriately for this new design.  The real question is whether it
has some other bad effect.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> Yet we're Emacs experts.  I think we're placing too little
> importance on the shock and emotional distress that such a bug
> [totally hosed fontification over an entire screen] will cause an
> ordinary Emacs user.  Even highlighting these parens might well
> puzzle, rather than help, this user.

Fontification is known not to be perfect, and there is an easy way
around if you feel shocked: change the input.  Such a change is
required for a number of other source code management tools like ctags
anyway.

This is a one-time effect.  In contrast, editing slowdowns are
permanent and incurable.

There are other modes, like LaTeX-mode, where the fontification engine
has no chance of figuring out everything correctly.  So one uses
simple hacks like

\def\xxx#1{\if$#1% $

where a comment is added that does nothing but close a $ math
environment that would otherwise be highlighted permanently.

People are _used_ to putting such stuff in if it annoys them too much.
It _is_ important, however, that there are easy workarounds.  For that
reason, AUCTeX matches dollar signs in line comments (one developer
made them ignored out of "correctness" concerns, and I vetoes this,
since it is impossible to get TeX fontification correct automatically,
anyway, and making the workarounds harder is a nuisance).

In the case of C-mode, there is a perfect workaround that requires no
undue amount of tap-dancing.

I don't see any reason to introduce little-tested, slow code that
already has lead to infinite loops and quite a bit of trouble at this
point of time in pretest.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Sean O'Rourke]

Richard Stallman <rms@gnu.org> writes:
> Of course, this change requires implementing c-beginning-of-defun
> appropriately for this new design.  The real question is whether it
> has some other bad effect.

I'm not sure about other bad effects, though I've been using my
change locally for months without trouble.  However, since
c-beginning-of-defun now seems to assume that
beginning-of-defun-function is *not* set, I don't think it will
require changes.

/s

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

> The right default for opic0ids in CC Mode is nil.  Please believe me on
> this point.

[ I agree, of course, since an open-paren or open-brace or open-bracket
  (almost?) never begins a defun in C. ]
Why is it so important?

>> If someone later comes up with a clever c-beginning-of-defun function,
>> we can switch it back.

> There is already an exceptionally clever c-beginning-of-defun function.
> Martin Stjernholm put a _lot_ of work into it.  It depends essentially on
> beginning-of-defun working "correctly" (i.e. syntactically) when opic0ids
> is set to nil.

Could you explain why it has to rely on beginning-of-defun working in
a particular way?  Can't you use syntax-ppss-toplevel-pos?
Is beginning-of-defun used indirectly via some other function over which you
have too little control?  If so which one?

I disagree that the behavior you want is the "correct" one.  It's not
necessarily wrong, but it has significant drawbacks and it seems that
syntax-ppss-toplevel-pos is preferable when you want that behavior.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

> Is it really so bad to revert the changes for now?  If the only files
> misfontified are Emacs source files, I don't think this "fix" is
> necessary for Emacs 22

The misfontification is surely not limited to Emacs source files.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

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

>> Is it really so bad to revert the changes for now?  If the only files
>> misfontified are Emacs source files, I don't think this "fix" is
>> necessary for Emacs 22
>
> The misfontification is surely not limited to Emacs source files.

And the performance problem is pretty much not limited to _any_ file
set.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

> M-> ran almost instantaneously before `beginning-of-defun-raw' changed.
> And you did fill CC Mode's cache already before the change.  I could
> imagine it's because font-locking runs `beginning-of-defun' repeatedly
> for decreasing buffer positions and the `syntax-ppss' cache doesn't
> handle these cases optimally.

I thought "no, can't be, 'cause I was careful to treat this correctly", so
I went and measured it to give a clear proof of it, and much to my surprise
you seemed to be right:

parse-partial-sexp on the whole buffer takes about 0.08 seconds on my laptop
(1.2GHz P-III M, maybe running at 800MHz at that time).  syntax-ppss on that
same buffer takes about 0.24s (about 3 times slower: not great, but seemed
possible), but

           (syntax-ppss (point-max))
           (syntax-ppss (- (point-max) 2000))
           (syntax-ppss (- (point-max) 4000))

takes a good 1.8s which doesn't make any sense.  After the shock (thinking,
god, I was wrong, how could that be?), I realized that these numbers didn't
make any sense.  And sure enough the problem was that in xdisp.c
syntax-begin-function is not nil but set to c-beginning-of-syntax which
doesn't make any sense: if you don't want to use a heuristic, then set the
damn thing to nil.

This variable is set to c-beginning-of-defun in cc-mode.el via
font-lock-beginning-of-syntax-function which is set via font-lock-defaults.

Try the patch below,


        Stefan


--- orig/lisp/progmodes/cc-mode.el
+++ mod/lisp/progmodes/cc-mode.el
@@ -597,7 +597,7 @@
 		       "font-lock-keywords-2" "font-lock-keywords-3")))
 	  nil nil
 	  ,c-identifier-syntax-modifications
-	  c-beginning-of-syntax
+	  nil
 	  (font-lock-lines-before . 1)
 	  (font-lock-mark-block-function
 	   . c-mark-function)))

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

> That makes me wonder whether find_defun_start should use
> syntax-begin-function.

Yes, that's what should happen.

> Does that work right?

I don't think we should try that at this stage.  It's post-22 AFAIC.

> If so, we could eliminate open-paren-in-column-0-is-defun-start.
> Stefan suggested something similar: making back_comment use syntax-ppss.

Actually what I intended to suggest is to use it in find_defun_start (which
is used by back_comment).

> How hard is that to implement?

I doubt it'll be a small local change.

> As for fixing the misfontifications of C mode, David Kastrup wrote:

>     The large slowdown most likely would not be necessary if information
>     was reused intelligently and not scanned repeatedly.  That requires a
>     good analysis and careful implementation/reimplementation.

I suspect that the patch I sent a minute ago which sets
font-lock-beginning-of-syntax-function to nil in cc-mode buffers will remove
a large part of the slowdown.

> Should C mode do (setq beginning-of-defun-function 'c-beginning-of-defun)
> Is there any reason that would be bad?

The only reason I could think of is if they want to use the default
beginning-of-defun in font-lock-beginning-of-syntax-function (and they don't
want to do that in cc-mode buffers) or when beginning-of-defun is misused.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 > ... I would say, rather, we should
 > attempt not to penalize people into whose files a columnd 0 ( strays.

One nice thing about the left-margin convention is that stray parens in
column zero have localized effect only.  When I have to rely on the
correctness of the entire syntax of my buffer in order to get reasonable
syntax-highlighting editing will get tedious: Suppose I deliberately
introduced some mis-balanced paren near the top of the buffer and to fix
that case I want to patch some code from the end of the buffer.  I would
feel penalized.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 >>M-> ran almost instantaneously before `beginning-of-defun-raw' changed.
 >>And you did fill CC Mode's cache already before the change.  I could
 >>imagine it's because font-locking runs `beginning-of-defun' repeatedly
 >>for decreasing buffer positions and the `syntax-ppss' cache doesn't
 >>handle these cases optimally.
 >
 >
 > I thought "no, can't be, 'cause I was careful to treat this correctly", so
 > I went and measured it to give a clear proof of it, and much to my surprise
 > you seemed to be right:
 >
 > parse-partial-sexp on the whole buffer takes about 0.08 seconds on my laptop
 > (1.2GHz P-III M, maybe running at 800MHz at that time).  syntax-ppss on that
 > same buffer takes about 0.24s (about 3 times slower: not great, but seemed
 > possible), but
 >
 >            (syntax-ppss (point-max))
 >            (syntax-ppss (- (point-max) 2000))
 >            (syntax-ppss (- (point-max) 4000))
 >
 > takes a good 1.8s which doesn't make any sense.

I got approximately that far when I wrote the lines above.

 > After the shock (thinking,
 > god, I was wrong, how could that be?), I realized that these numbers didn't
 > make any sense.

I simply felt puzzled and didn't dig any further.

 > And sure enough the problem was that in xdisp.c
 > syntax-begin-function is not nil but set to c-beginning-of-syntax which
 > doesn't make any sense: if you don't want to use a heuristic, then set the
 > damn thing to nil.
 >
 > This variable is set to c-beginning-of-defun in cc-mode.el via
 > font-lock-beginning-of-syntax-function which is set via font-lock-defaults.

Does that mean we got the `_c_-beginning-of-defun' behavior through the
back-door?  All that code existed before the open-paren-... change.

 > -	  c-beginning-of-syntax
 > +	  nil

Jumping to the end of xdisp.c still takes four seconds.  I continue to
feel puzzled.  Note that Alan binds `syntax-begin-function' to nil in
his `beginning-of-defun-raw' hence why should this have any impact in
the first place?  I must take another look in the basements of that.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

>>>M-> ran almost instantaneously before `beginning-of-defun-raw' changed.
>>>And you did fill CC Mode's cache already before the change.  I could
>>>imagine it's because font-locking runs `beginning-of-defun' repeatedly
>>>for decreasing buffer positions and the `syntax-ppss' cache doesn't
>>>handle these cases optimally.
>>
>> I thought "no, can't be, 'cause I was careful to treat this correctly", so
>> I went and measured it to give a clear proof of it, and much to my surprise
>> you seemed to be right:
>>
>> And sure enough the problem was that in xdisp.c
>> syntax-begin-function is not nil but set to c-beginning-of-syntax which
>> doesn't make any sense: if you don't want to use a heuristic, then set the
>> damn thing to nil.
>
> Jumping to the end of xdisp.c still takes four seconds.  I continue to
> feel puzzled.  Note that Alan binds `syntax-begin-function' to nil in
> his `beginning-of-defun-raw' hence why should this have any impact in
> the first place?  I must take another look in the basements of that.

There is indeed something fishy going on: xdisp.c has 722096
characters, and

(progn (goto-char 700000) (redisplay)) takes 0.25s
(progn (goto-char 720500) (redisplay)) takes 0.28s
(progn (goto-char 720522) (redisplay)) takes 0.26s
(progn (goto-char 720550) (redisplay)) takes 2.34s
(progn (goto-char 721000) (redisplay)) takes 2.24s
(progn (goto-char 722096) (redisplay)) takes 2.31s

Maybe a cache is overflowing somewhere.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

The slowdown comes from `open-paren-in-column-0-is-defun-start' being
nil when scanning lists backward.  You can try this with an Emacs before
Alan's change.  Open xdisp.c, put point after the braces in

void
syms_of_xdisp ()
{
...
}

and do a `backward-sexp'.  On my system it takes two seconds.  The
problem goes away when you set `open-paren-in-column-0-is-defun-start'
to t in C mode.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

To elaborate on my last mail: I think the bug is in back_comment and may
affect any mode setting `open-paren-in-column-0-is-defun-start' to nil.
My gdb is currently on strike, so could someone please try to debug
where back_comment spends its time when applying scan_lists to
syms_of_xdisp?  Maybe there's an easy solution.

Otherwise we'll have to set `open-paren-in-column-0-is-defun-start' to t
in C mode until someone fixes this.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

> To elaborate on my last mail: I think the bug is in back_comment and may
> affect any mode setting `open-paren-in-column-0-is-defun-start' to nil.
> My gdb is currently on strike, so could someone please try to debug
> where back_comment spends its time when applying scan_lists to
> syms_of_xdisp?  Maybe there's an easy solution.
>
> Otherwise we'll have to set `open-paren-in-column-0-is-defun-start' to t
> in C mode until someone fixes this.

It's the part in syntax.c:701 which says

 /* If we did not already find the defun start, find it now.  */
 if (defun_start == 0)
   {
     defun_start = find_defun_start (comment_end, comment_end_byte);
     defun_start_byte = find_start_value_byte;
   }

The trouble is that find_defun_start will scan from bob if
open_paren... is 0:

 static int
 find_defun_start (pos, pos_byte)
      int pos, pos_byte;
 {
   ...
   if (!open_paren_in_column_0_is_defun_start)
     {
       find_start_value_byte = BEGV_BYTE;
       return BEGV;
     }

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 > The trouble is that find_defun_start will scan from bob if
 > open_paren... is 0:

I see.  Does it do that for every single comment in `syms_of_xdisp' or
is there something particular which triggers it in certain comments
only?

Anyway, I'm afraid there's nothing we can do here at the moment.  I'm
convinced that this bug is responsible for C mode being slow over the
past year.  Eventually, Stefan will have to move syntax-ppss in here.
So far let's avoid setting `open-paren-in-column-0-is-defun-start' to
nil for _any_ mode.  Unless you have an idea ...

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

>> The trouble is that find_defun_start will scan from bob if
>> open_paren... is 0:
>
> I see.  Does it do that for every single comment in `syms_of_xdisp' or
> is there something particular which triggers it in certain comments
> only?

>From my understanding of the code, it will do it for every comment.
So this slowness is triggered by doing a backward scan_list over a
function which contains many comments and lives at the end of a long
file.

> Anyway, I'm afraid there's nothing we can do here at the moment.  I'm
> convinced that this bug is responsible for C mode being slow over the
> past year.  Eventually, Stefan will have to move syntax-ppss in here.
> So far let's avoid setting `open-paren-in-column-0-is-defun-start' to
> nil for _any_ mode.  Unless you have an idea ...

It might be possible to construct a "last known defun start position"
cache variable living in scan_list, so that back_comment can use that
instead of BOB.  But I don't know how desirable or general such a fix
is.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Chong Yidong <cyd@stupidchicken.com> writes:

> martin rudalics <rudalics@gmx.at> writes:
>
>> To elaborate on my last mail: I think the bug is in back_comment and may
>> affect any mode setting `open-paren-in-column-0-is-defun-start' to nil.
>> My gdb is currently on strike, so could someone please try to debug
>> where back_comment spends its time when applying scan_lists to
>> syms_of_xdisp?  Maybe there's an easy solution.
>>
>> Otherwise we'll have to set `open-paren-in-column-0-is-defun-start' to t
>> in C mode until someone fixes this.
>
> It's the part in syntax.c:701 which says
>
>  /* If we did not already find the defun start, find it now.  */
>  if (defun_start == 0)
>    {
>      defun_start = find_defun_start (comment_end, comment_end_byte);
>      defun_start_byte = find_start_value_byte;
>    }
>
> The trouble is that find_defun_start will scan from bob if
> open_paren... is 0:
>
>  static int
>  find_defun_start (pos, pos_byte)
>       int pos, pos_byte;
>  {
>    ...
>    if (!open_paren_in_column_0_is_defun_start)
>      {
>        find_start_value_byte = BEGV_BYTE;
>        return BEGV;
>      }

Well, it is not like an accident: this is defective by design.  Short
of having a column-0 heuristic, there is no way at all to find the
current nest level (and thus the function beginning) without counting
all braces from the start of the buffer.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

>> Anyway, I'm afraid there's nothing we can do here at the moment.  I'm
>> convinced that this bug is responsible for C mode being slow over the
>> past year.  Eventually, Stefan will have to move syntax-ppss in here.
>> So far let's avoid setting `open-paren-in-column-0-is-defun-start' to
>> nil for _any_ mode.  Unless you have an idea ...
>
> It might be possible to construct a "last known defun start position"
> cache variable living in scan_list, so that back_comment can use that
> instead of BOB.  But I don't know how desirable or general such a fix
> is.

This idea seems to work fairly well.  The patch to syntax.c is
attached; it adds an optional DEFUN_START argument to scan_lists which
is a position known to be outside any code structure.  The code in
beginning-of-defun-raw can be altered to use this as follows:

  (let* ((ppss (let (syntax-begin-function
                     font-lock-beginning-of-syntax-function)
                 (syntax-ppss)))
         (defun-start (nth 2 ppss))
         ...)
     ...
    (goto-char (scan-lists (point) (- arg) 0 defun-start))

With this additional change, (progn (goto-char (point-max)) (redisplay))
on xdisp.c takes 0.139s, down from 2.15s.

However M-> is still slow, because, for some reason, (recenter -3) at
the end of the buffer now takes about 2 seconds.  I don't know why.

However, I think all this should be post-22 work; for Emacs 22, let's
just set open-paren-in-column-0-is-defun-start to t, and leave the
existing beginning-of-defun-raw code there should anyone want to set
it to nil.

*** emacs/src/syntax.c.~1.203.~	2006-12-11 11:37:43.000000000 -0500
--- emacs/src/syntax.c	2006-12-16 15:56:24.000000000 -0500
***************
*** 98,107 ****
  
  static int find_defun_start P_ ((int, int));
  static int back_comment P_ ((EMACS_INT, EMACS_INT, EMACS_INT, int, int,
! 			     EMACS_INT *, EMACS_INT *));
  static int char_quoted P_ ((int, int));
  static Lisp_Object skip_chars P_ ((int, int, Lisp_Object, Lisp_Object, int));
! static Lisp_Object scan_lists P_ ((EMACS_INT, EMACS_INT, EMACS_INT, int));
  static void scan_sexps_forward P_ ((struct lisp_parse_state *,
  				    int, int, int, int,
  				    int, Lisp_Object, int));
--- 98,107 ----
  
  static int find_defun_start P_ ((int, int));
  static int back_comment P_ ((EMACS_INT, EMACS_INT, EMACS_INT, int, int,
! 			     EMACS_INT *, EMACS_INT *, int));
  static int char_quoted P_ ((int, int));
  static Lisp_Object skip_chars P_ ((int, int, Lisp_Object, Lisp_Object, int));
! static Lisp_Object scan_lists P_ ((EMACS_INT, EMACS_INT, EMACS_INT, int, int));
  static void scan_sexps_forward P_ ((struct lisp_parse_state *,
  				    int, int, int, int,
  				    int, Lisp_Object, int));
***************
*** 471,480 ****
     the returned value (or at FROM, if the search was not successful).  */
  
  static int
! back_comment (from, from_byte, stop, comnested, comstyle, charpos_ptr, bytepos_ptr)
       EMACS_INT from, from_byte, stop;
       int comnested, comstyle;
       EMACS_INT *charpos_ptr, *bytepos_ptr;
  {
    /* Look back, counting the parity of string-quotes,
       and recording the comment-starters seen.
--- 471,482 ----
     the returned value (or at FROM, if the search was not successful).  */
  
  static int
! back_comment (from, from_byte, stop, comnested, comstyle, charpos_ptr, bytepos_ptr,
! 	      defun_start)
       EMACS_INT from, from_byte, stop;
       int comnested, comstyle;
       EMACS_INT *charpos_ptr, *bytepos_ptr;
+      int defun_start;
  {
    /* Look back, counting the parity of string-quotes,
       and recording the comment-starters seen.
***************
*** 497,506 ****
    int comment_end_byte = from_byte;
    int comstart_pos = 0;
    int comstart_byte;
!   /* Place where the containing defun starts,
!      or 0 if we didn't come across it yet.  */
!   int defun_start = 0;
!   int defun_start_byte = 0;
    register enum syntaxcode code;
    int nesting = 1;		/* current comment nesting */
    int c;
--- 499,505 ----
    int comment_end_byte = from_byte;
    int comstart_pos = 0;
    int comstart_byte;
!   int defun_start_byte = defun_start ? CHAR_TO_BYTE (defun_start) : 0;
    register enum syntaxcode code;
    int nesting = 1;		/* current comment nesting */
    int c;
***************
*** 2153,2159 ****
  	  else if (code == Sendcomment)
  	    {
  	      found = back_comment (from, from_byte, stop, comnested, comstyle,
! 				    &out_charpos, &out_bytepos);
  	      if (found == -1)
  		{
  		  if (c == '\n')
--- 2152,2158 ----
  	  else if (code == Sendcomment)
  	    {
  	      found = back_comment (from, from_byte, stop, comnested, comstyle,
! 				    &out_charpos, &out_bytepos, 0);
  	      if (found == -1)
  		{
  		  if (c == '\n')
***************
*** 2204,2213 ****
     ? SYNTAX (c) : Ssymbol)
  
  static Lisp_Object
! scan_lists (from, count, depth, sexpflag)
       register EMACS_INT from;
       EMACS_INT count, depth;
!      int sexpflag;
  {
    Lisp_Object val;
    register EMACS_INT stop = count > 0 ? ZV : BEGV;
--- 2203,2212 ----
     ? SYNTAX (c) : Ssymbol)
  
  static Lisp_Object
! scan_lists (from, count, depth, sexpflag, defun_start)
       register EMACS_INT from;
       EMACS_INT count, depth;
!      int sexpflag, defun_start;
  {
    Lisp_Object val;
    register EMACS_INT stop = count > 0 ? ZV : BEGV;
***************
*** 2512,2518 ****
  	      if (!parse_sexp_ignore_comments)
  		break;
  	      found = back_comment (from, from_byte, stop, comnested, comstyle,
! 				    &out_charpos, &out_bytepos);
  	      /* FIXME:  if found == -1, then it really wasn't a comment-end.
  		 For single-char Sendcomment, we can't do much about it apart
  		 from skipping the char.
--- 2511,2517 ----
  	      if (!parse_sexp_ignore_comments)
  		break;
  	      found = back_comment (from, from_byte, stop, comnested, comstyle,
! 				    &out_charpos, &out_bytepos, defun_start);
  	      /* FIXME:  if found == -1, then it really wasn't a comment-end.
  		 For single-char Sendcomment, we can't do much about it apart
  		 from skipping the char.
***************
*** 2579,2585 ****
  	    make_number (last_good), make_number (from));
  }
  
! DEFUN ("scan-lists", Fscan_lists, Sscan_lists, 3, 3, 0,
         doc: /* Scan from character number FROM by COUNT lists.
  Returns the character number of the position thus found.
  
--- 2578,2584 ----
  	    make_number (last_good), make_number (from));
  }
  
! DEFUN ("scan-lists", Fscan_lists, Sscan_lists, 3, 4, 0,
         doc: /* Scan from character number FROM by COUNT lists.
  Returns the character number of the position thus found.
  
***************
*** 2593,2606 ****
  If the beginning or end of (the accessible part of) the buffer is reached
  and the depth is wrong, an error is signaled.
  If the depth is right but the count is not used up, nil is returned.  */)
!      (from, count, depth)
!      Lisp_Object from, count, depth;
  {
    CHECK_NUMBER (from);
    CHECK_NUMBER (count);
    CHECK_NUMBER (depth);
  
!   return scan_lists (XINT (from), XINT (count), XINT (depth), 0);
  }
  
  DEFUN ("scan-sexps", Fscan_sexps, Sscan_sexps, 2, 2, 0,
--- 2592,2611 ----
  If the beginning or end of (the accessible part of) the buffer is reached
  and the depth is wrong, an error is signaled.
  If the depth is right but the count is not used up, nil is returned.  */)
!      (from, count, depth, defun_start)
! Lisp_Object from, count, depth, defun_start;
  {
    CHECK_NUMBER (from);
    CHECK_NUMBER (count);
    CHECK_NUMBER (depth);
  
!   if (NILP (defun_start))
!     defun_start = make_number (0);
!   else
!     CHECK_NUMBER (defun_start);
! 
!   return scan_lists (XINT (from), XINT (count), XINT (depth), 0,
! 		     XINT (defun_start));
  }
  
  DEFUN ("scan-sexps", Fscan_sexps, Sscan_sexps, 2, 2, 0,
***************
*** 2620,2626 ****
    CHECK_NUMBER (from);
    CHECK_NUMBER (count);
  
!   return scan_lists (XINT (from), XINT (count), 0, 1);
  }
  
  DEFUN ("backward-prefix-chars", Fbackward_prefix_chars, Sbackward_prefix_chars,
--- 2625,2631 ----
    CHECK_NUMBER (from);
    CHECK_NUMBER (count);
  
!   return scan_lists (XINT (from), XINT (count), 0, 1, 0);
  }
  
  DEFUN ("backward-prefix-chars", Fbackward_prefix_chars, Sbackward_prefix_chars,

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 > It might be possible to construct a "last known defun start position"
 > cache variable living in scan_list, so that back_comment can use that
 > instead of BOB.  But I don't know how desirable or general such a fix
 > is.

Hmmm... It will work iff two assumptions apply:

(1) You're always within one and the same defun.  For a sequence of
short defuns each containing one comment it will fail because every
defun has a lower starting position.

(2) You have to invalidate the cache after each buffer change.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 >> static int
 >> find_defun_start (pos, pos_byte)
 >>      int pos, pos_byte;
 >> {
 >>   ...
 >>   if (!open_paren_in_column_0_is_defun_start)
 >>     {
 >>       find_start_value_byte = BEGV_BYTE;
 >>       return BEGV;
 >>     }
 >
 >
 > Well, it is not like an accident: this is defective by design.

Indeed.

 > Short
 > of having a column-0 heuristic, there is no way at all to find the
 > current nest level (and thus the function beginning) without counting
 > all braces from the start of the buffer.

We could, in principle, cache defun starts in a buffer and invalidate
the cached positions following a buffer change.  Stefan does that with
the syntax-ppss cache.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 > This idea seems to work fairly well.  The patch to syntax.c is
 > attached; it adds an optional DEFUN_START argument to scan_lists which
 > is a position known to be outside any code structure.  The code in
 > beginning-of-defun-raw can be altered to use this as follows:
 >
 >   (let* ((ppss (let (syntax-begin-function
 >                      font-lock-beginning-of-syntax-function)
 >                  (syntax-ppss)))
 >          (defun-start (nth 2 ppss))
 >          ...)
 >      ...
 >     (goto-char (scan-lists (point) (- arg) 0 defun-start))
 >
 > With this additional change, (progn (goto-char (point-max)) (redisplay))
 > on xdisp.c takes 0.139s, down from 2.15s.

This seems to prove that we have localized the bug.  The problem is,
however, that scan-lists is probably used in too many places and you
would reliably have to find the defun-start for each and every of them.

 > However M-> is still slow, because, for some reason, (recenter -3) at
 > the end of the buffer now takes about 2 seconds.  I don't know why.

Probably because somewhere scan-lists is called without the appropriate
defun-start.

 > However, I think all this should be post-22 work; for Emacs 22, let's
 > just set open-paren-in-column-0-is-defun-start to t, and leave the
 > existing beginning-of-defun-raw code there should anyone want to set
 > it to nil.

I agree.  More so, because, as I stated earlier, editing C code on my
system has become painstakingly slow for more than a year.  The recent
change to `beginning-of-defun-raw' was apparently just the straw
breaking the camel's back.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

>> The trouble is that find_defun_start will scan from bob if
>> open_paren... is 0:

> I see.  Does it do that for every single comment in `syms_of_xdisp' or
> is there something particular which triggers it in certain comments
> only?

The back_comment code tries to only use it when it can't detect the
beginning-of-comment on its own.  Problem is that in C modes this is the
case very often.  IIRC this is the case at least every time a /*..*/ comment
spans two or more lines (because it means it contains a \n which is
a comment marker for the other style of commenting, which makes it
particularly tricky to reliably figure out what's going on).

> convinced that this bug is responsible for C mode being slow over the
> past year.

Sounds credible.

> Eventually, Stefan will have to move syntax-ppss in here.

Anybody's free to take a crack at it.  But I really hope nobody tries to
port syntax-ppss to C code.  The saner approach is to make sure that
elisp code (in this case syntax-ppss) can be called from find_defun_start.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

> This idea seems to work fairly well.  The patch to syntax.c is
> attached; it adds an optional DEFUN_START argument to scan_lists which
> is a position known to be outside any code structure.  The code in
> beginning-of-defun-raw can be altered to use this as follows:

I still haven't seen any explanation for why this code needs to be in
beginning-of-defun-raw.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 > The saner approach is to make sure that
 > elisp code (in this case syntax-ppss) can be called from find_defun_start.

Actually, that's what I meant.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 >>This idea seems to work fairly well.  The patch to syntax.c is
 >>attached; it adds an optional DEFUN_START argument to scan_lists which
 >>is a position known to be outside any code structure.  The code in
 >>beginning-of-defun-raw can be altered to use this as follows:
 >
 >
 > I still haven't seen any explanation for why this code needs to be in
 > beginning-of-defun-raw.

Well, it doesn't harm either. Looking at cc-fonts.el we'd rather have to
investigate all call sequences à la

c-backward-syntactic-ws -> c-backward-sws -> c-backward-comments

which hardly seems feasible.  Does anyone know whether and how XEmacs
handles this?

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

>> I still haven't seen any explanation for why this code needs to be in
>> beginning-of-defun-raw.

> Well, it doesn't harm either.

Huh?  Have you looked at the beginning-of-defun-raw code before the change
and compared to the monster it has become?  All this (apparently) because
cc-mode wants to use syntax-ppss-toplevel-pos via this function rather than
calling it directly?

I want to remove that code and go back to what it was before, unless I can
find a *good* explanation for why it needs to be in beginning-of-defun-raw.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Chong Yidong]

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

>>> I still haven't seen any explanation for why this code needs to be in
>>> beginning-of-defun-raw.
>
>> Well, it doesn't harm either.
>
> Huh?  Have you looked at the beginning-of-defun-raw code before the change
> and compared to the monster it has become?  All this (apparently) because
> cc-mode wants to use syntax-ppss-toplevel-pos via this function rather than
> calling it directly?
>
> I want to remove that code and go back to what it was before, unless I can
> find a *good* explanation for why it needs to be in beginning-of-defun-raw.

I think one of the reasons it was put in there was because it doesn't
look like it belongs in cc-*.el either.  Since the code should
probably not be deleted outright (it seems to work OK for small files,
so individual users might want to turn it on), what about putting in a
function, call it "syntax-ppss-beginning-of-defun", in syntax.el?

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 >>>I still haven't seen any explanation for why this code needs to be in
 >>>beginning-of-defun-raw.
 >
 >
 >>Well, it doesn't harm either.
 >
 >
 > Huh?  Have you looked at the beginning-of-defun-raw code before the change
 > and compared to the monster it has become?

If you look at the various threads you will notice that I was the first
to fight the monster.

 > All this (apparently) because
 > cc-mode wants to use syntax-ppss-toplevel-pos via this function rather than
 > calling it directly?

Alan had a point because the old definition of `beginning-of-defun-raw'
was not entirely correct for `open-paren-in-column-0-is-defun-start' nil.

 > I want to remove that code and go back to what it was before, unless I can
 > find a *good* explanation for why it needs to be in beginning-of-defun-raw.

This won't resolve the problem that a `backward-sexp' may take two
seconds to execute.  _Before_ Alan's change.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

 >>I want to remove that code and go back to what it was before, unless I can
 >>find a *good* explanation for why it needs to be in beginning-of-defun-raw.
 >
 >
 > I think one of the reasons it was put in there was because it doesn't
 > look like it belongs in cc-*.el either.  Since the code should
 > probably not be deleted outright (it seems to work OK for small files,
 > so individual users might want to turn it on), what about putting in a
 > function, call it "syntax-ppss-beginning-of-defun", in syntax.el?

If we really wanted to do something about this we'd have to fix the
find_defun_start problem for `open-paren-in-column-0-is-defun-start'
equalling nil.  Everything else will be a non-issue after that.  M-v has
become unbearably slow in C mode.

Finding a good approximation for the find_defun_start problem is not
entirely trivial.  We'd have to define a `syntax-from-scratch' function
which repeatedly calls

(1) `parse-partial-sexp' on well (probably binomially) distributed
buffer positions, thereafter

(2) executes a second call from that position with a targetdepth zero
argument, before it finally

(3) executes a third call from the last position with a non-nil
stopbefore argument.

Essentially, we'd need something like

............. 1 .. 3 .........1 .. 3 ..... 1 .... 3 .. 1 . 3 .. 1 .. 3

where the "1"s correspond to the buffer positions where we asked
`parse-partial-sexp' to stop in the (1) steps, and the "3"s correspond
to the first `beginning-of-defun' found after that in a (3) step.  Next
we would have to cache the "3" positions and invalidate, in an
after-change-function, all positions following a buffer change.  And we
should reconcile this with syntax-ppss's cache to avoid duplicate work
(after all the "1" positions are similar to what syntax-ppss uses now).

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

martin rudalics <rudalics@gmx.at> writes:

> This won't resolve the problem that a `backward-sexp' may take two
> seconds to execute.  _Before_ Alan's change.

This is not as much of a problem as long as we expect backward-sexp to
only ever be called explicitly by the user, and as long as those "2
seconds" are not more than 2 seconds.

Font-locking, in contrast, works without user interaction.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Morning, Stefan!

On Sat, Dec 16, 2006 at 07:04:19PM -0500, Stefan Monnier wrote:
> >> I still haven't seen any explanation for why this code needs to be
> >> in beginning-of-defun-raw.
 
This isn't true, for any normal value of "needs".  I think it's more
accurate to say that you don't accept that the explanation that has been
given is a strong enough justification for the extra code in b-d-d-raw,
which is fair enough.
 
> > Well, it doesn't harm either.
 
> Huh?  Have you looked at the beginning-of-defun-raw code before the
> change and compared to the monster it has become?  All this
> (apparently) because cc-mode wants to use syntax-ppss-toplevel-pos via
> this function rather than calling it directly?
 
No, it's deeper than that.  It's to make the function match its
documentation (see "Left Margin Paren" in the Emacs manual).  It's to
make things work consistently when opic0ids is nil.  Everybody in this
discussion whose native language is English agrees on this point.  ;-)
To be explicit, here is the argument, which has already appeared in this
thread:

(i) The meaning of "beginning-of-defun" in a source file is, and always
has been, a paren at top level.  This proved too slow in early versions
of Emacs, and so the "column 0" heuristic was adopted.  This has been
documented in the Emacs manual since time immemorial; see page "Left
Margin Paren" in the Emacs 21 manual (i.e. before I made any changes to
it.  ;-)

(iii) This column 0 heuristic is just that - a heuristic, not a
definition of b-o-d.

(iv) The variable open-paren-in-column-0-is-defun-start was introduced
in the murky past, and there doesn't seem to be a record of why.  I
can't find any discussion of it in the emacs-devel archives.  However,
it's doc-string states: "Non-nil means an open paren in column 0 denotes
the start of a defun".  Otherwise put, "non-nil means that the column 0
heuristic applies".

(v) By pure logic, this MUST extend to "nil means the column 0 heuristic
does NOT apply".  Otherwise the variable would be meaningless.

(vi) When opic0ids is nil, b-o-d-raw must seek the beginning-of-defun
using the actual definition of b-o-d, namely a paren at top level.

That is the argument.  If you don't accept it, say so, and argue against
it.  But please stop saying that there is no argument.

> I want to remove that code and go back to what it was before, unless I
> can find a *good* explanation for why it needs to be in
> beginning-of-defun-raw.

Without that code, when opic0ids is nil, beginning-of-defun doesn't do
the right thing: it finds a column 0 parenthesis rather than one at top
level.  That has been the state up till now, and is _very_ confusing for
Elisp hackers.

That said, you and Martin R. have convinced me that setting opic0ids to
nil is the Wrong Thing to do in CC Mode.  Not only is it too sluggish
for big files, but bad parentheses early in a file.c would foul up the
fontification (and maybe even the syntactic analysis) of all defuns
after it.

I think we should strive to eliminate all this b-o-d confusion after
Emacs 22 has been released.

>         Stefan

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Hi, Martin!

On Sat, Dec 16, 2006 at 11:17:20AM +0100, martin rudalics wrote:
> > ... I would say, rather, we should
> > attempt not to penalize people into whose files a columnd 0 ( strays.
> 
> One nice thing about the left-margin convention is that stray parens in
> column zero have localized effect only.  When I have to rely on the
> correctness of the entire syntax of my buffer in order to get reasonable
> syntax-highlighting editing will get tedious: Suppose I deliberately
> introduced some mis-balanced paren near the top of the buffer and to fix
> that case I want to patch some code from the end of the buffer.  I would
> feel penalized.

OK, you've convinced me.  I now agree with you that CC Mode should not
itself set that horrible variable to nil, not even as a default.  I
should remove the offending line from cc-mode.el, and put some extra
explanation into cc-mode.texi and programs.texi.

Users may well want something like this in their c-mode-common-hook:

(if (< (- (point-max) (point-min)) 100000)
  (setq open-paren-in-column-0-is-defun-start nil))

, so the implemention of opic0ids code in beginning-of-defun-raw should
stay, IMAO.

Another possibility would be a new option in CC Mode (probably
implemented as a Clean-Up), which would guard against rogue (s and {s in
column 0.  Were this to be switched on, c-electric-paren,
c-electric-brace, and the filling routines would insert a \ at column 0
when needed.

The available evidence is that hackers cannot be expected to notice and
correct these things without help.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> Without that code, when opic0ids is nil, beginning-of-defun doesn't
> do the right thing: it finds a column 0 parenthesis rather than one
> at top level.  That has been the state up till now, and is _very_
> confusing for Elisp hackers.

Oh nonsense.  Whoever discovers this function will also discover its
DOC string.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> The available evidence is that hackers cannot be expected to notice
> and correct these things without help.

What evidence?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

On Sun, Dec 17, 2006 at 01:02:13PM +0100, David Kastrup wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > The available evidence is that hackers cannot be expected to notice
> > and correct these things without help.

> What evidence?

The existence (up until a few days ago) of the open paren in C0 in
syntax.c, of course.

If these things slip unnoticed past Emacs hackers for years, what hope
is there for ordinary programmers?

> David Kastrup, Kriemhildstr. 15, 44793 Bochum

-- 
Alan Mackenzie (Ittersbach, Germany)

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> On Sun, Dec 17, 2006 at 01:02:13PM +0100, David Kastrup wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > The available evidence is that hackers cannot be expected to notice
>> > and correct these things without help.
>
>> What evidence?
>
> The existence (up until a few days ago) of the open paren in C0 in
> syntax.c, of course.
>
> If these things slip unnoticed past Emacs hackers for years, what hope
> is there for ordinary programmers?

Do you want to imply that every program of yours immediately compiles
on first attempt?  If not, does that mean that hackers can't be
expected to understand C syntax?

One corrects syntax problems with the help of compiler feedback, and
one corrects the violation of indenting conventions with the feedback
of, for example, font-lock.  Now font-lock has not been the default
for long, and `beginning-of-defun' is not used so frequently as to
make editing a buffer die a horrible death (which is what happens with
syntax errors when compiling).

What you call "evidence" is, in my book, a very flimsy excuse for the
same.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Alan Mackenzie]

Hi, Mr. Kastrup,

On Sun, Dec 17, 2006 at 01:14:50PM +0100, David Kastrup wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > The existence (up until a few days ago) of the open paren in C0 in
> > syntax.c, of course.

> > If these things slip unnoticed past Emacs hackers for years, what hope
> > is there for ordinary programmers?

[ .... ]

> What you call "evidence" is, in my book, a very flimsy excuse for the
> same.

OK, I admit it.  I swallowd the bait, allowed myself to be set up as a
straw man, and now you've knocked me down.  Are you happy now?

Have a good 2007.

> David Kastrup, Kriemhildstr. 15, 44793 Bochum

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> Hi, Mr. Kastrup,
>
> On Sun, Dec 17, 2006 at 01:14:50PM +0100, David Kastrup wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > The existence (up until a few days ago) of the open paren in C0 in
>> > syntax.c, of course.
>
>> > If these things slip unnoticed past Emacs hackers for years, what hope
>> > is there for ordinary programmers?
>
> [ .... ]

Explanation conveniently snipped.

>> What you call "evidence" is, in my book, a very flimsy excuse for
>> the same.
>
> OK, I admit it.  I swallowd the bait, allowed myself to be set up as a
> straw man, and now you've knocked me down.  Are you happy now?

Ad hominem attacks don't make for a reasonable argument, either.  I'd
be happier if you did not feel the necessity to dilute your point with
hyperbole all the time.  It is tiresome to have to pick out the actual
facts and arguments from all that partly flippant verbiage that
carries across the notion that whoever disagrees with your assessment
must be wrong to start with.

You are trying to argue for a last-minute change far into the pretest,
one that has caused problems for people and does not seem easy to get
correct.  So we need really compelling reasons, and I have not seen
them, even though you act as if anybody with half a brain should
immediately recognize them.

I am all for good font-locking and movement commands, but one needs to
keep the associated cost in speed, stability, and release impact in
perspective and balance with the expected problems.  It is something
which I am trying to do in a manner I feel appropriate for a
conscientious developer.  I don't think that my different assessment
should single me out as a target for personal enmity.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[martin rudalics]

Hi Alan

 > OK, you've convinced me.  I now agree with you that CC Mode should not
 > itself set that horrible variable to nil, not even as a default.  I
 > should remove the offending line from cc-mode.el, and put some extra
 > explanation into cc-mode.texi and programs.texi.

Thank you.  I appreciate this as a wise and thoughtful step.

Here I want to apologize for accusing your `beginning-of-defun-raw'
change as the culprit of the misbehavior of C mode fontification over
the past weeks.  I really should have noticed the misbehavior before
when doing a `scroll-down', `backward-sexp', or `recenter' with a
negative argument.

I'd also kindly ask everyone to not argue against Alan's change any
more.  As far as I can tell from here, the misbehavior is caused by a
deficiency in back_comment when `open-paren-in-column-0-is-defun-start'
equals nil.  Alan's change simply revealed the misbehavior when jumping
to the end of a large buffer.

Using the default value for `open-paren-in-column-0-is-defun-start'
should make all operations in C mode sufficiently fast for the release.

 > Another possibility would be a new option in CC Mode (probably
 > implemented as a Clean-Up), which would guard against rogue (s and {s in
 > column 0.  Were this to be switched on, c-electric-paren,
 > c-electric-brace, and the filling routines would insert a \ at column 0
 > when needed.
 >
 > The available evidence is that hackers cannot be expected to notice and
 > correct these things without help.

Putting a `font-lock-warning-face' on such parens should be sufficient
for the moment.

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

>> All this (apparently) because cc-mode wants to use
>> syntax-ppss-toplevel-pos via this function rather than calling
>> it directly?

> Alan had a point because the old definition of `beginning-of-defun-raw'
> was not entirely correct for `open-paren-in-column-0-is-defun-start' nil.

The "not entirely correct" is only applicable in one particular
interpretation of the intent of beginning-of-defun and
open-paren-in-column-0-is-defun-start.  There's no good reason to choose
this interpretation.

>> I want to remove that code and go back to what it was before, unless I can
>> find a *good* explanation for why it needs to be in beginning-of-defun-raw.
> This won't resolve the problem that a `backward-sexp' may take two
> seconds to execute.  _Before_ Alan's change.

Indeed, it's unrelated.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

>> >> I still haven't seen any explanation for why this code needs to be
>> >> in beginning-of-defun-raw.
 
> This isn't true, for any normal value of "needs".  I think it's more
> accurate to say that you don't accept that the explanation that has been
> given is a strong enough justification for the extra code in b-d-d-raw,
> which is fair enough.

By "need" I meant "technical need" rather than a philosophical need to make
something work according to some interpretation of the doc.
So you seem to agree that it's not "needed" in the technical sense.
Good.
 
> No, it's deeper than that.  It's to make the function match its
> documentation (see "Left Margin Paren" in the Emacs manual).  It's to
> make things work consistently when opic0ids is nil.  Everybody in this
> discussion whose native language is English agrees on this point.  ;-)

I think it's a bit misleading: your original motivation was to get your code
working in cc-mode, not to fit the code to the doc.

> (i) The meaning of "beginning-of-defun" in a source file is, and always
> has been, a paren at top level.  This proved too slow in early versions
> of Emacs, and so the "column 0" heuristic was adopted.  This has been
> documented in the Emacs manual since time immemorial; see page "Left
> Margin Paren" in the Emacs 21 manual (i.e. before I made any changes to
> it.  ;-)

Other than the last historical paragraph, the emacs21 node only talks about
the fact that open-parens-in-column-0 should only be used when they mean
a beginning-of-defun.
I.e. open-paren-in-column-0-is-defun-start means "if you see a paren in
column-0, it's a defun" and not "all defuns should start at column-0".
So setting it to nil means "a paren in column-0 may be something else than
a defun" rather than "defuns may start in any column".

Furthermore beginning-of-defun is a command bound to C-M-a and thus
available in every major mode, so its meaning can't be nearly as limited as
you make it out to be since in many programming languages there is no
open-paren at the beginning of a defun.

> (iii) This column 0 heuristic is just that - a heuristic, not a
> definition of b-o-d.

Agreed.  Which is why I'd agree that b-o-d could be made to check
syntax-ppss *after finding a defun-like thingy* to check that it's not
inside a comment or string.

If you look at the current definition of b-o-d, you shoul dbe shocked at the
lack of consistency in its behavior: if defun-prompt-regexp is nil and
open-paren-in-column-0-is-defun-start is nil (i.e. in the new case you
added), then it works at the level of the parse tree, whereas for every
other setting it only looks at the level of the lex'd tokens (without paying
attention to any parsing state).  There's clearly something wrong there.

> (iv) The variable open-paren-in-column-0-is-defun-start was introduced
> in the murky past, and there doesn't seem to be a record of why.  I
> can't find any discussion of it in the emacs-devel archives.  However,
> it's doc-string states: "Non-nil means an open paren in column 0 denotes
> the start of a defun".

And the contrapositive of "Non-nil means an open paren in column 0 denotes
the start of a defun" is "nil means that an open paren in column 0 does not
necessarily denote the start of a defun".

> Otherwise put, "non-nil means that the column 0 heuristic applies".
> (v) By pure logic, this MUST extend to "nil means the column 0 heuristic
> does NOT apply".  Otherwise the variable would be meaningless.

Applying pure logic after adding a fuzzy (natural language) interpretation
doesn't seem very appealing to me.

> Without that code, when opic0ids is nil, beginning-of-defun doesn't do
> the right thing: it finds a column 0 parenthesis rather than one at top
> level.  That has been the state up till now, and is _very_ confusing for
> Elisp hackers.

Then let's fix it right for all uses (including those using
defun-prompt-regexp and beginning-of-defun-function) by checking syntax-ppss
after the fact and retrying if we find out we've been confused by some
defun-looking comment or string.


        Stefan

Re: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.

[Stefan Monnier]

> OK, you've convinced me.  I now agree with you that CC Mode should not
> itself set that horrible variable to nil, not even as a default.  I
> should remove the offending line from cc-mode.el, and put some extra
> explanation into cc-mode.texi and programs.texi.

> Users may well want something like this in their c-mode-common-hook:

> (if (< (- (point-max) (point-min)) 100000)
>   (setq open-paren-in-column-0-is-defun-start nil))

> , so the implemention of opic0ids code in beginning-of-defun-raw should
> stay, IMAO.

> Another possibility would be a new option in CC Mode (probably
> implemented as a Clean-Up), which would guard against rogue (s and {s in
> column 0.  Were this to be switched on, c-electric-paren,
> c-electric-brace, and the filling routines would insert a \ at column 0
> when needed.

If you set it back to non-nil, then please add back some font-lock rule to
highlight offending parens in comments and strings, like it used to be done
at some point (and is still done in elisp-mode).


        Stefan