New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify type inference rules for nullability annotations #30670

Merged
merged 3 commits into from Nov 6, 2018

Conversation

@gafter
Member

gafter commented Oct 23, 2018

Fixes #30433

@MadsTorgersen Does this also belong somewhere in the csharplang repo? https://github.com/dotnet/csharplang/blob/master/proposals/nullable-reference-types.md appears to be more than a year out of date.

@gafter gafter added this to the 16.0 milestone Oct 23, 2018

@gafter gafter self-assigned this Oct 23, 2018

@gafter gafter requested review from MadsTorgersen and cston Oct 23, 2018

@gafter gafter added this to Working/In Review in Compiler: Gafter Oct 23, 2018

- the invariant rule if the K's type parameter in the i position is invariant.
- the covariant rule if the K's type parameter in the i position is declared `out`.
- the contravariant rule if the K's type parameter in the i position is declared `in`.
Merging the array types T[] and U[] results in the type V[] where V is the result of merging T and U by the covariant rule.

This comment has been minimized.

@cston

cston Oct 23, 2018

Member

Merging array element nullability currently uses the invariant rule. For instance, we report "No best nullability" for the following:

    static void F(object[] notNullableArray, object?[] nullableArray)
    {
        var x = new[] { notNullableArray, nullableArray };
    }
```  #Resolved
@gafter

This comment has been minimized.

Member

gafter commented Oct 23, 2018

I've adjusted the rule for arrays to the rule as implemented. #Resolved

@cston

cston approved these changes Oct 23, 2018

LGTM. Thanks!

- Merging `dynamic` and `object` results in the type `dynamic`.
- Merging tuple types that differ in element names is specified elsewhere.
- Merging equivalent types that differ in nullability is performed as follows: merging the types Tn and Um (where n and m are differing nullability annotations) results in the type Vk where V is the result of merging T and U using the invariant rule, and k is as follows:

This comment has been minimized.

@jcouv

jcouv Nov 2, 2018

Member

nit: consider backquoting identifiers (here and below) #Resolved

- if either n or m are nullable, nullable.
- otherwise oblivious.
- Merging constructed generic types is performed as follows: Merging the types K<A1, A2, ...> and K<B1, B2, ...> results in the type K<C1, C2, ...> where Ci is the result of merging Ai and Bi by the invariant rule.
- Merging the array types T[] and U[] results in the type V[] where V is the result of merging T and U by the invariant rule.

This comment has been minimized.

@jcouv

jcouv Nov 2, 2018

Member

Are there other container types than arrays that might need to be spelled out? For instance, pointer types? #Resolved

@jcouv

jcouv approved these changes Nov 2, 2018

LGTM Thanks

@gafter

This comment has been minimized.

Member

gafter commented Nov 5, 2018

test roslyn-CI please

@gafter gafter closed this Nov 5, 2018

@gafter gafter reopened this Nov 5, 2018

@gafter gafter merged commit 862dcc7 into dotnet:master Nov 6, 2018

3 of 4 checks passed

windows_debug_vs-integration_prtest Build finished.
Details
license/cla All CLA requirements met.
Details
roslyn-CI #20181105.11 succeeded
Details
windows_release_vs-integration_prtest Build finished.
Details

gafter added a commit to gafter/roslyn that referenced this pull request Nov 6, 2018

@gafter gafter removed this from Working/In Review in Compiler: Gafter Nov 6, 2018

gafter added a commit that referenced this pull request Nov 9, 2018

Make nullabiulity inference associative and commutative (#30990)
Fixes #30966
Implements the spec change described in #30670
See also #30989
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment