Skip to content

Conversation

hkBst
Copy link
Member

@hkBst hkBst commented Jan 31, 2025

Fixes #77497

@rustbot
Copy link
Collaborator

rustbot commented Jan 31, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 31, 2025
impl<T: ?Sized> Eq for *const T {}

// Comparison for pointers
/// Pointers are compared as if they are usize
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Pointers are compared as if they are usize
/// Pointers are compared as an unsigned value with exposed provenance.

I think mentioning the provenance story may be worthwhile here, because aiui otherwise comparing pointers with different provenance would be unsound. @RalfJung is this accurate?

Could you add the same docs to PartialEq and PartialOrd? *mut should get similar changes

// Equality for pointers
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> PartialEq for *mut T {
#[inline(always)]
#[allow(ambiguous_wide_pointer_comparisons)]
fn eq(&self, other: &*mut T) -> bool {
*self == *other
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> Eq for *mut T {}
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> Ord for *mut T {
#[inline]
#[allow(ambiguous_wide_pointer_comparisons)]
fn cmp(&self, other: &*mut T) -> Ordering {
if self < other {
Less
} else if self == other {
Equal
} else {
Greater
}
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> PartialOrd for *mut T {
.

Copy link
Member

Choose a reason for hiding this comment

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

Provenance does not get exposed by comparison so the "with exposed provenance" part doesn't seem right. I don't even know what exactly it should mean though.

Copy link
Member

Choose a reason for hiding this comment

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

I think mentioning the provenance story may be worthwhile here, because aiui otherwise comparing pointers with different provenance would be unsound

why would this comparison be unsound?

Copy link
Member

@RalfJung RalfJung Feb 1, 2025

Choose a reason for hiding this comment

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

I think the easiest way to document this is to just say that pointers are compared by comparing the result of calling addr(). That covers most everything relevant:

  • Provenance does not get exposed
  • Provenance does not influence the result
  • The comparison happens at type usize, i.e., it is unsigned

It does not say what happens with wide pointers, but that is an orthogonal question anyway.

Copy link
Contributor

@tgross35 tgross35 Feb 1, 2025

Choose a reason for hiding this comment

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

Provenance does not get exposed by comparison

I said the wrong thing here; I meant to say that provenance is taken into account, not that it is exposed (sorry, provenance lingo is all still all a bit new to me...).

why would this comparison be unsound?

AIUI any kind of comparison of pointers across allocations would ride the non-deterministic/unsound line if provenance matters, but this isn't a problem based on the above.

Ralf or Jubille if you want to yoink this review feel free, I'm happy to though if you have no preference.

Copy link
Member

Choose a reason for hiding this comment

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

@tgross35 There's some nondeterminism always because, well, pointers are simply like that. Especially in const eval which can cause pointer comparisons to spuriously fail! That's why guaranteed_eq exists. But nondet != unsound.

afaik it is of course strictly unsound in C and C++, but we're not C or c++

@tgross35
Copy link
Contributor

tgross35 commented Feb 1, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 1, 2025

📌 Commit 55dc6db has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2025
@bors bors merged commit 15a5f5f into rust-lang:master Feb 1, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 1, 2025
@RalfJung
Copy link
Member

RalfJung commented Feb 2, 2025 via email

@hkBst hkBst deleted the ptr_cmp_docs branch February 2, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify (un)signed-ness of pointers for comparisons
6 participants