bug#22983: [ Patch ] Re: bug#22983: syntax-ppss returns wrong result.

[Alan Mackenzie <acm@muc.de>]

Hello, Dmitry.

On Mon, Sep 11, 2017 at 03:11:22 +0300, Dmitry Gutov wrote:
> On 9/10/17 2:36 PM, Alan Mackenzie wrote:

> >>> The solution I propose is to introduce a second cache into syntax-ppss,
> >>> and this cache would be used whenever (not (eq (point-min) 1)).
> >>> Whenever point-min changes, and isn't 1, this second cached would be
> >>> calculated again from scratch.

> > Here is a patch implementing this.  Comments about it would be welcome.

> Thank you. It seems to hold up to the main test scenario I had in mind, 
> so I don't have any complaints behavior-wise.

Thanks.

> It looks pretty big, though. With lots of new global variables.

> Before, we had syntax-ppss-cache and syntax-ppss-last. The patch adds 8 
> new ones.

Yes.  But each one has a very single purpose, and there are no loops in
the new code, which makes it easier to be sure it is correct.

> I propose two avenues for simplification:

> 1) Use a cons structure for the (PPSS-CACHE . PPSS-LAST) structure. We 
> will have three global variables total: syntax-ppss-data-wide, 
> syntax-ppss-data-narrow, syntax-ppss-data-narrow-point-min. syntax-ppss 
> would bind a local variable syntax-ppss-data to one of the first two 
> depending on the value of the third (and then modify its car and cdr 
> during the course of execution).

I'm in favour rather of setting syntax-ppss-{cache,last} to the
appropriate stored cache.  This will avoid needing to change the
function syntax-ppss much.

A disadvantage of using such a cons is in debugging.  It is more
difficult to understand a cons like this when it is printed out, than
the two component lists (which are difficult enough themselves).

> 2) Some extra vars serve to delay the actual clearing of the unused 
> cache until it's used again. It's a valid idea, but what if we try 
> without it at first? So syntax-ppss-flush-cache would always clear both 
> caches eagerly.

When there's a lot of buffer changing going on, it is an overhead having
to clear both (or several) caches continually.  (I'm thinking about the
possible extension to using an alist of caches, which could be quite
long.)

Also clearing both caches at the same time would be a bigger change to
syntax-ppss-flush-cache than it's suffered so far.

But I'm really not sure which way is better.

> The advantages:

> - Less code, easier to reason about.

> - Any package than advises syntax-ppss will have to juggle fewer global 
> variables.

I was intending that the new variables be purely internal, and that no
external elisp would need to access them.  I suppose I really ought to
have put "--" in the middle of their names.

> So Vatalie's polymode will have an easier time of it. It could even
> reuse some of the cache-while-narrowed logic by substituting the
> values of syntax-ppss-data-narrow and
> syntax-ppss-data-narrow-point-min as appropriate.

That sounds a little dangerous.  

> The obvious downside is, of course, extra indirection, which translates 
> to extra overhead. We don't know how significant it will be, though.

I wouldn't be keen on seeing lots of (car compound-variable) and (cdr
compound-variable) throughout the syntax-ppss function.  I think it
would make it significantly more difficult to understand.

> Would you like to see the code?

Yes, why not?

But just to make my position clear, I'm not particularly fixed on my
patch as submitted.  It was optimised for simplicity and correctness
rather than elegance, though I don't think it's too bad.  I'm fairly
open on whether we use your suggestions or Stefan's suggestion of having
an alist of caches.

-- 
Alan Mackenzie (Nuremberg, Germany).