bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting

[Dmitry Gutov <dgutov@yandex.ru>]

Hi folks,

Sorry I'm late to this party.

On 13.08.2021 16:36, João Távora wrote:
>>>> By separating the filtering and mutation
>>>> (highlighting, scoring) my patch addresses the problem at hand in the
>>>> proper way.
>>>> [ ... ]
>>>> Mutation would be a reasonable choice here if the problem could not be
>>>> solved in a proper way.  But in fact it can be solved in a proper way
>>>> without mutating the strings at all as my patch shows.
>>> "proper" is just an reasonably empty adjective.  There are different ways to
>>> go about this, of course.  What's "proper" and isn't is hard to debate
>>> objectively.
>> You are contradicting yourself here. You agree that string mutation is
>> better be avoid. If we define "proper" as avoids string mutation if this
>> is easily possible, then my patch implements a proper solution to the> problem.
> I didn't say it's better avoided, though of course I will avoid_any_  change if
> I can. I said I have identified one drawback with doing it.  Then I
> have addressed
> that drawback. So that's what I said.
> 
> I am unaware of_other_  drawbacks.  They might exist, but I am unaware of
> them.  Perhaps you are, and indeed you state they exist, but you refuse to
> let me know about them.  Or perhaps others know of them and will let me know.
> In my long-running discussion with Dmitry they were not presented (again,
> except for the one I identified).

I thought I explained the problem with this previously.

It's basically this: we cannot mutate what we don't own. Across all of 
completion functions out there, there will be such that return "shared" 
strings (meaning, not copied or newly allocated) from their completion 
tables. And modifying them is bad, with consequences which can present 
themselves in unexpected, often subtle ways.

Since up until now completion-pcm--hilit-commonality copied all strings 
before modifying, completion tables such as described (with "shared" 
strings) have all been "legal". Suddenly deciding to stop supporting 
them would be a major API breakage with consequences that are hard to 
predict. And while I perhaps agree that it's an inconvenience, I don't 
think it's a choice we can simply make as this stage in c-a-p-f's 
development.

To give an example of a subtle consequence:

   1. (setq s (symbol-name 'car))

   2. (put-text-property 1 3 'face 'error s)

   3. Switch to a buffer in fundamental mode

   4. (insert (symbol-name 'car)) --> see the error face in the buffer

Now imagine that some completion table collects symbol names by passing 
obarray through #'symbol-name rather than #'all-completions, and voila, 
if the completion machinery adds properties (any properties, not just 
face) to those strings, you have just modified a bunch of global values. 
That's not good.

And in the example above, the values are those that the 
lispref/objects.texi says we should not change (though it gives 
(symbol-name 'cons) as example). "Not mutable", in its parlance. IIRC 
the related discussions mentioned that modifying such values could lead 
to a segfault in some previous Emacs versions. Maybe not anymore, but 
it's still not a good idea.