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

Non-nullable value detected as nullable #58211

Closed
NiJeTi opened this issue Dec 9, 2021 · 6 comments
Closed

Non-nullable value detected as nullable #58211

NiJeTi opened this issue Dec 9, 2021 · 6 comments
Labels
Area-Compilers Resolution-By Design The behavior reported in the issue matches the current design untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@NiJeTi
Copy link

NiJeTi commented Dec 9, 2021

Version Used:
Version: 6.0.0
Commit: 4822e3c3aa

Steps to Reproduce:

object? GetPossibleNull1() => throw new NotImplementedException();
object? GetPossibleNull2() => throw new NotImplementedException();
void MethodAcceptingNotNull(object obj) => throw new NotImplementedException();

var object1 = GetPossibleNull1();
var object2 = GetPossibleNull2();

if (object1 is not null || object2 is not null)
    MethodAcceptingNotNull(object1 ?? object2);

Expected Behavior:
No warnings in build output

Actual Behavior:
Warning CS8604 : Possible null reference argument for parameter 'obj' in 'void MethodAcceptingNotNull(object obj)'.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 9, 2021
@NiJeTi NiJeTi changed the title Non-nullable value detected is nullable Non-nullable value detected as nullable Dec 9, 2021
@Youssef1313
Copy link
Member

Youssef1313 commented Dec 9, 2021

This is unfortunately by design. Similar issue: #36927

You can re-write the code as:

if (object1 is not null)
    MethodAcceptingNotNull(object1);
else if (object2 is not null)
    MethodAcceptingNotNull(object2);

or use null-forgiving operator !.

@jcouv
Copy link
Member

jcouv commented Dec 13, 2021

Youssef is correct, this is by-design. Flow analysis keeps track of a state for each variable in the different branches of the code. When we get inside the if, the state is that both object1 and object2 are maybe-null.

@jcouv jcouv added the Resolution-By Design The behavior reported in the issue matches the current design label Dec 13, 2021
@NiJeTi
Copy link
Author

NiJeTi commented Dec 13, 2021

@jcouv As I understand, this happens because in my case both variables WAS NOT marked by flow analyzer as not-null. Am I right?

@Youssef1313
Copy link
Member

@NiJeTi Kind of, but marking both as not-null is incorrect too. If both are marked not-null then:

if (object1 is not null || object2 is not null)
    MethodAcceptingNotNull(object1); // since both are not-null, this won't be an error while it should.

The request here, per my understanding, is a different type of flow analysis as stated in #36927 (comment).

@jcouv
Copy link
Member

jcouv commented Dec 13, 2021

@NiJeTi Sorry if this is too much details. The analysis of || goes as follows: we compute two states after the first expression (whenTrue1 and whenFalse1). In whenTrue1 we'll have object1 as not-null, but in whenFalse1 we'll have object1 as maybe-null. Then we take the whenFalse1 case as starting point and analyze the second expression. This results in two states whenTrue2 where object2 is not-null and whenFalse2 where object2 is maybe-null. And in both those states, object1 is still maybe-null.
The state for the consequence block of the if is constructed by joining whenTrue1 and whenTrue2 (ie. taking the worse case from both states). That state has object1 and object2 as both maybe-null.

Because states just track whenever each local can be null, there is no way to represent "object2 is not-null when object1 is maybe-null".

@NiJeTi
Copy link
Author

NiJeTi commented Dec 13, 2021

@jcouv thanks for the explanation. All works as I understood. I think issue can be closed for the moment. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Resolution-By Design The behavior reported in the issue matches the current design untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

3 participants