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

Implement writeToMemory/readFromMemory for pointers #11734

Merged
merged 1 commit into from
Feb 19, 2023

Conversation

Vexu
Copy link
Member

@Vexu Vexu commented May 26, 2022

This is in direct contradiction to parts of #9646 and these two test cases https://github.com/ziglang/zig/blob/master/test/behavior/comptime_memory.zig#L319-L371.
As discussed in the self hosted compiler meetings the justification for this is change is that it seems like a lot of work for little value and making it an error also avoids the complications brought by comptime only types.

cc @SpexGuy

Closes #11211

@SpexGuy
Copy link
Contributor

SpexGuy commented May 26, 2022

Shoot, I shouldn't have missed the meeting 😅
@ptrToInt at comptime is something that sort of needs to work. Without it, there's a pretty strange line for what compiles and what doesn't. For example, code which does a direct ptrCast from *T to [*]u8 in order to offset a pointer would be fine at comptime, but code which uses @ptrToInt and adds a byte offset would not.

Additionally, you aren't really saving any complexity. This code is still well defined:

pub fn betterPtrToInt(comptime ptr: anytype) usize {
    return @ptrCast(*const usize, &ptr).*;
}

@Vexu
Copy link
Member Author

Vexu commented May 27, 2022

@ptrToInt at comptime is something that sort of needs to work. Without it, there's a pretty strange line for what compiles and what doesn't. For example, code which does a direct ptrCast from *T to [*]u8 in order to offset a pointer would be fine at comptime, but code which uses @ptrToInt and adds a byte offset would not.

The things that can be done with @ptrCast are more limited and guaranteed to stay within the original allocation. Is @ptrToInt(&a) + 1 guaranteed to not overflow? What's stopping @ptrToInt from returning std.math.maxInt(usize)?

Additionally, you aren't really saving any complexity. This code is still well defined:

This can easily be made to have an error like reinterpreting pointers to comptime variables is not allowed or something.
What does this code do?

test {
    comptime var a = 1;
    comptime @compileLog(betterPtrToInt(&a));
}

@eLeCtrOssSnake
Copy link

This is in direct contradiction to parts of #9646 and these two test cases https://github.com/ziglang/zig/blob/master/test/behavior/comptime_memory.zig#L319-L371. As discussed in the self hosted compiler meetings the justification for this is change is that it seems like a lot of work for little value and making it an error also avoids the complications brought by comptime only types.

cc @SpexGuy

Closes #11211

This features should exist. There is no justification for removing it.
Inspecting own PE Header for example is one reason. Getting PEB on Windows requires a hard coded address.

@Vexu
Copy link
Member Author

Vexu commented Sep 9, 2022

This features should exist. There is no justification for removing it.
Inspecting own PE Header for example is one reason. Getting PEB on Windows requires a hard coded address.

I don't understand what you're trying to say.

@andrewrk
Copy link
Member

andrewrk commented Feb 16, 2023

@Vexu if you wouldn't mind rebasing this and getting the CI checks passing, I'll prioritize accepting it / rejecting it. I'm leaning towards accepting it, and amending the spec, as long as it doesn't break any valuable use case.

@eLeCtrOssSnake
Copy link

This features should exist. There is no justification for removing it.
Inspecting own PE Header for example is one reason. Getting PEB on Windows requires a hard coded address.

I don't understand what you're trying to say.

I confused myself on that one. I agree, in comptime it's hard to understand what compiler might come up with, so removing @ptrToInt for comptime variables makes sense to not allow "C hacks" in comptime.
The only use case I am a bit worried about is negative indexing from pointer, I am not sure how useful it is, but it might be a point for consideration as zig does not allow negative array index, so the only real way to do it is through pointer math(but negative indexing is kind of bad anyway). Again, not sure if it's a useful thing in comptime, more of a real code(not comptime) C interaction thing.
I am not thinking of any other use case for comptime pointer math as there are better alternatives with for/while loops, many item ptr's etc. All in all I think it makes sense.

@Vexu Vexu force-pushed the ptrToInt-comptime-var branch 2 times, most recently from 349d132 to 545478d Compare February 18, 2023 20:13
@Vexu
Copy link
Member Author

Vexu commented Feb 18, 2023

The current behavior of @ptrToInt of only evaluating at comptime if the pointer value has a comptime representation is more in line with the rest of the language (currently) and solves the problem I originally had. I implemented reinterpreting pointers to declarations to also become runtime operations to completely solve the issue for now.

Whether this should be a compile error can be decided when solving #2471 as far as I'm concerned.

@Vexu Vexu changed the title Make @ptrToInt on address of comptime variable an error Implement writeToMemory/readFromMemory for pointers Feb 18, 2023
@andrewrk andrewrk merged commit f109505 into ziglang:master Feb 19, 2023
@Vexu Vexu deleted the ptrToInt-comptime-var branch February 19, 2023 19:40
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.

stage2: compiler panic when using @ptrToInt in bitwise AND
4 participants