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

map: use voidptr-based key equality and meta index methods #7320

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

ntrel
Copy link
Contributor

@ntrel ntrel commented Dec 14, 2020

Part of #6991.

Every operation on a map key needs to take a voidptr, not a string. This pull encapsulates:

  • The 'keys equal' operation on voidptr keys.
  • The 'meta index' operation on a voidptr key.

Add map::key_bytes field.

@ntrel ntrel requested a review from ka-weihe December 14, 2020 11:50
@ntrel ntrel marked this pull request as ready for review December 14, 2020 12:03
V currently does this automatically, but we shouldn't rely on it.
@ntrel ntrel marked this pull request as draft December 14, 2020 12:15
@ntrel ntrel changed the title map: use voidptr based keys_eq method instead of fast_string_eq map: use voidptr-based key equality and meta index methods Dec 14, 2020
C.memcpy(pval, value, m.value_bytes)
}
return
}
index += 2
meta += probe_inc
}
kv_index := m.key_values.push(key, value)
kv_index := m.key_values.push(&key, value)
Copy link
Contributor Author

@ntrel ntrel Dec 14, 2020

Choose a reason for hiding this comment

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

This fixes a mistake from one of the DenseArray voidptr pulls. In practice this line change has no effect, because V automatically takes the address. I filed #7291 because I don't think it should.

@ntrel ntrel marked this pull request as ready for review December 14, 2020 12:29
@spytheman spytheman merged commit 89ef316 into vlang:master Dec 14, 2020
@ntrel ntrel deleted the key-eq2 branch December 14, 2020 14:55
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.

3 participants