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

rename ptr::memcpy, memmove, memset #4203

Closed
brson opened this issue Dec 16, 2012 · 5 comments
Closed

rename ptr::memcpy, memmove, memset #4203

brson opened this issue Dec 16, 2012 · 5 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Milestone

Comments

@brson
Copy link
Contributor

brson commented Dec 16, 2012

Unlike the C versions these copy a number of elements, not bytes. Seemed like a good idea at the time but confused me today when I couldn't remember. Let's rename them to be clear that they are not the same as C.

I don't have any great ideas for the names at this moment.

@jruderman
Copy link
Contributor

shallow_copy and podset? (Assuming you can copy structs with @ but not with ~, and can't set either.)

@brson
Copy link
Contributor Author

brson commented Jan 10, 2013

These are unsafe functions, so they can copy anything, not just POD. I also noticed that vec::raw::memcpy, etc. have this same issue, they copy elements, not bytes. Interestingly, the vec docs incorrectly claim that they copy bytes.

@brson
Copy link
Contributor Author

brson commented Jan 10, 2013

As discussed on IRC, calling these anything other than memcpy and memmove makes it less clear what the distinction between the two is. The pair have very well known properties regarding overlapping memory.

So any other name should indicate that distinction. I'm wondering if it's best to keep these names after all.

@brson
Copy link
Contributor Author

brson commented Jan 10, 2013

@nikomatsakis suggested "copy_memory" and "copy_overlapping_memory"

@catamorphism
Copy link
Contributor

I've merged pull request #4411, which fixes this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

3 participants