emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Permission requested to merge branch comment-cache into master.


From: Stefan Monnier
Subject: Re: Permission requested to merge branch comment-cache into master.
Date: Mon, 14 Mar 2016 11:58:26 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux)

>> The only case where it calls to user function is via syntax-propertize.
>> In the case of back_comment this should never happen (because we're
>> moving backward, so syntax-propertize has already been called on
>> positions further down and hence isn't needed at this position), ....
> That's not true.  If you have just moved forward in the buffer to a part
> where syntax-properties haven't yet been applied, and do a
> (forward-comment -1), syntax-propertize would get called for that buffer
> position.

This should generally not be the case, no, because syntax-propertize
will have been called during the SETUP_SYNTAX_TABLE call earlier.
There might be corner cases where it is called, tho, admittedly.
Some of them may be bugs, obviously.

>> but in case it were to happen it could only be because it's necessary
>> *for correctness*.
> It might be necessary to apply those properties for correctness, but
> applying them is NOT the job of a primitive.

Depends if you want the primitive to work correctly.

>> So, again: when my patch to back_comment calls syntax-ppss, it will not
>> call syntax-propertize-function.
> It will.

The only case I know of where this can happen is if
parse-sexp-lookup-properties is nil (which is unusual since as soon as
syntax-propertize-function is called, it sets
parse-sexp-lookup-properties to t).  If needed we could try and plug
this corner case, tho it hasn't been a problem so far.

> I have an idea.  How about rewriting the cache mechanism in C, caching
> the value MUCH more frequently (say, every 128 bytes) and using a data
> structure suitable for that amount of data?

Be my guest.  It's always been part of the possible future I was
imagining for syntax-ppss, so I'm definitely not opposed to it.

That won't remove the need to call syntax-propertize, of course.
And I'm not sure it's worth the trouble since I haven't seen any
indication that the performance of the current implementation of
syntax-ppss is a problem.

> That's the job of high level code, most likely part of a major mode.
> The primitive should _respect_ the syntax-table properties which are
> there, but has no business applying them itself.  Indeed the way they
> are applied and when they are applied is entirely for the major mode to
> decide.

That's indeed what CC-mode does.  All other modes just use
syntax-propertize which takes care of that for them.  I have no reason
to believe that CC-mode couldn't also use syntax-propertize (tho it
would take a lot of work to restructure CC-mode accordingly, IIUC).

But it doesn't really matter: for various reasons, CC-mode does it
differently, and that's OK.  It means that in your case, syntax-ppss
will not call syntax-propertize-function, so the problems you're so
worried about won't affect you.

> OK, sorry about the invective, but take that away and what I said is
> accurate.  You have construed the problem as much more limited in scope
> than I have, and you would leave back_comment largely unchanged.

That's right, my patch leaves back_comment largely unchanged, and I do
think you're making mountains of mole-hills.

>> Now, I'm far from opposed to changing the behavior so as to treat the
>> last /*   '"'"  */ as a comment in the narrowed region.  I think this
>> choice doesn't really matter much, except for a few specific
>> circumstances, where the caller needs to provide more info in order to
>> clarify which behavior she wants.
> Here's where we differ.  I think primitives should be rigorous and
> always work, even in awkward corner cases.

Then you'd better start looking at the whole narrowing issue much
more seriously, because we don't handle it consistently at all:
syntax-ppss is just one of many culprits.

> And that is wholly uncalled for.  My solution is rock-solid and
> bullet-proof, barring any remaining bugs in it due to fact that it's new
> code.

Of course it's not.  According to your own description, it'll break when
syntax-table is changed; it will break when category properties are
changed; it will break when the buffer's text is changed while
inhibit-modification-hooks is non-nil.

Additionally to that it will break code which relies on
`forward-comment' (and all other primitives that depend on
parse-sexp-ignore-comments, such as scan-sexp, forward-sexp, up-list,
...) working *within* the narrowed region without widening.

I don't fault your code for that, mind you.  It's just that this is
Elisp we're talking about, not Coq.  The foundations on which you build
this have fuzzy&complex semantics.


        Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]