Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disallow modification of immutable in constructor after calling base ctor #19573

Open
dlangBugzillaToGithub opened this issue May 31, 2019 · 7 comments
Labels

Comments

@dlangBugzillaToGithub
Copy link

Steven Schveighoffer (@schveiguy) reported this on 2019-05-31T14:38:23Z

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

CC List

Description

This breaks immutability:

import std.stdio;
class C
{
    void foo() { writeln("C");}
    this()
    {
        foo();
    }
}

class D : C
{
    immutable int x;
    this()
    {
        super();
        x = 5;
        foo();
    }
    override void foo()
    {
        writeln("D: ", x);
    }
}

void main()
{
    new D;
}

x could be considered final once any base ctor is called, or really any function which accepts a path back to x (like if you call some external bar(this), it should be the same thing).
@dlangBugzillaToGithub
Copy link
Author

razvan.nitu1305 commented on 2019-06-04T10:12:24Z

I don't think this issue is valid. If we disallow modification of immutable fields after a base class ctor is called then it will be impossible to initialize that field after a super call, which in my opinion is unacceptable behavior. What happens here is that foo is called before the field is actually initialized so it reads the default value of x, then it is called again after x has been initialized. This behavior is correct. The code in the original post is similar to this:

=============================
import std.stdio : writeln;

struct A
{
    immutable int x;
    this(int)
    {
        foo();
        x = 8;
        foo();
    }

    void foo()
    {   
        writeln(x);
    }       
}

void main()
{
    A a = A(2);
}
=============================

This code compiles and runs successfully. I don't see why it wouldn't. An alternative approach would be to consider that the first use of x locks down the variable and future accesses to it are considered modifications, but this leads to other problems: the constructor will not be able to initialize x and it would require inter-function compilation to determine this; dmd does not support inter-function compilation.

I suggest we close this as invalid.

@dlangBugzillaToGithub
Copy link
Author

andrei (@andralex) commented on 2019-06-04T15:52:23Z

This does seem to be a problem. This is liable to cause problems:

import std.stdio : writeln;

struct A
{
    immutable int x;
    this(int)
    {
        foo();
        x = 8;
        foo();
    }

    void foo()
    {   
        passToAnotherThread(&x);
    }       
}

The observing thread assumes the immutable(int)* it receives is, well, immutable. In reality that value will change over time.

@dlangBugzillaToGithub
Copy link
Author

andrei (@andralex) commented on 2019-06-04T15:58:11Z

A solution seems to be disallowing the use of "this" and "this.xyz" (as in passing as an argument to methods or function) until the object is cooked. Cooked means all immutable (or otherwise restricted) fields are initialized.

Implications for other qualifiers and combinations need to be analyzed.

@dlangBugzillaToGithub
Copy link
Author

andrei (@andralex) commented on 2019-06-04T16:01:49Z

Hm, this does not solve the original problem though.

@dlangBugzillaToGithub
Copy link
Author

andrei (@andralex) commented on 2019-06-04T16:05:27Z

So a solution to the super() issue would be to require the immutable fields are assigned before calling super(). It is odd but it works.

@dlangBugzillaToGithub
Copy link
Author

schveiguy (@schveiguy) commented on 2019-06-04T20:49:02Z

The reason you can modify immutable in a constructor is because nothing else has access to it. As soon as you give access to it elsewhere, it can cause a problem. Andrei is correct, calling any function which allows access to the immutable provides a vector of breaking immutability.

(In reply to RazvanN from comment #1)
> I don't think this issue is valid. If we disallow modification of immutable
> fields after a base class ctor is called then it will be impossible to
> initialize that field after a super call, which in my opinion is
> unacceptable behavior.

It's tricky, and definitely difficult to deal with. It would disallow certain solutions that seem otherwise valid. But that might be the cost of having a correct immutable implementation.

I will note that Swift enforces all members are initialized (look for 2-phase initialization) before calling a super ctor:
https://docs.swift.org/swift-book/LanguageGuide/Initialization.html

Java is the opposite, it requires you call any super ctor FIRST before initializing members: https://stackoverflow.com/questions/15682457/initialize-field-before-super-constructor-runs

But they also don't have a concept of immutable which is implicitly sharable.

@dlangBugzillaToGithub
Copy link
Author

andrei (@andralex) commented on 2019-06-04T20:58:49Z

I'm glad Swift provides a precedent and inspiration. We should follow their model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant