A Definite Assignment Anomaly

UPDATE: I have discovered that this issue is considerably weirder than the initial bug report led me to believe. I've rewritten the examples in this article; the previous ones did not actually demonstrate the bug.

Every struct has a default constructor which initializes its fields to their default values; this frobs the zero handle.

What about this?

S s2;s2.Frob();

Looks like a definite assignment error, doesn’t it?

An interesting and little-known fact about the C# compiler is that this is reported as a definite assignment error if the struct is compiled from source code, but not if the struct is in a referenced assembly! What is the reasoning behind this seemingly anomalous behaviour?

Well, consider the following related situation:

struct SS{ public int x; public int y; public void Bar() {…}}

Here we have the pure unadulterated evil that is a mutable value type. Is this legal?

SS ss;ss.x = 123;ss.y = 456;ss.Bar();

Yes, that is legal. The local variable s essentially has two sub-variables, x and y. If x and y are both definitely assigned, then s is also considered to be definitely assigned. When checking definite assignment on a struct, the compiler actually checks to see whether we know that every field of the struct has been initialized. A constructor call is assumed to automatically initialize every field, but if there is not a constructor call, then we simply track every field independently and then pool the results. (And if the fields are themselves of value type, we do so recursively of course.)

The anomaly happens because when the compiler sees that the type in question is in source code, it tracks the definite assignment of all the fields, detects that “s1.blah” is an unassigned field, and gives a definite assignment error on s1. But when the type in question is from a referenced assembly, the compiler ignores inaccessible fields of reference type. We see that there were zero fields initialized out of the zero accessible fields that needed to be initialized, and declare that s1 must be fully assigned!

UPDATE: Initially I thought this behaviour was odd and undesirable but plausibly justifiable. However I have since learned that in fact, for reasons still obscure to me, the compiler only ignores inaccessible fields of reference type. Inaccessible fields of value type are still tracked for definite assignment. This weird inconsistency points strongly in the direction of this being a bug, not a feature. Continuing with my original text...

This behaviour is undoubtedly weird; whether it is desirable or not is a judgment call. There certainly is an argument to be made that the user has no ability to do anything about a private field on an imported type; a type they almost certainly did not author themselves, do not know the details of, and cannot change. Whereas if the type is in source code, then the user running the compiler has personal knowledge of its internals. So, the argument goes, we should give the user a pass on having to prove that the fields they cannot access and know nothing about are assigned; just let them be assigned to their default values and don’t worry about them.

There is also the (more compelling to me) argument that we ought to be consistent here and either consider allthe fields all the time, or consider only the accessible fields all the time, rather than swapping back and forth between the two choices depending on the details of the compilation process. My choice would be the former; force the user to call the default constructor or use the “default(T)” operator if they really want the all-zero default version of the struct. That makes the intention very clear in the code.

Unfortunately, though I think I’ve argued that this behaviour is plausibly undesirable, we’re stuck with it. There are numerous BCL structs which have no public fields, and lots of existing code which uses them without calling the default constructor. Changing now to make this an error is a breaking change with no compelling benefit, and we’re not going to do that. Particularly since the anomaly is essentially harmless; whether we force you to say “s1 = new S()” or not, the handle field is in practice always initialized to its default value.

Nor do I think we should change this by making the code legal in all cases, even if the type is in source code; that’s making a bad situation worse merely in order to achieve consistency. This seems like the very definition of “foolish consistency”.

This is an interesting case. I was not aware there were situations where the compiler would allow you to declare a local instance without either initializing it or calling a constructor explicitly. Back in my C++ days, I recall many a time when local instances would be declared using a constructorless notation:

S s1; // default constructor automatically called

I always found this notation confusing - particularly since to following is also legal, but much more obvious:

S s1 = S(); // also legal but clearer

Pavel Minaev

18 Jan 2010 11:25 AM

@Eric:

When compiling the version which is in the same assembly, does it complain regardless of the contents of Frob(). Or does it actually check whether Frob() references fields, and complain about those specifically?

To be more precise, say I have Frob implemented thus:

void Frob() { handle = ...; }

Would it still complain about the lack of definite initialization? If not, and if I call Frob on uninitialized handle, will it recognize s.handle as initialized after Frob returns for the purposes of definite initialization? Or does it generally assume that any method call on a struct will access all its fields?

(I kinda have to ask because the behavior that you describe only makes some sense to me if compiler actually does cross-method boundary analysis for definite assignment for methods for which it has the source code).

@Leo: your second C++ snippet requires a copy constructor to exist (even if it's optimized away, which it legally can be), while the first one does not, so there is a difference. I don't really have a problem with the parentheses-less syntax as such; the real evilness of C++ is the distinction between POD and non-POD, and initialization thereof.

We do not do cross-method analysis. Basically, it's analyzed as though the code

h1.Frob()

was actually written as

MyHandle.Frob(ref h1);

Since we are passing a variable as "ref", it must be definitely assigned before the method call. We check definite assignment on structs by verifying that all known fields of the struct are fully assigned. In the "import from metadata" case, we don't even load information about private fields off of the disk, so as far as we know, its fully assigned. But in source code, we have all the private field info available in memory already, and we use it. -- Eric

Tom

18 Jan 2010 1:14 PM

Is this behavior in line with the specification?

Good question. The spec says "A struct-type variable is considered definitely assigned if each of its instance variables is considered definitely assigned." Notice that it does not say "each of its accessible instance variables'. So technically, I suppose this is a violation of the specification. -- Eric

Obviously (obvious to the programmer, not to the compiler), this is what I meant to do albeit a bit awkward. I don't really want to assign color since it's a calculated value. But I have to either do

Point p = new Point();

or

p.Color = dummy;

I was willing to accept that - I prefer calling the constructor anyway. But now you tell me that if Color was a struct in a different assembly it would work - now that suddenly doesn't make sense.

Darrell

18 Jan 2010 1:28 PM

[quote]Every struct has a default constructor which initializes its fields to their default values[/quote]

This is the real problem IMHO. As far as I'm concerned, there should never be an implicit parameter-less (default) constructor - if the dev wanted the object to be called with a parameter-less constructor, it would be in the definition. I've been bitten by this back in my C++ days (though for the life of me I don't remember the context, only that it was a PITA to track down), and hoped that C# was immune to it.

I'm sure there is some reason for it, but does that reason outweigh the problems?

Neil Whitaker

18 Jan 2010 3:24 PM

An important case to consider in this discussion is this one:

S[] sArray = new S[1000000];

If you require me to call S's constructor, how will you make sure I call it for all the values of this array?

In the case of arrays of classes, it's simple, because they're initialized to null, which means to class exists yet. But in the case of a value type, as soon as you allocate space, you have a struct.

Gabe

18 Jan 2010 3:33 PM

Neil: the only way to assure that all elements of an array get their constructors called is to have the compiler generate loops for each array to call the default constructor on each element. This would mean that every array element would get initialized at least twice -- first to all-zeroes by the CLR, then by the default constructor, then possibly by the actual intended value for each element. That should pretty much answer Darrell's question.

> Unfortunately, though I think I’ve argued that this behaviour is plausibly undesirable, we’re stuck with it.

still apply, or is it now officially in the defect-to-be-fixed land?

My argument stands: there are plenty of value types in the BCL that have reference type private fields and existing code that uses them as locals without explicit implementation. The buggy behaviour is safe and simply results in the fields being initialized to nulls, exactly as though the local were instead a field initialized to the default value. With no compelling up side to fixing it, and potentially large down sides, it's an easy call to make; we won't fix it. -- Eric

Gabe

19 Jan 2010 11:10 AM

So the problem is just that private reference types in members of structs from other assemblies might be usable without the other assembly's author explicitly setting them to null? I think I can live with that.

pete.d

19 Jan 2010 4:44 PM

I'm less familiar with the CLR spec than the C# spec. C#'s definite assignment rules make any implementation detail moot, but in the CLR, are we _guaranteed_ that value type local variables will in fact be initialized to zeroes? Or is that simply a coincidental fact of the current implementation?

I realize that, just as this bug won't be fixed for fear of breaking a bunch of existing buggy BCL code, there may be an implicit requirement at this point that stack variables be initialized to zeroes. But does the spec actually require that?

See Partition III section 3.4.7, "localloc". I quote: "If the localsinit flag on the method is true, the block of memory returned is initialized to 0; otherwise, the initial value of that block of memory is unspecified." -- Eric

pete.d

19 Jan 2010 9:30 PM

Thanks for the pointer. So, the way I read that as it relates to my question is that the answer is "no, there is no guarantee locals will be initialized to zeros". That is, the C# specification doesn't, as near as I can tell, require that the run-time option "localsinit" be used (*), and so whether they are or not is entirely up to the specific implementation.

IMHO, that could in fact be an argument for going ahead and fixing this bug, in spite of the effort that would be required in the BCL to fix up all the uninitialized variables. After all, _some_ of those uninitialized variables could in fact be bugs themselves, even if not all are. What better way to improve the overall quality of the BCL implementation than to take advantage of a correct implementation of the definite assignment rules?

(*) Indeed, the C# specification seems to mainly be agnostic about a specific run-time implementation; the .NET Framework is assumed and of course certain primitives are explicitly used (System.Int32, System.Boolean, System.Threading.Monitor, etc.), but otherwise it seems that there's no specific target language for the compilation output, never mind use of specific features of the target language.

The C# compiler is responsible for implementing the semantics of the C# language, and the semantics of the language are such that it *should* be impossible for the runtime's initialization implementation details of locals to be unobservable (modulo of course using debuggers to peer into variables before they are assigned to). Now, I think it is plausibly argued that what we have here is a bug; with this, one can observe the fact that our implementation of the compiler does in fact ensure that fields of struct locals are initialized to zeroes. But it would be rather strange for the spec to say "a correct implementation must ensure that local initialization details are non-observable, but an incorrect implementation is required to ensure that they're initialized to zeroes". The spec doesn't say anything at all about what an incorrect implementation is required to do. -- Eric

Pavel Minaev

20 Jan 2010 10:51 AM

> My argument stands: there are plenty of value types in the BCL that have reference type private fields and existing code that uses them as locals without explicit implementation. The buggy behaviour is safe and simply results in the fields being initialized to nulls, exactly as though the local were instead a field initialized to the default value. With no compelling up side to fixing it, and potentially large down sides, it's an easy call to make; we won't fix it.

By "fixing" this I rather meant switching to "guaranteed null initialization for structs" (or any other intermediate semantics that would ensure consistent behavior between structs in current project and ones from referenced assemblies). I understand that a "proper fix" in the spirit of language spec would be out of question for compat reasons.

"Verification assumes that the CLI zeroes all memory other than the evaluation stack before it is made visible to programs. A conforming implementation of the CLI shall provide this observable behavior. Furthermore, verifiable methods shall have the localsinit bit set, see Partition II (Flags for Method Headers). If this bit is not

set, then a CLI might throw a Verification exception at any point where a local variable is accessed, and where the assembly containing that method has not been granted SecurityPermission.SkipVerification."