133 lines
No EOL
3.6 KiB
Markdown
133 lines
No EOL
3.6 KiB
Markdown
|
|
# C# LDM for Feb. 5, 2020
|
|
|
|
## Agenda
|
|
|
|
1. Dependent nullability attribute
|
|
2. Null checking in unconstrained generics
|
|
|
|
## Discussion
|
|
|
|
### Nullability
|
|
|
|
Dependent calls:
|
|
|
|
We'd like to be able to support patterns like:
|
|
|
|
```C#
|
|
if (x.HasValue)
|
|
{
|
|
x.Value // should not warn when HasValue is true
|
|
}
|
|
```
|
|
|
|
We would add attributes to support these use cases: `EnsuresNotNull` and `EnsuresNotNullWhen` (to parallel the existing `NotNull` attributes). The
|
|
proposal as stands is to name the fields or properties and that would be
|
|
the inputs and outputs for the flow analysis. We propose that the lookup
|
|
rules for resolving the names in these instances would be similar to the
|
|
rules for the `DefaultMemberAttribute`.
|
|
|
|
This could also be used for helpers in constructor initialization, where today constructors which
|
|
only call `base` are required to initialize all members, even if a helper initializes some of the
|
|
members.
|
|
|
|
There's a follow-up question: should you be able to specify that members of the parameter are
|
|
not-null after the call? For example,
|
|
|
|
```C#
|
|
class Point
|
|
{
|
|
public object? X;
|
|
}
|
|
static void M([EnsuresNotNull(nameof(Point.X))]Point p) { ... }
|
|
```
|
|
|
|
We could also allow it for nested annotation
|
|
|
|
```C#
|
|
class Point
|
|
{
|
|
public object? X;
|
|
}
|
|
class Line
|
|
{
|
|
public Point P1;
|
|
public Point P2;
|
|
}
|
|
static void M([EnsuresNotNull("P1.X", "P1.Y")]Line l) { ... }
|
|
static bool M([EnsuresNotNullWhen(true, "P1.X", "P1.Y")]Line l) { ... }
|
|
```
|
|
|
|
The nested names could also be used for return annotations.
|
|
|
|
However, we're not sure this is worth it. We see the usefulness in theory,
|
|
but we're not sure how often it would actually be used. If we want to leave
|
|
the space for later, we could produce an error when writing an attribute with
|
|
an unsupported string form.
|
|
|
|
Similarly, if we don't want to support referring to members of types through
|
|
the parameters, as in the first example, we can also provide an error for these
|
|
scenarios. Or, we could say that the initial proposal is qualitatively different
|
|
from all these scenarios:
|
|
|
|
```C#
|
|
[MemberNotNull(nameof(X))]
|
|
void M() { }
|
|
```
|
|
|
|
For the situations in parameters and return types we are referring to the target the attribute is
|
|
being applied to, while the original proposal is returning to the containing type, somewhat
|
|
unrelated to the location of the attribute.
|
|
|
|
We're also not sure exactly what the name or shape of these attributes would
|
|
look like. We think this could be valuable, but we'd like to decide with
|
|
the full context of other attributes we're considering.
|
|
|
|
**Conclusion**
|
|
|
|
We see the usefulness of the original scenario, but the broadening we're not sure
|
|
on the return on investment. Let's support the original scenario through a new
|
|
attribute, `MemberNotNull`, that only has members as valid attribute targets to start.
|
|
|
|
If we find users still hit significant limitations without the parameter and return
|
|
type support, we can consider broadening in a future release.
|
|
|
|
### Pure non-null check in generic
|
|
|
|
```C#
|
|
public T Id<T>(T t)
|
|
{
|
|
if (t is null) Console.WriteLine();
|
|
return t; // warn?
|
|
}
|
|
```
|
|
|
|
The question here is whether we should consider `T` to move to `MaybeDefault` after
|
|
checking for `null`. We never do this today.
|
|
|
|
The scenario where a "pure" null check would come into play is:
|
|
|
|
```C#
|
|
public T Id<T>(T t) where T : notnull
|
|
{
|
|
if (t is null) Console.WriteLine();
|
|
return t;
|
|
}
|
|
```
|
|
|
|
It appears that this does not warn today, which looks like a bug. The analogous
|
|
scenario for `T??` is
|
|
|
|
```C#
|
|
public T ID<T>(T t)
|
|
{
|
|
if (t is default) Console.WriteLine();
|
|
return t;
|
|
}
|
|
```
|
|
|
|
However, this is illegal as there is no way to check if `T` is `default(T)`.
|
|
|
|
**Conclusion**
|
|
|
|
The original scenario should not warn. |