constructor delegation and initializer locals

[CeylonMigrationBot]

[@gavinking] The typechecker accepts this code:

class Super {
    String name;
    shared new ()  {}
    shared new New() {
        name = "Gavin";
    }
    shared new Default() 
            extends New() {}
}

The backend produces this error:

Variable name might not have been initialized

[Migrated from ceylon/ceylon-compiler#2171] [Closed at 2015-07-30 10:46:50]

[CeylonMigrationBot]

[@tombentley] This breaks the heuristic I was using for cope with capture in the presence of constructor delegation.

Consider these:

class Smashing {
    String name;//captured
    shared new ()  {}
    shared new New() {
        name = "Gavin";
    }
    shared new Default() 
            extends New() {
        print(name);
    }
}
class Great {
    String name;// not captured
    shared new ()  {
        name = "Jim;
    }
    shared new New() {
        name = "Gavin";
    }
    shared new Default() 
            extends New() {}
}

I think a better rule would be when a function or value is referenced by more than one constructor in the chain of extends clauses then we need to consider it captured (since we use the member to pass the value between the two), but if it's just referenced by one constructor in each chain then it's not captured.

[CeylonMigrationBot]

[@FroMage] @gavinking can you explain why the typechecker accepts this? Looks to me like the default constructor will leave name uninitialised, no?

[CeylonMigrationBot]

[@FroMage] Oh, because it isn't used I suppose?

[CeylonMigrationBot]

[@FroMage] Well it's not so clear. This is accepted:

class Super() {
    String name;
    if(true){
        name = "bar";
    }
}

But this not:

class Super() {
// ERROR:forward declaration may not occur in declaration section: 'name'
    String name;
}

So it's not totally clear to me why the default constructor would be allowed to leave things uninitialised.

[CeylonMigrationBot]

[@FroMage] This is accepted, though:

class Super {
    String name;
    shared new ()  {}
}

Making it further weird that the previous was not.

[CeylonMigrationBot]

[@FroMage] Well, I can fix the capturing. Sounds to me like only constructors which delegate are capturing, not those which are delegated to (the case ATM).

But we have many more issues related to definite initialisation of fields throws by javac, in the case of partial initialisation:

class Super {
    String name;
    String name2;
    shared new constr() {
        name = "Gavin";
// here javac complains that "name2" is not initialised
    }
    shared new default() 
            extends constr() {
// here javac complains that "name2" may already be initialised
        name2 = "";
        print(name);
        print(name2);
    }
}

So it sounds to me like we should add the @NoCheckInit to fields even in the absence of abstract initialisers, because even non-abstract initialisers may be partial initialisers.

[CeylonMigrationBot]

[@FroMage] Right, in Java it's the non-delegating (to this) constructor's job to initialise every final field. You can't delegate to another constructor (of this) and initialise any final field.

[CeylonMigrationBot]

[@FroMage] Now, is this a compiler (language) limit, or JVM limit, I'm not sure.

[CeylonMigrationBot]

[@gavinking] You are allowed to have non-captured declarations in the initializer that are not definitely initialized at the end of the initializer section. Just like you're allowed to have the same thing in a function, or in a method in Java.

However, you're not allowed to have a forward declaration in the declaration section.

So all this is working as specified.

[CeylonMigrationBot]

[@FroMage] Oh OK, it's the statements after it that make it part of the init section, I get it. Thanks.

[CeylonMigrationBot]

[@gavinking]

But we have many more issues related to definite initialisation of fields throws by javac, in the case of partial initialisation:

That code example should not have been accepted by the typechecker, and I'm shocked that it was. WTF? I have tests for that!

UPDATE: ignore this comment.

[CeylonMigrationBot]

[@gavinking] OIC, name2 isn't shared. So sure, it should be accepted. My bad.

[CeylonMigrationBot]

[@FroMage] And I'm back in MethodOrValueReferenceVisitor which I still think is written backwards, grrrr…

It's not enough to make delegating constructors capturing: the statements after them are also capturing, but not of everything.

[CeylonMigrationBot]

[@FroMage] This is in fact extremely hard to get right. If I don't want to capture too many things I have to only mark things as captured if they end up used in two different generated constructors, UNLESS they are actually also captured by something else that captures, and there's absolutely no mechanism in place to differentiate the two sorts of capturers.

[CeylonMigrationBot]

[@FroMage] I have made some progress, but my mind just melted. Hope I'll have better luck tomorrow.

[CeylonMigrationBot]

[@FroMage] So I got the constructor capturing somewhat under control. Now I broke 88 tests, though.

[CeylonMigrationBot]

[@FroMage] Man, one of those days I'll have to rewrite that capturing visitor into something comprehensible and fast. It's so backwards and pointless it's a wonder it works at all. I know I keep saying this and I don't do it, but damn…

[CeylonMigrationBot]

[@FroMage] Done. And now I'm starting to understand how constructors work :)

[CeylonMigrationBot]

[@tombentley]

my mind just melted.

I sympathize, because it made my mind do the same thing when I was trying to get this stuff to work. Well done that you managed to close this one!

imo how we handle capture is one of the weakest, and yet most important parts of the JVM backend, and we should find time to improve it because it's getting very hard to maintain.