GNU bug report logs - #79669
[PATCH] Add end position to symbols-with-position

Previous Next

To reply to this bug, email your comments to 79669 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add end position to symbols-with-position
Date: Tue, 21 Oct 2025 16:39:21 +0200

[Message part 1 (text/plain, inline)]

Tags: patch

Hi,

This draft patch extends SWP with an end position.  Without such an
extension, code that uses read-positioning-symbols needs to work hard to
"recover" the end position of a SWP (by going to its start position and
reading/searching forward for its end, as we now do for highlighting,
see commit bb54174c219).

If this looks reasonable, I'll also update the documentation and tests,
and post another patch.


Thanks,

Eshel

[0001-Add-end-position-to-symbols-with-position.patch (text/patch, attachment)]

Message #8 received at 79669 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: acm <at> muc.de, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 79669 <at> debbugs.gnu.org
Subject: Re: bug#79669: [PATCH] Add end position to symbols-with-position
Date: Tue, 21 Oct 2025 19:50:34 +0000

Hello, Eshel.

On Tue, Oct 21, 2025 at 16:39:21 +0200, Eshel Yaron wrote:
> Tags: patch

> Hi,

> This draft patch extends SWP with an end position.  Without such an
> extension, code that uses read-positioning-symbols needs to work hard to
> "recover" the end position of a SWP (by going to its start position and
> reading/searching forward for its end, as we now do for highlighting,
> see commit bb54174c219).

> If this looks reasonable, I'll also update the documentation and tests,
> and post another patch.

My gut feeling about this idea is "why?".

It would add some complexity to what, at the moment, is brutally simple.
For example, there would be the error condition (<= symbol-with-pos-end
symbol-with-pos-beg), which would need to be tested for somewhere or
other, and there would need to be an error symbol for when the condition
triggers.  None of that is needed at the moment.

What is the problem with the code "working hard" to find a symbol's end
position?  Surely all that is needed is to take symbol-with-pos-pos and
add the length of symbol-with-pos-sym.

What is the motivation for the proposed change?  Is it run time
optimisation?  If so, how much (quantatively) does the patch speed
something up?

Also, symbols with position is a documented feature in the elisp manual,
so there is a chance, small but non-zero, this could make some other
package non-compatible in either the last or next version of Emacs.

So, my feeling at the moment is that the disadvantages of this patch
outweigh the advantages.  I'm open to being persuaded otherwise.

> Thanks,

> Eshel

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #11 received at 79669 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eshel Yaron <me <at> eshelyaron.com>, 79669 <at> debbugs.gnu.org
Subject: Re: bug#79669: [PATCH] Add end position to symbols-with-position
Date: Tue, 21 Oct 2025 16:54:04 -0400

> It would add some complexity to what, at the moment, is brutally simple.

AFAICT Eshel's patch is brutally simple: it adds a field to the object
and then fills it with the readily-available value

    make_fixnum (readchar_offset)

and that's about it.  Seems a lot simpler than the rest of the existing
SWP code from where I stand.  So, I think we're just witnessing the fact
that complexity is in the eye of the beholder.

> What is the problem with the code "working hard" to find a symbol's end
> position?  Surely all that is needed is to take symbol-with-pos-pos and
> add the length of symbol-with-pos-sym.

No, the length of the symbol's name is not always equal to the
length of the text that represents the symbol.  Try for example:

   (mapcar (lambda (str)
             (cons (length str)
                   (length (symbol-name (car (read-from-string str))))))
           '("foo\\ bar" "##" "#:foo"))

And of course if you use `read-symbol-shorthands`.

> Also, symbols with position is a documented feature in the elisp manual,
> so there is a chance, small but non-zero, this could make some other
> package non-compatible in either the last or next version of Emacs.

Since the new field can be ignored, it should be easy to make the patch
fully backward compatible.  AFAICT the patch he sent is backward
compatible except for `position-symbol` where it removes the possibility
to pass an integer as the POS arg, but I don't know why.

> What is the motivation for the proposed change?  Is it run time
> optimisation?  If so, how much (quantatively) does the patch speed
> something up?

It'd be good to see such numbers, indeed.


        Stefan

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Alan Mackenzie <acm <at> muc.de>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 79669 <at> debbugs.gnu.org
Subject: Re: bug#79669: [PATCH] Add end position to symbols-with-position
Date: Wed, 22 Oct 2025 13:04:36 +0200

Hi,

Stefan Monnier writes:

>> It would add some complexity to what, at the moment, is brutally simple.
>
> AFAICT Eshel's patch is brutally simple: it adds a field to the object
> and then fills it with the readily-available value
>
>     make_fixnum (readchar_offset)
>
> and that's about it.  Seems a lot simpler than the rest of the existing
> SWP code from where I stand.  So, I think we're just witnessing the fact
> that complexity is in the eye of the beholder.
>
>> What is the problem with the code "working hard" to find a symbol's end
>> position?  Surely all that is needed is to take symbol-with-pos-pos and
>> add the length of symbol-with-pos-sym.
>
> No, the length of the symbol's name is not always equal to the
> length of the text that represents the symbol.  Try for example:
>
>    (mapcar (lambda (str)
>              (cons (length str)
>                    (length (symbol-name (car (read-from-string str))))))
>            '("foo\\ bar" "##" "#:foo"))
>
> And of course if you use `read-symbol-shorthands`.

Right.  Nice examples. :)

>> Also, symbols with position is a documented feature in the elisp manual,
>> so there is a chance, small but non-zero, this could make some other
>> package non-compatible in either the last or next version of Emacs.
>
> Since the new field can be ignored, it should be easy to make the patch
> fully backward compatible.  AFAICT the patch he sent is backward
> compatible except for `position-symbol` where it removes the possibility
> to pass an integer as the POS arg, but I don't know why.

Exactly, thanks.  As for the incompatible change in position-symbol,
what's important is to have some end position.  Requiring an existing
SWP gives us that, and makes for a nice and simple interface, I think.
But we can also preserve compatibility by setting the end position to
the start position, if we're only given the latter.  Additionally we can
have position-symbol accept a cons cell (BEG . END) as POS.  WDYT?

But I'm kinda struggling to think of (or find existing) good use cases
for calling position-symbol with a position instead of an existing SWP,
which is why I believe that this incompatibility might be acceptable.

>> What is the motivation for the proposed change?  Is it run time
>> optimisation?

Yes, the main motivation is optimizing semantic highlighting.  
(A bonus benefit would be simplifying the code.)

>> If so, how much (quantatively) does the patch speed
>> something up?
>
> It'd be good to see such numbers, indeed.

Yeah, I'll run some benchmarks and share the results.


Best,

Eshel

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Alan Mackenzie <acm <at> muc.de>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 79669 <at> debbugs.gnu.org
Subject: Re: bug#79669: [PATCH] Add end position to symbols-with-position
Date: Sun, 26 Oct 2025 15:35:05 +0100

Eshel Yaron <me <at> eshelyaron.com> writes:

>>> What is the motivation for the proposed change?  Is it run time
>>> optimisation?
>
> Yes, the main motivation is optimizing semantic highlighting.  
> (A bonus benefit would be simplifying the code.)
>
>>> If so, how much (quantatively) does the patch speed
>>> something up?
>>
>> It'd be good to see such numbers, indeed.
>
> Yeah, I'll run some benchmarks and share the results.

Well, so far my benchmarks do not show a significant speedup. :|
I tried two test cases:

1. (font-lock-fontify-region (point-min) (point-max)) in minibuffer.el
2. (elisp-scope-analyze-form #'elisp-fontify-symbol) on the definition
   of 'perform-replace'.

The main expected performance gain is avoiding the 'goto-char'+'read' in
'elisp-fontify-symbol', but it seems like the rest of the work we do in
this function (adding text properties) greatly outweighs this cost.

So I think we can hold off on this patch for now, and focus on other
optimizations first.


Eshel

Message #26 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: acm <at> muc.de, "Stefan Monnier via Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 79669 <at> debbugs.gnu.org
Subject: Re: bug#79669: [PATCH] Add end position to symbols-with-position
Date: Sun, 26 Oct 2025 14:44:29 +0000

Hello, Eshel.

On Sun, Oct 26, 2025 at 15:35:05 +0100, Eshel Yaron wrote:
> Eshel Yaron <me <at> eshelyaron.com> writes:

> >>> What is the motivation for the proposed change?  Is it run time
> >>> optimisation?

> > Yes, the main motivation is optimizing semantic highlighting.  
> > (A bonus benefit would be simplifying the code.)

> >>> If so, how much (quantatively) does the patch speed
> >>> something up?

> >> It'd be good to see such numbers, indeed.

> > Yeah, I'll run some benchmarks and share the results.

> Well, so far my benchmarks do not show a significant speedup. :|
> I tried two test cases:

> 1. (font-lock-fontify-region (point-min) (point-max)) in minibuffer.el
> 2. (elisp-scope-analyze-form #'elisp-fontify-symbol) on the definition
>    of 'perform-replace'.

> The main expected performance gain is avoiding the 'goto-char'+'read' in
> 'elisp-fontify-symbol', but it seems like the rest of the work we do in
> this function (adding text properties) greatly outweighs this cost.

> So I think we can hold off on this patch for now, and focus on other
> optimizations first.

Thanks for letting us know!

> Eshel

-- 
Alan Mackenzie (Nuremberg, Germany).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.