Feature: Check `ref` variables for `null`

[NickStrupat]

Wondering if it's something you've thought about.

Semantically, I'm thinking it'd be equivalent to this:

public void Foo(ref int i)
{
    if (Unsafe.IsNullRef(ref i))
        throw ...;
    ...
}

[mikernet]

I am leaning towards this being a similar situation as null pointers, in that explicit checks are better here, if you deem them necessary. You normally shouldn't really be worrying about null refs, and in the few cases where you're doing something unsafe that may produce a null ref then I think there is value in having the checks be explicit.

(CORRECTED, see below) In v1 I had it so you could annotate pointer types with [AllowNull] / [DisallowNull] but it didn't always reliably flow nullable context since the compiler ignores pointer types when considering how to annotate things.

[mikernet]

Do you have an example of where this would be useful?

I am potentially open to adding configuration properties to the <RuntimeNullables> element that turn on additional checks. I could maybe see checking refs in debug being useful. Not sure it has much value in release code.

EDIT: Actually yeah, after some more thought, it could be nice to have this default to on for debug builds (given that receiving null refs is typically unexpected) and off for release builds, similar to how output checks work currently. You would then be able to override the behavior with CheckRefs="true" if you wanted it on in release builds as well.

Thoughts? Would that cover what you are looking for?

[mikernet]

Ah, I forgot about something...per v2 notes:

Removed support for checking pointers (nullable context does not flow correctly/reliably in some cases and this can't be fixed).

It is almost a certainty that the same problem will apply to ref parameters, so we end up in the same place as before with pointers: might as well just do it explicitly if needed.

[mikernet]

I suppose you could make the argument that ref params/returns should never be a 0 / null address (Roslyn assumes the same) so flowing nullable context is irrelevant in this case. I could probably get behind that.

[NickStrupat]

Yea I think in general there should never be a 0/null ref. That said, there are some framework methods that return null refs, such as CollectionsMarshal.GetValueRefOrNullRef and Unsafe.NullRef<T>.

As far as I can tell, neither of those methods provide nullability attributes/metadata, so it would require a hack to properly deal with those cases (and whatever others there might be).

[NickStrupat]

A hard-coded list of framework methods that might return/assign null refs would "work", but it wouldn't be amazing haha.

Also, hey, I grew up in London ❤

[mikernet]

Yeah, you can get null/0 refs with unsafe methods, but IMO you shouldn't be returning/outputting/passing those outside of internal optimizations within your method body so I don't think it is a problem. The checks would catch exactly this kind of bad behavior. Is there a different use-case for this?

If you really wanted to override it to allow that (i.e. you are writing your own unsafe method intended for public consumption that is clearly indicated as being unsafe) then you would do [NullChecks(false)] on the method in question. I expect such cases to be exceedingly rare.