KEMBAR78
Implement compile-time boundary checks around inline arrays access. by AlekseyTs · Pull Request #68178 · dotnet/roslyn · GitHub
Skip to content

Implement compile-time boundary checks around inline arrays access. #68178

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 15, 2023
Merged

Implement compile-time boundary checks around inline arrays access. #68178

merged 2 commits into from
May 15, 2023

Conversation

@AlekseyTs
Copy link
Contributor

No description provided.

@AlekseyTs AlekseyTs requested review from cston and jjonescz May 12, 2023 13:32
@AlekseyTs AlekseyTs requested a review from a team as a code owner May 12, 2023 13:32
@ghost ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label May 12, 2023
@AlekseyTs
Copy link
Contributor Author

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

jjonescz
jjonescz approved these changes May 15, 2023
View reviewed changes
src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

// PROTOTYPE(InlineArrays): Check bounds
// Check bounds
if (convertedIndex.ConstantValueOpt is { SpecialType: SpecialType.System_Int32, Int32Value: int constIndex })
Copy link
Member

@jjonescz jjonescz May 15, 2023 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

Does this cover integer constants of other types, e.g., Int16? #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.

Does this cover integer constants of other types, e.g., Int16?

We do not need to cover scenarios like that here. We can end up here only with three types: int, System.Index and System.Range. See ``tryImplicitConversionToInlineArrayIndex````, for example.

@AlekseyTs
Copy link
Contributor Author

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

cston
cston reviewed May 15, 2023
View reviewed changes
src/Compilers/CSharp/Test/Semantic/Semantics/InlineArrayTests.cs
_ = f[new System.Index(0, true)];
_ = f[new System.Index(-1, true)];
_ = f[new System.Index(int.MinValue, true)];
}
Copy link
Contributor

@cston cston May 15, 2023 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

}

Consider testing:

f[new System.Index()];
f[new System.Index(value: -1)];
f[new System.Index(value: -1, fromEnd: false)];
f[new System.Index(fromEnd: false, value: -1)];
``` #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 testing: ...

Added.

@AlekseyTs AlekseyTs enabled auto-merge (squash) May 15, 2023 19:44
@AlekseyTs AlekseyTs merged commit 1431be1 into dotnet:features/InlineArrays May 15, 2023
@allisonchou allisonchou mentioned this pull request Jun 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjonescz jjonescz jjonescz approved these changes

+1 more reviewer

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Compilers Feature - Inline Arrays untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AlekseyTs @jjonescz @cston