bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 143 bytes --]

'concat' hasn't guaranteed the allocation of a new string since 1997 at least (79d137ffe7d). Remove claims to the contrary from the manual.


[-- Attachment #2: 0001-Correct-concat-manual-entry.patch --]
[-- Type: application/octet-stream, Size: 1658 bytes --]

From 3c6d4f9cefc7571df26e390b94f1c39da08a78e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 9 Jul 2020 17:32:13 +0200
Subject: [PATCH] Correct 'concat' manual entry

* doc/lispref/strings.texi (Creating Strings): 'concat' does not
necessarily return a newly allocated string.  This has been the case
at least since 1997 (Emacs 20.3).
---
 doc/lispref/strings.texi | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 2ef88b9025..0aab326dc7 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -248,7 +248,7 @@ Creating Strings
 @defun concat &rest sequences
 @cindex copying strings
 @cindex concatenating strings
-This function returns a new string consisting of the characters in the
+This function returns a string consisting of the characters in the
 arguments passed to it (along with their text properties, if any).  The
 arguments may be strings, lists of numbers, or vectors of numbers; they
 are not themselves changed.  If @code{concat} receives no arguments, it
@@ -269,10 +269,6 @@ Creating Strings
 @end example
 
 @noindent
-This function always constructs a new string that is not @code{eq} to
-any existing string, except when the result is the empty string (to
-save space, Emacs makes only one empty multibyte string).
-
 For information about other concatenation functions, see the
 description of @code{mapconcat} in @ref{Mapping Functions},
 @code{vconcat} in @ref{Vector Functions}, and @code{append} in @ref{Building
-- 
2.21.1 (Apple Git-122.3)

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Thu, 9 Jul 2020 17:54:10 +0200
> 
> 'concat' hasn't guaranteed the allocation of a new string since 1997 at least (79d137ffe7d). Remove claims to the contrary from the manual.

This loses too much useful information, IMO.  Let's please say that
the result is sometimes ("frequently"?), but not always, a new string,
and that programs should not rely on that aspect.

Thanks.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

9 juli 2020 kl. 19.31 skrev Eli Zaretskii <eliz@gnu.org>:

> This loses too much useful information, IMO.  Let's please say that
> the result is sometimes ("frequently"?), but not always, a new string,
> and that programs should not rely on that aspect.

Here is a new version that adds this information, and recommends copy-sequence for mutability.


[-- Attachment #2: 0001-Correct-concat-manual-entry.patch --]
[-- Type: application/octet-stream, Size: 1768 bytes --]

From 4fcd3b13509ab94ddd552d3478c7c3cd9fe95279 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 9 Jul 2020 17:32:13 +0200
Subject: [PATCH] Correct 'concat' manual entry

* doc/lispref/strings.texi (Creating Strings): 'concat' does not
necessarily return a newly allocated string.  This has been the case
at least since 1997 (Emacs 20.3).
---
 doc/lispref/strings.texi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 2ef88b9025..455f3efc27 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -248,7 +248,7 @@ Creating Strings
 @defun concat &rest sequences
 @cindex copying strings
 @cindex concatenating strings
-This function returns a new string consisting of the characters in the
+This function returns a string consisting of the characters in the
 arguments passed to it (along with their text properties, if any).  The
 arguments may be strings, lists of numbers, or vectors of numbers; they
 are not themselves changed.  If @code{concat} receives no arguments, it
@@ -269,9 +269,9 @@ Creating Strings
 @end example
 
 @noindent
-This function always constructs a new string that is not @code{eq} to
-any existing string, except when the result is the empty string (to
-save space, Emacs makes only one empty multibyte string).
+The returned string cannot be relied upon to be unique or newly
+allocated; it should be considered immutable.  Use
+@code{copy-sequence} on the result if a mutable string is needed.
 
 For information about other concatenation functions, see the
 description of @code{mapconcat} in @ref{Mapping Functions},
-- 
2.21.1 (Apple Git-122.3)

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Thu, 9 Jul 2020 19:56:59 +0200
> Cc: 42296@debbugs.gnu.org
> 
> > This loses too much useful information, IMO.  Let's please say that
> > the result is sometimes ("frequently"?), but not always, a new string,
> > and that programs should not rely on that aspect.
> 
> Here is a new version that adds this information, and recommends copy-sequence for mutability.

That's not really what I asked for.

And how does mutability enter the picture?  We could say something
about it (but then we'd have to be less terse), but that doesn't in
any way replace the need to say that in many cases the value will be a
new string, IMO.

Thanks.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

9 juli 2020 kl. 20.51 skrev Eli Zaretskii <eliz@gnu.org>:

> That's not really what I asked for.

Then I misunderstood; would you explain what you mean in a different way?
The old text incorrectly stated that the return value always is a new string; the last patch changes this to saying that the value may or may not be a new string, and that the user therefore shouldn't assume that the value be mutable. If that was not what you asked for, then what was it, and why?

> And how does mutability enter the picture?  We could say something
> about it (but then we'd have to be less terse), but that doesn't in
> any way replace the need to say that in many cases the value will be a
> new string, IMO.

Sorry, but I still don't understand. What salient quality is there other than mutability? There is identity (uniqueness), but that is included as well. No user is ever worried about that the returned value may actually be a new string; it's very much the other way around.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> Feedback-ID:mattiase@acm.or
> From: Mattias Engdegård <mattiase@acm.org>
> Date: Thu, 9 Jul 2020 21:17:14 +0200
> Cc: 42296@debbugs.gnu.org
> 
> 9 juli 2020 kl. 20.51 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > That's not really what I asked for.
> 
> Then I misunderstood; would you explain what you mean in a different way?

I tried to explain that by showing the text I had in mind; I sent that
a minute ago.

> > And how does mutability enter the picture?  We could say something
> > about it (but then we'd have to be less terse), but that doesn't in
> > any way replace the need to say that in many cases the value will be a
> > new string, IMO.
> 
> Sorry, but I still don't understand. What salient quality is there other than mutability? There is identity (uniqueness), but that is included as well. No user is ever worried about that the returned value may actually be a new string; it's very much the other way around.

To my mind, immutability (or, rather, the adverse effects of modifying
the result of 'concat') is the result of the identity.  So it is
conceptually wrong to talk about mutability _instead_ of the identity.
Identity is the basic reason, immutability is the side effect.  Which
is what I tried to say in the text I suggested.

Thanks.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

9 juli 2020 kl. 21.24 skrev Eli Zaretskii <eliz@gnu.org>:

>  This function frequently, but not always, constructs a new string
>  that is not @code{eq} to any existing string.  Lisp programs should
>  not rely on the result being a new string nor on it being @code{eq}
>  to an existing string.
> 
>  When this function returns a string @code{eq] to another, changing
>  the result will also change that other string; to avoid that, use
>  @code{copy-sequence} on the result.

Thank you! First a minor detail: the word 'frequently' doesn't convey any useful information since the user isn't supposed to take any chances -- either the returned value is always new and unaliased, or there is no such guarantee. The frequency isn't relevant, and we shouldn't encourage the user to act as if it were by talking about it.

> To my mind, immutability (or, rather, the adverse effects of modifying
> the result of 'concat') is the result of the identity.  So it is
> conceptually wrong to talk about mutability _instead_ of the identity.
> Identity is the basic reason, immutability is the side effect.  Which
> is what I tried to say in the text I suggested.

I understand what you mean, but do not necessarily agree in every respect. A string may be unsuitable for modification for several (overlapping) reasons:

* It is a string literal that occurs in the program.
* It is a string produced by macro-expansion.
* It is a string resulting from other compile-time evaluation (eg, constant folding).
* It is a string residing in read-only storage of some kind (via dumping).
* It aliases another string in the program.
* It is an actual read-only string value (which we don't really have today, but we should not preclude its introduction in the future).

Thus mutability and identity are related concepts but neither clearly dominates the other; stating both is probably useful, for clarity if nothing else. Saying that mutating the return value merely entails changing some other string is understating and misleading: the consequences are not well-defined, and may include mutating programs and the raising of errors. (It will never reach C levels of undefined behaviour if we can help it, however.)

There is also the question whether the function can return one of its arguments. If so, the caller not only needs to abstain from mutating the returned value, but also the argument, even though it may be safely created by the caller. Currently, (concat x) never returns a string argument, unless it's a compile-time constant that cannot be mutated anyway. It's unclear whether we can guarantee this behaviour.

But let's be constructive: Taking your proposed text and performing some light surgery, we might end up with:

 This function does not always allocate a new string.  Callers should
 not rely on the result being a new string nor on it being @code{eq}
 to an existing string.

 In particular, the returned value should not be altered. To obtain
 a string that can be mutated, use @code{copy-sequence} on the result.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 10 Jul 2020 19:04:48 +0200
> Cc: 42296@debbugs.gnu.org
> 
> 9 juli 2020 kl. 21.24 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> >  This function frequently, but not always, constructs a new string
> >  that is not @code{eq} to any existing string.  Lisp programs should
> >  not rely on the result being a new string nor on it being @code{eq}
> >  to an existing string.
> > 
> >  When this function returns a string @code{eq] to another, changing
> >  the result will also change that other string; to avoid that, use
> >  @code{copy-sequence} on the result.
> 
> Thank you! First a minor detail: the word 'frequently' doesn't convey any useful information since the user isn't supposed to take any chances -- either the returned value is always new and unaliased, or there is no such guarantee. The frequency isn't relevant, and we shouldn't encourage the user to act as if it were by talking about it.

"Frequently" describes what actually happens.  Describing facts is not
"encouraging" users to do anything, especially since the very next
sentence tells them not to draw any far-reaching conclusions.

IOW, we should treat our users as grown-up adults, not as children
from whom we need to hide information.

>  This function does not always allocate a new string.  Callers should
>  not rely on the result being a new string nor on it being @code{eq}
>  to an existing string.
> 
>  In particular, the returned value should not be altered. To obtain
>  a string that can be mutated, use @code{copy-sequence} on the result.

Fine with me, except that "should not be altered": I object to that,
unless we explain why.  My proposed text included such an explanation;
without it, this looks like another dogma that someone sooner or later
will come up and challenge.

Thanks.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

10 juli 2020 kl. 20.08 skrev Eli Zaretskii <eliz@gnu.org>:

> Fine with me, except that "should not be altered": I object to that,
> unless we explain why.  My proposed text included such an explanation;
> without it, this looks like another dogma that someone sooner or later
> will come up and challenge.

Let's try again then, elaborating the second paragraph:

 This function does not always allocate a new string.  Callers should
 not rely on the result being a new string nor on it being @code{eq}
 to an existing string.

 In particular, the returned value should not be altered, because doing
 so may inadvertently change another string, alter a constant string in
 the program, or even raise an error.  To obtain a string that can be
 mutated, use @code{copy-sequence} on the result.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 11 Jul 2020 13:15:39 +0200
> Cc: 42296@debbugs.gnu.org
> 
>  In particular, the returned value should not be altered, because doing
>  so may inadvertently change another string, alter a constant string in
>  the program, or even raise an error.

Too definitive to my palate, sorry.

You may wish reading the recent long discussions of what is/should be
and what isn't/shouldn't be immutable.  Turns out some of us are very
sensitive to "should not" being used where "we recommend you not" or
"make sure you know what you are doing" is what actually is the truth.

Once again, we are not talking to kindergarten kids.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

11 juli 2020 kl. 13.52 skrev Eli Zaretskii <eliz@gnu.org>:

> Too definitive to my palate, sorry.

Apologies -- I do my best to read your mind.

> You may wish reading the recent long discussions of what is/should be
> and what isn't/shouldn't be immutable.

Thank you, but reading it once was quite enough.

>  Turns out some of us are very
> sensitive to "should not" being used where "we recommend you not" or
> "make sure you know what you are doing" is what actually is the truth.

Very well, here is a revised proposal:

 This function does not always allocate a new string.  Callers are
 advised not rely on the result being a new string nor on it being @code{eq}
 to an existing string.

 In particular, mutating the returned value may inadvertently change another
 string, alter a constant string in the program, or even raise an error.
 To obtain a string that can be mutated, use @code{copy-sequence} on the result.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 11 Jul 2020 14:51:06 +0200
> Cc: 42296@debbugs.gnu.org
> 
> 11 juli 2020 kl. 13.52 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > Too definitive to my palate, sorry.
> 
> Apologies -- I do my best to read your mind.

I posted my proposed text so that you won't have to.

>  This function does not always allocate a new string.  Callers are
>  advised not rely on the result being a new string nor on it being @code{eq}
>  to an existing string.
> 
>  In particular, mutating the returned value may inadvertently change another
>  string, alter a constant string in the program, or even raise an error.
>  To obtain a string that can be mutated, use @code{copy-sequence} on the result.

Fine with me, with one correction: last sentence will sound better if
reworded like this:

  To obtain a string that you can safely mutate, use
  @code{copy-sequence} on the result.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

11 juli 2020 kl. 15.07 skrev Eli Zaretskii <eliz@gnu.org>:

> Fine with me, with one correction:

Thank you -- correction applied, and result pushed to master (by mistake) and then to emacs-27.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 11 Jul 2020 15:16:24 +0200
> Cc: 42296-done@debbugs.gnu.org
> 
> Thank you -- correction applied, and result pushed to master (by mistake) and then to emacs-27.

Thanks.

Next time you have to do something like this, use "git cherry-pick -x"
to bring commits from master to the release branch, since that helps
merging in the opposite direction (I think the way you did it might
cause merge conflicts).

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Mattias Engdegård]

11 juli 2020 kl. 15.37 skrev Eli Zaretskii <eliz@gnu.org>:

> Next time you have to do something like this, use "git cherry-pick -x"
> to bring commits from master to the release branch, since that helps
> merging in the opposite direction (I think the way you did it might
> cause merge conflicts).

Understood.

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Drew Adams]

> >  This function does not always allocate a new string.  Callers are
> >  advised not rely on the result being a new string nor on it being
> @code{eq}
> >  to an existing string.
> >
> >  In particular, mutating the returned value may inadvertently change
> another
> >  string, alter a constant string in the program, or even raise an
> error.
> >  To obtain a string that can be mutated, use @code{copy-sequence} on
> the result.
> 
> Fine with me, with one correction: last sentence will sound better if
> reworded like this:
> 
>   To obtain a string that you can safely mutate, use
>   @code{copy-sequence} on the result.

FTR, I think that the doc of `concat' should really
make clear that it (now, and apparently for a while
now) has this odd (yes) behavior of not guaranteeing
a new string.  Not sure why we made this change, but
I imagine it was in a zeal to optimize.

At a minimum, please make this very clear, and say
explicitly that to ensure that you get a new string
you can use `copy-sequence' (as you've mentioned),
and that you can alternatively use `seq-concatenate':

 (seq-concatenate 'string &rest SEQUENCES)

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >  To obtain a string that can be mutated, use @code{copy-sequence} on the result.

  > Fine with me, with one correction: last sentence will sound better if
  > reworded like this:

  >   To obtain a string that you can safely mutate, use
  >   @code{copy-sequence} on the result.

Yes, it is almost always better to switch to active voice
and state the subject.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#42296: 27.0.91; Correct manual entry for 'concat' w.r.t. allocation [PATCH]

[Eli Zaretskii]

> Date: Thu, 09 Jul 2020 21:51:19 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 42296@debbugs.gnu.org
> 
> That's not really what I asked for.
> 
> And how does mutability enter the picture?  We could say something
> about it (but then we'd have to be less terse), but that doesn't in
> any way replace the need to say that in many cases the value will be a
> new string, IMO.

Here's what I had in mind:

  This function frequently, but not always, constructs a new string
  that is not @code{eq} to any existing string.  Lisp programs should
  not rely on the result being a new string nor on it being @code{eq}
  to an existing string.

  When this function returns a string @code{eq] to another, changing
  the result will also change that other string; to avoid that, use
  @code{copy-sequence} on the result.

WDYT?