Skip to content
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

Bogus nullable warning for Volatile.Write used in constructor #47663

Closed
jkotas opened this issue Sep 13, 2020 · 5 comments · Fixed by #47952
Closed

Bogus nullable warning for Volatile.Write used in constructor #47663

jkotas opened this issue Sep 13, 2020 · 5 comments · Fixed by #47952

Comments

@jkotas
Copy link
Member

jkotas commented Sep 13, 2020

See dotnet/runtime#42174 (comment) for full context.

Version Used: 3.8.0-3.20452.3 (.NET SDK 5.0.100-rc.1.20454.5)

Steps to Reproduce:

using System;
using System.Runtime.CompilerServices;
using System.Diagnostics.CodeAnalysis;

class C
{
#nullable enable
    static void X<T>([NotNullIfNotNull("value")] ref T location, T value) where T : class?
    { }

    private readonly double[] foo;

    C()
    {
        if (Environment.CommandLine.Length > 0) foo = new double[5];

        double[] bar = new double[3];
        X(ref foo, bar); // gives CS8601, with `ref foo!`, the warning disappears. 
    }
}

Expected Behavior: No nullable warning

Actual Behavior: Nullable warning

@jkotas
Copy link
Member Author

jkotas commented Sep 13, 2020

cc @jcouv

@jcouv
Copy link
Member

jcouv commented Sep 13, 2020

The proposal under discussion is to have ref foo contribute type double[]? to method type inference (because the value of foo is maybe-null in the invocation) rather than double[] as it does currently.

FYI, I updated the OP example with where T : class?.

Note to self: I'm working on this in branch ref-inference

@jcouv jcouv added this to the 16.8 milestone Sep 13, 2020
@jcouv jcouv moved this from Misc to Active/Investigating in Compiler: Julien's umbrellas Sep 14, 2020
@am11
Copy link
Member

am11 commented Sep 14, 2020

@jcouv, thanks for the correction. Yes it was a typo where T : class? (nullable).

@jcouv jcouv moved this from Active/Investigating to Language in Compiler: Julien's umbrellas Sep 17, 2020
@jcouv jcouv moved this from Language to Active/Investigating in Compiler: Julien's umbrellas Sep 17, 2020
@RikkiGibson
Copy link
Contributor

@jcouv If you're still working on this, could you please self-assign it?

@jcouv
Copy link
Member

jcouv commented Sep 25, 2020

Fixed in 16.8p4. Note that there was a bit of impact on the runtime repo. It is due to a known issue with inference with null literal (does not properly contribute its nullability) which we're hoping to address in 16.9.

@jcouv jcouv closed this as completed Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Compiler: Julien's umbrellas
Active/Investigating
Development

Successfully merging a pull request may close this issue.

4 participants