From 94eef2a17295973a20c86be985984e041746407e Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 14 Jan 2019 13:15:37 -0800 Subject: [PATCH] Responding to LDM feedback --- proposals/null-arg-checking.md | 154 ++++++++++++++++++++++++++++++--- 1 file changed, 140 insertions(+), 14 deletions(-) diff --git a/proposals/null-arg-checking.md b/proposals/null-arg-checking.md index 19aab56..fa32f76 100644 --- a/proposals/null-arg-checking.md +++ b/proposals/null-arg-checking.md @@ -1,4 +1,4 @@ -# Simplified Null Argument Checknig +# Simplified Null Argument Checking ## Summary This proposal provides a simplified syntax for validating method arguments are not `null` and @@ -12,6 +12,8 @@ gave us the desire to explore a minimal syntax for argument `null` validation in anticipated to pair often with NRT the proposal is independent of it. ## Detailed Design + +### Null validation The bang operator, `!`, can be positioned after any identifier in a parameter list and this will cause the C# compilet to emit standard `null` checking code for that parameter. For example: @@ -48,43 +50,167 @@ void G(T arg!) { } ``` -In the case of a constructor that uses `this` or `base` to chain to another constructor the -`null` validation will occur after such a call. +In the case of a constructor the `null` validation will occur before any other code in the constructor. That includes: + +- Chaining to other constructors with `this` or `base` +- Field initializers which implicitly occur in the constructor ``` csharp class C { - C(string name!) :this(name) { + string field = GetString(); + C(string name!): this(name) { ... } } ``` -Will be translated into: +Will be roughly translated into the following: ``` csharp class C { - C(string name!) :this(name) { + C(string name) if (name is null) { throw new ArgumentNullException(nameof(name)); } + field = GetString(); + :this(name); ... - } } ``` +Note: this is not legal C# code but instead just an approximation of what the implementation does. + The `!` operator can only be used for parameter lists which have an associated method body. This means it cannot be used in an `abstract` method, `interface`, `delegate` or `partial` method definition. +### Extending is null +The types for which the expression `is null` is valid will be extended to include unconstrained type parameters. This +will allow it to fill the intent of checking for `null` on all types which a `null` check is valid. That is types +which are not definitely known to be value types. Type parameters which are constrained to `struct` cannot be +used with this syntax. + +``` csharp +void NullCheck(T1 p1, T2 p2) where T2 : struct { + if (p1 is null) { + ... + } + + // Error + if (p2 is null) { + ... + } +} +``` + +The behavior of `is null` on a type parameter will be the same as `== null` today. In the cases where the type parameter +is instantiated as a value type the code will be evaluated as `false`. For cases where it is a reference type the +code will do a proper `is null` check. + +### Intersection with Nullable Reference Types +Any parameter which has a `!` operator applied to it's name will start with the nullable state being not `null`. This is +true even if the type of the parameter itself is potentially `null`. That can occur with an explicitly nullable type, +such as say `string?`, or with an unconstrained type parameter. + +When a `!` operator on parameters is combined with an explicitly nullable type on the parameter then a warning will +be issued by the compiler: + +``` csharp +void WarnCase( + string? name!, // Warning: combining explicit null checking with a nullable type + T value1 // Okay +) +``` + ## Open Issuess -- There is no way to check the `value` argument of a property setter for `null` using this syntax. +None ## Considerations -- In the case a constructor chains to another constructor with `this` or `base` the `null` -validation could occur before the chaining call. This is a stronger guarantee the developer could -desire. However C# code cannot be authored that way today and it seems unlikely that it's a -significant issue. Leaving the check after the chaining call means existing code can be ported -to the new pattern without fear of compat concerns. + +### Constructors +The code generation for constructors means there is a small, but observable, behavior change when moving from standard +`null` validation today and the `null` validation parameter syntax (`!`). The `null` check in standard validation +occurs after both field initializers and any `base` or `this` calls. This means a developer can't necessarily migrate +100% of their `null` validation to the new syntax. Constructors at least require some inspection. + +After discussion though it was decided that this is very unlikely to cause any significant adoption issues. It's more +logical that the `null` check run before any logic in the constructor does. Can revisit if significant compat issues +are discovered. + +### Warning when mixing ? and ! +There was a lengthy discussion on whether or not a warning should be issued when the `!` syntax is applied to a +parameter which is explicitly typed to a nullable type. On the surface it seems like a non-sensical declaration by +the developer but there are cases where type hierarchies could force developers into such a situation. + +Consider the following class hierarchy across a series of assemlbies (assuming all are compiled with `null` checking +enabled): + +``` csharp +// Assembly1 +abstract class C1 { + protected abstract void M(object o); +} + +// Assembly2 +abstract class C2 : C1 { + +} + +// Assembly3 +abstract class C3 : C2 { + protected override void M(object o!) { + ... + } +} +``` + +Here the author of `C3` decided to add `null` validation to the parameter `o`. This is completely in line with how the +feature is intended to be used. + +Now imagine at a later date the author of Assembly2 decides to add the following override: + +``` csharp +// Assembly2 +abstract class C2 : C1 { + protected override void M(object? o) { + ... + } +} +``` + +This is allowed by nullable reference types as it's legal to make the contract more flexible for input positions. The +NRT feature in general allows for reasonable co/contravariance on parameter / return nullability. However the language +does the co/contravariance checking based on the most specific override, not the original declaration. This means the +author of Assembly3 will get a warning about the type of `o` not matching and will need to change the signature to the +following to eliminate it: + +``` csharp +// Assembly3 +abstract class C3 : C2 { + protected override void M(object? o!) { + ... + } +} +``` + +At this point the author of Assembly3 has a few choices: + +- They can accept / suppress the warning about `object?` and `object` mismatch. +- They can accept / suppress the warning about `object?` and `!` mismatch. +- They can just remove the `null` validation check (delete `!` and do explicit checking) + +This is a real scenario but for now the idea is to move forward with the warning. If it turns out the warning happens +more frequently than we anticipate then we can remove it later (the reverse is not true). + +### Implicit property setter arguments +The `value` argument of a parameter is implicit and does not appear in any parameter list. That means it cannot be a +target of this feature. The property setter syntax could be extended to include a parameter list to allow the `!` +operator to be applied. But that cuts against the idea of this feature making `null` validation simpler. As such the +implicit `value` argument just won't work with this feature. ## Future Considerations -None \ No newline at end of file +None + +- iterators +- ctors +- lambda parameters \ No newline at end of file