https://issues.dlang.org/show_bug.cgi?id=5207

RazvanN <razvan.nitu1305@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |razvan.nitu1305@gmail.com
         Resolution|---                         |WONTFIX

--- Comment #4 from RazvanN <razvan.nitu1305@gmail.com> ---
There is nothing wrong this code. Variables are default initialized so it is up to the user to use the variable inside the constructor however he pleases (this includes using the variable with the default initialized value). This is not going to change.

Closing as WONTFIX.

--

https://issues.dlang.org/show_bug.cgi?id=5207

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|WONTFIX                     |---

--- Comment #5 from hsteoh@quickfur.ath.cx ---
Are you sure?  I think the complaint here is that `i` should only be initializable once in the ctor, and should not be readable until then. Otherwise, while inside the ctor immutability is violated, and strange things can happen (e.g., the ctor can call a function that receives an immutable value, then mutate it afterwards, causing an inconsistent state).

The fact that the ctor can initialize the immutable is not the complaint here.

--

https://issues.dlang.org/show_bug.cgi?id=5207

--- Comment #6 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to hsteoh from comment #5)
> Are you sure?  I think the complaint here is that `i` should only be initializable once in the ctor, and should not be readable until then. Otherwise, while inside the ctor immutability is violated, and strange things can happen (e.g., the ctor can call a function that receives an immutable value, then mutate it afterwards, causing an inconsistent state).
> 

I don't see how this could happen. The value is immutable and therefore it cannot be changed by anyone else other than the constructor. And that is true only for the first assignment. In the reported example how is it possible to change i from a different function?

> The fact that the ctor can initialize the immutable is not the complaint here.

I understand what the enhancement request asks for, however, this cannot be enforced. Imagine this case:

struct A
{
    immutable int i;
    this(int k)
    {
        doSomething();
        i = k;
    }

    void doSomething()
    {
        int a = i;
    }
}

To reject this case the compiler should look at doSomething's body and do some complicated dataflow analysis. And for what gain? It is fine to read un-initialized variables because they have a default value. What advantage would we gain if we forbid that? I think that this enhancement request does actually bring any benefit, it just complicates the dataflow analysis for no gain.

--

https://issues.dlang.org/show_bug.cgi?id=5207

--- Comment #7 from hsteoh@quickfur.ath.cx ---
Here's an example of a problematic case:

----------
import std;

struct S {
        int x;
}

immutable S s;
immutable(S)* ptr;

void fun(immutable(S)* p) {
        ptr = p;
}

shared static this() {
        fun(&s);

        writeln(ptr.x); // prints 0

        s.x = 1;
        writeln(ptr.x); // prints 1

        s.x = 2;
        writeln(ptr.x); // prints 2: immutability broken
}
----------

Since `ptr` points to an immutable value, one expects that `ptr.x` should not change. Correspondingly, an optimizing compiler reserves the right to elide subsequent loads, thus causing different output.

And indeed, if the above code is refactored as follows:

----------
import std;

struct S {
        int x;
}

immutable S s;
immutable S* ptr;

shared static this() {
        ptr = &s;

        writeln(ptr.x); // prints 0

        s.x = 1;
        writeln(ptr.x); // prints 1

        s.x = 2;
        writeln(ptr.x); // prints 2: immutability broken
}

void main() {
        writeln(ptr.x);
}
----------

Then the output becomes:

----------
0
0
0
2
----------

Clearly, the compiler has optimized away the loads of `ptr.x` because of the immutability of `ptr`, but actually that was a wrong assumption because the supposedly immutable value *did* change, as proven by what main() sees afterwards. This causes inconsistent behaviour in either case, the root cause of which is an immutable value being assignable multiple times inside a ctor.

Immutable values should not be readable before initialization (the first writeln should be illegal), and should only be initializable once.

--

https://issues.dlang.org/show_bug.cgi?id=5207

--- Comment #8 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to hsteoh from comment #7)
> Here's an example of a problematic case:

In the code that you have posted there seems to be an error with regards to module constructors, because if you put the same code in a normal constructor:

import std;

struct S {
        int x;
        this(int k)
        {
            fun(&s);

            writeln(ptr.x); // prints 0

            s.x = 1;
            writeln(ptr.x); // prints 1

            s.x = 2;
            writeln(ptr.x); // prints 2: immutability broken

        }

}

immutable S s;
immutable(S)* ptr;

void fun(immutable(S)* p) {
        ptr = p;
}

you will get errors when trying to modify `s`. The bug in the example that you posted is that `s` is not typechecked correctly in the module constructor, not the fact that you should not be able to read an immutable variable before initialization.

--

https://issues.dlang.org/show_bug.cgi?id=5207

--- Comment #9 from RazvanN <razvan.nitu1305@gmail.com> ---
Thinking more about this, isn't the idea of module constructors to not type check qualifiers because it is considered that they are used to initialize data before the program is actually run? For example, this works:

immutable int a;

shared static this()
{
    a = 2;
    a = 3;
}

I guess this should be documented.

--

https://issues.dlang.org/show_bug.cgi?id=5207

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P3

--

https://issues.dlang.org/show_bug.cgi?id=5207

Nick Treleaven <nick@geany.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nick@geany.org

--- Comment #10 from Nick Treleaven <nick@geany.org> ---
> isn't the idea of module constructors to not type check qualifiers because it is considered that they are used to initialize data before the program is actually run?

No, because the compiler does check the immutable qualifier for `s` below.

struct S
{
    int* p;
}
immutable S s;

shared static this()
{
    int* p = new int;
    s = S(p); // Error: cannot implicitly convert expression `S(p)` of type `S`
to `immutable(S)`
}

It would also be inconsistent with class constructor immutable field initialization if immutable was completely ignored in a shared static constructor - see issue 24449.

--

https://issues.dlang.org/show_bug.cgi?id=5207

--- Comment #11 from Nick Treleaven <nick@geany.org> ---
(In reply to hsteoh from comment #7)
> immutable S s;
> immutable S* ptr;
> 
> shared static this() {
> 	ptr = &s;
> 
> 	writeln(ptr.x); // prints 0
> 
> 	s.x = 1;

Interestingly, if it worked like struct/class constructors, you would get an error:

struct S
{
    immutable int x;
    immutable int* p;
    this(int)
    {
        p = &x; // compiler counts this as initializing x!
        x = 5; // Error: immutable field `x` initialized multiple times
    }
}

It also says 'Previous initialization is here.' for the `p = &x` line.

--