28

I think this is a compiler bug.

The following console application compiles und executes flawlessly when compiled with VS 2015:

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            var x = MyStruct.Empty;
        }

        public struct MyStruct
        {
            public static readonly MyStruct Empty = new MyStruct();
        }
    }
}

But now it's getting weird: This code compiles, but it throws a TypeLoadException when executed.

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            var x = MyStruct.Empty;
        }

        public struct MyStruct
        {
            public static readonly MyStruct? Empty = null;
        }
    }
}

Do you experience the same issue? If so, I will file an issue at Microsoft.

The code looks senseless, but I use it to improve readability and to achieve disambiguation.

I have methods with different overloads like

void DoSomething(MyStruct? arg1, string arg2)

void DoSomething(string arg1, string arg2)

Calling a method this way...

myInstance.DoSomething(null, "Hello world!")

... does not compile.

Calling

myInstance.DoSomething(default(MyStruct?), "Hello world!")

or

myInstance.DoSomething((MyStruct?)null, "Hello world!")

works, but looks ugly. I prefer it this way:

myInstance.DoSomething(MyStruct.Empty, "Hello world!")

If I put the Empty variable into another class, everything works okay:

public static class MyUtility
{
    public static readonly MyStruct? Empty = null;
}

Strange behavior, isn't it?


UPDATE 2016-03-29

I opened a ticket here: http://github.com/dotnet/roslyn/issues/10126


UPDATE 2016-04-06

A new ticket has been opened here: https://github.com/dotnet/coreclr/issues/4049

Peter Perot
  • 921
  • 1
  • 8
  • 15
  • 2
    Can be [reproduced with .NET fiddle](https://dotnetfiddle.net/WEWMmY), looks like a bug at a first glance. Note: Please remove your `using` directives, they are not needed for the example an unnecessarily clutter your code example. – Heinzi Mar 25 '16 at 14:47
  • 4
    Personally, I'd call it `MyStruct.Null`. People expect something different when taking about "empty" in the context of structs. – Heinzi Mar 25 '16 at 14:48
  • 7
    I think it is potentially a bug that it is allowed to compile. The same in VS2013 errors on compile with exception `CS0523: Struct member 'ConsoleApplication1.Program.MyStruct.Empty' of type 'System.Nullable' causes a cycle in the struct layout`, which I would half expect – Rhumborl Mar 25 '16 at 14:50
  • 1
    @Rhumborl: But I think this is a compiler bug, too. The error message would be okay, if the variable was **non-static**. But since it is **static** there is no cycle in the layout. – Peter Perot Mar 25 '16 at 15:06
  • @Heinzi: You are right. In the first example "Empty" is perfect, in the second "Null" would be more appropriate. But that won't solve the compiler bug. ;-) – Peter Perot Mar 25 '16 at 15:10
  • 7
    Nice find. That sure looks like a bug. – Eric Lippert Mar 25 '16 at 15:11
  • Wow! How often do we see questions where the problem must be a bug in the compiler - this one really is!! – n8wrl Mar 25 '16 at 15:37
  • @PeterPerot - Nice find. Once you post it to MS, can you please paste the link to issue here? (would like to see MS take on this) – Rohit Vats Mar 25 '16 at 16:06
  • I opened a ticket here: https://github.com/dotnet/roslyn/issues/10126 – Peter Perot Mar 29 '16 at 16:48
  • @Peter Perot, it is not the declaration that is wrong. What is wrong is invoking `MyStruct.Empty`. Because of the `dot operator`, this is invoking a static field on the type MyStruct. Which is invoking `null` on a type. Will the the CLR allow you to invoke a null on a type? When you call `var x = MyStruct.Empty;` in the `Main` method that is `invocation` rather than simple `assignment` of a value. Will the CLR allow this invocation? – Julius Depulla Mar 30 '16 at 03:16
  • @Peter Perot, The `dot operator` represents `invocation` I don't see how CLR can invoke a `null` – Julius Depulla Mar 30 '16 at 03:30
  • 1
    @Eric Lippert I do not think it is bug. This is null reference invocation. `MyStruct.null` literally. The use of the dot operator represents invocation rather than simple assignment and asking the CLR to invoke a null reference, a no type, will result in the TypeLoadException – Julius Depulla Mar 30 '16 at 04:03
  • 2
    @JuliusDepulla Kid, you might want to take a quick look at who Eric Lippert is. If he says it's a bug, it's a bug. Your logic is flawed anyway, since there is **no invocation of the null entity** involved. How would you ever store a null value in a member if that were true? – Corey Mar 31 '16 at 07:32
  • Ask @Eric Lippert to analyse my explanation and he will tell you it is not a bug and I am right. No need to be rude – Julius Depulla Mar 31 '16 at 07:56
  • 2
    @JuliusDepulla You're the one being rude here. You ask Eric if you like, I'm not going to bother him with trivialities like this. In fact if you post your stuff as a question about the way the language works he will likely answer. But be aware that he helped write the C# language, so it's likely he has a much better understanding of it than you do. – Corey Mar 31 '16 at 08:46
  • At first when you look at the syntax you will think it is a bug but when you analyse you realize it is not a bug. I have read books authored by Eric Lippert, currently reading Essential C# 6.0 by Mark Michaelis and Eric Lippert. I know about Eric and Anders Hejlsberg. But this is about a post that is misleading and I am saying it is not a bug. – Julius Depulla Mar 31 '16 at 08:59
  • 3
    First off, whether it is a bug or not is up to the C# team, of which I am no longer a member. Second, the analysis in the bug by user "coderealm" is nonsensical; I suspect that coderealm there and Julius here are the same person. The dot operator is not an invocation operator, it is a member access operator. An *invocation* only happens if the member accessed is a property, an invoked method, or a field of delegate type which is invoked. (Or a few other similar cases.) In this case none of the above is true; there is no invocation here. – Eric Lippert Mar 31 '16 at 13:39
  • @peter-perot, Corey, Having read the blog from [Are private members a part of the API surface?](http://blog.paranoidcoding.com/2016/02/15/are-private-members-api-surface.html). It is an implementation of the CLR and your code behaviour is expected. Going back to Eric Lippert options for the cause of the exception. The last option is the correct answer. The actual program itself. It is not a bug in the CLR a but desired implementation by Microsoft. – Julius Depulla Apr 01 '16 at 07:58
  • One thing is for sure, my explanation is not how the CLR handled this code. But I knew from the onset that the code was the issue and not the CLR. – Julius Depulla Apr 01 '16 at 08:04
  • 2
    Oh my Gingerbread-Jesus, what's up, @JuliusDepulla?! My code is not the bug here – period. This discussion leads to nothing since your arguments don't bring in something new here. – Peter Perot Apr 01 '16 at 16:17
  • @Peter Perot Well done. Excellent find. I have thrown in the towel. I personally email Jared and he responded. See his comments in the Github thread . – Julius Depulla Apr 01 '16 at 17:20
  • Thank you! :-) :-) – Peter Perot Apr 06 '16 at 16:56

3 Answers3

18

First off, it is important when analyzing these issues to make a minimal reproducer, so that we can narrow down where the problem is. In the original code there are three red herrings: the readonly, the static and the Nullable<T>. None are necessary to repro the issue. Here's a minimal repro:

struct N<T> {}
struct M { public N<M> E; }
class P { static void Main() { var x = default(M); } }

This compiles in the current version of VS, but throws a type load exception when run.

  • The exception is not triggered by use of E. It is triggered by any attempt to access the type M. (As one would expect in the case of a type load exception.)
  • The exception reproduces whether the field is static or instance, readonly or not; this has nothing to do with the nature of the field. (However it must be a field! The issue does not repro if it is, say, a method.)
  • The exception has nothing whatsoever to do with "invocation"; nothing is being "invoked" in the minimal repro.
  • The exception has nothing whatsoever to do with the member access operator ".". It does not appear in the minimal repro.
  • The exception has nothing whatsoever to do with nullables; nothing is nullable in the minimal repro.

Now let's do some more experiments. What if we make N and M classes? I will tell you the results:

  • The behaviour only reproduces when both are structs.

We could go on to discuss whether the issue reproduces only when M in some sense "directly" mentions itself, or whether an "indirect" cycle also reproduces the bug. (The latter is true.) And as Corey notes in his answer, we could also ask "do the types have to be generic?" No; there is a reproducer even more minimal than this one with no generics.

However I think we have enough to complete our discussion of the reproducer and move on to the question at hand, which is "is it a bug, and if so, in what?"

Plainly something is messed up here, and I lack the time today to sort out where the blame ought to fall. Here are some thoughts:

  • The rule against structs containing members of themselves plainly does not apply here. (See section 11.3.1 of the C# 5 specification, which is the one I have present at hand. I note that this section could benefit from a careful rewriting with generics in mind; some of the language here is a bit imprecise.) If E is static then that section does not apply; if it is not static then the layouts of N<M> and M can both be computed regardless.

  • I know of no other rule in the C# language that would prohibit this arrangement of types.

  • It might be the case that the CLR specification prohibits this arrangement of types, and the CLR is right to throw an exception here.

So now let us sum up the possibilities:

  • The CLR has a bug. This type topology should be legal, and it is wrong of the CLR to throw here.

  • The CLR behaviour is correct. This type topology is illegal, and it is correct of the CLR to throw here. (In this scenario it may be the case that the CLR has a spec bug, in that this fact may not be adequately explained in the specification. I don't have time to do CLR spec diving today.)

Let us suppose for the sake of argument that the second is true. What can we now say about C#? Some possibilities:

  • The C# language specification prohibits this program, but the implementation allows it. The implementation has a bug. (I believe this scenario to be false.)

  • The C# language specification does not prohibit this program, but it could be made to do so at a reasonable implementation cost. In this scenario the C# specification is at fault, it should be fixed, and the implementation should be fixed to match.

  • The C# language specification does not prohibit the program, but detecting the problem at compile time cannot be done at reasonable cost. This is the case with pretty much any runtime crash; your program crashed at runtime because the compiler couldn't stop you from writing a buggy program. This is just one more buggy program; unfortunately, you had no reason to know it was buggy.

Summing up, our possibilities are:

  • The CLR has a bug
  • The C# spec has a bug
  • The C# implementation has a bug
  • The program has a bug

One of these four must be true. I do not know which it is. Were I asked to guess, I'd pick the first one; I see no reason why the CLR type loader ought to balk on this one. But perhaps there is a good reason that I do not know; hopefully an expert on the CLR type loading semantics will chime in.


UPDATE:

This issue is tracked here:

https://github.com/dotnet/roslyn/issues/10126

To sum up the conclusions from the C# team in that issue:

  • The program is legal according to both the CLI and C# specifications.
  • The C# 6 compiler allows the program, but some implementations of the CLI throw a type load exception. This is a bug in those implementations.
  • The CLR team is aware of the bug, and apparently it is hard to fix on the buggy implementations.
  • The C# team is considering making the legal code produce a warning, since it will fail at runtime on some, but not all, versions of the CLI.

The C# and CLR teams are on this; follow up with them. If you have any more concerns with this issue please post to the tracking issue, not here.

Eric Lippert
  • 612,321
  • 166
  • 1,175
  • 2,033
  • Interesting. There was an informative [article](http://blog.paranoidcoding.com/2016/02/15/are-private-members-api-surface.html) by @JaredPar a few weeks ago were he points out this construction is legal in C# but illegal in the CLR, but let's it pass without comment as to whether this correct/desirable. Suffice to say, I never thought of private fields in structs as being part of their contract. – Mike Zboray Mar 31 '16 at 22:53
  • 3
    @mikez: Oh for heaven's sake, if Jared is already on it then what am I messing around here for? Let him sort it out, that's what they pay him the big bucks for! :-) (And this is what I get for being behind in my blog reading.) – Eric Lippert Mar 31 '16 at 23:08
  • I believe there is a fault in the minimal repro here. One difference between your minimal repro and the OP's use of `Nullable` is that the `Nullable` also contains a field, of type `T`. In your case you didn't do that, which pretty much nullifies most of your arguments towards this being legal (in regards to the original posting). In the case of the field being present, your minimal repro struct `M` would contain a struct `N` which also contains `M`, which in turns means that indirectly struct `M` contains itself and thus has an unbounded size. – Lasse V. Karlsen Apr 01 '16 at 08:35
  • 1
    Ironically you can compile this code with the Microsoft compiler, but you can't run it with the .NET runtime -- and while you *can't* compile it with the Mono compiler, you *can* run it with the Mono runtime. (Tested on Windows 10, .NET 3.5, and the version of Mono 2.0 that comes with Unity 3D 5.4.0f3.) – yoyo Oct 20 '16 at 00:38
  • C++ have some restrictions too. The dreaded "error: field has incomplete type" – Luiz Felipe Dec 28 '20 at 20:09
  • What's the size of an recursive structure ? its infinite of course, unless you have a way to stop the beta-reduction. Do you happen to have a real Turing Machine? just kidding. How can you run it on mono? it surely is doing something wrong, perhaps it just created a 0-size struct? so it can call an instance member on it, I don't see why this is correct behavior. C# compiler is also wrong, because that is an error. Do note that you can have structs with non-instance members, they don't make the type incomplete in C++. So that's also a bug in the CLR if it doesn't allow for static members. – Luiz Felipe Dec 28 '20 at 20:43
11

This is not a bug in 2015 but a possibly a C# language bug. The discussion below relates to why instance members cannot introduce loops, and why a Nullable<T> will cause this error, but should not apply to static members.

I would submit it as a language bug, not a compiler bug.


Compiling this code in VS2013 gives the following compile error:

Struct member 'ConsoleApplication1.Program.MyStruct.Empty' of type 'System.Nullable' causes a cycle in the struct layout

A quick search turns up this answer which states:

It's not legal to have a struct that contains itself as a member.

Unfortunately the System.Nullable<T> type which is used for nullable instances of value types is also a value type and must therefore have a fixed size. It's tempting to think of MyStruct? as a reference type, but it really isn't. The size of MyStruct? is based on the size of MyStruct... which apparently introduces a loop in the compiler.

Take for instance:

public struct Struct1
{
    public int a;
    public int b;
    public int c;
}

public struct Struct2
{
    public Struct1? s;
}

Using System.Runtime.InteropServices.Marshal.SizeOf() you'll find that Struct2 is 16 bytes long, indicating that Struct1? is not a reference but a struct that is 4 bytes (standard padding size) longer than Struct1.


What's not happening here

In response to Julius Depulla's answer and comments, here is what is actually happening when you access a static Nullable<T> field. From this code:

public struct foo
{
    public static int? Empty = null;
}

public void Main()
{
    Console.WriteLine(foo.Empty == null);
}

Here is the generated IL from LINQPad:

IL_0000:  ldsflda     UserQuery+foo.Empty
IL_0005:  call        System.Nullable<System.Int32>.get_HasValue
IL_000A:  ldc.i4.0    
IL_000B:  ceq         
IL_000D:  call        System.Console.WriteLine
IL_0012:  ret         

The first instruction gets the address of the static field foo.Empty and pushes it on the stack. This address is guaranteed to be non-null as Nullable<Int32> is a structure and not a reference type.

Next the Nullable<Int32> hidden member function get_HasValue is called to retrieve the HasValue property value. This cannot result in a null reference since, as mentioned previously, the address of a value type field must be non-null, regardless of the value contained at the address.

The rest is just comparing the result to 0 and sending the result to the console.

At no point in this process is it possible to 'invoke a null on a type' whatever that means. Value types do not have null addresses, so method invocation on value types cannot directly result in a null object reference error. That's why we don't call them reference types.

Community
  • 1
  • 1
Corey
  • 13,462
  • 1
  • 29
  • 60
  • Yes, you are right. The issue is discussed here: http://stackoverflow.com/questions/9296251/cycle-in-the-struct-layout-that-doesnt-exist. But since it is static, this is no memory layout issue. – Peter Perot Mar 25 '16 at 15:13
  • @PeterPerot Yes, which is why at the end of my answer I suggest you submit it as a language bug. The rest of the post is essentially a discussion on why this is a problem for instance members... sorry if that was unclear. – Corey Mar 25 '16 at 15:18
  • +1. I'll submit a bug repost. Another discussion is here by the way: https://social.msdn.microsoft.com/Forums/vstudio/en-US/e133fd1f-e68c-42e4-8606-3071c4b210b2/recursive-declaration-of-a-generic-struct?forum=clr#4ceaff6a-00e4-444c-831f-796ebaeac4e4 – Peter Perot Mar 25 '16 at 15:26
  • 1
    @PeterPerot Updated the answer to better reflect this. – Corey Mar 25 '16 at 15:30
  • This answer is not right in my opinion. I have provided explanation to why it is not a bug below. – Julius Depulla Mar 31 '16 at 04:12
1

Now that we've had a lengthy discussion about what and why, here's a way to work around the issue without having to wait on the various .NET teams to track down the issue and determine what if anything will be done about it.

The issue appears to be restricted to field types that are value types which reference back to this type in some way, either as generic parameters or static members. For instance:

public struct A { public static B b; }
public struct B { public static A a; }

Ugh, I feel dirty now. Bad OOP, but it demonstrates that the problem exists without invoking generics in any way.

So because they are value types the type loader determines that there is a circularity involved that should be ignored because of the static keyword. The C# compiler was smart enough to figure it out. Whether it should have or not is up to the specs, on which I have no comment.

However, by changing either A or B to class the problem evaporates:

public struct A { public static B b; }
public class B { public static A a; }

So the problem can be avoided by using a reference type to store the actual value and convert the field to a property:

public struct MyStruct
{
    private static class _internal { public static MyStruct? empty = null; }
    public static MyStruct? Empty => _internal.empty;
}

This is a bunch slower because it's a property instead of a field and calls to it will invoke the get method, so I wouldn't use it for performance-critical code, but as a workaround it at least lets you do the job until a proper solution is available.

And if it turns out that this doesn't get resolved, at least we have a kludge we can use to bypass it.

Corey
  • 13,462
  • 1
  • 29
  • 60
  • Cool workaround, @Cory. However, since `Nullable`is a struct itself and no reference type, each invocation of the property getter will create a copy. So you can simply write `public struct MyStruct { public static MyStruct? Empty => null; }`; there is no need to back up a `null` of a nullable type in an internal variable. – Peter Perot Apr 01 '16 at 07:44
  • @PeterPerot Of course. I always forget the copy effect of value types. Comes from being a C programmer in my youth I guess. – Corey Apr 01 '16 at 22:01