Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix Dictionary CopyTo regression#17300

Merged
stephentoub merged 1 commit into
dotnet:masterfrom
benaadams:dict-fixes
Mar 28, 2018
Merged

Fix Dictionary CopyTo regression#17300
stephentoub merged 1 commit into
dotnet:masterfrom
benaadams:dict-fixes

Conversation

@benaadams
Copy link
Copy Markdown
Member

Also List set version

/cc @jkotas @stephentoub

Copy link
Copy Markdown
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@danmoseley
Copy link
Copy Markdown
Member

The + i caused worse codegen?

@benaadams
Copy link
Copy Markdown
Member Author

The + i caused worse codegen?

Better codegen, but items could be skipped when hashCode < 0 so the array index doesn't always increase; however i always increases so it would copy both empty entries (dict side) and out of bounds (array side)

@stephentoub stephentoub merged commit fe867dd into dotnet:master Mar 28, 2018
dotnet-bot pushed a commit to dotnet/corert that referenced this pull request Mar 28, 2018
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Mar 29, 2018
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Mar 30, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Mar 30, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Mar 30, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
@jkotas
Copy link
Copy Markdown
Member

jkotas commented Mar 31, 2018

@Anipik This was missed by the CoreCLR->CoreFX mirror.

@benaadams
Copy link
Copy Markdown
Member Author

benaadams commented Mar 31, 2018

Think there were 3 that had it but they weren't merged dotnet/corefx#28632, dotnet/corefx#28633, dotnet/corefx#28634

dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Mar 31, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
@Anipik
Copy link
Copy Markdown

Anipik commented Mar 31, 2018

@jkotas @benaadams I was testing something with mirror and somehow skipped this PR. I have started the Pr for it

@benaadams benaadams deleted the dict-fixes branch April 1, 2018 00:40
Anipik pushed a commit to dotnet/corefx that referenced this pull request Apr 1, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
MichalStrehovsky pushed a commit to MichalStrehovsky/corert that referenced this pull request Apr 6, 2018
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants