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

Roslyn is unable to correcltly infer variables implicitly checked for null when expression under cast is not trivial #36695

Closed
TessenR opened this issue Jun 24, 2019 · 2 comments

Comments

@TessenR
Copy link

TessenR commented Jun 24, 2019

Version Used:

Branch master (21 Jun 2019)
Latest commit 898bed by Heejae Chang:
added NFW to get some data on incremental parsing bug where source si� (#36620)

  • added NFW to get some data on incremental parsing bug where source size and tree size is different

  • more comments

Steps to Reproduce:

Compile the following code

#nullable enable
interface IV<T> { T V { get; } }

class C
{
  void M1<T,V>(V v) where T : V where V : class, IV<V>
  {
    // this check correctly infers that 'v' is not null in then branch
    if (((T) v)?.ToString() != null)
    {
        v.ToString();
    }
      
    // this check also correctly infers that 'v' is not null in then branch
    if (((T) v?.V) != null)
    {
        v.ToString();
    }
 
    // this check does not infer that 'v' is not null in then branch
    if (((T) v?.V)?.ToString() != null)
    {
        v.ToString();
    }

    // if you dereference the result instead of using it in a conditional access combined with a null check
    // Roslyn will correctly infer that result of ((T) v?.V) being not null guarantees that 'v' is not null too
    ((T) v?.V).ToString();
    v.ToString();
  }
}

Expected Behavior:
Roslyn applies the same rules to infer implicitly null-checked variables when it processes dereference e.g. x.ToString() and comparison with null e.g. x != null.

Actual Behavior:
Roslyn warns about v possibly containing a null value after checking that ((T) v?.V)?.ToString() != null

Notes
Roslyn already checks implicitly checked variables through casts and null conditional accesses as demonstrated by the first if in the code above.
However using conditional access in the casted expression prevents Roslyn from correctly unwrapping it when processing null checks even though the same works with explicit dereferences

@RikkiGibson
Copy link
Contributor

I think this is a situation where the corresponding scenario in definite assignment isn't fixed by the "improved definite assignment" proposal. Particularly because when you look at an expression like ((T)v?.V)?.ToString(), the "non-conditional" counterpart is just ((T)v?.V).ToString().

Will try to take a closer look, but it's a scenario we may end up punting on.

@RikkiGibson
Copy link
Contributor

Ended up being fixed by "improved definite assignment" 🎉 #51463 SharpLab

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

No branches or pull requests

4 participants