[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Pip Cet
Subject: Re: 31395511: "Don’t attempt to modify constant strings"
Date: Thu, 04 Jun 2020 07:26:03 +0000
User-agent: Gnus/5.13 (Gnus v5.13)

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 6/3/20 5:43 PM, Paul Eggert wrote:
>> On 6/3/20 4:48 PM, João Távora wrote:
>>> I think I'd rather this previous behavior were retained, or at least
>>> achievable by request.
>> It's tricky, as make-text-button in emacs-27 (and earlier) modifies
>> its string
>> argument, which is buggy because string constants are not always unique.
> On second thought, I'll work on coming up with a better workaround for the
> problem along the lines I suggested in my previous message:
> make-text-button can
> copy the button label string only if the string's not mutable. I hope
> this helps
> with SLY (as I hope SLY is not trying to modify string literals...).

It is. 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.

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. (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

> In the
> meantime I've reverted the change by installing the attached patch; this means
> the bug I mentioned in my previous message remains unfixed.

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. Maybe we're looking at a more complicated
bug, or maybe we haven't tested with the same kind of button (I entered
an unbound symbol and got the restart options, and those were buttons
with my patch but plain text without).

reply via email to

[Prev in Thread] Current Thread [Next in Thread]