On 10/19/2015 02:44 PM, Jonathan M Davis wrote:
> On Monday, 19 October 2015 at 18:26:45 UTC, Adam D. Ruppe wrote:
>> On Monday, 19 October 2015 at 18:16:15 UTC, Andrei Alexandrescu wrote:
>>> Tangentially related: since when we allow field initialization with
>>> new? I was surprised to see that this works:
>>
>> Since CTFE started supporting it... this might actually be an
>> unintentional feature.
>>
>> Since the initializer is in a static context, the right hand side gets
>> CTFE'd, which means that actually points to an array in the data
>> segment... the *same* array in the data segment for all
>> initializations (the pointer is just blitted over with the rest of
>> init), which might be a bit surprising.
>>
>> I've seen a lot of people do this with classes not realizing it makes
>> a static instance!
>
> Yeah. It makes sense when you're dealing with an immutable object/arrays
> (and possibly const, assuming that it wasn't initialized with a mutable
> variable that was directly initialized via new), but it really makes no
> sense for mutable objects/arrays - not unless you have some weird case
> where you want each instance of your object to be able to share that
> member variable initially and then possibly stop sharing later. But
> given the high probability that someone is going to do this and shoot
> themselves in the foot, I think that we'd be a lot better off if we
> disallowed it.

Urgh. That looks like quite a bit of a bummer. -- Andrei

On Monday, 19 October 2015 at 19:53:14 UTC, Andrei Alexandrescu wrote:
> struct A {
>     int[] x = new int[10];
> }
>
> void main() {
>     import std.stdio;
>     A a;
>     a.x[1] = 42;
>     writeln(a.x);
> }
>
> Looks like a bona fide runtime array to me.

It is still in the static data segment. Try this:


struct A {
    int[] x = new int[10];
}

void main() {
    import std.stdio;
    A a;
    a.x[1] = 42;
    writeln(a.x);

    A a2;
    writeln(a2.x);

    assert(a.ptr is a2.ptr); // passes
}


The `new int[10]` is done at compile time and the pointer it produces is put into the .init for the struct. So the same pointer gets blitted over to ALL instances of `A`, meaning they alias the same data (until the slice gets reallocated by append or whatever).

On 10/19/2015 09:57 PM, Adam D. Ruppe wrote:
> On Monday, 19 October 2015 at 19:53:14 UTC, Andrei Alexandrescu wrote:
>> struct A {
>>     int[] x = new int[10];
>> }
>>
>> void main() {
>>     import std.stdio;
>>     A a;
>>     a.x[1] = 42;
>>     writeln(a.x);
>> }
>>
>> Looks like a bona fide runtime array to me.
>
> It is still in the static data segment. Try this:
>
>
> struct A {
>      int[] x = new int[10];
> }
>
> void main() {
>      import std.stdio;
>      A a;
>      a.x[1] = 42;
>      writeln(a.x);
>
>      A a2;
>      writeln(a2.x);
>
>      assert(a.ptr is a2.ptr); // passes
> }
>
>
> The `new int[10]` is done at compile time and the pointer it produces is
> put into the .init for the struct. So the same pointer gets blitted over
> to ALL instances of `A`, meaning they alias the same data (until the
> slice gets reallocated by append or whatever).

This is the worst part:

class C{
    int[] x=[1,2,3];
}

void main(){
    auto mut=new C;
    auto imm=new immutable(C);
    assert(imm.x[0]==1);
    mut.x[0]=2;
    assert(imm.x[0]==2);
}

On Monday, 19 October 2015 at 23:37:09 UTC, Timon Gehr wrote:
> This is the worst part:
>
> class C{
>     int[] x=[1,2,3];
> }
>
> void main(){
>     auto mut=new C;
>     auto imm=new immutable(C);
>     assert(imm.x[0]==1);
>     mut.x[0]=2;
>     assert(imm.x[0]==2);
> }

Oooo. Ouch. Yeah, that pushes it from being a good idea to disallow this in order to avoid bugs to a necessity to disallow it in order to avoid breaking the type system.

- Jonathan M Davis

On Tuesday, 20 October 2015 at 01:49:08 UTC, Jonathan M Davis wrote:
> On Monday, 19 October 2015 at 23:37:09 UTC, Timon Gehr wrote:
>> This is the worst part:
>>
>> class C{
>>     int[] x=[1,2,3];
>> }
>>
>> void main(){
>>     auto mut=new C;
>>     auto imm=new immutable(C);
>>     assert(imm.x[0]==1);
>>     mut.x[0]=2;
>>     assert(imm.x[0]==2);
>> }
>
> Oooo. Ouch. Yeah, that pushes it from being a good idea to disallow this in order to avoid bugs to a necessity to disallow it in order to avoid breaking the type system.

Yeah. But IMO it should still be allowed for immutable objects, and the error/deprecation message should say so, because for those it can actually be useful and there would be no other way to do it.

On Tuesday, 20 October 2015 at 11:36:36 UTC, Marc Schütz wrote:
> On Tuesday, 20 October 2015 at 01:49:08 UTC, Jonathan M Davis wrote:
>> On Monday, 19 October 2015 at 23:37:09 UTC, Timon Gehr wrote:
>>> This is the worst part:
>>>
>>> class C{
>>>     int[] x=[1,2,3];
>>> }
>>>
>>> void main(){
>>>     auto mut=new C;
>>>     auto imm=new immutable(C);
>>>     assert(imm.x[0]==1);
>>>     mut.x[0]=2;
>>>     assert(imm.x[0]==2);
>>> }
>>
>> Oooo. Ouch. Yeah, that pushes it from being a good idea to disallow this in order to avoid bugs to a necessity to disallow it in order to avoid breaking the type system.
>
> Yeah. But IMO it should still be allowed for immutable objects, and the error/deprecation message should say so, because for those it can actually be useful and there would be no other way to do it.

I believe that it's supposed to be possible to initialize an immutable member variable in a struct's constructor. So, the only thing that allowing member variables that are immutable objects or arrays to be directly initialized would add would be to make it so that they can be part of the init value. And in general, I'd strongly argue that it's bad practice to have either const or immutable member variables in a struct, because then it can never be assigned to even if the struct as a whole is supposed to be mutable. But that's a problem with member variables in general, not just those which refer to objects on the heap, so disallowing directly initializing with immutable objects/arrays on that basis wouldn't make sense IMHO. It just means that in most cases, it would be a bad idea for different reasons.

What I _do_ think is useful is having a member variable which is Rebindable so that it refers to an immutable object but isn't actually immutable itself (SysTime is supposed to do that but can't at the moment due to a compiler bug). But regardless, having a member variable directly initialized with an immutable object or array should be safe and correct. So, I agree that we should allow it. const could be depending (though it's probably easier not to), but mutable definitely has to go.

- Jonathan M Davis

On Monday, 19 October 2015 at 18:16:15 UTC, Andrei Alexandrescu wrote:
>
> Tangentially related: since when we allow field initialization with new? I was surprised to see that this works:
>

Thanks for this notice. I edited question on SO and I removed field initialization with new (replace class with struct). It is not related to my question and actually I wrote this code occasionally when I tried to reduce code to minimum working sample.

On 10/19/15 9:49 PM, Jonathan M Davis wrote:
> On Monday, 19 October 2015 at 23:37:09 UTC, Timon Gehr wrote:
>> This is the worst part:
>>
>> class C{
>>     int[] x=[1,2,3];
>> }
>>
>> void main(){
>>     auto mut=new C;
>>     auto imm=new immutable(C);
>>     assert(imm.x[0]==1);
>>     mut.x[0]=2;
>>     assert(imm.x[0]==2);
>> }
>
> Oooo. Ouch. Yeah, that pushes it from being a good idea to disallow this
> in order to avoid bugs to a necessity to disallow it in order to avoid
> breaking the type system.

Please file, thanks. -- Andrei

On 10/21/2015 12:55 PM, Andrei Alexandrescu wrote:
> On 10/19/15 9:49 PM, Jonathan M Davis wrote:
>> On Monday, 19 October 2015 at 23:37:09 UTC, Timon Gehr wrote:
>>> This is the worst part:
>>>
>>> class C{
>>>     int[] x=[1,2,3];
>>> }
>>>
>>> void main(){
>>>     auto mut=new C;
>>>     auto imm=new immutable(C);
>>>     assert(imm.x[0]==1);
>>>     mut.x[0]=2;
>>>     assert(imm.x[0]==2);
>>> }
>>
>> Oooo. Ouch. Yeah, that pushes it from being a good idea to disallow this
>> in order to avoid bugs to a necessity to disallow it in order to avoid
>> breaking the type system.
>
> Please file, thanks. -- Andrei
>

(I've filed it in 2013.
https://issues.dlang.org/show_bug.cgi?id=10376 .)

What should be the expected behaviour?

- Don't allow default initialization with mutable indirections in aggregates.

- Clone the mutable referenced data for each instance.

- Have different "init" for different mutability.

- ... ?

On 10/21/2015 07:40 AM, Timon Gehr wrote:
> On 10/21/2015 12:55 PM, Andrei Alexandrescu wrote:
>> On 10/19/15 9:49 PM, Jonathan M Davis wrote:
>>> On Monday, 19 October 2015 at 23:37:09 UTC, Timon Gehr wrote:
>>>> This is the worst part:
>>>>
>>>> class C{
>>>>     int[] x=[1,2,3];
>>>> }
>>>>
>>>> void main(){
>>>>     auto mut=new C;
>>>>     auto imm=new immutable(C);
>>>>     assert(imm.x[0]==1);
>>>>     mut.x[0]=2;
>>>>     assert(imm.x[0]==2);
>>>> }
>>>
>>> Oooo. Ouch. Yeah, that pushes it from being a good idea to disallow this
>>> in order to avoid bugs to a necessity to disallow it in order to avoid
>>> breaking the type system.
>>
>> Please file, thanks. -- Andrei
>>
>
> (I've filed it in 2013.
> https://issues.dlang.org/show_bug.cgi?id=10376 .)
>
> What should be the expected behaviour?
>
> - Don't allow default initialization with mutable indirections in
> aggregates.
>
> - Clone the mutable referenced data for each instance.
>
> - Have different "init" for different mutability.
>
> - ... ?

The quickest way to stop the bleeding is to disallow the code. It's incorrect for immutable data and misleading for mutable data. (What an user might expect is that each data comes with a distinct array.)

I can't believe I didn't know about this huge hole.


Andrei