Skip to content

Conversation

@RikkiGibson
Copy link
Member

Closes #48574

Basically, entering the this parameter caused the declared state to be written to the slots for the struct fields, so we need to makeNotNullMembersMaybeNull() after the EnterParameter, not before. I think it also makes more sense to move the call into the !_isSpeculative block, since in speculative analysis we should just load a snapshot that already has the "state before we call base.Scan()" plugged into it. (if I'm reading the code right @333fred.)

I think we should revisit the way we populate state when assigning to variables of struct types eventually (to solve #47596, for example).

@dotnet/roslyn-compiler for review please

@RikkiGibson RikkiGibson requested a review from a team as a code owner October 15, 2020 20:21
@RikkiGibson RikkiGibson requested a review from a team as a code owner October 15, 2020 21:58
@RikkiGibson
Copy link
Member Author

New warnings in dotnet/runtime:

#nullable disable warnings
public ConflictResolution(string errorMessage) : this()
=> ErrorMessage = errorMessage;
#nullable enable warnings
Copy link
Member Author

Choose a reason for hiding this comment

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

@CyrusNajmabadi for a spot check on this change. This PR fixes a bug that caused us to miss nullable field initialization warnings on a struct with a : this() initializer.

@jcouv jcouv self-assigned this Oct 16, 2020
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Compiler change LGTM Thanks (iteration 3)

@RikkiGibson
Copy link
Member Author

RikkiGibson commented Oct 16, 2020

running a VS validation insertion before merging here.

update: we built VS successfully so will merge as soon as we get sign-off for IDE changes. https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/281961

@RikkiGibson RikkiGibson merged commit 405d1a5 into dotnet:master Oct 19, 2020
@ghost ghost added this to the Next milestone Oct 19, 2020
@RikkiGibson RikkiGibson deleted the fix-48574 branch October 19, 2020 16:51
@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
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.

Missing nullability warnings about member initialization in constructors in structs with default struct ctor calls

5 participants