Warning is not issued for uninitialized non-nullable reference type fields in structs

[sfuller]

Version Used:
Version: 3.0.100
Commit: 04339c3a26

Steps to Reproduce:
Compile the following code:

#nullable enable

public struct Foo
{
    public string Value;
    
    public int DoStuff()
    {
        return Value.Length;
    }
}

Expected Behavior:
A CS8618 or similar warning is issued due to Value possibly not being initialized.
Example: warning CS8618: Non-nullable field 'Value' is uninitialized. Consider declaring the field as nullable.

Actual Behavior:
No warnings are issued.

Note
When Foo is changed from a struct to a class, the warning is reported.
I was able to reproduce this with sharp lab: link

[jaredpar]

This behavior is currently "By Design". This was an explicit decision where we traded off correctness for convenience in the language.

Moving to csharplang as it's a language issue.

[sfuller]

Thank you for moving!

I can understand the convenience factor for existing code with structs, or those who use structs sparingly, however the ability for null values from default struct values to bypass the nullable reference type checker seems like it undermines the nullable reference types system as a whole.

Could we consider adding this as an opt-in feature? I believe that having the nullable reference type system cover struct default values would help inform better usage of structs for those who desire to turn the feature on.

[yaakov-h]

You can always write your own Analyzer to warn when you declare a non-nullable field or property in a struct type.

[DavidArno]

@yaakov-h, sure working around the issue with a custom analyzer is one option. Fixing it in the language is better option though for those of us who value consistently and readability over convenience.

[Joe4evr]

The warning for this case already exists at the call-site, not the declaration-site:

(Image by @Kukkimonsuta at this comment)

[sfuller]

That seems to be a Visual Studio specific hint, and not an actual compiler warning. I'll need to check again, but I believe other IDEs and the compiler itself do not show a warning in that case.

[canton7]

@Joe4evr I can't reproduce that. Maybe that's an old compiler version?

[canton7]

I do see the problem here: structs can be default-initialized, which means you can't catch all of the places where people might assign null to S.Str (using the example above).

I guess you could have a bit of flow state attached to local variables which are structs, which marks all of the members as "maybe null" if the struct has been default-initialized. However, that's 1) presumably complex to implement, 2) probably of limited value, and 3) gets complicated fast, since public string Str => _str ?? "" can never return null (and so wouldn't need the "maybe null" state attached to it), but the compiler has to go delving through the struct's members to figure that out.

It feels like a more accurate approach would be to complain if a struct has any fields which are reference types which are not declared as nullable (including auto-property backing fields). That at least would open the struct author's eyes to the possibility that these references might be null, despite their best efforts. However I'm not sure what the syntax to suppress this would look like...

[DavidArno]

@canton7,

It feels like a more accurate approach would be to complain if a struct has any fields which are reference types which are not declared as nullable (including auto-property backing fields)

Spot on. That is what I'd expect the compiler to do. With NRTs enabled, all reference fields in structs are T? by the very nature of the way structs are initialised, so I feel the compiler should complain when they aren't decorated as such.

[sfuller]

@DavidArno I agree, it is intuitive for NRT to consider the nullability of reference types in structs, and I was baffled when I discovered that the compiler did not issue such warnings.

Another way this could be solved is by introducing a new NRT warning that is issued for reference type fields in structs that are not annotated with ?. If users would prefer the current behavior, this warning could be disabled. Or this could be an opt-in warning. Either way, this would give power to the users who want NRT analysis to be accurate and consider struct field nullability correctly, while allowing other users who cautiously use structs to do their own thing.

(edit: I just re-read @canton7 's comment, they suggest the same thing)

[IS4Code]

If a struct promises its member to be non-nullable, then it shouldn't give false promises when it cannot be upheld. I wouldn't find the presence of the warning inconvenient, as it signalizes that it should be a property instead:

struct S
{
    string? field;

    public string Property => field ?? throw new InvalidOperationException();
}

[canton7]

I imagine there are probably cases in e.g. Unity where large structs are passed around by ref, and simply aren't default-initialized in practice. Struct fields might need to be fields for perf reasons, or so that they can be accessed by ref. In cases like this, I think authors might not want this behaviour.

[parched]

So, has anyone created an analyzer for this, I couldn't find one?

[CyrusNajmabadi]

Doesn't seem like it. Probably a fairly niche need. You could pretty trivially write such a thing yourself if you wanted :)

[jnm2]

(The amount I ran into this myself makes me wonder about the 'niche' part)