# C# LDM Notes for April 24, 2019

## Agenda

1. MaybeNull and related attributes

## Discussion

There are two proposals on the table:
- https://github.com/cston/csharplang/blob/MaybeNull/proposals/MaybeNull.md
- https://gist.github.com/MadsTorgersen/e94bc6802b96ce9cc65bc3dd39b7f6a2
 
The proposals are primarily focused on providing nullable information for the caller of methods
where certain nullable contracts are outside the ability of our current annotation syntax to
express.

We've also gathered the following information on nullable contracts as they currently
exist in CoreFX:

> We’ve annotated most of corelib at this point.  I just quickly went through looking at some of the ~600 TODO-NULLABLE follow-up comments we have for examples of public API signatures that are currently less-than-correct and that would benefit from further ability to annotate/attribute.  I did not include things that just impact locals or fields or other non-API stuff.

> Generic methods or method on generic type that might return default(T), e.g.

> - Array.Find<T>(T[] array, Predicate<T> match).  Returns default(T) if there isn’t a match.
> - Lazy<T>.ValueForDebugDisplay.  Returns default(T) if the Lazy<T> hasn’t yet been initialized.
> - Marshal.PtrToStructure<T>(IntPtr).  If you pass in IntPtr.Zero, you’ll get back default(T).

> Generic methods accepting an unconstrained T with argument that should not be null
 
> - Marshal.GetFunctionPointerForDelegate

> Try- methods on generic type that outs default(T), e.g.

> - ConcurrentQueue<T>.TryDequeue(out T)
> - IProducerConsumerCollection<T>.TryTake(out T)
> - WeakReference<T>.TryGetTarget(out T target)
> Try- methods with non-generics, e.g.
> - Version.TryParse
> - Semaphore.TryOpenExisting

> Try- methods that return a struct wrapper around a reference type that’ll be non-null if returning true

> - MemoryMarshal.TryGetArray(…, out ArraySegment<T>)

 

> Interfaces where T should be nullable on some methods but non-nullable on others, e.g.

> - IEqualityComparer<T>: Equals(T x, T y) should have nullable arguments, but GetHashCode(T obj) should have a non-nullable argument

 

> Fields/properties of type T that start life as default(T), and maybe become default(T) again

> - AsyncLocal<T>.Value
> - ThreadLocal<T>.Value
> - StrongBox<T>.Value 

> Array slots that need to be settable to default(T)

> - Pretty much every collection we have, nulling out a slot when an element is removed

> Methods where whether the return value can be null is impacted by whether an argument is null

> - RegistryKey.GetValue
> - Path.GetFullName
> - Path.ChangeExtension
> - Path.GetExtension
> - Path.GetFileNameWithoutExtension
> - Convert.ToString(string?)
> - Delegate.Combine

 

> Methods where whether the return value can be null is impacted by whether an argument is true/false
> - Type.GetType(…, bool throwOnError)

 

> Methods where one argument will be non-null if another is non-null
> - Volatile.Write
> - Interlocked.Exchange
> - Interlocked.CompareExchange (this one’s more complicated still)

 

> Methods that accept an delegate<object?> and object?, but where the argument to Action<object?> will be non-null iff the object? is non-null
> - Task.Factory.StartNew
> - ExecutionContext.Run
> - SynchronizationContext.Post
> - CancellationToken.Register
> - ThreadPool.QueueUserWorkItem
> - Task ctor
> - IValueTaskSource.OnCompleted

 

> Ref arguments that will be non-null upon return
> - Array.Resize(ref T[]);
> - LazyInitializer.EnsureInitialized(ref T) (this one’s complicated if T is a nullable value type)


The first question is whether or not the annotation of the method
changes, e.g.

```C#
public static class Enumerable
{
    [return: MaybeNull]
    public static T FirstOrDefault<T>(this IEnumerable<T> src) {...}

    public static T Identity<T>(T t) => t;
}
```

Here `FirstOrDefault<string>` does not produce a `string?`, but the flow state of values returned
from calls is a string with a MaybeNull state. Following through, for
`Identity(FirstOrDefault<string>(...))`, the inferred type of `T` for `Identity<T>` would be
`string?` because the flow state is used to construct the annotation of the substituted type
argument.

As written, if you were to provide a `FirstOrDefault<string>` override for an analogous
`FirstOrDefault<T>` instance method, you could not write `string?` for the return type, despite
the `MaybeNull` attribute.

The proposal does not attempt to address modifying what the methods accept, only
what they produce, meaning that providing `MaybeNull` on a parameter does not
indicate that the input can be nullable.

However, we think `MaybeNull` may be common or impactful enough to consider syntax, like allowing
`T?` on an unconstrained type parameter or `T : object?`, to express `MaybeNull`. Importantly,
this would not address all the patterns we've found while annotating CoreFX, like
`Interlocked.CompareExchange` or `Debug.Assert`.

The other problem is that the list of attributes which are needed to represent all
patterns is unbounded for all the code, but even the set of attributes needed for
just what we know is common is quite large.

**Conclusion**

We think the best approach right now is to try to address ~80% of the common cases
using a mixture of attributes and special casing for extremely complicated contracts
like `Interlocked.CompareExchange`.

For `MaybeNull` and `T?` specifically, we are conflicted between the potential value
that `T?` can provide, especially in annotating nested types, and the additional
complexity in both language and implementation. One thing we're particularly worried
about is the design work in adopting `T?` and revisiting some previous decisions.
We're going to examine that work and if it's low in comparison to the `MaybeNull`
attribute work, there's substantial support for implementing the feature. If we do,
implement `T?`, we do not want to include the `MaybeNull`, specifically.