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: Alan Mackenzie
Subject: Re: Permission requested to merge branch comment-cache into master.
Date: Mon, 14 Mar 2016 13:28:06 +0000
User-agent: Mutt/1.5.24 (2015-08-30)

Hello, Stefan.

On Sun, Mar 13, 2016 at 09:11:04PM -0400, Stefan Monnier wrote:
> > syntax-ppss is simply unsuitable for use in a primitive, and using it
> > never entered into my considerations.  It is difficult to understand,
> > difficult to maintain high level lisp

> Huh?  syntax-ppss is very simple code, and I have no idea where you
> could have gotten the idea that it's "difficult to maintain".

OK, I take that back.  I personally found it difficult to understand,
and would not want to have to maintain it.

> > and it calls out to user functions via a hook.  It is crazy and in bad
> > taste to make a primitive depend on such a thing.  It is just the
> > Wrong Thing to do.

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

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

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

It will.

> Of course, it still means we call an Elisp function, which could be
> adviced and whatnot.  If you want to rewrite it in C go for it, but
> I'd oppose it unless you have good *concrete* reasons for it, rather
> than vague allegations about it being "crazy" or "wrong".

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?

> > Anything we use in primitives should be robust and rigorous, and as
> > far as possible, simple.

> Exactly.  But in my book, robust includes "has the needed syntax-table
> properties applied".

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.

> > Stefan's idea for a solution is to construe the problem as something
> > more limited, leave the backward scanning in place, and just make bug
> > #22884 go away (until the next bug happens).

> Alan, you're a good coder and good thinker.  I know you can make
> a much better case against my code than that kind of FUD.

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.

> I have acknowledged all the weaknesses you point out (hell, I could give
> you a few more if you were more welcoming).  But syntax-ppss was
> introduced in Emacs-22.1, i.e. it has close to 9 years of wide use.
> So these issues aren't nearly as serious as you make it out to be.
> And furthermore, if/when they need to be fixed, they can be probably be
> fixed rather easily.

> >     char foo[] = "asdf asdf" "asdf"; /* "asdf" */ /*  */  /*   '"'"  */
> >                       ^
> > , narrow it such that point-min is at the marked point.  syntax-ppss
> > then sees the following strings, but no comments:

> As mentioned, depending on the *reason* behind narrowing, what you say
> that syntax-ppss does actually makes perfect sense.  Better yet: that's
> also what (forward-comment -1) does in Emacs-24.  And it's not because
> of a bug in (forward-comment -1) but because (forward-comment -1) also
> decided to start parsing from (point-min) in that particular case.

Let's fix it.

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

[ .... ]

> > How about, as a compromise, merging comment-cache into master for now,
> > while syntax-ppss gets sorted out, then deciding what to do once that
> > has happened?

> You make it sound like your solution is rock-solid bullet-proof whereas
> syntax-ppss is completely wobbly.  This verges on the ridiculous.
> They're both wobbly, by the very nature of their work.

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.  If you see any bugs in it, please let me (and everybody else)
know.  I would like to get the code merged into master specifically to
exercise it and find those bugs, have people measure performance, and so
on.  As for syntax-ppss, its problems are currently under discussion
elsewhere.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



reply via email to

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