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

Remove all usages of std.mem.copy and remove std.mem.set #18143

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

Rexicon226
Copy link
Contributor

My reasoning for making this PR is 2 reasons.

  1. This should have been simply removed/@compileError the first time, and now there is just no point in keeping it around.

  2. In my opinion this creates a bad practice function alias. By aliasing copyForward, we are creating possible bugs that are not caught and corrupt data due to overlap. The pointer overlap detection of @memcpy is more important than I believe given credit to. So it leads to the thought that all uses of std.mem.copy (after it was aliased/depricated), were made out of ignorance of the dangerous it could pose to the codebase. If the writer wanted to ensure overlap, they would have / should use copyForward, and if they did not want overlap, they need to use @memcpy.

@Rexicon226
Copy link
Contributor Author

cc @Vexu for aro

@squeek502
Copy link
Collaborator

squeek502 commented Nov 27, 2023

Just FYI, the typical procedure is to alias things with a Deprecated doc comment for one release cycle and then remove the alias during the next release cycle, so this is definitely fair game since the deprecation note was included in 0.11.0.

@Rexicon226
Copy link
Contributor Author

For the CI failures, should I just change them all to copyForward? Or is there something more indepth that I should do?

@squeek502
Copy link
Collaborator

The slices need to be the same length to use @memcpy, see #15481 for how things were updated when the change was made.

@Rexicon226
Copy link
Contributor Author

ah i see, they were just done using [0..source.len] inside of the @memcpy. I'll do that rn!

@Rexicon226
Copy link
Contributor Author

i know what to fix, i just wanna test a couple things tomorrow 😄

i wish there was a way for me to cancel ci actions, because i feel bad about wasting resources 😭

Copy link
Member

@Vexu Vexu left a comment

Choose a reason for hiding this comment

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

Please submit a patch upstream as well!

@Rexicon226 Rexicon226 changed the title Remove all usages of std.mem.copy Remove all usages of std.mem.copy and remove std.mem.set Nov 27, 2023
@andrewrk andrewrk merged commit 1e42a3d into ziglang:master Nov 29, 2023
10 checks passed
voluntas added a commit to shiguredo/media-processors that referenced this pull request Dec 9, 2023
linusg added a commit to linusg/zig-args that referenced this pull request Jan 4, 2024
linusg added a commit to linusg/zig-args that referenced this pull request Jan 4, 2024
linusg added a commit to linusg/zig-args that referenced this pull request Jan 5, 2024
linusg added a commit to linusg/zig-args that referenced this pull request Jan 5, 2024
ikskuh pushed a commit to ikskuh/zig-args that referenced this pull request Jan 5, 2024
RossComputerGuy pushed a commit to ExpidusOS-archive/zig that referenced this pull request Jan 6, 2024
@Rexicon226 Rexicon226 deleted the patch-3 branch July 22, 2024 12:57
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.

4 participants