Skip to content

Learn from non-null tests on as operator#41419

Merged
jcouv merged 3 commits intodotnet:masterfrom
jcouv:nullable-as
Feb 6, 2020
Merged

Learn from non-null tests on as operator#41419
jcouv merged 3 commits intodotnet:masterfrom
jcouv:nullable-as

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Feb 5, 2020

If we learn that x as Type is not null, then we can learn that x was not null.

Fixes #38522
Closes #39424

@jcouv jcouv added this to the 16.6.P1 milestone Feb 5, 2020
@jcouv jcouv self-assigned this Feb 5, 2020
@jcouv jcouv marked this pull request as ready for review February 5, 2020 15:39
@jcouv jcouv requested a review from a team as a code owner February 5, 2020 15:39
@RikkiGibson
Copy link
Copy Markdown
Member

Done with review pass (iteration 2)

Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments didn't show up because I used GitHub incorrectly. Sorry about that.


[Fact]
[WorkItem(39424, "https://github.com/dotnet/roslyn/issues/39424")]
public void LearnFromNullCoalescingFalse()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this test just added in this PR? It looks like one I added when I added support for learning from null coalescing

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just added it.
After looking for tests with ?? false, I found ConditionalBranching_19 which covers this scenario. So I'll remove this test and close the issue (as already fixed and verified)

}

[Fact, WorkItem(38522, "https://github.com/dotnet/roslyn/issues/38522")]
public void AsType_LearningFromNotNullTest()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test that demonstrates when (if ever) this check should be pure or not?

I would be interested in cases where the as type is exactly the same as the expression, and in places where it’s a different type

@jcouv jcouv requested a review from RikkiGibson February 5, 2020 21:40
@jcouv jcouv merged commit b4314fd into dotnet:master Feb 6, 2020
@jcouv jcouv deleted the nullable-as branch February 6, 2020 00:16
333fred added a commit to 333fred/roslyn that referenced this pull request Feb 6, 2020
* dotnet/master: (918 commits)
  Pure null test on unconstrained type (dotnet#41384)
  Fix file headers, bootsrap build issues.
  Make `elementLocations` accept an array of nullable locations in the public api.
  Learn from non-null tests on as operator (dotnet#41419)
  Use Microsoft.NET.Sdk.WindowsDesktop for XAML projects (dotnet#40234)
  Address feedback.
  Introduce `GetRequiredBinder`.
  Add missing `NotNullWhen` annotations.
  Annotate more of the binder
  Add version of zlib library to deterministic build inputs (dotnet#41385)
  [master] Update dependencies from dotnet/arcade (dotnet#41354)
  Simplify
  Simplify
  Do not simplify interpolation expressions on implicit receivers.
  Fix local function crash + add tests
  Substituted symbol equality (dotnet#41123)
  Fix test failures
  Rename from IncludeNonNullableReferenceTypeModifier to IncludeNotNullableReferenceTypeModifier (dotnet#41332)
  Set TEMP environment variable to $TempDir on CI machines (dotnet#41361)
  Document placeholders
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possible bug in the flow analysis of nullable reference types Null check with 'as' does not update nullable state

3 participants