Re: 31395511: "Don’t attempt to modify constant strings"

[Paul Eggert]

On 6/4/20 4:11 AM, Basil L. Contovounesios wrote:

> How would make-text-button detect whether its first argument is mutable?

It could try to mutate the string, and catch the error that is thrown when it's
not mutable. No such error is thrown now and Emacs can crash or worse - but I
plan to arrange for one to be thrown.

> Would it not suffice to clarify in its documentation that it modifies
> its argument, in the same way that we warn about passing immutable lists
> to nconc?

We could do that, yes. Some code passes string literals to make-text-button,
though, and we'd need to change it. The first example I found was ibuf-ext.el's
ibuffer-old-saved-filters-warning, which calls (make-text-button "here" ...).
Such code is already "broken" in some sense, so we'll need to fix it anyway 
somehow.

On 6/4/20 12:26 AM, Pip Cet wrote:

> I'm not sure the copy-sequence-unless-mutable semantics really
> make sense, though, as that might make bugs such as this one even harder
> to find.

True.

> I think we should add a new function with clean semantics, and throw an
> error in the old function if the string isn't "mutable", whatever that
> means in this context.

Throwing an error matches Basil's suggestion. What sort of clean semantics did
you have in mind?

> (I guess I can't modify the string contents or
> add text properties, but can I modify existing properties?  What about
> cons cells deep within the properties? If they're recursively immutable,
> what about markers and other objects that change state behind your
> back?)

The test I was thinking of is pretty simple: you can't modify the string object
itself, but you can modify the objects it points at. We could come up with
fancier tests later involving immutable property lists, but one thing at a time
and maybe this one thing is good enough (at least it should avoid the undefined
behavior).

> I'm still surprised my patch fixed the problem here (for some buttons,
> at least, for others there are a few more places that do the same
> thing...) but not for João.

There are several instances of the same problem in SLY. I found the ones in the
attached patch, and I expect there are others. So perhaps João was running into
one of the other problems.

sly.diff
Description: Text Data