www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Why allow initializers of non-static members that allocate?

reply Bastiaan Veelo <Bastiaan Veelo.net> writes:
I have been foolish enough to make a mistake like this:
```d
struct S
{
     int[] arr = new int[](5);
}
```

This is terrible because
```d
S s1;
S s2;
s2.arr[0] = 42;
writeln(s1.arr[0]); // 42 Gotcha!
```

Of course there are less obvious variants of the same mistake:
```d
import std;

struct S
{
     A a = A(5);
}

struct A
{
     int[] arr;
     this (int l)
     {
         arr.length = l;
     }
}

void main()
{
     S s1;
     S s2;
     s2.a.arr[0] = 42;
     writeln(s1.a.arr[0]); // 42 :-(
}
```

Is there a use case where this makes sense? I would have much 
appreciated the compiler slapping me on the fingers, but it 
doesn't. I understand that it is safe and that the compiler can 
allow this, but why would anyone want that? D-scanner does not 
check for this either.

I think a helpful error message would be: "Error: The initializer 
`A(5)` allocates memory that is shared among all instances of 
`S`. If you want that, make `S.a` `static`."

-- Bastiaan.
Jun 10 2022
next sibling parent reply Mike Parker <aldacron gmail.com> writes:
On Friday, 10 June 2022 at 07:35:17 UTC, Bastiaan Veelo wrote:


 Is there a use case where this makes sense? I would have much 
 appreciated the compiler slapping me on the fingers, but it 
 doesn't. I understand that it is safe and that the compiler can 
 allow this, but why would anyone want that? D-scanner does not 
 check for this either.


Any initialization of a member field is overriding the field's 
`.init` value for the type. If a dynamic allocation set a 
different value per instance, then you'd have inconsistent 
behavior with, e.g., `int a = 5`.



 I think a helpful error message would be: "Error: The 
 initializer `A(5)` allocates memory that is shared among all 
 instances of `S`. If you want that, make `S.a` `static`."


I understand that it's not something that people expect, but 
making it an error can't be the answer. And making it a static 
field is not the same thing.

I think this is a case where having a warning that's on by 
default, and which can be explicitly disabled, is useful. "Blah 
blah .init blah blah. See link-to-something-in-docs. Is this what 
you intended?"
Jun 10 2022
next sibling parent reply Mike Parker <aldacron gmail.com> writes:
On Friday, 10 June 2022 at 07:46:36 UTC, Mike Parker wrote:


 I think this is a case where having a warning that's on by 
 default, and which can be explicitly disabled, is useful. "Blah 
 blah .init blah blah. See link-to-something-in-docs. Is this 
 what you intended?"


And it *is* documented:


 Struct fields are by default initialized to whatever the 
 Initializer for the field is, and if none is supplied, to the 
 default initializer for the field's type.
 The default initializers are evaluated at compile time.


https://dlang.org/spec/struct.html#default_struct_init
Jun 10 2022
next sibling parent Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Friday, 10 June 2022 at 07:49:43 UTC, Mike Parker wrote:

 And it *is* documented:


 Struct fields are by default initialized to whatever the 
 Initializer for the field is, and if none is supplied, to the 
 default initializer for the field's type.
 The default initializers are evaluated at compile time.


 https://dlang.org/spec/struct.html#default_struct_init


Yes, that section I find open for interpretation, because I don't 
think pointer values can be determined at compiler time. I assume 
S.init is constructed at program initialization, which is then 
blitted into any new instance of S.

-- Bastiaan.
Jun 10 2022
prev sibling parent reply matheus <matheus gmail.com> writes:
On Friday, 10 June 2022 at 07:49:43 UTC, Mike Parker wrote:

 ...
 And it *is* documented:


 Struct fields are by default initialized to whatever the 
 Initializer for the field is, and if none is supplied, to the 
 default initializer for the field's type.
 The default initializers are evaluated at compile time.


 https://dlang.org/spec/struct.html#default_struct_init


Hmm I understand the text but for this intializers I was 
expecting new address with length 5, for example:

import std.stdio;

struct S{
     int[] arr = new int[](5);
     int[2] arr2;
}

void main(){
     S s1, s2;

     s2.arr[0] = 42;
     writeln(s1.arr[0], " address: ", &s1.arr[0]);
     writeln(s2.arr[0], " address: ", &s2.arr[0]);

     s2.arr2[0] = 10;
     writeln(s1.arr2[0], " address: ", &s1.arr2[0]);
     writeln(s2.arr2[0], " address: ", &s2.arr2[0]);
}

Prints:

42 address: 5586F5BD9CC0
42 address: 5586F5BD9CC0
  0 address: 7FFFC65C7A20
10 address: 7FFFC65C7A40


And as it can be seen: s1.arr and s2.arr shares the same address.

So, in the case of "int[] arr = new int[](5)", an array of length 
5 of type int will be instantiated and its address will be shared 
among whoever instantiates "S" and be pointed and accessed 
through arr.

In the second case, "int[2] arr2", 2 consecutive integer spaces 
in memory will be allocate independently for each "instantiation" 
of "S", so different address.

I never saw this before (I mean I never wrote the first case), 
I'm used to the "int[2] arr2;" way of declaring it, but if I had 
looked this code without knowing this, I'd be certain that s1.arr 
and s2.arr would have different addresses.

This is a bit weird (At least for a newbie like me), I really 
think the compiler should emit an warning about this.

Matheus.
Jun 10 2022
parent reply Mike Parker <aldacron gmail.com> writes:
On Saturday, 11 June 2022 at 01:14:06 UTC, matheus wrote:


 So, in the case of "int[] arr = new int[](5)", an array of 
 length 5 of type int will be instantiated and its address will 
 be shared among whoever instantiates "S" and be pointed and 
 accessed through arr.

 In the second case, "int[2] arr2", 2 consecutive integer spaces 
 in memory will be allocate independently for each 
 "instantiation" of "S", so different address.

 I never saw this before (I mean I never wrote the first case), 
 I'm used to the "int[2] arr2;" way of declaring it, but if I 
 had looked this code without knowing this, I'd be certain that 
 s1.arr and s2.arr would have different addresses.


That's because static arrays are allocated as part of the 
instance:

```d
struct Foo {
     int[] dyn;
}

struct Bar {
     int[10] stat;
}

assert(Foo.sizeof == 16);
assert(Bar.sizeof == 40);
```


 This is a bit weird (At least for a newbie like me), I really 
 think the compiler should emit an warning about this.


At it's core this is just a matter of knowing how two specific 
language features behave (allocation of static vs. dynamic arrays 
coupled with member field initialization). If you aren't aware of 
it and it bites you, then you learn about it and you know it. So 
would you then really want a warning every time you initialize a 
static array field?

People getting bit by `new` in field initialization often enough 
that I think a warning would be helpful. But any such warnings 
need to be enabled by default to be useful, and must have an off 
switch for people who don't need them. So the question in each 
case would be, where's the line between helpful and annoying?

The compiler should be as helpful as it can, but it has to be 
helpful without getting in the way. There's a significant amount 
of learning by trial and error in any programming language. So I 
think there has to be a distinction between things like "easy to 
do by accident even when you know the deal"  and "once you learn 
it, you're unlikely to do it again".
Jun 10 2022
next sibling parent reply matheus <matheus gmail.com> writes:
On Saturday, 11 June 2022 at 01:52:58 UTC, Mike Parker wrote:

 ...
 That's because static arrays are allocated as part of the 
 instance:
 ...


Yes I understood the problem, but the naive me was thinking that 
in this example:

struct S{
     int[] arr = new int[](5);
}

For some reason this would be transformed as a statically "int[5] 
arr" in the case of structs.

But now I see that's not the case and "arr" points to whatever 
was allocated by "new", and will share across the instances of S 
(Since it's the same address).


 ... So would you then really want a warning every time you 
 initialize a static array field?


Well would this be annoying? Yes, mainly if I already know this, 
but if not, then it would be like a nice to know warning for 
newbies like myself. By the way this would be only in the cases 
that a static array field being initialized with dynamic 
allocation like in the case of struct declaration, not on every 
place.


 ...
 So the question in each case would be, where's the line between 
 helpful and annoying?


Well I like to think about this like the "-Wall" flag in C 
compilers, you need to go for it, and you just dismiss the 
warning in such case if you don't agree or know what you're doing.


 ...
 The compiler should be as helpful as it can, but it has to be 
 helpful without getting in the way. There's a significant 
 amount of learning by trial and error in any programming 
 language. So I think there has to be a distinction between 
 things like "easy to do by accident even when you know the 
 deal"  and "once you learn it, you're unlikely to do it again".


Yes no doubt about it, by the way like I said, I never used this 
form to initialize fields in structs, I always used the "T[n] 
member", but in this particular case the result would have been 
very different from what I expected.

Anyway I'm always lurking around this Forum and learning new 
things.

Thanks,

Matheus.

PS: I'm ESL so sorry for my English mistakes.
Jun 10 2022
parent Salih Dincer <salihdb hotmail.com> writes:
On Saturday, 11 June 2022 at 02:22:28 UTC, matheus wrote:

 Well would this be annoying? Yes, mainly if I already know 
 this, but if not, then it would be like a nice to know warning 
 for newbies like myself. By the way this would be only in the 
 cases that a static array field being initialized with dynamic 
 allocation like in the case of struct declaration, not on every 
 place.


I agree...

I'd be wrong too if I didn't know that while reading the code. I 
think there should be a warning when compiling.

Really, is it so hard to put up a warning?

SDB 79
Jun 10 2022
prev sibling parent reply Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Saturday, 11 June 2022 at 01:52:58 UTC, Mike Parker wrote:

 People getting bit by `new` in field initialization often 
 enough that I think a warning would be helpful.


The problem is so much bigger because it is not just a case of 
being aware not to use `new` in member initialisers. As you see 
in my second example in 
https://forum.dlang.org/post/ogvubzgprghefclgluce forum.dlang.org 
there is no `new` anywhere. In fact you could say an effort has 
been made to do the right thing in `struct A`  where the 
allocation has been moved to the constructor (a strategy that is 
not always available because structs don’t have default 
constructors) yet we fell into the same trap.

My point is that this problem can be buried deep down under 
multiple layers and you can’t really ever be sure that there 
isn’t a problem in your massive code base.


 But any such warnings need to be enabled by default to be 
 useful, and must have an off switch for people who don't need 
 them. So the question in each case would be, where's the line 
 between helpful and annoying?


So that’s why I used “why” in the title of this thread, which I 
haven’t seen an answer to yet. What is the practical case where 
that warning would be annoying? When would you actually want this 
behaviour?

— Bastiaan.
Jun 11 2022
parent reply Mike Parker <aldacron gmail.com> writes:
On Saturday, 11 June 2022 at 10:37:30 UTC, Bastiaan Veelo wrote:


 So that’s why I used “why” in the title of this thread, which I 
 haven’t seen an answer to yet. What is the practical case where 
 that warning would be annoying? When would you actually want 
 this behaviour?


After actually stopping to think about it (my earlier responses 
were reflexive), I agree with your initial assessment that it 
should be an error. It really only makes sense to allow the 
dynamic allocation if the fields are immutable and, in the case 
of arrays, the initializer is a literal.
Jun 11 2022
parent Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Saturday, 11 June 2022 at 10:52:58 UTC, Mike Parker wrote:

 I agree with your initial assessment that it should be an 
 error. It really only makes sense to allow the dynamic 
 allocation if the fields are immutable and, in the case of 
 arrays, the initializer is a literal.


đź‘Ť
Jun 11 2022
prev sibling next sibling parent Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Friday, 10 June 2022 at 07:46:36 UTC, Mike Parker wrote:

 On Friday, 10 June 2022 at 07:35:17 UTC, Bastiaan Veelo wrote:


 Is there a use case where this makes sense? I would have much 
 appreciated the compiler slapping me on the fingers, but it 
 doesn't. I understand that it is safe and that the compiler 
 can allow this, but why would anyone want that? D-scanner does 
 not check for this either.


 Any initialization of a member field is overriding the field's 
 `.init` value for the type. If a dynamic allocation set a 
 different value per instance, then you'd have inconsistent 
 behavior with, e.g., `int a = 5`.


Yes, I understand that the compiler can't do what I was expecting 
it to do, it was a mistake.


 I think a helpful error message would be: "Error: The 
 initializer `A(5)` allocates memory that is shared among all 
 instances of `S`. If you want that, make `S.a` `static`."


 I understand that it's not something that people expect, but 
 making it an error can't be the answer. And making it a static 
 field is not the same thing.


It's not the same thing, therefore I was hoping to see a use case 
for it -- but just because I'm curious; Preventing the mistake is 
my main thing.


 I think this is a case where having a warning that's on by 
 default, and which can be explicitly disabled, is useful. "Blah 
 blah .init blah blah. See link-to-something-in-docs. Is this 
 what you intended?"


That would be fine too. By the way, if there is 
something-in-docs, I don't think it is prominent enough...


-- Bastiaan.
Jun 10 2022
prev sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/10/22 3:46 AM, Mike Parker wrote:

 On Friday, 10 June 2022 at 07:35:17 UTC, Bastiaan Veelo wrote:
 

 Is there a use case where this makes sense? I would have much 
 appreciated the compiler slapping me on the fingers, but it doesn't. I 
 understand that it is safe and that the compiler can allow this, but 
 why would anyone want that? D-scanner does not check for this either.


 
 Any initialization of a member field is overriding the field's `.init` 
 value for the type. If a dynamic allocation set a different value per 
 instance, then you'd have inconsistent behavior with, e.g., `int a = 5`.


Yes, it can't be done the way that is expected (which is common to many 
languages): allocate one for every instance.


 I think a helpful error message would be: "Error: The initializer 
 `A(5)` allocates memory that is shared among all instances of `S`. If 
 you want that, make `S.a` `static`."


 
 I understand that it's not something that people expect, but making it 
 an error can't be the answer. And making it a static field is not the 
 same thing.
 
 I think this is a case where having a warning that's on by default, and 
 which can be explicitly disabled, is useful. "Blah blah .init blah blah. 
 See link-to-something-in-docs. Is this what you intended?"


Here the language is being extremely unsafe.

Not only is the field shared between instances, it's shared across 
instances in *different threads*.

Discovered circa 2009: https://issues.dlang.org/show_bug.cgi?id=2947

It should be illegal to declare a field this way that has mutable 
references without being `shared`. End of story.

-Steve
Jun 10 2022
next sibling parent reply Mike Parker <aldacron gmail.com> writes:
On Friday, 10 June 2022 at 14:56:24 UTC, Steven Schveighoffer 
wrote:


 Discovered circa 2009: 
 https://issues.dlang.org/show_bug.cgi?id=2947

 It should be illegal to declare a field this way that has 
 mutable references without being `shared`. End of story.

 -Steve


The docs do say that:


 The default initializers may not contain references to mutable 
 data.
Jun 10 2022
parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/10/22 1:20 PM, Mike Parker wrote:

 On Friday, 10 June 2022 at 14:56:24 UTC, Steven Schveighoffer wrote:
 

 Discovered circa 2009: https://issues.dlang.org/show_bug.cgi?id=2947

 It should be illegal to declare a field this way that has mutable 
 references without being `shared`. End of story.


 
 The docs do say that:
 

 The default initializers may not contain references to mutable data.




I guess it was added but no implementation done for it? There's not even 
a warning!

https://github.com/dlang/dlang.org/pull/2331/files#diff-dee70e840ada3a820b3a373d649812b1157fd584ed0837e4a937ea3418f093e3R157

Anyways, a documented rule that's so easily enforced but is not actually 
enforced is pretty worthless.

-Steve
Jun 10 2022
prev sibling parent Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Friday, 10 June 2022 at 14:56:24 UTC, Steven Schveighoffer 
wrote:

 On 6/10/22 3:46 AM, Mike Parker wrote:

 I think this is a case where having a warning that's on by 
 default, and which can be explicitly disabled, is useful. 
 "Blah blah .init blah blah. See link-to-something-in-docs. Is 
 this what you intended?"


 Here the language is being extremely unsafe.

 Not only is the field shared between instances, it's shared 
 across instances in *different threads*.

 Discovered circa 2009: 
 https://issues.dlang.org/show_bug.cgi?id=2947


Thanks for the pointer. #dbugfix 2947


 It should be illegal to declare a field this way that has 
 mutable references without being `shared`. End of story.
Jun 10 2022
prev sibling parent reply Salih Dincer <salihdb hotmail.com> writes:
On Friday, 10 June 2022 at 07:35:17 UTC, Bastiaan Veelo wrote:

 I have been foolish enough to make a mistake like this:
 ```d
 struct S
 {
     int[] arr = new int[](5);
 }
 ```


Well, if the b's may not be equal, there's a simple solution.  
But why are a's like that, they're not static!

```d
void main()
{
   struct S(size_t size)
   {
     int[] arr = new int[size];
   }

   S!5 a1, a2;
   assert(a1.arr.ptr == a2.arr.ptr);

   S!5 b1;
   S!6 b2;
   assert(b1.arr.ptr != b2.arr.ptr);
}
```
SDB 79
Jun 10 2022
parent Arafel <er.krali gmail.com> writes:
On 10/6/22 14:58, Salih Dincer wrote:

 On Friday, 10 June 2022 at 07:35:17 UTC, Bastiaan Veelo wrote:

 I have been foolish enough to make a mistake like this:
 ```d
 struct S
 {
     int[] arr = new int[](5);
 }
 ```


 
 Well, if the b's may not be equal, there's a simple solution. But why 
 are a's like that, they're not static!
 
 ```d
 void main()
 {
    struct S(size_t size)
    {
      int[] arr = new int[size];
    }
 
    S!5 a1, a2;
    assert(a1.arr.ptr == a2.arr.ptr);
 
    S!5 b1;
    S!6 b2;
    assert(b1.arr.ptr != b2.arr.ptr);
 }
 ```
 SDB 79


Because the `arr` are created for each instantiation of the template.

All S!5 share one default value, so you could also:

```d
assert (a1.arr.ptr == b1.arr.ptr);
```

However, S!6 becomes a completely different struct, and thus gets a 
different default `arr`.

Note that this would also happen with static members:

```d
struct S(int T) {
     static int foo;
}

static assert(&S!1.foo !is &S!2.foo);

void main() { }
```
Jun 10 2022