ceylon / ceylon-spec

DEPRECATED
Apache License 2.0
108 stars 34 forks source link

singleton constructor can leak `this` ref in initializer #1368

Closed sgalles closed 9 years ago

sgalles commented 9 years ago

I believe that this code should be rejected

class Foo {
    shared new foo {}
    value f = foo;
}
gavinking commented 9 years ago

I agree. But on precisely what grounds should it be rejected?

pthariensflame commented 9 years ago

Circularity? A constructor of a class shouldn't be used to initialize a non-late, non-lazy property of that class.

sgalles commented 9 years ago

yes and also this is a problem :

class Bar(){
    value f = Foo.foo;
}

class Foo extends Bar{
    shared new foo extends Bar(){}
}

So, I don't know if it answers your question @gavinking, but I guess that a singleton constructor should not be referenced in any other constructors of its class, or any constructors of its super classes. But maybe your question was more general ?

gavinking commented 9 years ago

Well,I was going to say should be treated as a case of foo not being definitely initialized within the constructor of Foo. But now with @sgalles second example, I'm not sure if that is the best way to treat it after all.

pthariensflame commented 9 years ago

I think it's still a problem of definite initialization. After all, the same problem occurs if we allow

class Bar() {
    value f = Foo.foo();
}
class Foo extends Bar {
    shared new foo() extends Bar() {}
}
gavinking commented 9 years ago

The thing is that "definite initialization" is a notion that I only really define within a local scope. In this case these two classes could be in different compilation units, in different modules.

sgalles commented 9 years ago

@pthariensflame I may be mistaken, but I'm not sure that it's exactly the same problem. You're example will end up in stackoverflow. Eventually the user will detect this problem (at runtime) because Foo just can't be instanciated.

On the other hand, with my example everything seems fine but actually f is null.

I'm not sure that the compiler can always detect these infinite loops in general. For instance

class Foo(){
    value b = Bar();
}
class Bar(){
    value b = Foo();
}

also ends up with an infinite loop. I see your example as a special case of this example. That being said I happily admit that I don't know the exact and formal definition of "definite initialization"

sgalles commented 9 years ago

Oh wait, actually my second example with an extends is also a particular case ! This doesn't work either :cry:

class Bar(){
    value f = Foo.foo;
}

class Foo {
    value b = Bar();
    shared new foo{}
}
gavinking commented 9 years ago

I have pushed a provisional implementation of this restriction.

gavinking commented 9 years ago

I have documented this restriction in the spec. Closing.