Why don't I get a warning about possible dereference of a null in C# 8 with a class member of a struct?
Note that there is no reason for there to be a warning on the call to Console.WriteLine()
. The reference type property is not a nullable type, and so there's no need for the compiler to warn that it might be null.
You might argue that the compiler should warn about the reference in the struct
itself. That would seem reasonable to me. But, it doesn't. This seems to be a loophole, caused by the default initialization for value types, i.e. there must always be a default (parameterless) constructor, which always just zeroes out all the fields (nulls for reference type fields, zeroes for numeric types, etc.).
I call it a loophole, because in theory non-nullable reference values should in fact always be non-null! Duh. :)
This loophole appears to be addressed in this blog article: Introducing Nullable Reference Types in C#
Avoiding nulls So far, the warnings were about protecting nulls in nullable references from being dereferenced. The other side of the coin is to avoid having nulls at all in the nonnullable references.
There are a couple of ways null values can come into existence, and most of them are worth warning about, whereas a couple of them would cause another “sea of warnings” that is better to avoid:
…
- Using the default constructor of a struct that has a field of nonnullable reference type. This one is sneaky, since the default constructor (which zeroes out the struct) can even be implicitly used in many places. Probably better not to warn [emphasis mine - PD], or else many existing struct types would be rendered useless.
In other words, yes this is a loophole, but no it's not a bug. The language designers are aware of it, but have chosen to leave this scenario out of the warnings, because to do otherwise would be impractical given the way struct
initialization works.
Note that this is also in keeping with the broader philosophy behind the feature. From the same article:
So we want it to complain about your existing code. But not obnoxiously. Here’s how we are going to try to strike that balance:
…
- There is no guaranteed null safety [emphasis mine - PD], even if you react to and eliminate all the warnings. There are many holes in the analysis by necessity, and also some by choice.
To that last point: Sometimes a warning is the “correct” thing to do, but would fire all the time on existing code, even when it is actually written in a null safe way. In such cases we will err on the side of convenience, not correctness. We cannot be yielding a “sea of warnings” on existing code: too many people would just turn the warnings back off and never benefit from it.
Also note that this same issue exists with arrays of nominally non-nullable reference types (e.g. string[]
). When you create the array, all of the reference values are null
, and yet this is legal and won't generate any warnings.
So much for explaining why things are the way the are. Then the question becomes, what to do about it? That's a lot more subjective, and I don't think there's a right or wrong answer. That said…
I personally would treat my struct
types on a case-by-case basis. For those where the intent is actually a nullable reference type, I would apply the ?
annotation. Otherwise, I would not.
Technically, every single reference value in a struct
should be "nullable", i.e. include the ?
nullable annotation with the type name. But as with many similar features (like async/await in C# or const
in C++), this has an "infectious" aspect, in that you'll either need to override that annotation later (with the !
annotation), or include an explicit null check, or only ever assign that value to another nullable reference type variable.
To me, this defeats a lot of the purpose of enabling nullable reference types. Since such members of struct
types will require special-case handling at some point anyway, and since the only way to truly safely handle it while still being able to use non-nullable reference types is to put null checks everywhere you use the struct
, I feel that it's a reasonable implementation choice to accept that when code initializes the struct
, it is that code's responsibility to do so correctly and make sure the non-nullable reference type member is in fact initialized to a non-null value.
This can be aided by providing an "official" means of initialization, such as a non-default constructor (i.e. one with parameters) or factory method. There will still always be the risk of using the default constructor, or no constructor at all (as in array allocations), but by providing a convenient means to initialize the struct
correctly, this will encourage code that uses it to avoid null references in non-nullable variables.
That said, if what you want is 100% safety with respect to nullable reference types, then clearly the correct approach for that particular goal is to always annotate every reference type member in a struct
with ?
. This means every field and every auto-implemented property, along with any method or property getter that directly returns such values or the product of such values. Then the consuming code will need to include null checks or the null-forgiving operator at every point where such values are copied into non-nullable variables.
In the light of the excellent answer by @peter-duniho it seems that as of Oct-2019 it's best to mark all non-value-type members a nullable reference.
#nullable enable
public class C
{
public int P1 { get; }
}
public struct S
{
public C? Member { get; } // Reluctantly mark as nullable reference because
// https://devblogs.microsoft.com/dotnet/nullable-reference-types-in-csharp/
// states:
// "Using the default constructor of a struct that has a
// field of nonnullable reference type. This one is
// sneaky, since the default constructor (which zeroes
// out the struct) can even be implicitly used in many
// places. Probably better not to warn, or else many
// existing struct types would be rendered useless."
}
public class Program
{
public static void Main()
{
var instance = new S();
Console.WriteLine(instance.Member.P1); // Warning
}
}