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

Fix missing hashcode set in OrderedDictionary.SetAt #103643

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

stephentoub
Copy link
Member

Fixes #103637

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

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

Thanks!

@stephentoub
Copy link
Member Author

CI appears to be in a bad state

@stephentoub stephentoub reopened this Jun 18, 2024
@stephentoub
Copy link
Member Author

/ba-g ci is horked and this bug is blocking another p6 pr

@stephentoub stephentoub merged commit a582842 into dotnet:main Jun 18, 2024
70 of 83 checks passed
@stephentoub stephentoub deleted the fixodhash branch June 18, 2024 23:54
@@ -809,8 +809,8 @@ public void SetAt(int index, TKey key, TValue value)
}

// The key doesn't match that index. If it exists elsewhere in the collection, fail.
uint _ = 0, collisionCount = 0;
if (IndexOf(key, ref _, ref collisionCount) >= 0)
uint hashCode = 0, collisionCount = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Just curious why do we use uint for collision count? In general we use signed everywhere unless we need the full range.

Copy link
Member Author

Choose a reason for hiding this comment

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

This just copies what Dictionary<TKey, TValue> does. I believe it uses uint just to make it very clear there's no possibility of overflow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OrderedDictionary.SetAt bug
3 participants