Re: Mysterious fontification/C++ context issue

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

Hi everybody!

On Mon, 4 Dec 2006, martin rudalics wrote:

> > I see this too a lot, even in plain C files.  A backtrace when this
> > happen looks like this:
> >
> > Debugger entered--Lisp error: (scan-error "Containing expression ends
> > prematurely" 107612 107612)
> >   scan-lists(107699 -1 0)
> >   forward-list(-1)
> >   backward-list(1)
> >   beginning-of-defun-raw(nil)
> >   beginning-of-defun()
> >   c-get-fallback-start-pos(108117)
> >   c-parse-state()
> >   c-electric-semi&comma(nil)
> >   call-interactively(c-electric-semi&comma)
> >   call-last-kbd-macro(nil kmacro-loop-setup-function)
> >   kmacro-call-macro(nil nil)
> >   kmacro-end-and-call-macro(nil)
> >   call-interactively(kmacro-end-and-call-macro)
> >
> > Usually moving the cursor up a few lines, hitting tab to indent that
> > line, and then move back to the offending line cures it.

That bit of the CC Mode code is establishing a cache when there isn't 
already one, so the buggy behaviour won't be readily predictable or easily 
repeated.

> It's a consequence of the recent change to `beginning-of-defun-raw'.
> Compare the threads "font-locking and open parens in column 0" and
> "emacs hangs in jit-lock".

More precisely, it's a bug in that change.  The doc-string of 
`beginning-of-defun' states that point should just get left at BOB when 
there's no defun to go back over.

Here's a tentative fix for the bug, not that well tested, but tested a 
little bit:

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

 	* emacs-lisp/lisp.el (beginning-of-defun-raw): In the seeking a
 	non-nested open-paren case, check there are enough lists to move
 	over.  Return the correct value.

Index: lisp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/lisp.el,v
retrieving revision 1.75
diff -c -r1.75 lisp.el
*** lisp.el	8 Nov 2006 19:19:52 -0000	1.75
--- lisp.el	4 Dec 2006 20:59:34 -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
***************
*** 242,271 ****
         (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.
--- 247,288 ----
         (save-restriction
   	(widen)
   	;; Get outside of any string or comment.
! 	(if (and (nth 8 pps-state) (/= arg 0))
   	    (goto-char (nth 8 pps-state)))

! 	(condition-case nil ; to catch there being too few defuns to move over.
! 	    (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)
! 		  t
! 		(goto-char floor)
! 		nil))
! 
! 	     ((< 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)
! 		  t
! 		(goto-char ceiling)
! 		nil))
! 
! 	     (t))
! 	  (error (goto-char
! 		  (if (> arg 0) floor ceiling))
! 		 nil)))))))

   (defvar end-of-defun-function nil
     "If non-nil, function for function `end-of-defun' to call.

-- 
Alan Mackenzie, Ittersbach Germany
"Still having problems sending and receiving Email."

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

This change seems reasonable.  Would you please install it?

But we still face the question of whether to make another change in
the way C mode finds defuns, for efficiency.
That is an important question, I think.

Re: Mysterious fontification/C++ context issue

[martin rudalics]

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

 > Here's a tentative fix for the bug, not that well tested, but tested a
 > little bit:
 >
 > 2006-12-04  Alan Mackenzie  <acm@muc.de>
 >
 >     * emacs-lisp/lisp.el (beginning-of-defun-raw): In the seeking a
 >     non-nested open-paren case, check there are enough lists to move
 >     over.  Return the correct value.

Thanks for taking care of this.  Please note:

1. You don't widen the buffer before calling `syntax-ppss'.

2. A construct built along

(foo
) (bar)

with point at "bar" might derail font-lock (remember,
`c-beginning-of-defun-1' calls `beginning-of-defun', not
`beginning-of-defun-raw').

3. All this is still awfully slow and expensive: Open a larger buffer
like xdisp.c, jump to its end, hit M-v a couple of times, and look at
CPU consumption.

4. You could avoid the tedious `up-list' steps by using a variant of the
tentative fix I attached.

5. Consider making `open-paren-in-column-0-is-defun-start' a real user
option in C mode.  Let, for example, users decide whether they want to
respect GNU coding standards as mentioned in

http://lists.gnu.org/archive/html/bug-gnu-emacs/2006-11/msg00037.html

This should also permit binding C-M-a / C-M-e to `c-beginning-of-defun'
/ `c-end-of-defun' when `open-paren-in-column-0-is-defun-start' is nil.

6. In any case, the new behavior should be documented since it applies
whenever `open-paren-in-column-0-is-defun-start' is set to nil.

[-- Attachment #2: lisp.patch --]
[-- Type: text/plain, Size: 2375 bytes --]

*** lisp.el	Thu Nov  9 07:55:26 2006
--- lisp.el	Wed Dec  6 08:53:18 2006
***************
*** 229,271 ****
  			       "^\\s(")
  			     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.
--- 229,263 ----
  			       "^\\s(")
  			     nil 'move arg)
  	 (progn (goto-char (1- (match-end 0)))) t))
     (t
!     (let ((floor (point-min))
! 	  (ceiling (point-max)))
        (save-restriction
  	(widen)
! 	(let* ((ppss (let (syntax-begin-function
! 			   font-lock-beginning-of-syntax-function)
! 		       (syntax-ppss)))
! 	       (pos (or (let ((pos (car (nth 9 ppss))))
! 			  (when pos		; Within defun.
! 			    (when (> arg 0) (setq arg (1- arg)))
! 			    pos))
! 			(nth 8 ppss)		; Within string or comment.
! 			(point))))		; Must be at outermost level.
! 	  (goto-char pos)
! 	  (condition-case nil
! 	      (progn
! 		(forward-list (- arg))
! 		(cond
! 		 ((< pos floor)
! 		  (goto-char floor)
! 		  nil)
! 		 ((> pos ceiling)
! 		  (goto-char ceiling)
! 		  nil)
! 		 (t)))
! 	    (error
! 	     (goto-char (if (< arg 0) ceiling floor))
! 	     nil))))))))

  (defvar end-of-defun-function nil
    "If non-nil, function for function `end-of-defun' to call.

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

_______________________________________________
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug

Re: Mysterious fontification/C++ context issue

[Kim F. Storm]

martin rudalics <rudalics@gmx.at> writes:

> 3. All this is still awfully slow and expensive: Open a larger buffer
> like xdisp.c, jump to its end, hit M-v a couple of times, and look at
> CPU consumption.

I can second that.  Working on xdisp.c has become awfully slow lately.
We definitely need to fix this before the release!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Mysterious fontification/C++ context issue

[Chong Yidong]

storm@cua.dk (Kim F. Storm) writes:

>> 3. All this is still awfully slow and expensive: Open a larger buffer
>> like xdisp.c, jump to its end, hit M-v a couple of times, and look at
>> CPU consumption.
>
> I can second that.  Working on xdisp.c has become awfully slow lately.
> We definitely need to fix this before the release!

I'm third.

Could we set open-paren-in-column-0-is-defun-start to t for c mode?
This gives the old behavior, which, though incorrect in corner cases,
is at least usable.  Individual users can set it back to nil if
slow-but-correct fontification is desired (and it's irrelevant for
anyone but Emacs hackers anyway).

As for RMS's suggestion of writing a beginning-of-defun-function, I
don't see any dramatic optimizations that we could employ to make that
a solution for this (apart from, well, assuming that open parens in
column zero are defun starts :-).

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

    Could we set open-paren-in-column-0-is-defun-start to t for c mode?

If no one sees a better alternative, let's do that.  But let's wait
a few days and see what else is suggested.

Re: Mysterious fontification/C++ context issue

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     Could we set open-paren-in-column-0-is-defun-start to t for c mode?
>
> If no one sees a better alternative, let's do that.  But let's wait
> a few days and see what else is suggested.

It seems that no better ideas are forthcoming, so I think we should go
ahead and do this.

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>> Could we set open-paren-in-column-0-is-defun-start to t for c mode?
>> If no one sees a better alternative, let's do that.  But let's wait
>> a few days and see what else is suggested.

> It seems that no better ideas are forthcoming, so I think we should go
> ahead and do this.

Could we revert the change to beginning-of-defun instead?


        Stefan

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

    >> Could we set open-paren-in-column-0-is-defun-start to t for c mode?
    >> If no one sees a better alternative, let's do that.  But let's wait
    >> a few days and see what else is suggested.

    > It seems that no better ideas are forthcoming, so I think we should go
    > ahead and do this.

    Could we revert the change to beginning-of-defun instead?

It seems clearly incorrect for beginning-of-defun to ignore
open-paren-in-column-0-is-defun-start.

For what other reason does C mode set open-paren-in-column-0-is-defun-start?

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>>> Could we set open-paren-in-column-0-is-defun-start to t for c mode?
>>> If no one sees a better alternative, let's do that.  But let's wait
>>> a few days and see what else is suggested.

>> It seems that no better ideas are forthcoming, so I think we should go
>> ahead and do this.

>     Could we revert the change to beginning-of-defun instead?

> It seems clearly incorrect for beginning-of-defun to ignore
> open-paren-in-column-0-is-defun-start.

Not to me.

> For what other reason does C mode set open-paren-in-column-0-is-defun-start?

Don't know.
So that forward-comment (when going backward) doesn't get confused by
a stray open-paren in column 0?

Or because it misunderstood what open-paren-in-column-0-is-defun-start
is for?


        Stefan


PS: AFAIK open-paren-in-column-0-is-defun-start should be marked
    as obsolete.

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

[ Expanding on my previous message ]

>     Could we revert the change to beginning-of-defun instead?
> It seems clearly incorrect for beginning-of-defun to ignore
> open-paren-in-column-0-is-defun-start.

[ The change I'd like to revert only influences the case where
  defun-prompt-regexp is nil so the non-nil case is implicitly outside of
  this discussion]

Does it?

The docstring doesn't mention anything about it.  And really, when
defun-prompt-regexp is nil, then beginning-of-defun considers that "defun
starts when there is a char with open-parenthesis syntax at the beginning of
a line".  So if open-paren-in-column-0-is-defun-start is nil, it's hard to
figure out what should be considered as a defun start.

It's kind of like saying "I won't tell you what it is, I'll just tell you
that it's not what you think".

Note that open-paren-in-column-0-is-defun-start is only used in back_comment
in syntax.c (where we should remove it by making back_comment use
syntax-ppss) and in beginning-of-defun where it was only used to add the
empty string to defun-prompt-regexp.

> For what other reason does C mode set open-paren-in-column-0-is-defun-start?

Maybe because in C defuns do not start with a paren (neither in column 0 nor
elsewhere)?

Or maybe because Alan uses beginning-of-defun for something for which it was
not designed, and thus ends up having to work around beginning-of-defun's
functionality by first setting open-paren-in-column-0-is-defun-start and
defun-prompt-regexp to some usually unused combination and then putting the
code he really wants to use in beginning-of-defun (whereas he should just
use this code directly without going through beginning-of-defun)?
[ I know this sounds like I'm accusing Alan of plotting.  I'm not.  ]


        Stefan

Re: Mysterious fontification/C++ context issue

[Chong Yidong]

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

> The docstring doesn't mention anything about it.  And really, when
> defun-prompt-regexp is nil, then beginning-of-defun considers that "defun
> starts when there is a char with open-parenthesis syntax at the beginning of
> a line".  So if open-paren-in-column-0-is-defun-start is nil, it's hard to
> figure out what should be considered as a defun start.
>
> It's kind of like saying "I won't tell you what it is, I'll just tell you
> that it's not what you think".

We could explicitly document that this situation (defun-prompt-regexp
and o-p-i-c-0-i-d-s both nil) means "figure it out using
syntactically", i.e. using ppss as Alan implemented.

We have to bring this to a resolution; intricate changes can be
postphone till after the release.  I recommend the following:

* Updating the relevant docstrings to say what defun-prompt-regexp and
  o-p-i-c-0-i-d-s both nil means.

* Updating the Elisp manual, and mentioning that such a combination
  can be slow.

* Checking in Martin's optimization to Alan's code.

* Setting o-p-i-c-0-i-d-s to t for cc mode.

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>> The docstring doesn't mention anything about it.  And really, when
>> defun-prompt-regexp is nil, then beginning-of-defun considers that "defun
>> starts when there is a char with open-parenthesis syntax at the beginning of
>> a line".  So if open-paren-in-column-0-is-defun-start is nil, it's hard to
>> figure out what should be considered as a defun start.
>> 
>> It's kind of like saying "I won't tell you what it is, I'll just tell you
>> that it's not what you think".

> We could explicitly document that this situation (defun-prompt-regexp
> and o-p-i-c-0-i-d-s both nil) means "figure it out using
> syntactically", i.e. using ppss as Alan implemented.

We could, but I'm very much not in favor since it makes o-p-i-c-0-i-d-s more
important than it deserves and it's very unclear that this behavior is of
any use to beginning-of-defun.  It seems to only be "useful" in the case
where you actually wanted to use syntax-ppss and
syntax-ppss-toplevel-pos instead.

> * Updating the relevant docstrings to say what defun-prompt-regexp and
>   o-p-i-c-0-i-d-s both nil means.

I think the current docstring is quite clear.

> * Updating the Elisp manual, and mentioning that such a combination
>   can be slow.

> * Checking in Martin's optimization to Alan's code.

> * Setting o-p-i-c-0-i-d-s to t for cc mode.

* revert the change and live with the resulting occasional problems for now
  (problems which have existed for ever anyway).


-- Stefan

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

    It seems that no better ideas are forthcoming, so I think we should go
    ahead and do this.

Please do it; but then we need to see if that change makes
some other change necessary.

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

Hi, Richard!

On Wed, Dec 06, 2006 at 11:59:26PM -0500, Richard Stallman wrote:
>     Could we set open-paren-in-column-0-is-defun-start to t for c mode?

This would nullify the fix for the bug reported by Martin a few weeks
ago, namely: Load syntax.c and immediately jump forward to a certain
function.  The fontification was seriously wrong.

> If no one sees a better alternative, let's do that.  But let's wait
> a few days and see what else is suggested.

Currently CC Mode sets open-paren-in-column-0-is-defun-start
unconditionally to nil.  I now think this isn't necessarily a good idea.

The optimal setting of opic0ids depends on what your source file looks
like:

(i) (eq opic0ids nil) is good for small to medium sized source files,
(probably) for files with hanging defun braces, and for files (like
syntax.c) with syntactical anomalies.

(ii) (eq opic0ids t) is good for large source files with defun braces in
column 0.
 
I think I now agree with Martin R: opic0ids should become a fully user
settable option, and CC Mode should respect it.  However, I think it
needs more than two values, t and nil; it should have three values,
"definitely nil", "definitely t" and "don't really care".  The last of
these would be an instruction to the major mode to use a mode-dependent
default value, the latter defaulting to non-nil.  Should it not also
become a buffer local variable?

I'm not 100% sure what effect this would have on CC Mode, but it'll be
something like this: c-beginning-of-defun will continue to look for an
unnested {, but with (eq opic0ids t), its results will be probabalistic
rather than rigorous.  But it worked well enough before, when
beginning-of-defun was a bit squidgy.

I'll need to amend cc-mode.texi.

I think the CC Mode default for opic0ids should be nil, and a typical
Emacs hacker's .emacs would contain something like this:

(setq open-paren-in-column-0-is-default-start
  (and (not equal (filename-nondirectory
                   (filename-sans-extension (buffer-file-name)))
		  "syntax.c")
       (> (buffer-size) 200000)))

What do people think?

-- 
Alan.

Re: Mysterious fontification/C++ context issue

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> I think I now agree with Martin R: opic0ids should become a fully user
> settable option, and CC Mode should respect it.  However, I think it
> needs more than two values, t and nil; it should have three values,
> "definitely nil", "definitely t" and "don't really care".  The last of
> these would be an instruction to the major mode to use a mode-dependent
> default value, the latter defaulting to non-nil.  Should it not also
> become a buffer local variable?

If there is bad fontification for only one file in Emacs, out of the
universe of C files, why not set open-paren-in-column-0-is-defun-start
to be nil for that file, using a file-local variable?

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

Hi, Chong!

(By the way, apologies if I should be calling you "Yidong".  Please tell
me if I've made a mistake.)

On Sat, Dec 09, 2006 at 08:11:11PM -0500, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > I think I now agree with Martin R: opic0ids should become a fully user
> > settable option, and CC Mode should respect it.  However, I think it
> > needs more than two values, t and nil; it should have three values,
> > "definitely nil", "definitely t" and "don't really care".  The last of
> > these would be an instruction to the major mode to use a
> > mode-dependent default value, the latter defaulting to non-nil.
> > Should it not also become a buffer local variable?

> If there is bad fontification for only one file in Emacs, out of the
> universe of C files, why not set open-paren-in-column-0-is-defun-start
> to be nil for that file, using a file-local variable?

There will be bad fontification in lots of files.  Setting opic0ids to
nil makes Emacs analyse a CC Mode buffer rigorously, correctly and a bit
slowly.  Setting it to t does a rough and ready job which sometimes gives
wierd results.

With Martin Rudalics's optimisation to c-beginning-of-defun-raw, I don't
feel that the slowness is too bad any more (though he does ;-).  In the
extreme case xdisp.c, scrolling to EOB and M-v is mildly sluggish (about
half a second), on my 5 year old Athlon 1.2 GHz machine.  A typical new
PC now is, say, 3 GHZ.  In the time Emacs 22 is the current release, a
typical new PC will come to be around 20 GHz, and this slowness will not
matter.

-- 
Alan.

Re: Mysterious fontification/C++ context issue

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> With Martin Rudalics's optimisation to c-beginning-of-defun-raw, I
> don't feel that the slowness is too bad any more (though he does
> ;-).  In the extreme case xdisp.c, scrolling to EOB and M-v is
> mildly sluggish (about half a second), on my 5 year old Athlon 1.2
> GHz machine.  A typical new PC now is, say, 3 GHZ.  In the time
> Emacs 22 is the current release, a typical new PC will come to be
> around 20 GHz, and this slowness will not matter.

Please, arguments like that are just not acceptable.  If examples with
barely tolerable behavior can be come across easily, you can bet that
there will be normal use cases where the the behavior will be quite
intolerable.  And even on faster machines, editing should try avoiding
draining unnecessary CPU power: editing is by far the most important
application where you want to have a long battery life for laptops.

I am working with a 600MHz laptop, and my occasionally used desktop
system has a 233MHz processor.  The preview-latex
<URL:http://www.gnu.org/software/auctex/preview-latex.html> package
has been streamlined to operate well on such machines.  It would be
really a bad hoax if syntax highlighting (which does a lot less) would
make such machines unusable for editing with an up-to-date Emacs with
default settings.

When stuff becomes sluggish on a machine of _your_ power, that most
likely implies an O(n^2) or worse behavior.  And that means that it
does not take much of a change in the highlighted file to get
intolerable behavior on your machine or even better ones.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

Guten Tag, David!

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

> > With Martin Rudalics's optimisation to c-beginning-of-defun-raw, I
> > don't feel that the slowness is too bad any more (though he does
> > ;-).  In the extreme case xdisp.c, scrolling to EOB and M-v is
> > mildly sluggish (about half a second), on my 5 year old Athlon 1.2
> > GHz machine.  A typical new PC now is, say, 3 GHZ.  In the time
> > Emacs 22 is the current release, a typical new PC will come to be
> > around 20 GHz, and this slowness will not matter.

> Please, arguments like that are just not acceptable.  If examples with
> barely tolerable behavior can be come across easily, you can bet that
> there will be normal use cases where the the behavior will be quite
> intolerable.  And even on faster machines, editing should try avoiding
> draining unnecessary CPU power: editing is by far the most important
> application where you want to have a long battery life for laptops.

> I am working with a 600MHz laptop, and my occasionally used desktop
> system has a 233MHz processor.  The preview-latex
> <URL:http://www.gnu.org/software/auctex/preview-latex.html> package
> has been streamlined to operate well on such machines.  It would be
> really a bad hoax if syntax highlighting (which does a lot less) would
> make such machines unusable for editing with an up-to-date Emacs with
> default settings.

Hey, just calm down a bit and take a few long deep breaths!  You've
snipped my argument and left only the supporting discussion.  I'm NOT
arguing that anybody should have this "barely tolerable behaviour"
thrust upon him; the user should be able to set his system up as he
wishes.  I'm merely saying that in a few years time the point will be
moot.  And that for Emacs 23, we will be able to set the default value
of open-paren-...-start  to nil, just as in Emacs 22 font-locking is
enabled by default.

Hopefully Emacs will always be such that wierdos who, for whatever
reason, want to run with font-lock disabled, or on bare TTYs, or on
early 1990s machines can do so comfortably.  ;-)

[ .... ]

> -- 
> David Kastrup, Kriemhildstr. 15, 44793 Bochum

-- 
Alan Mackenzie (Ittersbach, Germany)

Re: Mysterious fontification/C++ context issue

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> Guten Tag, David!
>
> On Sun, Dec 10, 2006 at 01:46:40PM +0100, David Kastrup wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > With Martin Rudalics's optimisation to c-beginning-of-defun-raw, I
>> > don't feel that the slowness is too bad any more (though he does
>> > ;-).  In the extreme case xdisp.c, scrolling to EOB and M-v is
>> > mildly sluggish (about half a second), on my 5 year old Athlon 1.2
>> > GHz machine.  A typical new PC now is, say, 3 GHZ.  In the time
>> > Emacs 22 is the current release, a typical new PC will come to be
>> > around 20 GHz, and this slowness will not matter.
>
>> Please, arguments like that are just not acceptable.  If examples with
>> barely tolerable behavior can be come across easily, you can bet that
>> there will be normal use cases where the the behavior will be quite
>> intolerable.  And even on faster machines, editing should try avoiding
>> draining unnecessary CPU power: editing is by far the most important
>> application where you want to have a long battery life for laptops.
>
>> I am working with a 600MHz laptop, and my occasionally used desktop
>> system has a 233MHz processor.  The preview-latex
>> <URL:http://www.gnu.org/software/auctex/preview-latex.html> package
>> has been streamlined to operate well on such machines.  It would be
>> really a bad hoax if syntax highlighting (which does a lot less) would
>> make such machines unusable for editing with an up-to-date Emacs with
>> default settings.
>
> Hey, just calm down a bit and take a few long deep breaths!  You've
> snipped my argument and left only the supporting discussion.  I'm NOT
> arguing that anybody should have this "barely tolerable behaviour"
> thrust upon him; the user should be able to set his system up as he
> wishes.

Disagree.  The _default_ should be such that the behavior is
tolerable, without the user having to set up anything.

> I'm merely saying that in a few years time the point will be moot.

Disagree.  Non-local O(n^2) (or worse) behavior will exhaust any
advance of computing power eventually.  You've snipped my argument.

> And that for Emacs 23, we will be able to set the default value of
> open-paren-...-start to nil, just as in Emacs 22 font-locking is
> enabled by default.

We only enabled font-locking after hunting down the cases where it
lead to intolerable behavior.  Since the original behavior was not
deemed acceptable, I don't see why we should now act cavalier about
regressions.

> Hopefully Emacs will always be such that wierdos who, for whatever
> reason, want to run with font-lock disabled, or on bare TTYs, or on
> early 1990s machines can do so comfortably.  ;-)

Hopefully Emacs will work tolerably for all use cases without having
to strip it down to "weirdo level" from its default settings.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Mysterious fontification/C++ context issue

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

>> If there is bad fontification for only one file in Emacs, out of the
>> universe of C files, why not set open-paren-in-column-0-is-defun-start
>> to be [t] for that file, using a file-local variable?
>
> There will be bad fontification in lots of files.  Setting opic0ids to
> nil makes Emacs analyse a CC Mode buffer rigorously, correctly and a bit
> slowly.  Setting it to t does a rough and ready job which sometimes gives
> wierd results.
>
> With Martin Rudalics's optimisation to c-beginning-of-defun-raw, I don't
> feel that the slowness is too bad any more (though he does ;-).

The slowdown 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.  If
someone later comes up with a clever c-beginning-of-defun function, we
can switch it back.

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?

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]

 >>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.

[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.

[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.

[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.

[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.

[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

What `opic0ids' really is [was: Mysterious fontification/C++ context issue - Patch for beginning-of-defun-raw.]

[Stuart D. Herring]

> 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!

I've not been in this thread thus far, but I feel that the point may be
being missed here.  open-paren-in-column-0-is-defun-start is purely an
optimization:

1. If the user knows that "bad" C0 parens are sufficiently rare
("sufficiently" for his purposes) that assuming them to be defun-start
points, he should set the variable to t (perhaps as a file-local in a long
C file which would otherwise be a problem).

2. If another user knows that code she deals with does not follow good
indentation standards or is using some preprocessor or so that makes that
assumption invalid, she should set the variable to nil (again, perhaps
locally).

3. If a mode knows that its language requires that the rule be followed
(or that it is followed in all halfway-sensible cases), it MAY (see RFC
2119) set opic0ids to t locally in its buffers, but it SHOULD NOT assume
that the variable is non-nil (in that, perhaps, it assumes that low-level
functions involved in `beginning-of-defun' will act as they do when it is
non-nil) unless it specifically dynamically binds the variable so.

4. If a mode knows that its language requires or merely often involves
that other C0 parens appear, it SHOULD set opic0ids to nil locally in its
buffers.  If a user overrides this setting, the mode SHOULD NOT fail or
raise errors, and MUST NOT make incorrect textual buffer modifications,
but may of course produce and/or cache incorrect answers about the
structure of text in the buffer.

All that I think is common sense or follows from the definition of
opic0ids as an optimization control.  My own suggested guidelines follow.

5. A mode MAY ignore opic0ids in its own code (and bind it conveniently
when calling into standard routines) if parens are not associated with
defuns in its language.  A mode MAY similarly ignore the variable if (for
some operation) it provides a more powerful, faster, more convenient, or
otherwise superior (e.g., updates useful internal data) mechanism for
finding the beginnings of defuns that incurs no user-perceptible
performance penalty for any operation on a well-formed file of a size and
complexity ever encountered in practice on the overwhelming majority of
modern hardware.

6. Any feature or behavior without which a mode is usable (even if
crippled) and which has the potential in any plausible scenario to delay
editing (particularly repeatedly) MUST be controllable by a user option
(although the default value SHOULD enable it so that it is available
except when problematic and is known in the first place).

7. A mode SHOULD interpret the assumption implied by opic0ids non-nil as
appropriate to that mode; for instance, in C mode, where functions begin
not with their opening { but rather with the return type of the function,
a C0 paren (with opic0ids set) may be assumed to be the beginning of a
function's root block, but the mode should still arrange to move before
the argument list, name, and return type (as possible) when performing
`beginning-of-defun' or the equivalent.

8. A mode MUST NOT use the value of opic0dis to decide whether to make
other unrelated assumptions or simplifications, but it MAY provide a value
of a user option which means "enable <feature> iff opic0ids is set".

9. All code MUST NOT assume the converse -- that all starts of defuns are
heralded by C0 parens -- as a result of the value of opic0ids.

10. All code SHOULD NOT assume that converse ever, unless their language
requires it to be the case.  If it is prohibitively expensive not to
assume it, it may be assumed, but this variance MUST be documented and
SHOULD be controlled by a user option (whose default value MAY allow the
assumption) if feasible.

I note that in 21.3.1 (the only version I have available at the moment)
Emacs Lisp mode violates #10.

Sorry that this is something of a rant, but I want to put out a set of
rules such that either
A) we come to agree that they are a good representation of how things
should be, so that we now have a guide for how to resolve things;
B) we discover that we are not in agreement about what the goals/effects
of this user option are/should be, so that we can address them instead of
wasting time talking at cross purposes; or
C) we find that I was right to stay out of this thread because I do not
understand the subject or have uselessly unique ideas about it, so that I
will resume silence and not waste anyone's time with lots of shorter
messages that would be no more helpful.

So, er, which is it?
Davis

-- 
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.

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.

[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.

[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.

[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.

[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:

> 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 c-basic-common-init

[Alan Mackenzie]

Hi, Martin, and everybody else.

On Sun, Dec 17, 2006 at 12:55:22PM +0000, Alan Mackenzie wrote:

[ .... ]

> 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.

Here's a pair of patches to do the job.  I think most people will be
happy about them.  ;-)

Richard, please review my change to programs.texi.

I'll have a look at cc-mode.texi ASAP.

Thanks to Chong, Stefan and Martin for talking me through this at such
great length and persuading me to do the right thing.


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

	* progmodes/cc-mode.el (c-basic-common-init): don't set
	open-paren-in-column-0-is-defun-start to nil any more.


Index: cc-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-mode.el,v
retrieving revision 1.50
diff -c -r1.50 cc-mode.el
*** cc-mode.el	3 Dec 2006 00:59:33 -0000	1.50
--- cc-mode.el	17 Dec 2006 18:10:38 -0000
***************
*** 531,539 ****
    ;; heuristic that open parens in column 0 are defun starters.  Since
    ;; we have c-state-cache, that heuristic isn't useful and only causes
    ;; trouble, so turn it off.
!   (when (memq 'col-0-paren c-emacs-features)
!     (make-local-variable 'open-paren-in-column-0-is-defun-start)
!     (setq open-paren-in-column-0-is-defun-start nil))
  
    (c-clear-found-types)
  
--- 531,541 ----
    ;; heuristic that open parens in column 0 are defun starters.  Since
    ;; we have c-state-cache, that heuristic isn't useful and only causes
    ;; trouble, so turn it off.
! ;; 2006/12/17: This facility is somewhat confused, and doesn't really seem
! ;; helpful.  Comment it out for now.
! ;;   (when (memq 'col-0-paren c-emacs-features)
! ;;     (make-local-variable 'open-paren-in-column-0-is-defun-start)
! ;;     (setq open-paren-in-column-0-is-defun-start nil))
  
    (c-clear-found-types)
  


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

	* programs.texi (Left Margin Paren): Remove the bit which says
	that CC Mode sets open-paren-in-column-0-is-defun-start to nil.
	Discuss some of the issues of setting this option to nil.


Index: programs.texi
===================================================================
RCS file: /cvsroot/emacs/emacs/man/programs.texi,v
retrieving revision 1.121
diff -c -r1.121 programs.texi
*** programs.texi	30 Sep 2006 21:07:21 -0000	1.121
--- programs.texi	17 Dec 2006 18:28:40 -0000
***************
*** 156,178 ****
  @cindex open-parenthesis in leftmost column
  @cindex ( in leftmost column
    Emacs assumes by default that any opening delimiter found at the
! left margin is the start of a top-level definition, or defun.  You can
! override this default by setting this user option:
! 
! @defvar open-paren-in-column-0-is-defun-start
! If this user option is set to @code{t} (the default), opening
! parentheses or braces at column zero always start defuns.  When it's
! @code{nil}, defuns are found by searching for parens or braces at the
! outermost level.  Some major modes, including C and related modes, set
! @code{open-paren-in-column-0-is-defun-start} buffer-locally to
! @code{nil}
! @end defvar
! 
!   In modes where @code{open-paren-in-column-0-is-defun-start} is
! @code{t}, @strong{don't put an opening delimiter at the left margin
! unless it is a defun start}.  For instance, never put an
  open-parenthesis at the left margin in a Lisp file unless it is the
! start of a top-level list.
  
    If you don't follow this convention, not only will you have trouble
  when you explicitly use the commands for motion by defuns; other
--- 156,166 ----
  @cindex open-parenthesis in leftmost column
  @cindex ( in leftmost column
    Emacs assumes by default that any opening delimiter found at the
! left margin is the start of a top-level definition, or defun.
! Therefore, @strong{don't put an opening delimiter at the left margin
! unless it should have that significance}.  For instance, never put an
  open-parenthesis at the left margin in a Lisp file unless it is the
! start of a top-level list.  
  
    If you don't follow this convention, not only will you have trouble
  when you explicitly use the commands for motion by defuns; other
***************
*** 182,191 ****
  
    The most likely problem case is when you want an opening delimiter
  at the start of a line inside a string.  To avoid trouble, put an
! escape character (@samp{\}, in Emacs Lisp, @samp{/} in some other Lisp
! dialects) before the opening delimiter.  This will not affect the
! contents of the string, but will prevent that opening delimiter from
! starting a defun.  Here's an example:
  
  @example
    (insert "Foo:
--- 170,179 ----
  
    The most likely problem case is when you want an opening delimiter
  at the start of a line inside a string.  To avoid trouble, put an
! escape character (@samp{\}, in C and Emacs Lisp, @samp{/} in some
! other Lisp dialects) before the opening delimiter.  This will not
! affect the contents of the string, but will prevent that opening
! delimiter from starting a defun.  Here's an example:
  
  @example
    (insert "Foo:
***************
*** 197,202 ****
--- 185,209 ----
  highlights confusing opening delimiters (those that ought to be
  quoted) in bold red.
  
+ If you need to override this convention, you can so by setting this
+ user option:
+ 
+ @defvar open-paren-in-column-0-is-defun-start
+ If this user option is set to @code{t} (the default), opening
+ parentheses or braces at column zero always start defuns.  When it's
+ @code{nil}, defuns are found by searching for parens or braces at the
+ outermost level.
+ @end defvar
+ 
+   Usually, you shouldn't need to set
+ @code{open-paren-in-column-0-is-defun-start} to @code{nil}.  However,
+ if your buffer contains parentheses or braces in column zero which
+ don't start defuns and this confuses Emacs, it sometimes helps to set
+ the option to @code{nil}.  Be aware, though, that this will make
+ scrolling and display in large buffers quite sluggish, and that
+ parentheses and braces must be correctly matched throughout the buffer
+ for it to work properly.
+ 
    In the earliest days, the original Emacs found defuns by moving
  upward a level of parentheses or braces until there were no more
  levels to go up.  This always required scanning all the way back to
***************
*** 1557,1566 ****
  @table @kbd
  @item C-c C-@key{DEL}
  @itemx C-c @key{DEL}
! @findex c-hungry-backspace
  @kindex C-c C-@key{DEL} (C Mode)
  @kindex C-c @key{DEL} (C Mode)
! @code{c-hungry-backspace}---Delete the entire block of whitespace
  preceding point.
  
  @item C-c C-d
--- 1564,1573 ----
  @table @kbd
  @item C-c C-@key{DEL}
  @itemx C-c @key{DEL}
! @findex c-hungry-delete-backwards
  @kindex C-c C-@key{DEL} (C Mode)
  @kindex C-c @key{DEL} (C Mode)
! @code{c-hungry-delete-backwards}---Delete the entire block of whitespace
  preceding point.
  
  @item C-c C-d

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for c-basic-common-init

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> Hi, Martin, and everybody else.
>
>> 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.
>
> Here's a pair of patches to do the job.  I think most people will be
> happy about them.  ;-)

Looks good.  Please check it in, and thanks!

Re: Mysterious fontification/C++ context issue - Patch for c-basic-common-init

[Alan Mackenzie]

Hi, Chong!

On Sun, Dec 17, 2006 at 01:45:59PM -0500, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hi, Martin, and everybody else.

> >> 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.

> > Here's a pair of patches to do the job.  I think most people will be
> > happy about them.  ;-)
 
> Looks good.  Please check it in, and thanks!

Done.

-- 
Alan.

Re: Mysterious fontification/C++ context issue - Patch for c-basic-common-init

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> Hi, Chong!
>
> On Sun, Dec 17, 2006 at 01:45:59PM -0500, Chong Yidong wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > Hi, Martin, and everybody else.
>
>> >> 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.
>
>> > Here's a pair of patches to do the job.  I think most people will be
>> > happy about them.  ;-)
>  
>> Looks good.  Please check it in, and thanks!

I also checked in the optimization/simplification to the
beginning-of-defun-raw code.

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

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.

[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.

[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]

 > 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]

> 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]

 >>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 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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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

[Stefan Monnier]

> In the time Emacs 22 is the current release, a typical new PC will come to
> be around 20 GHz, and this slowness will not matter.

Actually, "recent" trends indicate that this is not true.  We'll probably
see typical new PCs with 4-16 CPUs, each one running at 4-5GHz, but that
won't help Emacs much.


        Stefan

Re: Mysterious fontification/C++ context issue

[Kim F. Storm]

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

>> In the time Emacs 22 is the current release, a typical new PC will come to
>> be around 20 GHz, and this slowness will not matter.
>
> Actually, "recent" trends indicate that this is not true.  We'll probably
> see typical new PCs with 4-16 CPUs, each one running at 4-5GHz, but that
> won't help Emacs much.

And most laptops still run at < 2GHz.  This is not likely to improve
much, as the trend is towards longer battery life rather than
(usually unnecessary) faster CPUs.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

On Sun, Dec 10, 2006 at 04:39:03PM -0500, Stefan Monnier wrote:
> > In the time Emacs 22 is the current release, a typical new PC will come to
> > be around 20 GHz, and this slowness will not matter.
 
> Actually, "recent" trends indicate that this is not true.  We'll probably
> see typical new PCs with 4-16 CPUs, each one running at 4-5GHz, but that
> won't help Emacs much.
 
Hmmm.  One processor for foreground editing, another for doing garbage
collection, yet another four or five for background font-locking and
syntactic cacheing, and so on.

That could be quite a fast system.


>         Stefan

-- 
Alan.

Re: Mysterious fontification/C++ context issue

[Miles Bader]

Alan Mackenzie <acm@muc.de> writes:
> Hmmm.  One processor for foreground editing, another for doing garbage
> collection, yet another four or five for background font-locking and
> syntactic cacheing, and so on.

Hmmm I wonder, maybe allocate separate processors to analyze separate
regions of the file -- of course that assumes a reasonable beginning-of-defun
actually gets sorted out by then... :-)

-miles
-- 
`The suburb is an obsolete and contradictory form of human settlement'

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>> > In the time Emacs 22 is the current release, a typical new PC will come to
>> > be around 20 GHz, and this slowness will not matter.
 
>> Actually, "recent" trends indicate that this is not true.  We'll probably
>> see typical new PCs with 4-16 CPUs, each one running at 4-5GHz, but that
>> won't help Emacs much.
 
> Hmmm.  One processor for foreground editing, another for doing garbage
> collection, yet another four or five for background font-locking and
> syntactic cacheing, and so on.

> That could be quite a fast system.

I'm anxiously awaiting your patch to Emacs's C code base to make such
concurrency possible.


        Stefan

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 > If there is bad fontification for only one file in Emacs, out of the
 > universe of C files, why not set open-paren-in-column-0-is-defun-start
 > to be nil for that file, using a file-local variable?

It would be simpler to make syntax.c obey the coding standard by
removing the conflicting open paren in column zero.  Alas, the problem
is merely ideological: If we abandon the standard, people will more
frequently write files that don't obey the open paren in column zero
convention.  I think:

- Programmers should avoid putting open parens in column zero unless
   these start a defun.  To enforce this behavior, major modes should
   always issue a warning by highlighting such parens (where "always"
   means "as long as the according coding standard persists").

- Major modes may try to avoid messing up fontification when they
   encounter a misplaced open paren.  If this slows down fontification,
   it should be a user option, though.

In either case, it'd be advisable to tell users that they may influence
this by setting `open-paren-in-column-0-is-defun-start' accordingly or,
whatever they do has no impact on the major mode's behavior.

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>> If there is bad fontification for only one file in Emacs, out of the
>> universe of C files, why not set open-paren-in-column-0-is-defun-start
>> to be nil for that file, using a file-local variable?

> It would be simpler to make syntax.c obey the coding standard by
> removing the conflicting open paren in column zero.  Alas, the problem
> is merely ideological: If we abandon the standard, people will more
> frequently write files that don't obey the open paren in column zero
> convention.  I think:

> - Programmers should avoid putting open parens in column zero unless
>   these start a defun.  To enforce this behavior, major modes should
>   always issue a warning by highlighting such parens (where "always"
>   means "as long as the according coding standard persists").

I disagree in general.  What I agree to is that programmers should avoid
putting things that look like defuns inside comments and strings.

I.e. in Lisp, an open-paren-in-col0 is indeed a bad idea.  In C it's not
a problem (maybe an open-brace-in-col0 is a problem there, but not an open
paren).


        Stefan

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 > I disagree in general.  What I agree to is that programmers should avoid
 > putting things that look like defuns inside comments and strings.

Commented-out code may look like a defun.

 > I.e. in Lisp, an open-paren-in-col0 is indeed a bad idea.  In C it's not
 > a problem (maybe an open-brace-in-col0 is a problem there, but not an open
 > paren).

The Emacs manual states

In modes where `open-paren-in-column-0-is-defun-start' is `t',
*don't put an opening delimiter at the left margin unless it is a defun
start*.

and you say `open-paren-in-column-0-is-defun-start' is obsolete ;-)

BTW, in Elisp an open bracket in column zero of a string should not
confuse Emacs.  Yet you highlight it with `font-lock-warning-face'.

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>> I disagree in general.  What I agree to is that programmers should avoid
>> putting things that look like defuns inside comments and strings.
> Commented-out code may look like a defun.

Indeed, so programmers should be careful to comment out in a way that makes
it not look like a defun.  That's generally more or less true of code
commented out with M-x comment-region.

>> I.e. in Lisp, an open-paren-in-col0 is indeed a bad idea.  In C it's not
>> a problem (maybe an open-brace-in-col0 is a problem there, but not an open
>> paren).
> The Emacs manual states
> In modes where `open-paren-in-column-0-is-defun-start' is `t',
> *don't put an opening delimiter at the left margin unless it is a defun
> start*.
> and you say `open-paren-in-column-0-is-defun-start' is obsolete ;-)

Well I didn't say it's obsolete: I said I'd like to declare it obsolete
(but that'll require a few changes).  Note that its current effect is
extremely minor anyway (it only adds "^\\|" to defun-prompt-regexp and
slightly tweaks the way back_comment deals with worst-case scenarios).

> BTW, in Elisp an open bracket in column zero of a string should not
> confuse Emacs.  Yet you highlight it with `font-lock-warning-face'.

Indeed, it *shouldn't*, but it does confuse Emacs, which is why it's
highlighted ;-(.  We should fix it so it doesn't confuse Emacs any more (and
doesn't get highlighted either).


        Stefan

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 >>BTW, in Elisp an open bracket in column zero of a string should not
 >>confuse Emacs.  Yet you highlight it with `font-lock-warning-face'.
 >
 >
 > Indeed, it *shouldn't*, but it does confuse Emacs, which is why it's
 > highlighted ;-(.  We should fix it so it doesn't confuse Emacs any more (and
 > doesn't get highlighted either).

Due to the fact that "\\s(" is hardwired in `font-lock-compile-keywords'
and can't be set individually by a major mode.  A minor issue showing
that it's not entirely trivial to give a simple recipe fitting all major
modes.

What I meant is "stay away from putting an open paren in column zero
unless it starts a defun" in any programming mode where "open paren" is
what the syntax table says it is.  It's one constraint for the user,
easy to explain, understand, and implement, difficult to defend, tho.

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>>> BTW, in Elisp an open bracket in column zero of a string should not
>>> confuse Emacs.  Yet you highlight it with `font-lock-warning-face'.
>> Indeed, it *shouldn't*, but it does confuse Emacs, which is why it's
>> highlighted ;-(.  We should fix it so it doesn't confuse Emacs any more (and
>> doesn't get highlighted either).
> Due to the fact that "\\s(" is hardwired in `font-lock-compile-keywords'
> and can't be set individually by a major mode.

It's only hardwired in font-lock-compile-keywords for the (good) reason that
it's hardwired in beginning-of-defun.  Of course, if we fix
beginning-of-defun, we should adjust font-lock-compile-keywords accordingly.

The problem really fundamentally lies in beginning-of-defun.
`font-lock-compile-keywords' only reflects it.


        Stefan

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

>> Could we set open-paren-in-column-0-is-defun-start to t for c mode?

> This would nullify the fix for the bug reported by Martin a few weeks
> ago, namely: Load syntax.c and immediately jump forward to a certain
> function.  The fontification was seriously wrong.

These limitations have existed for years.   In elisp (and in c-mode at some
point during the development of Emacs-22), we addressed those problems not
by fixing them but instead by highlighting in `warning' face the pieces of
code that can cause such problems.

Such limitations are not necessarily that bad.  After all, if you write your
code in a sufficiently odd way, the indentation engine will also get
completely confused.  Emacs only provides good support for code written in
a sufficiently idiomatic way.

> I think I now agree with Martin R: opic0ids should become a fully user
> settable option, and CC Mode should respect it.

I think I agree.  But I'd do it slightly differently.
The "open-paren-in-col0" is a heuristic that makes sense in some modes but
not all.  Other modes may want another heuristic.

The heuristic itself is currently specified/enabled via
`syntax-begin-function' whose value may use things like beginning-of-defun.
I think the user's control should be on syntax-begin-function (e.g. by
adding a Custom var that forces Emacs to ignore the syntax-begin-function
setting).

> I'm not 100% sure what effect this would have on CC Mode, but it'll be
> something like this: c-beginning-of-defun will continue to look for an
> unnested {, but with (eq opic0ids t), its results will be probabalistic
> rather than rigorous.  But it worked well enough before, when
> beginning-of-defun was a bit squidgy.

And if the user chooses to let the major mode use some heuristic, it'd be
good for the major mode to flag (to the extent possible) those places where
the heuristic is in error.

In the same area: the change in beginning-of-defun which treats
open-paren-in-column-0-is-defun-start specially when defun-prompt-regexp is
nil (and which currently leads to oddball cases where point can end up in
the middle of an parenthesized expression, as shown by Martin's examples)
should be replaced by a new var "b-d-f-check-syntax" which if non-nil causes
beginning-of-defun to skip the false matches (i.e. parens-in-col0 or
defun-prompt-regexp matches that are not at toplevel) by checking
syntax-ppss.


        Stefan

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

    >     Could we set open-paren-in-column-0-is-defun-start to t for c mode?

    This would nullify the fix for the bug reported by Martin a few weeks
    ago, namely: Load syntax.c and immediately jump forward to a certain
    function.  The fontification was seriously wrong.

Could you please explain this more clearly?  What was that fix,
and why would this change interfere with it?  I don't remember
the previous discussion any more.

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

    6. In any case, the new behavior should be documented since it applies
    whenever `open-paren-in-column-0-is-defun-start' is set to nil.

Where are documentation changes are needed for this?

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 >     6. In any case, the new behavior should be documented since it applies
 >     whenever `open-paren-in-column-0-is-defun-start' is set to nil.
 >
 > Where are documentation changes are needed for this?

In the Elisp description of `beginning-of-defun' I presume.  I'd explain
the following issues:

1. `beginning-of-defun' moves to the beginning of the line where it
expects the beginning of a defun.  That position is not necessarily part
of that defun and maybe not even at the "outermost" level as I explained
in the example of point 2 of my previous mail.  This was a non-issue
when searching for an open paren in column zero (think of a couple of
parenthesized expressions on the same line).

2. That a user's idea of "defun" for her programming language does not
necessarily coincide with that of `beginning-of-defun'.

foo (...)
...
{
...
}

will now get you two defuns instead of one.  This should be explained
for C mode at least.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

    1. `beginning-of-defun' moves to the beginning of the line where it
    expects the beginning of a defun.  That position is not necessarily part
    of that defun and maybe not even at the "outermost" level as I explained
    in the example of point 2 of my previous mail.

I don't understand this point (and I don't remember the previous mail).
What do you think it should say here?

    2. That a user's idea of "defun" for her programming language does not
    necessarily coincide with that of `beginning-of-defun'.

The major mode is supposed to try to make them correspond.
It seems strange to assume that it doesn't succeed.

    foo (...)
    ...
    {
    ...
    }

    will now get you two defuns instead of one.

We're still considering how to change the way C mode handles
this case, and I don't think that will be true when we are done.

Anyway, this is not the right place to discuss what happens in
specific modes.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 > I don't understand this point (and I don't remember the previous mail).
 > What do you think it should say here?

Suppose you have a construct like

(foo

...) (bar ...)

and you invoke `beginning-of-defun' from somewhere after "bar".
`beginning-of-defun-raw' will determine that the "defun" starts at the
open paren preceding "bar" but `beginning-of-defun' may return the
position at line beginning which is inside the parens of "foo".

This was a non-issue with the old `beginning-of-defun' which detected a
defun iff there was an opening paren at column zero.

 >     2. That a user's idea of "defun" for her programming language does not
 >     necessarily coincide with that of `beginning-of-defun'.
 >
 > The major mode is supposed to try to make them correspond.
 > It seems strange to assume that it doesn't succeed.

If it really succeeded why should C mode ever bother to provide its own
`c-beginning-of-defun'?

 >
 >     foo (...)
 >     ...
 >     {
 >     ...
 >     }
 >
 >     will now get you two defuns instead of one.
 >
 > We're still considering how to change the way C mode handles
 > this case, and I don't think that will be true when we are done.

We're still considering how to handle this within `beginning-of-defun',
not necessarily in C mode.

 > Anyway, this is not the right place to discuss what happens in
 > specific modes.

The current change affects the behavior of `beginning-of-defun' which
may affect other modes as well.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Richard Stallman]

     >     2. That a user's idea of "defun" for her programming language does not
     >     necessarily coincide with that of `beginning-of-defun'.
     >
     > The major mode is supposed to try to make them correspond.
     > It seems strange to assume that it doesn't succeed.

    If it really succeeded why should C mode ever bother to provide its own
    `c-beginning-of-defun'?

Maybe C mode should arrange to use `c-beginning-of-defun'
as the beginning-of-definition-function.  Is there a reason why
this would give bad results?

     > Anyway, this is not the right place to discuss what happens in
     > specific modes.

    The current change affects the behavior of `beginning-of-defun' which
    may affect other modes as well.

Yes, if they set `open-paren-in-column-0-is-defun-start' to t.

Re: Mysterious fontification/C++ context issue

[martin rudalics]

> Maybe C mode should arrange to use `c-beginning-of-defun'
> as the beginning-of-definition-function.  Is there a reason why
> this would give bad results?

I think it has to set up its own cache first.  This might slow down
fontification even more.

>     The current change affects the behavior of `beginning-of-defun' which
>     may affect other modes as well.
> 
> Yes, if they set `open-paren-in-column-0-is-defun-start' to t.

"... to nil".  The default value is t.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

Hi, Richard, hi, Martin!

On Sat, Dec 09, 2006 at 11:24:54PM -0500, Richard Stallman wrote:
>      >     2. That a user's idea of "defun" for her programming
>      >     language does not necessarily coincide with that of
>      >     `beginning-of-defun'.
>      >
>      > The major mode is supposed to try to make them correspond.  It
>      > seems strange to assume that it doesn't succeed.
 
beginning-of-defun unfortunately conflates (i) "beginning of a function" and
(ii) "outermost level of parens", this arising because they coincide in
the most important language (Lisp).  Having
"beginning-of-defun-function" makes things even worse.

Typically, b-o-d (sense (i)) is going to be calling
foo-beginning-of-defun (the value of b-o-d-function) which is going to
be recursively calling beginning-of-defun (sense (ii)), getting one's
knickers horribly twisted.

Maybe, for Emacs 23, we could disentangle these things.

>     If it really succeeded why should C mode ever bother to provide
>     its own `c-beginning-of-defun'?

c-beginning-of-defun puts point at the beginning of the C function, i.e.
at the "int" in the following, regardless of whether the brace is hung
or at column 0:

int foo (bar)
{
  ....
}

> Maybe C mode should arrange to use `c-beginning-of-defun'
> as the beginning-of-definition-function.  Is there a reason why
> this would give bad results?

Yes.

You mean syntax-begin-function?  c-beginning-of-defun is a user level
function, and it's slow.  About 90% of its runtime is spent determining
where point is initially (inside a defun / inside "int foo (bar)" / at
"int foo (bar)"/ ... ) and finally moving from the open brace to the
"int".  Only ~10% is used in finding the outermost open brace, which is
all that's needed for font-locking and so on.

>      > Anyway, this is not the right place to discuss what happens in
>      > specific modes.

>     The current change affects the behavior of `beginning-of-defun' which
>     may affect other modes as well.

> Yes, if they set `open-paren-in-column-0-is-defun-start' to t.

You mean setting it to nil.  ;-)

The behaviour of beginning-of-defun has been changed ONLY for when
opic0ids gets set to nil.  The only other file.el within Emacs which
does this is progmodes/python.el.  Any Python users here?  Dave Love,
are you listening?

There is also this baffling comment in net/tramp.el:

;; Fontification is messed up when
;; `open-paren-in-column-0-is-defun-start' set to t.

-- 
Alan Mackenzie (Ittersbach, Germany)

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 > The behaviour of beginning-of-defun has been changed ONLY for when
 > opic0ids gets set to nil.  The only other file.el within Emacs which
 > does this is progmodes/python.el.  Any Python users here?  Dave Love,
 > are you listening?

`python-mode' provides it's own `beginning-of-defun-function' which
doesn't seem to rely on `beginning-of-defun-raw'.  Hence, setting that
in `python-mode' is likely just paranoia.

 > There is also this baffling comment in net/tramp.el:
 >
 > ;; Fontification is messed up when
 > ;; `open-paren-in-column-0-is-defun-start' set to t.

That comment should probably serve to justify why the subsequent code in
the doc-string is indented.

Anyway: `open-paren-in-column-0-is-defun-start' is a user option
explained thoroughly in the Emacs manual.  If a major mode does not
override the default setting, users should know where
`beginning-of-defun' gets them when they customize it to nil.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Slawomir Nowaczyk]

On Sun, 10 Dec 2006 12:58:42 +0100
martin rudalics <rudalics@gmx.at> wrote:

#>  > The behaviour of beginning-of-defun has been changed ONLY for when
#>  > opic0ids gets set to nil. The only other file.el within Emacs
#>  > which does this is progmodes/python.el. Any Python users here?
#>  > Dave Love, are you listening?
#> 
#> `python-mode' provides it's own `beginning-of-defun-function' which
#> doesn't seem to rely on `beginning-of-defun-raw'. Hence, setting that
#> in `python-mode' is likely just paranoia.

Yes, it seems so. I would suggest the following patch, so we can be
sure:

**********************************************************************

--- EmacsCVS/lisp/progmodes/python.el       2006-12-10 11:49:05.479332800 +0100
+++ Emacs/lisp/progmodes/python.el   2006-12-10 15:26:11.657208000 +0100
@@ -2236,7 +2236,7 @@
           symbol-end))
   (set (make-local-variable 'outline-heading-end-regexp) ":\\s-*\n")
   (set (make-local-variable 'outline-level) #'python-outline-level)
-  (set (make-local-variable 'open-paren-in-column-0-is-defun-start) nil)
+  ;;(set (make-local-variable 'open-paren-in-column-0-is-defun-start) nil)
   (make-local-variable 'python-saved-check-command)
   (set (make-local-variable 'beginning-of-defun-function)
        'python-beginning-of-defun)

**********************************************************************

Or, if this is assumed too risky at this point in time, maybe this at
least:

**********************************************************************

--- EmacsCVS/lisp/progmodes/python.el       2006-12-10 11:49:05.479332800 +0100
+++ Emacs/lisp/progmodes/python.el   2006-12-10 15:27:56.628148800 +0100
@@ -2236,6 +2236,7 @@
           symbol-end))
   (set (make-local-variable 'outline-heading-end-regexp) ":\\s-*\n")
   (set (make-local-variable 'outline-level) #'python-outline-level)
+  ;; TODO: isn't setting opic0ids noop?
   (set (make-local-variable 'open-paren-in-column-0-is-defun-start) nil)
   (make-local-variable 'python-saved-check-command)
   (set (make-local-variable 'beginning-of-defun-function)

**********************************************************************

-- 
 Best wishes,
   Slawomir Nowaczyk
     ( slawomir.nowaczyk.847@student.lu.se )

Christmas: Be Naughty - save Santa the trip.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

> beginning-of-defun unfortunately conflates (i) "beginning of a function" and
> (ii) "outermost level of parens", this arising because they coincide in
> the most important language (Lisp).  Having
> "beginning-of-defun-function" makes things even worse.

If you want a reliable solution for (ii), then use syntax-ppss (e.g. use the
syntax-ppss-toplevel-pos I proposed yesterday).

Beginning-of-defun *does not* conflate the two.  It has always been defined
as the meaning (i).  In just so happens that for performance reasons, people
have used it as a heuristic in syntax-begin-function to get an approximation
of (ii).  If you don't want that approximation, then don't set
syntax-begin-function.


        Stefan

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Alan Mackenzie]

Evening, Martin!

On Wed, Dec 06, 2006 at 10:04:23AM +0100, martin rudalics wrote:

[ .... ]

> Thanks for taking care of this.  Please note:

> 1. You don't widen the buffer before calling `syntax-ppss'.

You mean I don't have to, and would get the same result without
widening?  I need to widen anyway, in case point is inside a
comment/string and (point-min) is after the c/s's start.

> 2. A construct built along

> (foo
> ) (bar)

> with point at "bar" might derail font-lock (remember,
> `c-beginning-of-defun-1' calls `beginning-of-defun', not
> `beginning-of-defun-raw').

I think it's best to keep the discussion of beginning-of-defun and the CC
Mode stuff separate.  Once b-o-d works right, the question is then how CC
Mode should use it.  You discuss this below.

> 3. All this is still awfully slow and expensive: Open a larger buffer
> like xdisp.c, jump to its end, hit M-v a couple of times, and look at
> CPU consumption.

UURRRKkk!!  That's not good!  It took me ~6 seconds, and that's not even
on my (deceased) 166 MHz box.  That file is the best part of a megabyte,
though.

> 4. You could avoid the tedious `up-list' steps by using a variant of the
> tentative fix I attached.

(nth 9 ppss) is "Internal data for continuing the parsing".  If we're
going to be using that thing's car as "the least nested paren", why
don't we give up the pretence of an "internal" variable and document it
properly?  I know it's used in some other places too, but it seems to
be being economical with our integrity to use it ourselves whilst
telling everybody else "it's internal stuff".  Or has this been
discussed already?

Your patch is more compact than mine was, and makes scrolling in xdisp.c
acceptably (to me) fast.  In fact, that 6 seconds has been reduced to
about half a second.  So it gets my vote.  (I've got a 1.2 GHz Athlon
machine, by the way.)  Just a small point, though: your patch doesn't
handle the silly case (beginning-of-defun 0).  I think this should mean
"point doesn't move".

Is it the `up-list' calls which slow my version down so much?

> 5. Consider making `open-paren-in-column-0-is-defun-start' a real user
> option in C mode.  Let, for example, users decide whether they want to
> respect GNU coding standards [ .... ].

I think I might agree with this, now.  Or is your version of
beginning-of-defun-raw fast enough so that this doesn't really matter?
I think there are lots of programmers who hang braces which open defuns.
I think Java hackers do it a lot.  But we need to implement this in a
way which doesn't repeat the unholy hassle we had with
`require-final-newline'.

I don't think there should be a variable `c-open-paren....-start" - the
name's long enough already.  ;-)  Maybe CC Mode should make
o-p-i-c-o-i-d-s local in each of its buffers.  

> This should also permit binding C-M-a / C-M-e to `c-beginning-of-defun'
> / `c-end-of-defun' when `open-paren-in-column-0-is-defun-start' is nil.

Indeed.

> 6. In any case, the new behavior should be documented since it applies
> whenever `open-paren-in-column-0-is-defun-start' is set to nil.

What new behaviour?  But Richard asked that.

-- 
Alan Mackenzie (Ittersbach, Germany)

Re: Mysterious fontification/C++ context issue

[martin rudalics]

 > Evening, Martin!

Evening, Alan!

 >>1. You don't widen the buffer before calling `syntax-ppss'.
 >
 >
 > You mean I don't have to, and would get the same result without
 > widening?  I need to widen anyway, in case point is inside a
 > comment/string and (point-min) is after the c/s's start.

No, I mean you have to widen _before_ calling `syntax-ppss'.

 >
 >
 >>2. A construct built along
 >
 >
 >>(foo
 >>) (bar)
 >
 >
 >>with point at "bar" might derail font-lock (remember,
 >>`c-beginning-of-defun-1' calls `beginning-of-defun', not
 >>`beginning-of-defun-raw').
 >
 >
 > I think it's best to keep the discussion of beginning-of-defun and the CC
 > Mode stuff separate.  Once b-o-d works right, the question is then how CC
 > Mode should use it.  You discuss this below.

In my example `beginning-of-defun' would move to the beginning of the
line and thus not to the outermost level.

 >>4. You could avoid the tedious `up-list' steps by using a variant of the
 >>tentative fix I attached.
 >
 >
 > (nth 9 ppss) is "Internal data for continuing the parsing".  If we're
 > going to be using that thing's car as "the least nested paren", why
 > don't we give up the pretence of an "internal" variable and document it
 > properly?  I know it's used in some other places too, but it seems to
 > be being economical with our integrity to use it ourselves whilst
 > telling everybody else "it's internal stuff".  Or has this been
 > discussed already?

I don't know.  But it is used in `syntax-ppss' and since you already
call that we probably shouldn't care.  Just add the same comment as
`syntax-ppss' does and let things evolve.

 > Your patch is more compact than mine was, and makes scrolling in xdisp.c
 > acceptably (to me) fast.  In fact, that 6 seconds has been reduced to
 > about half a second.  So it gets my vote.  (I've got a 1.2 GHz Athlon
 > machine, by the way.)

You win, mine is a 1 GHz Athlon.

 > Just a small point, though: your patch doesn't
 > handle the silly case (beginning-of-defun 0).  I think this should mean
 > "point doesn't move".

That's what I meant by "tentative" ;-) you'll find more of that.

 > Is it the `up-list' calls which slow my version down so much?

Depends on your mileage, parsing backward is not entirely trivial, you
always have to check for stray comments.

 >>5. Consider making `open-paren-in-column-0-is-defun-start' a real user
 >>option in C mode.  Let, for example, users decide whether they want to
 >>respect GNU coding standards [ .... ].
 >
 >
 > I think I might agree with this, now.  Or is your version of
 > beginning-of-defun-raw fast enough so that this doesn't really matter?

I don't think so.

 > I think there are lots of programmers who hang braces which open defuns.
 > I think Java hackers do it a lot.  But we need to implement this in a
 > way which doesn't repeat the unholy hassle we had with
 > `require-final-newline'.
 >
 > I don't think there should be a variable `c-open-paren....-start" - the
 > name's long enough already.  ;-)  Maybe CC Mode should make
 > o-p-i-c-o-i-d-s local in each of its buffers.

In any case, it should be the user's responsibility - and I'm a user
here myself ...


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Mysterious fontification/C++ context issue

[Stefan Monnier]

> (nth 9 ppss) is "Internal data for continuing the parsing".  If we're
> going to be using that thing's car as "the least nested paren", why
> don't we give up the pretence of an "internal" variable and document it
> properly?  I know it's used in some other places too, but it seems to
> be being economical with our integrity to use it ourselves whilst
> telling everybody else "it's internal stuff".  Or has this been
> discussed already?

When I decided to use this info in `syntax-ppss' I faced this exact same
problem, and decided to just go ahead with it: it's a sufficiently internal
function that if parse-partial-sexp is ever changed, syntax-ppss can be
changed as well.

I agree that in the case of cc-mode the situation is different.  Maybe,
instead of documenting this behavior, syntax.el should provide a function
(syntax-toplevel-pos PPSS) which returns the nearest previous position
that's outside of anything (string, comment, parenthesis).


        Stefan

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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.

[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.

[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.

[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.

[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.

[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