Some structs are required to chain the paramerless constructor

[scottdorman]

In the process of upgrading some projects to support .NET Core (by simply moving to the new project.json format) and, once done, the following code failed with compiler error CS0188: The ‘this’ object cannot be used before all of its fields are assigned to.

Prior to the change to .NET Core, this code compiled just fine in a PCL targeting .NET 4.0.

The code in question (edited for brevity) is:

[StructLayout(LayoutKind.Sequential)]
public partial struct Comb : IFormattable, IComparable, IComparable<Comb>, IEquatable<Comb>
{
     public Comb(byte[] array)
     {
         this.a = ((int)array[0] << 24) | ((int)array[1] << 16) | ((int)array[2] << 8) | array[3];
         this.b = (short)(((int)array[4] << 8) | array[5]);
         this.dateTime = this.GetDateTimeOffset(); // <== This line causes the error.
     }

     private DateTimeOffset GetDateTimeOffset()
     {
         var buffer = this.ToByteArray();
         var msecsArray = new byte[]  { ... };

         var days = buffer[6] + (buffer[0] << 8) + (buffer[4] << 16) + (buffer[5] << 24);
         var msecs = BitConverter.ToDouble(msecsArray, 0);
         var date = Comb.MinDate.AddDays(days).AddMilliseconds(msecs * Comb.Accuracy);
         return date;
     }
 }
}

The line (this.dateTime = this.GetDateTimeOffset()) compiles just fine in every version of the Framework except .NET Core.

This is actually easy for me to fix by making sure the constructor chains a call to the parameterless constructor:

public Comb(byte[] array) : this()

I’d argue that the new code is actually more correct and that it’s how the code should have been written in the first place. The issue is that by simply upgrading the project and doing nothing else, I suddenly get a compiler error when there weren’t any before. In fact, there weren’t even any warnings about this before.

Apparently this was similar to an issue with auto properties in structs as well, all the way back to 2009, with the same fix/workaround. According to some of the answers to questions about this on StackOverflow, it was fixed in C#6.

Either way, it’s a regression because what used to compile doesn’t anymore just by upgrading the project format. I'm not certain if it's related to .NET Core or to Roslyn.

[BillWagner]

Tagging @jskeet because I remember discussing a similar issue with him, but I can't find any notes or the resolution of this issue. (It might have been in the context of the ECMA spec reviews.

[jskeet]

What's in the rest of the partial Comb definition? If you can give us a complete example (ideally not using partial types, unless that's required to reproduce it) it will be much simpler to reason about.

[scottdorman]

Sure. Full source code for the class in question is at:

• .csproj - https://github.com/scottdorman/cadru/blob/master/src/Cadru.Core/Comb.cs.
• project.json - https://github.com/scottdorman/cadru/blob/netcore/src/Cadru.Core/Comb.cs

[jskeet]

I don't think it should ever have compiled. Here's a similar example:

struct Foo
{
    int a;

    public Foo(int x)
    {
        this.a = Method();
    }

    int Method()
    {
        return 5;
    }
}

That gives me the same error ("The 'this' object cannot be used before all of its fields are assigned to"), without using DNX.

I wonder whether this is actually to do with Code Contracts... I'll see if I can build your csproj-based code myself...

[scottdorman]

@jskeet This code has been in GitHub for almost 2 years like that and it's always compiled just fine.

[jskeet]

Hmm... yes, it builds for me, even after removing the code contracts. Will try to reduce it to a simpler example - hopefully in doing so I'll find out what's going on.

[jskeet]

Okay, narrowing it down somewhat, small repro:

• Create a new solution
• Create a new PCL project targeting .NET 4.5 and Windows Phone 8.1
• Add the following code:

using T = System.Guid;

public partial struct Demo
{
    private T field;

    public Demo(int ignored)
    {
        this.field = this.Method();
    }

    private T Method()
    {
        return default(T);
    }
}

That builds, but shouldn't.

• If you change the using directive for T to make it System.Int32 instead of System.DateTimeOffset, it doesn't compile. Ditto System.String. But System.Guid is fine.
• If you add another field which is only assigned after the call to GetMethod(), it doesn't compile

I dread to think what the underlying cause of this is, but I think it's a reasonable bug to be fixed in the compiler - making your code invalid. Just add the this() :)

[scottdorman]

@jskeet Thanks for figuring out a repro for this issue. The extra information about some types working and others not for T and adding another field after the call to GetMethod() is interesting/puzzling/scary all at the same time.

In general, I agree that the code I had probably should never have compiled and that I should have always had the chained constructor call to this(), but I didn't and everything compiled so there's somewhat of an expectation that the code would continue to compile...especially since all I did was change project file formats.

[jaredpar]

I took a look at this and one curiosity jumps out: System.Guid in the specific PCL @jskeet specified (profile 111) is actually an empty struct. It is given to the compiler from this DLL:

C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework.NETPortable\v4.5\Profile\Profile111\System.Runtime.dll

That definition has all members stripped out. Hence this is essentially testing whether or not this can be used before a field, with no members, is definitely assigned.

I believe this behavior is hence by design. But definitive assignment in struct ctors is an area I've been known to get tripped up on. Interested to hear what others think.

CC @gafter

[jskeet]

Right. In that case, it feels like if the compiler is meant to understand reference assemblies, then either:

• It should always assume that any struct it only has the reference assembly for has fields
• The reference assembly needs some indication (e.g. via an attribute) of structs that don't have any fields

How unreasonable would be to simply assume that any struct outside the assembly you're currently building always contains fields? (For the purpose of definite assignment, at the very least.) Perhaps special-case known structs which don't have any fields? (I'm thinking of Void to start with... are there any others that are relevant?)

[jaredpar]

@jskeet the compiler doesn't understand reference assemblies though, it just understands assemblies. There is no differentiation to us.

The negotiation of reference vs. implementation assemblies, profiles, etc ... is all handled by MSBuild. It makes a determination of what should be referenced and sends it to the compiler (who looks no further).

How unreasonable would be to simply assume that any struct outside the assembly you're currently building always contains fields?

I think this would have been a very reasonable behavior if it had been adopted in C# from the beginning. My fear in adopting it now though is that it would be a breaking change. Given the tendency for reference assembly tools to strip private fields and the subtle nature of struct ctor definitive assignment, it's likely one that would have a noticeable impact.

[jaredpar]

Another possible fix is for PCLs to stop stripping out private fields on structs. Rightly or wrongly it's an aspect of structs that C# has taken a dependency on. By stripping away private fields the reference generator tools have altered the semantic meaning of the type (as consumed by C#). From the perspective of C# the original assembly and reference assemblies, when this occur, are semantically different.

[davkean]

Having owned/caused this issue, I'll add some background:

About ~5 years ago, when we made the decision to strip the fields[1], portable libraries could not use unsafe code or security critical members; it was explicitly disabled due to Silverlight's sandbox. I believe we discuss this particular issue at a design meeting (though can't find the notes), decided that due to lack of unsafe code, private fields weren't needed. We were unaware that C# used them in their definite assignment analysis.

My initial thought was that even with unsafe code, as long as we marked the struct with the correct size (via the pseudo StructLayoutAttribute), then we could still skip the fields - but @jaredpar pointed out that C# still uses the fields to determine if a given struct is "blittable" and hence, can be a valid pointer[2].

Given this definite assignment issue and interop reasons, I agree that the fields need to be added to the reference assemblies.// cc @weshaggard @ericstj.

[1] We didn't strip per se, just didn't generate them - the reference assemblies were built from generated C# code.
[2] If you have a reference inside the structure, C# doesn't let you take a pointer to it (as such a reference could be moved around on you by the GC).

[VSadov]

Another example of semantical significance of all struct fields:

    struct S2<T>
    {
        // if ref assembly does not disclose this field, 
        // S1 will compile
        private T field;
    }

    struct S1
    {
        private S2<S1> field;
    }

[davkean]

@VSadov Nice one!

[VSadov]

IMO - since structs are basically chunks of data without any point of indirection, the shape/layout of that data constitutes a kind of a contract.
Accessibility modifiers on struct fields are mostly just that - they control accessibility. If a struct field is private it is not meant to be used directly, the fact that the field exists could still be observable.

[davkean]

@VSadov And yet we allow accessibility on struct members...

[jaredpar]

Going to move this bug over to corefx. The root problem here is in the definition of the reference assemblies. Roslyn is behaving to spec based on the data it is being given. Need to correct the reference assemblies here so this, and other bugs, can be fixed.

https://github.com/dotnet/corefx/issues/6185

For the curious here is a full write up of the various ways private struct fields produce observable behavior.

[scottdorman]

Thanks @jaredpar. I'll keep an eye on the new issue.

[coderealm]

@jskeet @scottdorman @BillWagner @scottdorman @jaredpar I will appreciate if you big guns of C# have a swap at this stackoverflow code and give us your view. @ericlippert has given a view but this is one subtle behaviour either in C# or CLR . It is also available here on Roslyn Issues

I know @jskeet will love this one @peter-perot. This is somewhat related to this thread.

[ericlippert]

I have just learned that Jared already commented on the issue that coderealm refers to:

http://blog.paranoidcoding.com/2016/02/15/are-private-members-api-surface.html

Jared seems to have this one well in hand; no need for further input from me on this one.