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

Compute more pointer comparisons in CTFE #113052

Closed
wants to merge 1 commit into from

Conversation

joboet
Copy link
Member

@joboet joboet commented Jun 26, 2023

IIUC, pointers from the same allocation can be reliably compared by just looking at their offsets. This PR implements this in CTFE and therefore makes it possible to write e.g. pointer-based slice iterators by substituting the pointer comparisons with calls to guaranteed_eq.

As the FIXMEs in the current code pointed out, function pointers and vtables do not have stable addresses, so the comparison still fails in those cases.

@rustbot label +A-const-eval

@rustbot
Copy link
Collaborator

rustbot commented Jun 26, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) labels Jun 26, 2023
@petrochenkov
Copy link
Contributor

r? @RalfJung I guess

@rustbot rustbot assigned RalfJung and unassigned petrochenkov Jun 27, 2023
Comment on lines +326 to +328
// to the same allocation. Function pointers and vtables do not
// have stable addresses (see #73722), so the result of comparisons
// cannot be known during CTFE.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// to the same allocation. Function pointers and vtables do not
// have stable addresses (see #73722), so the result of comparisons
// cannot be known during CTFE.
// to the same allocation. Function pointers and vtables do not
// have stable addresses (see #73722), so the result of comparisons
// cannot be known during CTFE -- hence we check `get_alloc_info`
// to exclude those cases.

@RalfJung
Copy link
Member

Cc @rust-lang/wg-const-eval

My concern here is that this allocation might be another previously evaluated const, and those actually can get duplicated as well, so I am not sure if they are truly guaranteed to be equal at runtime even if they point to the same allocation with the same offset.

@RalfJung
Copy link
Member

makes it possible to write e.g. pointer-based slice iterators by substituting the pointer comparisons with calls to guaranteed_eq

#112837 offers another way to make that possible: ptr2.wrapping_offset_from(ptr1) == 0.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 28, 2023

Yea, I don't think we should do this. AllocIds are an implementation artifact that doesn't really map to user-facing provenance. Pointer based iterators would probably randomly work or not work depending on what kind of pointer you input. That is very much not how guaranteed_eq should be used. guaranteed_eq does not enable new features, it allows you to write performance optimizations that work at runtime, and end up taking the slow path at compile-time.

@joboet
Copy link
Member Author

joboet commented Jun 29, 2023

I should have approached this another way: What I wish for is a way to compare pointers at compile time to allow e.g. making slice::Iter const Iterator (once const-traits have been figured out). ATM, my preferred method of doing this would be to make PartialEq on pointers const, but to abort with a nice error when comparing pointers to different allocated objects (since that's hard/impossible to achieve at compile-time). I assumed implementing the comparison in guaranteed_eq would be a good first step in that direction, but apparently I misunderstood the intended purpose of these methods...

As such, I'll close this and open a Zulip discussion instead. Should I also file a PR removing the FIXMEs, since they are a bit misleading?

Yea, I don't think we should do this. AllocIds are an implementation artifact that doesn't really map to user-facing provenance. Pointer based iterators would probably randomly work or not work depending on what kind of pointer you input. That is very much not how guaranteed_eq should be used. guaranteed_eq does not enable new features, it allows you to write performance optimizations that work at runtime, and end up taking the slow path at compile-time.

Just for future reference, is there a right provenance indicator in the CTFE engine that could be used for this?

@joboet joboet closed this Jun 29, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jun 29, 2023

Just for future reference, is there a right provenance indicator in the CTFE engine that could be used for this?

Unfortunately not. Constants are inherently fickle to work with. We're balancing performance issues vs good deduplication. The current implementation is probably a local optimum that we can only get out of with a major project that changes a lot of things at the same time.

Let's chat on zulip about your original problem.

@RalfJung
Copy link
Member

Where is that Zulip discussion?

FWIW my hope is that #112837 is sufficient for slice iterators in const.

There is the interesting observation that one can use offset_from(...) == 0 to test if two pointers are equal, but I can't see how that would cause a problem... but then one could also argue than any two pointers for which calling offset_from is allowed at const-time, we might as well return a result in ptr_guaranteed_eq/ne? I don't like that conclusion but I am not sure where the argument goes wrong.

@joboet
Copy link
Member Author

joboet commented Jun 30, 2023

Opened a Zulip thread. I hope I picked the right stream...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants