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

Improve definite assignment to understand coalescing with false and nullable comparison with true #3365

Closed
jnm2 opened this issue Apr 17, 2020 · 6 comments

Comments

@jnm2
Copy link
Contributor

jnm2 commented Apr 17, 2020

These are forms of the same logic from most preferred to least preferred. The compiler does not consider them to be the same for the purpose of definite assignment, but it should.

The ?? false and == true guarantee that execution cannot move inside the if block without first calling TryGetValue and assigning the out parameter:

using System;
using System.Collections.Generic;

class C
{
    void M(IReadOnlyDictionary<int, int> expr)
    {
        if (expr?.TryGetValue(42, out var value1) ?? false)
        {
            // ❌ CS0165: Use of unassigned local variable 'value1'
            Console.WriteLine(value1);   
        }
        
        if (expr?.TryGetValue(42, out var value2) == true)
        {
            // ❌ CS0165: Use of unassigned local variable 'value2'
            Console.WriteLine(value2);   
        }
        
        if (expr is { } && expr.TryGetValue(42, out var value3))
        {
            Console.WriteLine(value3);   
        }
    }
}

Similar but for pattern matching rather than out parameters, and not involving nullability: #801

@jnm2 jnm2 changed the title Improve definite assignment to understand coalescing with false and comparing with true Improve definite assignment to understand coalescing with false and nullable comparison with true Apr 17, 2020
@hez2010
Copy link

hez2010 commented Apr 17, 2020

Seems that Roslyn always forgot to analyze patterns like if (a?.b ?? false) and if (a?.b == true), even in NRT analysis 😄

Maybe you should open this issue in https://github.com/dotnet/roslyn/issues

@jnm2
Copy link
Contributor Author

jnm2 commented Apr 17, 2020

@hez2010 My understanding is that the C# definite assignment spec needs to be updated before Roslyn can do anything about the examples I gave in this issue, unlike the nullability warnings you mention.

@333fred
Copy link
Member

333fred commented Apr 17, 2020

That's correct @jnm2. While I'm interested in championing this, I'd like to sit down with the definite assignment spec and see the level of change we'd need to make to support it, so before I actually tag it and such I'll work on that.

@jnm2
Copy link
Contributor Author

jnm2 commented Jul 28, 2020

My first example is a duplicate of #916. Is my second example close enough to close this as a dupe? (Hate to lose the 13 upvotes though..)

@333fred
Copy link
Member

333fred commented Jul 29, 2020

Yeah, it's pretty much exactly the same as those. I will say that while I still want to come up with a spec for how this would work, I have not had the time (as you might be able to tell). If a community member wanted to make a proposal for how to modify the definite assignment rules, that's probably what would drive progress here.

@jnm2
Copy link
Contributor Author

jnm2 commented Jul 29, 2020

Okay, closing this in favor of the original at #916. Please upvote and follow there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants