Skip to content

made "foreach to for" to only support rank1 array.#26750

Merged
heejaechang merged 1 commit intodotnet:masterfrom
heejaechang:supportRank1
May 15, 2018
Merged

made "foreach to for" to only support rank1 array.#26750
heejaechang merged 1 commit intodotnet:masterfrom
heejaechang:supportRank1

Conversation

@heejaechang
Copy link
Copy Markdown
Contributor

otherwise, it will pick up IList but Array implementation only support Rank==1 on runtime even if there is no compile time error.

fixes - #26748

otherwise, it will pick up IList but Array implementation only support Rank==1 on runtime even if there is no compile time error.
@heejaechang heejaechang requested a review from a team as a code owner May 9, 2018 23:13
{
void Method()
{
foreach [||] (int a in new int[,] { {1, 2} })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

note: this is somewhat dangerous. If we ever changed the span to be only teh 'foreach', then this test would continue failing, but not for the right reason. Code action tests try to pick a span that should always be safe even in the presence of future refinements to the applicability span.

@heejaechang
Copy link
Copy Markdown
Contributor Author

retest this please

@jcouv jcouv added the Area-IDE label May 11, 2018
@heejaechang
Copy link
Copy Markdown
Contributor Author

@jinujoseph can I get approval for 15.8?

Copy link
Copy Markdown
Contributor

@ivanbasov ivanbasov left a comment

Choose a reason for hiding this comment

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

:shipit:

@jinujoseph
Copy link
Copy Markdown
Contributor

Approved to merge for 15.8.Preview3

@heejaechang
Copy link
Copy Markdown
Contributor Author

thank you!

@heejaechang heejaechang merged commit 832d21f into dotnet:master May 15, 2018
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.

5 participants