Compiler emits incorrect visibility modifier for internal constructors of abstract class

[axel-habermaier]

Compile the following code:

[<AbstractClass>]
type X internal (i : int) =
    internal new () = X 1
    new (f : float32) = X 1

Note that we've defined an abstract class with two internal constructors and one public one. When you disassemble the code you can see that the following code was generated by the compiler:

[AbstractClass]
[CompilationMapping(SourceConstructFlags.ObjectType)]
[Serializable]
public abstract class X
{
    protected X(int i)
    {
      base..ctor();
      Program.X x = this;
    }

    protected X()
    {
      this..ctor(1);
    }

    protected X(float f)
    {
      this..ctor(1);
    }
}

Note how the constructors' visibilities were incorrectly changed to protected; that's only ok for the public constructor, but definitely wrong for the internal ones. The internal constructors should still be internal. I noticed that problem while deriving from such a class in a C# project where all of a sudden I had the possibility to choose between different base constructors to call, whereas only the public one of the F# class should have been visible.

Happens with VS2013 and VS2015 RC in both debug and release builds. When you remove the [<AbstractClass>] attribute, the compiler emits the correct visibilities.

I realize that fixing this bug would be a breaking change, but then again, I consider this bug to be pretty serious.

[latkin]

Problem is here, all ctors of abstract classes are naïvely set to family (i.e. protected). That's wrong for this case, and very wrong for private ctors.

Similar bug here, although this is even worse because this code sets them all to assembly (i.e. internal), which I assume was just a typo.

For the internal guys, we could set access to ILMemberAccess.FamilyAndAssembly. Consumers must be in derived class AND same assembly. Note this is not expressible in C# - protected internal in C# maps to FamilyOrAssembly. This option seems behaviorally correct and in line with existing conversion to protected, however this is a rare accessibility due to lack of support in C#, so a bit risky. private ctors would also need to be specially handled, and kept private.

I think the better option is to simply stop altering the access modifiers entirely. Although the current conversion from public to protected is "safe", it's not necessary from IL standpoint. The C# compiler does not perform such a conversion. From the reflection perspective, the conversion is harmful: code indicates public but reflection tells you it's protected.

@dsyme, do you know of a compelling reason to keep the accessibility conversion, or can we take it out?

[latkin]

While testing this I learned that F# private members always compile to IL internal. Interesting.

So my earlier statement

private ctors would also need to be specially handled, and kept private.

is not quite accurate. They would be translated to internal.

[braden]

I was looking into minimizing the metadata the compiler emits and noticed that everything compiles to internal, is there a reason for this? If things could be private it seems like we could trivially omit a lot of metadata (it's significant for fsharp.compiler for example)

[dsyme]

@braden, @latkin, As background: we did make a decision in F# 1.0 (and confirmed it in 2.0) to not try to make use of "private" in IL but rather use "internal" as the minimum accessibility annotation we emitted. This was a deliberate choice, and was simplifying at the time (or seemed like it), especially when taking signature files into account.

[MarcSigrist]

I am glad this is fixed; it makes the behavior of the F# compiler more predictable in multi-language solutions. Thanks.