6504 – Regression(2.041): "str" ~ [arr] allows string literal to be modified

for (int i=0; i<3; ++i)
{
 char[] x2 = "xxx" ~ ['c'];
 assert(x2[1] == 'x');
 x2[1] = 'q';
}
The assertion fails the second time through the loop.
What happens, is that "xxx" ~ ['c'] becomes a string literal of value "xxxc" and type char[]. Thus, it is modifiable.
This code passes on 2.040.

This bug was introduced in this commit, which fixed bug 5852.
https://github.com/D-Programming-Language/dmd/commit/5c7c6b51e27d9cd394ddda4f7940cdf9c1610953
This means that "xxx" ~ ['c'] is a string literal of type char[]
(that is, it's a _mutable_ string literal). Either this needs to be changed, or else a partial fix of bug 2356 needs to be made.

I'm going to argue this is not a bug.
While "xxx" is immutable, "xxx"~['c'] is mutable. Otherwise, it would be an error to use it to initialize x2.
Hence, x2 can modify it. Since x2 is a reference to the initializer, not a copy of it, the initializer is modified.
The spec says that string literals in the source code are immutable, not incidental string literals in the compiler that result from other operations.

(In reply to comment #2)
> I'm going to argue this is not a bug.
> 
> While "xxx" is immutable, "xxx"~['c'] is mutable. Otherwise, it would be an
> error to use it to initialize x2.
> 
> Hence, x2 can modify it. Since x2 is a reference to the initializer, not a copy
> of it, the initializer is modified.
> 
> The spec says that string literals in the source code are immutable, not
> incidental string literals in the compiler that result from other operations.
Reopened, because in every other respect, it behaves as an immutable string literal.
On Linux, the test case causes a segfault -- it's an attempt to modify an read-only literal.
On Windows, it's a secret global variable. You can even mark it as pure.
I don't think we can afford to have something so simple causing undefined behaviour.
Fixing this bug isn't a big deal, btw. Now that I've put an isCtfe flag in array literals, this case can go back to being an ArrayLiteral without hurting CTFE performance.

I found out it causes the seg fault after posting last night. I don't quite understand your suggestion. Do you have a fix that can be pulled?
I did try just not constant folding "string"~['c'] and letting that be done at runtime, but that caused interpret3.d to fail, as it relies on the constant folder to do that.
So I think that can work if you enhance CTFE to handle the "string"~['c'] case.

(In reply to comment #4)
> I found out it causes the seg fault after posting last night. I don't quite
> understand your suggestion. Do you have a fix that can be pulled?
I thought there was an existing pull request, but I was wrong (the closed pull457 is what I was thinking of, it's closely related but a little different).
> 
> I did try just not constant folding "string"~['c'] and letting that be done at
> runtime, but that caused interpret3.d to fail, as it relies on the constant
> folder to do that.
> 
> So I think that can work if you enhance CTFE to handle the "string"~['c'] case.
Yes, I think that I need to create CTFE-specific appender code anyway. It's the only case left where CTFE makes useless copies of array literals.
I will work on it tonight.

Commit pushed to master at https://github.com/D-Programming-Language/dmd
https://github.com/D-Programming-Language/dmd/commit/5fd81965b2155abd6c5f8bbfcb9d7b1460a36b87
6504 Regression(2.041): "str" ~ [arr] allows string literal to be modified
(a) Move slicing functions from interpret.c into constfold.c
(b) move existing string ~ [arr] -> string into interpret.c
(c) create string ~ [arr] -> [arr] for non-CTFE functions.
The CTFE slicing functions are used for the conversion, to avoid code
duplication.
Fixes bug 6504.

Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd
https://github.com/D-Programming-Language/dmd/commit/467cf6687f60dbb4180df036bd0508b9ce408d19
fix issue 6504, fix issue 7419