API proposal: Unsafe.NullRef, IsNullRef

[jkotas]

Proposed API

namespace System.Runtime.CompilerServices
{
    public class Unsafe
    {
        public bool IsNullRef<T>(ref T);
        public ref T NullRef<T>();
    }
}

ref cannot be null pointer in C#, but it is perfectly valid in IL. ref containing null pointers are useful for low-level optimizations. For example, dotnet/coreclr#27195 uses them to make Dictionary lookups about 10% faster. There are several other examples in the .NET Core framework where we have used ref containing null pointers.

The Unsafe class does have building blocks that allow operations on null ref pointers (e.g. Unsafe.AsPointer<T>(null) creates null ref), but it obfuscates the intent and it is not as straightforward and lean as it can be. The proposed APIs will make it easier.

[jkotas]

cc @GrabYourPitchforks @benaadams @VSadov

[GrabYourPitchforks]

Bwahaha, this lives! 😈

Though upon further thought I'm curious as to whether for usability's sake it would be better to take "ref" instead of "in". I'm wondering if there will be pits of failure where folks pass non-refs to this method, the compiler fixes up the call site to create a reference to the copied local, and the method always returns false.

[jkotas]

it would be better to take "ref" instead of "in"

Agree.

[VSadov]

I think it should take ref.

ref guarantees aliasing.

in allows you to pass rvalues by passing a ref to a copy . There is a little point to test those for null. It would be most likely a bug.

in allows to pass lvalue of TDerived type by passing a ref to a copy. You would get an NRE instead of true

Basically - most cases that in enables you do not want enabled.

[tannergooding]

At the same time, in is more appropriate because it does not mutate the ref and not taking in forces users who have a readonly ref to use ref Unsafe.As(in value)

An analyzer would likely be sufficient to catch the issues of passing an rvalue etc.

[GrabYourPitchforks]

API review - This really should be in from a purity perspective, but should sit down with language folks to figure out exactly what pitfalls might exist and whether the language itself could be evolved to address any concerns. I'll punt this back to myself.

[jkotas]

FWIW, there are two reasons why this should take ref:

• Consistency with the existing APIs: All existing S.R.CS.Unsafe APIs for ref pointer arithmetic take ref.
• Pit of success: in makes it easy to introduce accidental aliasing as pointed above.

[tannergooding]

Consistency with the existing APIs

Many of the APIs were added before in existed (C# 7.2) and it causes issues anytime you want to work with readonly references as it requires you to convert using ref T Unsafe.As<T>(in T). Not only does this add another API call but it strips the readonly from the ref which can make code harder to follow/maintain. There are a number of other APIs (not on Unsafe) where this can also be problematic (one example is Volatile.Read) but for which we can't change due to back-compat (the C# language doesn't allow in methods to be called using the ref keyword, even though it is narrowing, so it is source breaking).

in makes it easy to introduce accidental aliasing as pointed above.

This is true for any usage of in and is a side effect of the compiler allowing rvalues. There exists some well known analyzers (such as ErrorProne.NET) which help avoid these issues.

Personally, I would much rather have an analyzer enabled that calls out places where I may be doing inefficient things than have to insert the equivalent of const_cast all over the place.

However, maybe as an alternative, we could have both IsNullRef(ref T) and IsNullRefReadonly(in T) ?

[jkotas]

it causes issues anytime you want to work with readonly references as it requires you to convert using ref T Unsafe.As<T>(in T)

I have written quite a bit of code against S.R.CS.Unsafe and I have not found this to be a nuisance. I find the unsafe byref-arithmetic with ref to be more readable than with readonly ref/in.

However, maybe as an alternative, we could have both IsNullRef(ref T) and IsNullRefReadonly(in T) ?

Yes, this sounds better to me.

[broudy3]

What about out reference? Could the null check be added also for that? Something like:

public static bool IsNullOut<T>(out T value)
{
   SkipInit(out value);
   return IsNullRef(ref value);
}

example : IsNullOut(out NullRef<int>());

[GrabYourPitchforks]

@broudy3 what's the scenario for caring about 'out'?

Edit: some further thoughts - methods that accept 'out' parameters basically have a contract "I'll write to this address before the method returns." It would be a caller error to pass null for these addresses, akin to saying "in the x64 calling convention, I'm expecting a struct return value but passing nullptr for rcx." The method should expect to AV upon returning.

[tannergooding]

That's really the C# contract however, not necessarily the IL contract. There are plenty of native methods, for example, which take a T** result where the other inputs are validated or special values are looked up if result is null and otherwise they perform the operation as normal.

[GrabYourPitchforks]

I agree, but we're talking about an API specifically for C# callers.

[tannergooding]

for C# callers.

and F#, IL, C++/CLI, or any of the other languages the framework supports.

It is incredibly niche, but one you get into interop land, it's entirely reasonable, and if we are adding a version for IsNullRef and IsNullReadOnlyRef, it only seems reasonable to just expose IsNullOut for parity.
We only need NullRef, however, because ref can be narrowed to either in or out.

[GrabYourPitchforks]

It is incredibly niche

That sounds like a good argument against adding it, especially since it's already doable by stringing together existing APIs. :) In IL and C++/CLI you can already trivially compare the reference against null using existing language syntax. No special function needed. I don't know enough about F# to make a statement there.

and if we are adding a version for IsNullRef and IsNullReadOnlyRef

I'm still not personally convinced on the utility of the in function. But I'm willing to cede that point.

[jkotas]

It is incredibly niche

That sounds like a good argument against adding it, especially since it's already doable by stringing together existing APIs

+1 . S.R.CS.Unsafe is designed around C# refs. refs are the safest type to write unsafe byref arithmetic with. If you have something else, we can add methods to convert it to ref.

I'm still not personally convinced on the utility of the in function.

+1. in is optional and easy to get make mistakes in when writing unsafe byref arithmetic code.

[GrabYourPitchforks]

FWIW, we do seem to have a bunch more folks using byref arithmetic in high-performance code bases. I know that Tanner has been fielding questions along these lines lately. Is there any chance we might get first-class language support to make these scenarios a bit less error-prone? It should also obviate the need for some of the APIs as proposed here.

Strawman example:

public unsafe void DoIt(int* ip)
{
    // 'ip' is non-gc tracked pointer to int
    int* jp = i + 1; // jp points to an address 4 bytes after ip
    bool isNull = (ip == null);
    int deref = *ip;
}

public unsafe void DoIt(int& ip)
{
    // 'ip' is gc tracked pointer to int
    int& jp = i + 1; // jp points to an address 4 bytes after ip, still gc-tracked
    bool isNull = (ip == null);
    int deref = *ip;
}

One pit of failure from this is that the second example resembles a standard pointer, but it's a gc-tracked pointer. If you perform the arithmetic incorrectly you could end up making the GC very angry. But you already have that situation today if you use the Unsafe APIs improperly.

[jkotas]

This sounds like a proposal for https://github.com/dotnet/csharplang. I do not see one on this yet.

[jkotas]

It should also obviate the need for some of the APIs as proposed here.

FWIW, Unsafe has been originally designed as a shortcut to get things done before we have something better. Some of the Unsafe APIs were obliviated by language features already.

[tannergooding]

Is there anything blocking this from being marked ready-for-review? Also, I think this should be milestone future as (AFAIK) it isn't critical for 5.0

[jkotas]

This was API reviewed once already and sent back with the ref vs in question. Do we like the proposal at the top after the follow up discussion?

I agree that this is not critical for 5.0.

[GrabYourPitchforks]

Your API as proposed LGTM.

[terrajobst]

Video

• Looks good as proposed

namespace System.Runtime.CompilerServices
{
    public class Unsafe
    {
        public bool IsNullRef<T>(ref T);
        public ref T NullRef<T>();
    }
}