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

Auto-default ref fields with a null ref-assignment #63065

Merged
merged 5 commits into from
Aug 9, 2022

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Jul 29, 2022

Vlad mentioned that the fixed statement can null-out references. Found that code in LocalRewriter_FixedStatement.cs and factored it out into a NullRef helper.
This is emitted using the same IL as ref T Unsafe.NullRef<T>(), namely ldc.i4.0 followed by conv.u.

No spec update required, as I think the desired behavior is already covered by auto-default-structs.md:

  1. When the this variable itself does not meet the requirements, then all unassigned instance variables within this at all points where requirements are violated are implicitly initialized to the default value (§9.3) in an initialization phase before any other code in the constructor runs.
  2. When an instance variable v within this does not meet the requirements, or any instance variable at any level of nesting within v does not meet the requirements, then v is implicitly initialized to the default value in an initialization phase before any other code in the constructor runs.

Fixes #63018

Relates to test plan #59194

@jcouv jcouv changed the base branch from main to release/dev17.4 July 29, 2022 20:30
@jcouv jcouv marked this pull request as ready for review July 29, 2022 20:31
@jcouv jcouv requested a review from a team as a code owner July 29, 2022 20:31
@@ -12592,5 +12592,198 @@ public void ScopedReserved_Alias_Escaped()
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using @scoped = System.Int32;").WithLocation(1, 1)
);
}

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

@cston cston Jul 29, 2022

Choose a reason for hiding this comment

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

InitRefField_AutoDefault()

Consider moving above ScopedReserved_* tests, to avoid a conflict with #62783. #Resolved

@jcouv jcouv changed the base branch from release/dev17.4 to main August 2, 2022 05:18
@jcouv jcouv marked this pull request as draft August 2, 2022 05:37
@jcouv jcouv marked this pull request as ready for review August 5, 2022 15:33
@jcouv
Copy link
Member Author

jcouv commented Aug 5, 2022

@cston FYI, I skipped one test following integration with runtime flag change (that test needs to use Net60 framework and use the test hook...).

@jcouv
Copy link
Member Author

jcouv commented Aug 5, 2022

@dotnet/roslyn-compiler for second review. Thanks

@jcouv
Copy link
Member Author

jcouv commented Aug 5, 2022

@dotnet/roslyn-compiler for second review. Thanks

@jaredpar jaredpar merged commit dac7e5c into dotnet:main Aug 9, 2022
@ghost ghost added this to the Next milestone Aug 9, 2022
@dibarbet dibarbet modified the milestones: Next, 17.4 P2 Sep 1, 2022
@jcouv jcouv deleted the auto-default branch September 12, 2022 17:37
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.

Auto-default structs cause runtime exceptions in explicit constructors when a ref field is not initialized
4 participants