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 hashmap put all for create + test case #1696

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomaskallup
Copy link
Contributor

@tomaskallup tomaskallup commented Dec 17, 2024

I based these changes on the implementation of @each, which does an inner while loop, checking for entry.next.

Closes: #1695

@@ -455,8 +455,10 @@ fn void HashMap.put_all_for_create(&map, HashMap* other_map) @private
if (!other_map.count) return;
foreach (Entry *e : other_map.table)
{
if (!e) continue;
map.put_for_create(e.key, e.value);
while (e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you fix it to match the other coding style, so Allman braces?

@lerno
Copy link
Collaborator

lerno commented Dec 18, 2024

Can you also add mention in the release notes?

@lerno
Copy link
Collaborator

lerno commented Dec 18, 2024

And the same bug is presumably in map.c3 as well, can you fix it?

@tomaskallup tomaskallup force-pushed the fix-hashmap-put-all-for-create branch from b809bcb to 15af01c Compare December 18, 2024 22:40
@tomaskallup tomaskallup requested a review from lerno December 18, 2024 22:40
@tomaskallup
Copy link
Contributor Author

Fixed it up, I'll try to also add unit tests for the regular Map.

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

Successfully merging this pull request may close these issues.

HashMap temp_init_from_map skips over some elements
2 participants