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

[Proposal]: Improved Definite Assignment Analysis #4465

Open
4 of 6 tasks
Tracked by #829
RikkiGibson opened this issue Feb 25, 2021 · 2 comments
Open
4 of 6 tasks
Tracked by #829

[Proposal]: Improved Definite Assignment Analysis #4465

RikkiGibson opened this issue Feb 25, 2021 · 2 comments
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Milestone

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Feb 25, 2021

Improved Definite Assignment Analysis

Summary

Definite assignment analysis as specified has a few gaps which have caused users inconvenience. In particular, scenarios involving comparison to boolean constants, conditional-access, and null coalescing.

Related discussions and issues

csharplang discussion of this proposal: #4240

Probably a dozen or so user reports can be found via this or similar queries (i.e. search for "definite assignment" instead of "CS0165", or search in csharplang).
https://github.com/dotnet/roslyn/issues?q=is%3Aclosed+is%3Aissue+label%3A%22Resolution-By+Design%22+cs0165

I have included related issues in the scenarios below to give a sense of the relative impact of each scenario.

Scenarios

As a point of reference, let's start with a well-known "happy case" that does work in definite assignment and in nullable.

#nullable enable

C c = new C();
if (c != null && c.M(out object obj0))
{
    obj0.ToString(); // ok
}

public class C
{
    public bool M(out object obj)
    {
        obj = new object();
        return true;
    }
}

Comparison to bool constant

if ((c != null && c.M(out object obj1)) == true)
{
    obj1.ToString(); // undesired error
}

if ((c != null && c.M(out object obj2)) is true)
{
    obj2.ToString(); // undesired error
}

Comparison between a conditional access and a constant value

This scenario is probably the biggest one. We do support this in nullable but not in definite assignment.

if (c?.M(out object obj3) == true)
{
    obj3.ToString(); // undesired error
}

Conditional access coalesced to a bool constant

This scenario is very similar to the previous one. This is also supported in nullable but not in definite assignment.

if (c?.M(out object obj4) ?? false)
{
    obj4.ToString(); // undesired error
}

Conditional expressions where one arm is a bool constant

It's worth pointing out that we already have special behavior for when the condition expression is constant (i.e. true ? a : b). We just unconditionally visit the arm indicated by the constant condition and ignore the other arm.

Also note that we haven't handled this scenario in nullable.

if (c != null ? c.M(out object obj4) : false)
{
    obj4.ToString(); // undesired error
}

Specification

The specification has moved to https://github.com/dotnet/csharplang/blob/master/proposals/improved-definite-assignment.md

Drawbacks

It feels odd to have the analysis "reach down" and have special recognition of conditional accesses, when typically flow analysis state is supposed to propagate upward. We are concerned about how a solution like this could intersect painfully with possible future language features that do null checks.

Alternatives

Two alternatives to this proposal:

  1. Introduce "state when null" and "state when not null" to the language and compiler. This has been judged to be too much effort for the scenarios we are trying to solve, but that we could potentially implement the above proposal and then move to a "state when null/not null" model later on without breaking people.
  2. Do nothing.

Unresolved questions

public class C {
    public int M0(object obj) => 0;

    public static void M1(C? c) {
        int x, y;
        _ = c?.M0(x = y = 0) <= 0
            ? x.ToString() // no warning
            : y.ToString(); // warning
    }
}

Design meetings

#4243

@RikkiGibson
Copy link
Contributor Author

The feature is now implemented and merged into the compiler.

@jnm2
Copy link
Contributor

jnm2 commented Jan 6, 2024

Could this be extended to work in switch expressions, a natural evolution from conditional expressions?

// Already works
public string ValidationMessage => validationResult?.IsError(out var errorMessage) is true ? errorMessage : null

// But this does not
public string ValidationMessage => validationResult?.IsError(out var errorMessage) switch
{
    // ❌ CS0165 Use of unassigned local variable 'errorMessage'
    //      ↓↓↓↓↓↓↓↓↓↓↓↓
    true => errorMessage,
    false => $"The backup path has been validated by {instanceName}",
    _ => "Checking the backup path..."
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Projects
None yet
Development

No branches or pull requests

4 participants