Skip to content

@memcpy: Don't check for aliasing if the type being copied has a bitSize of in 0 #21658

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

Conversation

JonathanHallstrom
Copy link
Contributor

Should close #21655
Unable to verify myself due to issues with building zig locally

@JonathanHallstrom JonathanHallstrom changed the title Don't check for aliasing if the type being copied has a bitSize of 0 @memcpy: Don't check for aliasing if the type being copied has a bitSize of in 0 Oct 10, 2024
Copy link
Contributor

@Rexicon226 Rexicon226 left a comment

Choose a reason for hiding this comment

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

I'm unsure if this is an issue that needs fixing. @memcpying requires memory to be involved, and ZSTs do not have any memory representation neither comptime nor runtime. Just my 2c.

@rohlem
Copy link
Contributor

rohlem commented Oct 10, 2024

I'm unsure if this is an issue that needs fixing.

The issue is #21655 , where correct code for a generic data structure in status-quo would need special-casing.
Discussion might make more sense there; I think it makes perfect sense to instead special-case the compiler builtin though.

@Rexicon226
Copy link
Contributor

I'm unsure if this is an issue that needs fixing.

The issue is #21655 , where correct code for a generic data structure in status-quo would need special casing. Discussion might make more sense there; I think it makes perfect sense to instead special-case the compiler builtin though.

Gotcha thanks, didn't see that.

@JonathanHallstrom
Copy link
Contributor Author

Seem CI is still failing, maybe not emitting anything in the case of zero bit types will fix it?

@JonathanHallstrom
Copy link
Contributor Author

This way of implementing it feels a little bit sketchy, maybe it should just be an early return?

@JonathanHallstrom JonathanHallstrom marked this pull request as ready for review October 11, 2024 13:20
@JonathanHallstrom
Copy link
Contributor Author

i seem to have messed something up in git

@alexrp alexrp requested review from mlugg and removed request for Rexicon226 January 26, 2025 18:24
@mlugg
Copy link
Member

mlugg commented Jan 26, 2025

This isn't quite safe due to some edge cases with comptime-only types. I'm going to wait until myself and @alexrp finish our discussion with Andrew on @memcpy semantics before proposing a path forward here -- hold tight.

@alexrp
Copy link
Member

alexrp commented Jan 29, 2025

@mlugg this was superseded by #22631, right?

@mlugg
Copy link
Member

mlugg commented Jan 29, 2025

Oops, I forgot to implement this into #22631. I'll look into this shortly.

@mlugg
Copy link
Member

mlugg commented Feb 1, 2025

Superseded by #22708.

@mlugg mlugg closed this Feb 1, 2025
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.

ArrayList(u0).appendSlice(list, &.{0, 0, 0}) panic: @memcpy arguments alias
6 participants