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

Implement definite assignment and region analysis for inline array types #68339

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

AlekseyTs
Copy link
Contributor

No description provided.

@AlekseyTs AlekseyTs requested review from jjonescz and cston May 26, 2023 14:29
@AlekseyTs AlekseyTs requested a review from a team as a code owner May 26, 2023 14:29
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label May 26, 2023
@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler Please review

2 similar comments
@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler Please review

@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler Please review

{
_element0 = 0;
this[0] = 1;
this[1] = 2;
Copy link
Member

@cston cston Jun 5, 2023

Choose a reason for hiding this comment

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

Do these statements affect definite assignment? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do these statements affect definite assignment?

Assigning to an element with non-zero index doesn't affect the assigned state for the inline array.

AssignImpl(elementAccess.Expression, null, isRef, written, read);
if (elementAccess.Expression.Type.HasInlineArrayAttribute(out int length) &&
(elementAccess.Argument.ConstantValueOpt is { SpecialType: SpecialType.System_Int32, Int32Value: 0 } ||
Binder.InferConstantIndexFromSystemIndex(compilation, elementAccess.Argument, length, out _) is 0))
Copy link
Member

@cston cston Jun 6, 2023

Choose a reason for hiding this comment

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

Why check for index 0? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because [0] and the element field are the same thing and assigning to that element should mark the field as assigned.

@AlekseyTs
Copy link
Contributor Author

@cston, @jjonescz, @dotnet/roslyn-compiler Please review.

Assert.Equal(refModifier != "" ? "x, y" : "y", GetSymbolNamesJoined(analysis.ReadOutside));

Assert.Equal("x", GetSymbolNamesJoined(analysis.WrittenInside));
Assert.Equal("x, y", GetSymbolNamesJoined(analysis.WrittenOutside));
Copy link
Member

@cston cston Jun 6, 2023

Choose a reason for hiding this comment

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

x

Why is x considered written (inside or outside)? Is it because a reference was taken to the contents of x, regardless of whether that reference was written through? Or because it is a parameter that is not out? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is x considered written (inside or outside)? Is it because a reference was taken to the contents of x, regardless of whether that reference was written through? Or because it is a parameter that is not out?

The latter: "because it is a parameter that is not out".

IL_0032: ret
}
");
}
Copy link
Member

@cston cston Jun 6, 2023

Choose a reason for hiding this comment

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

}

Consider compiling with TestOptions.Regular10 as well, to verify the compiler reports ERR_ParamUnassigned even though the field and all items have been assigned explicitly. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider compiling with TestOptions.Regular10 as well, to verify the compiler reports ERR_ParamUnassigned even though the field and all items have been assigned explicitly.

Added similar test scenario to DefiniteAssignment_34.

@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler For the second review.

@AlekseyTs AlekseyTs enabled auto-merge (squash) June 8, 2023 13:49
@AlekseyTs AlekseyTs merged commit 2c31d1c into dotnet:features/InlineArrays Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers New Feature - Inline Arrays untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants