unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Pip Cet <pipcet@gmail.com>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: "João Távora" <joaotavora@gmail.com>, emacs-devel <emacs-devel@gnu.org>
Subject: Re: 31395511: "Don’t attempt to modify constant strings"
Date: Thu, 04 Jun 2020 07:26:03 +0000	[thread overview]
Message-ID: <87d06f5m2c.fsf@gmail.com> (raw)
In-Reply-To: <e105032e-8f4f-e143-ecb5-de8ea7d5e032@cs.ucla.edu> (Paul Eggert's message of "Wed, 3 Jun 2020 18:19:54 -0700")

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
back?)

> 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	other threads:[~2020-06-04  7:26 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-03 21:52 31395511: "Don’t attempt to modify constant strings" João Távora
2020-06-03 22:41 ` Paul Eggert
2020-06-03 22:52   ` Pip Cet
2020-06-03 23:20     ` Paul Eggert
2020-06-03 23:20     ` Basil L. Contovounesios
2020-06-03 22:41 ` Pip Cet
2020-06-03 23:08   ` Basil L. Contovounesios
2020-06-03 23:31     ` Basil L. Contovounesios
2020-06-03 23:48   ` João Távora
2020-06-04  0:43     ` Paul Eggert
2020-06-04  1:19       ` Paul Eggert
2020-06-04  7:26         ` Pip Cet [this message]
2020-06-04 11:11           ` Basil L. Contovounesios
2020-06-04 19:46             ` Paul Eggert
2020-06-04 20:25               ` João Távora
2020-06-04 20:29                 ` Paul Eggert
2020-06-04 21:21                   ` Drew Adams
2020-06-04 20:43               ` Pip Cet
2020-06-04 21:27                 ` Stefan Monnier
2020-06-04 21:42                   ` Pip Cet
2020-06-04 23:10                 ` Paul Eggert
2020-06-05  2:09                   ` Clément Pit-Claudel
2020-06-05  6:44                     ` Paul Eggert
2020-06-05 12:44                       ` Stefan Monnier
2020-06-05 17:01                     ` Drew Adams
2020-06-05  9:48                   ` Pip Cet
2020-06-05 18:37                     ` Paul Eggert
2020-06-04 22:33               ` Basil L. Contovounesios
2020-06-05 15:25       ` João Távora
2020-06-05 17:14         ` Dmitry Gutov
2020-06-05 23:19           ` João Távora
2020-06-05 23:32             ` Dmitry Gutov
2020-06-06  1:34               ` FW: " Drew Adams
2020-06-06  0:23             ` Drew Adams
2020-06-06  1:43             ` Paul Eggert
2020-06-06  4:06               ` Richard Stallman
2020-06-06 11:41               ` João Távora
2020-06-06 11:47                 ` João Távora
2020-06-04  4:38     ` Pip Cet
2020-06-04  9:31       ` João Távora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d06f5m2c.fsf@gmail.com \
    --to=pipcet@gmail.com \
    --cc=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    --cc=joaotavora@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).