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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Sep 10, 2017 at 18:53:53 -0400, Stefan Monnier wrote:
> > +;; Several caches.
> > +;; Because `syntax-ppss' is equivalent to (parse-partial-sexp
> > +;; (POINT-MIN) x), we need either to empty the cache when we narrow
> > +;; the buffer, which is suboptimal, or we need to use several caches.

> I think that (parse-partial-sexp 1 x) is more often what the caller
> wants than (parse-partial-sexp (point-min) x), but if you're happy with
> the behavior described by the docstring, then that's fine.

I've never been happy with the specification, partly for that reason,
but we are where we are, with lots of use of syntax-ppss, so I think it
needs fixing according to that spec.

> > +;; The implementation which follows uses three caches, the current one
> > +;; (in `syntax-ppss-cache' and `syntax-ppss-last') and two inactive
> > +;; ones (in `syntax-ppss-{cache,last}-{wide,narrow}'), which store the
> > +;; former state of the active cache as it was used in widened and
> > +;; narrowed buffers respectively.

> Earlier in the thread, I suggested to use a single cache indexed by the
> position of point-min (or by the position and point-min and by the
> current syntax-table, so as to also handle changes in the syntax-table),
> i.e. a list of (POINT-MIN-POS . CACHE-DATA) or
> ((POINT-MIN-POS . SYNTAX-TABLE) . CACHE-DATA).  I think it would lead to
> less code duplication than your patch which only handles 2 different
> POINT-MIN-POS (and one of the two has to be equal to 1), but existing
> code trumps hypothetical designs.

I deliberately kept the patch simple, avoiding even an alist with the
point-min position as key.  This would necessitate having an arbitrary
maximum length of alist, and continual manipulation of this list.  Not
difficult, I agree, but do we need it?  How often are there going to be
nested or alternating narrowing with enough calls to syntax-ppss to
cause the establishment of syntax-ppss-cache (as opposed to merely
syntax-ppss-last, which my patch doesn't consider sufficient reason to
store a new narrow-cache)?  (These aren't rhetorical questions, by the
way, but real ones.  Which is the best way forward?)

However, the patch was deliberately contructed to make the replacement
of the two-cache cache by an arbitrary length alist simple.

> So, I have no objections to the patch.  But I think (parse-partial-sexp
> (point-min) x) is a design bug in syntax-ppss which we will need to fix
> sooner or later, which is why I never bothered to implement something
> like your patch, which only makes the code do what its doc says rather
> than what the caller needs.

I couldn't agree more.  However, syntax-ppss is established and there
are callers that depend on its literal specification.  Maybe a way
forward would be to introduce a new function equivalent to
(parse-partial-sexp 1 x) and deprecate syntax-ppss.  However, a name
would need to be found for this new function, not an easy task.  ;-)
(syntax-ppss is a very good name, but couldn't be reused.)

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).