Skip to content

Added tests for propagating escape errors through array elements#25855

Merged
OmarTawfik merged 1 commit intodotnet:dev15.7.xfrom
OmarTawfik:fix-25485-assert-escape-val
Apr 4, 2018
Merged

Added tests for propagating escape errors through array elements#25855
OmarTawfik merged 1 commit intodotnet:dev15.7.xfrom
OmarTawfik:fix-25485-assert-escape-val

Conversation

@OmarTawfik
Copy link
Copy Markdown
Contributor

@OmarTawfik OmarTawfik commented Mar 31, 2018

Closes #25485
The bug was already fixed in #25819
This PR just adds a test to close the bug.

cc @VSadov @dotnet/roslyn-compiler

@OmarTawfik OmarTawfik added Area-Compilers Test Test failures in roslyn-CI labels Mar 31, 2018
@OmarTawfik OmarTawfik added this to the 15.7 milestone Mar 31, 2018
@OmarTawfik OmarTawfik self-assigned this Mar 31, 2018
@OmarTawfik OmarTawfik requested review from a team and VSadov March 31, 2018 01:06
@svick
Copy link
Copy Markdown
Member

svick commented Mar 31, 2018

Is this test sufficiently different from the ArrayAccessRefStruct that was added in #25819?

roslyn/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs

Lines 3321 to 3344 in 3630339

[Fact]
[WorkItem(25398, "https://github.com/dotnet/roslyn/issues/25398")]
public void ArrayAccessRefStruct()
{
CreateCompilation(@"
ref struct S { }
class C
{
void M()
{
_ = ((S[])null)[0];
var a = ((S[])null)[0];
}
}", options: TestOptions.ReleaseDll).VerifyDiagnostics(
// (8,15): error CS0611: Array elements cannot be of type 'S'
// _ = ((S[])null)[0];
Diagnostic(ErrorCode.ERR_ArrayElementCantBeRefAny, "S").WithArguments("S").WithLocation(8, 15),
// (10,19): error CS0611: Array elements cannot be of type 'S'
// var a = ((S[])null)[0];
Diagnostic(ErrorCode.ERR_ArrayElementCantBeRefAny, "S").WithArguments("S").WithLocation(10, 19)
);
}

@VSadov
Copy link
Copy Markdown
Member

VSadov commented Mar 31, 2018

Not very different, but it uses the actual repro with a span (not with a ref struct).
I think it is ok to add this test - just in case.

VSadov
VSadov approved these changes Mar 31, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM

@OmarTawfik
Copy link
Copy Markdown
Contributor Author

OmarTawfik commented Apr 2, 2018

@dotnet/roslyn-compiler can I get another review?

1 similar comment
@OmarTawfik
Copy link
Copy Markdown
Contributor Author

OmarTawfik commented Apr 4, 2018

@dotnet/roslyn-compiler can I get another review?

@jcouv
Copy link
Copy Markdown
Member

jcouv commented Apr 4, 2018

Test-only change requires only one review. Good to go.

@OmarTawfik OmarTawfik merged commit 1168841 into dotnet:dev15.7.x Apr 4, 2018
@OmarTawfik OmarTawfik deleted the fix-25485-assert-escape-val branch April 4, 2018 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VSadov VSadov VSadov approved these changes

Assignees

@OmarTawfik OmarTawfik

Labels

Area-Compilers Test Test failures in roslyn-CI

Projects

None yet

Milestone

15.7

Development

Successfully merging this pull request may close these issues.

4 participants

@OmarTawfik @svick @VSadov @jcouv