Fsharp: Initialization soundness checks don't work for base constructor calls from primary constructors

[kbattocchi]

It appears that the compiler doesn’t check arguments to base class constructors for recursive references:

type Parent(o:obj) = class end
type Broken() as self = inherit Parent(self)

Attempting to call the constructor for Broken will fail with a null reference exception. However, I would have expected this to be picked up by F#’s initialization soundness checks, and to at least throw a more meaningful InvalidOperationException. This only appears to affect primary constructors. For instance, with the following definition of Broken, the expected error is generated:

type Broken = 
    inherit Parent
    new() as self = { inherit Parent(self) }

Worse, the broken soundness checks for primary base constructor calls also breaks some valid uses of self references:

type Parent<'t>(t:'t) = 
    member val T = t

type Ok() as self = inherit Parent<unit->Ok>(fun () -> self)

// inexplicably fails
Ok().T()

Note that once again using an explicit constructor improves things:

type Ok = 
    inherit Parent<unit->Ok>
    new() as self = { inherit Parent<unit->Ok>(fun () -> self) }

// works
Ok().T()

[kurtschelfthout]

I've been looking into this. I can reproduce the runtime behavior of the above report.

While looking around it seems there are some half-implemented warnings similar to the recursively defined value warning. See this todo comment - sort of: https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/TypeChecker.fs#L2715

The SelfRefObjCtor is defined (AreWithinCtorPreObj is not, before anyone gets too excited...), as well as some code to match it and return an appropriate warning text, looks like, but this exception is never raised. (A search for SelfRefObjCtor in the codebase shows as much)

So I guess the question is - should we try to enable the warning - it does seem kind of useful to have, and it would be consistent with recursively defined values. Or instead we can decide to remove the code, as it seems to be dead. If the former, I suggest to open a separate issue for it so it can be triaged separately.

[dsyme]

@kurtschelfthout I believe it's intended that we don't give the warning-code-40 recursively defined value warnings for the object-oriented constructs base/this/... At least, SelfRefObjCtor is not raised anywhere.

[kbattocchi]

@dsyme @kurtschelfthout I think not emitting warning FS0040 for object-oriented constructs might be fine. However, emitting error FS0031 (or some new error) when a self-reference is passed to a chained constructor would make sense, I think, since it is doomed to fail at runtime.

Also, note that the last paragraph of section 8.6.3 of the latest F# spec states that:

A warning is given if x occurs syntactically in or before the _additional-constr-init-expr_ of the construction expression. If any member is called before the completion of execution of the _additional-constr-init-expr_ within the _additional-constr-expr_ then an InvalidOperationException is thrown.

but this appears to be incorrect. For instance, there is no warning for the following code:

type X =
    new() as x= 
        let () = printfn "%O" x
        {  }

though attempting to execute X() does result in an InvalidOperationException.

[kurtschelfthout]

Leaving the warning discussion to one side for the moment, I am a bit stuck with investigating this.

The code that is being generated in the implicit constructor case is the following C# equivalent:

[CompilationMapping(SourceConstructFlags.ObjectType)]
    [Serializable]
    public class Broken : Parent
    {
        internal int init@4;

        public Broken()
        {
            FSharpRef<Broken> self = new FSharpRef<Broken>(null);
            FSharpRef<Broken> self2 = self2;
            this..ctor(LanguagePrimitives.IntrinsicFunctions.CheckThis<Broken>(self2.contents));
            self.contents = this;
            this.init@4 = 1;
        }
    }

And it's clear that this always dereferences null.

The code using an explicit ctor does not have the weird self2 reference and looks completely ok, as also evidenced by its behavior. As far as I can tell, type checking implicit ctor vs explicit ctor takes two very different code paths. So far I haven't been able to pinpoint exactly where things go wrong in the implicit constructor case.

What's also kind of strange is that with the F# 4.0 compiler at least the ref instance are called self and self2, while with the latest compiler they are called fsharpRef and fsharpRef2 which indicates that something else has changed on top of this wrt naming.

If this gives anyone any ideas, please share.