[CTFE] enum "char[]" not correctly duplicated when used.

[dlangBugzillaToGithub]

monarchdodra reported this on 2013-09-02T04:50:59Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=10950

Description

//----
enum char[] s = "hello".dup;

void main()
{
  auto s1 = s;
  auto s2 = s;
  assert (s1.ptr != s2.ptr); //Fails;

  s1[0] = 'j';

  assert(s != "jello"); //Fails
}
//----

This is a pernicious issue, as it effectively modifies what is expected to be immutable initialization. In particular, it is triggered by the real life code:
enum char[] s = to!(char[])("hello");
enum char[] s = to!(char[])(1);
Where the "to!char[]" is written *specifically* to allow un-aliased write access.

This behavior was seen on DMD.2.063.2 on windows. I'll test linux later.

[dlangBugzillaToGithub]

monarchdodra commented on 2013-09-02T11:51:17Z

(In reply to comment #0)
Or, the contrary:

//----
import std.exception;

enum string s = assumeUnique(['a', 'b']);

void main()
{
  auto s1 = s;
  auto s2 = s;
  assert (s1.ptr == s2.ptr); //Fails;
}
//----

We are hitting this in this pull:
https://github.com/D-Programming-Language/phobos/pull/1540

The "problem" seems to be reduced to:


//----
module test;

import std.array;

string toStr()
{
    auto w = appender!string();
    w.put('1');
    return w.data;
}

void main()
{
    enum string s = toStr();
    string s1 = s;
    string s2 = s;
    assert(s1 == s2);
    assert(s1.ptr is s2.ptr); //HERE
}
//----

This used to pass in 2.063.2, but fails in 2.064ALPHA.

I don't want to say it's a regression, just noticing the behavior changed.

[dlangBugzillaToGithub]

monarchdodra commented on 2013-09-02T11:57:56Z

I think the "root" issue is that when the enum is first instantiated, the compiler doesn't care what the "marked" type of the enum is, but rather, what the actual (ctfe) memory points to.

//----
module test;

char[] cc()
{
    return cast(char[])"hello";
}

string ss()
{
    return cast(string)['a', 'b'];
}

void main()
{
    auto cc1 = cc; //*Should* duplicate
    auto cc2 = cc; //*Should* duplicate
    auto ss1 = ss; //Should *not* duplicate
    auto ss2 = ss; //Should *not* duplicate
    assert(cc1 !is cc2); //Failed to duplicate
    assert(ss1  is ss2); //Failed to not duplicate
}
//----

[dlangBugzillaToGithub]

monarchdodra commented on 2013-09-02T12:04:55Z

(In reply to comment #0)
> This behavior was seen on DMD.2.063.2 on windows. I'll test linux later.

As I thought:

//########
enum char[] s = "hello".dup;

void main()
{
    char[] s1 = s;
    s1[0] = 'j';
}
//########
--- killed by signal 11
//########

*THAT* is quite the error case.

[dlangBugzillaToGithub]

monarchdodra commented on 2013-09-03T06:18:53Z

(In reply to comment #2)
> I think the "root" issue is that when the enum is first instantiated, the
> compiler doesn't care what the "marked" type of the enum is, but rather, what
> the actual (ctfe) memory points to.
> 
> //----
> module test;
> 
> char[] cc()
> {
>     return cast(char[])"hello";
> }
> 
> string ss()
> {
>     return cast(string)['a', 'b'];
> }
> 
> void main()
> {
>     auto cc1 = cc; //*Should* duplicate
>     auto cc2 = cc; //*Should* duplicate
>     auto ss1 = ss; //Should *not* duplicate
>     auto ss2 = ss; //Should *not* duplicate
>     assert(cc1 !is cc2); //Failed to duplicate
>     assert(ss1  is ss2); //Failed to not duplicate
> }
> //----

Sorry, the example should have been:

enum cc = cast(char[])"hello";
enum ss = cast(string)['a', 'b'];

void main()
{
    auto cc1 = cc; //*Should* duplicate
    auto cc2 = cc; //*Should* duplicate
    auto ss1 = ss; //Should *not* duplicate
    auto ss2 = ss; //Should *not* duplicate
    assert(cc1 !is cc2); //Failed to duplicate
    assert(ss1  is ss2); //Failed to not duplicate
}

[dlangBugzillaToGithub]

clugdbug commented on 2013-09-04T01:24:33Z

You seem to be expecting the .dup to act as a macro, and be automatically applied to any code that uses the enum? But it doesn't work like that. The expression is evaluated to form a value. How the value was generated is irrelevant. CTFE expressions never behave like macros.

There were bugs in how it worked before (long ago, they were terrible, and accidentally behaved like macros. See bug 2414).

Bug 4397 is another case very similar to this. It's not an implementation issue. The root cause is that enums of reference types *do not make sense*. It's an accident that they were ever accepted, and is related to bug 2414.

None of your test cases should compile.

[dlangBugzillaToGithub]

monarchdodra commented on 2013-09-04T02:00:40Z

(In reply to comment #5)
> You seem to be expecting the .dup to act as a macro, and be automatically
> applied to any code that uses the enum?

No, not at all.

I expect that when I use CTFE that evaluates a function to initialize an enum, for it work correctly.

> None of your test cases should compile.

Should this compile?

//----
import std.exception;

string getString()
{
    char[] s = ['a', 'b'];
    return assumeUnique(s);
}
enum string myString = getString();

char[] getArray()
{
  string s = "hello";
  return s.dup;
}
enum char[] myArray = getArray();

unittest
{
  //myString is a string enum, so there should be no allocation.
  string a = myString;
  string b = myString;
  assert(a is b); //This fails. It shouldn't.
}
unittest
{
  //myArray is a char[], so these should allocate.
  char[] a = myArray;
  char[] b = myArray;
  assert(a !is b); //This fails. It shouldn't.
}
//----

Such usage is used extensively throughout phobos. In particular, things like:

enum someString = format(XXXXXXXXX);

If this is not valid code, then I am *completely* lost.

It turns out that when doing this, using "someString" will trigger an allocation on every use, which is contrary to the whole "string literals don't allocate". Unless I'm missing something, "someString" *is* a string literal, right?

The second useage is rarer, but the consequences of the failure are more problematic (mutables referencing shared immutable data).

I didn't fully understand 4397, but it does look like it's root issue.

[dlangBugzillaToGithub]

clugdbug commented on 2013-09-04T04:53:51Z

My thinking on this is still not completely clear.

If you have an enum, by definition it does not have an address - it's a manifest constant. If you are using it in an expression which needs an address, it has to make one up. Something bad is inevitable. Currently in many cases it duplicates the value whenever it needs the address, so you may get a different address each time.

There are two possible approaches I can see.
(1) Treat strings as requiring an address, therefore banning it at declaration.
Most of these should be changed from 'enum' into 'static const'.

(2) Allow the declaration, but don't permit the address to be used.
So, for example, you could declare:
enum int [] x = [23, 5, 66, 123];

and then later in the code, you could use x[3]. It gets const-folded away, so it's OK - there is no address.
But auto z = x;  should not compile because x has no runtime address.

This is the only legitimate use of an enum reference type, that I can think of.
The benefit of it, is that you can store CTFE results, so that they don't need to be used again. But it would be a simple optimisation to make a no-parameter CTFE function to remember its result, so I don't think it's a big win.

So I currently favour resolution (1).

Regardless of this, almost all occurrences of 'enum string' in Phobos should actually be 'static string'. They are causing executable bloat.