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

Factual error in PartialEq documentation #91843

Open
QuineDot opened this issue Dec 13, 2021 · 4 comments
Open

Factual error in PartialEq documentation #91843

QuineDot opened this issue Dec 13, 2021 · 4 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@QuineDot
Copy link

The documentation for PartialEq states:

x.eq(y) can also be written x == y, and x.ne(y) can be written x != y.

This is not true, as #44762 illustrates, as well as this URLO-inspired example:

    let val = 1i32;
    let val_ref = &val;
    let is_equal = val.eq(val_ref); // Compiles
    let is_equal = val == val_ref; // Does not compile    

The other operator's documentation don't have this error. Shl (and Shr) also explicitly note some differences in inference between operators and the method call.

@rustbot label +A-docs

@rustbot rustbot added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Dec 13, 2021
@camelid
Copy link
Member

camelid commented Dec 13, 2021

I assume this is because auto(de)ref kicks in for the method version but not the operator version.

@camelid camelid added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 13, 2021
@QuineDot
Copy link
Author

I assume this is because auto(de)ref kicks in for the method version but not the operator version.

Yes (and experimenting with changing that is what #44762 is about).

@JanBeh
Copy link
Contributor

JanBeh commented Jan 13, 2023

The reference on comparison operators clarifies:

Unlike the arithmetic and logical operators above, these operators [the comparison operators] implicitly take shared borrows of their operands, evaluating them in place expression context:

# let a = 1;
# let b = 1;
a == b;
// is equivalent to
::std::cmp::PartialEq::eq(&a, &b);

This means that the operands don't have to be moved out of.

@QuineDot
Copy link
Author

QuineDot commented Feb 5, 2023

But that's also counterfactual!

// Commented code does not compile
fn main() {
    let v = 1;
    let x: *const u32 = &v;
    let y: &u32 = &1;
    let _ = dbg!(x == y);
    // let _ = dbg!(PartialEq::eq(x, y));
    // let _ = dbg!(PartialEq::eq(x, &y));
    // let _ = dbg!(PartialEq::eq(&x, y));
    // let _ = dbg!(PartialEq::eq(&x, &y));
    let _ = dbg!(PartialEq::eq(&x, &(y as _)));

    // But that can't be a desugaring...
    // let _ = dbg!(v == y);
    let _ = dbg!(&v == y);
    // let _ = dbg!(PartialEq::eq(&v, &(y as _)));
}

Operator resolution is its own thing with many niche differences and, in my opinion, all phrasing like "equivalent to" / "can be written as" / "is the same as" / "desugars" should be replaced with something more like "is notionally" / "is like" / etc, perhaps with a list of exceptions you're likely to hit.

Another example from the reference is compound assignment (+=):

  • They mention a place expression requirement -- great!
  • They mention order evaluation differs for primitives(!) -- great!
  • But then they blunder and say

    the following expression statements in example are equivalent:

Even modulo the exceptions they did cover, the remaining cases are not equivalent because += can participate in two-phase borrowing.

// commented code does not compile
use core::num::Wrapping;
fn main() {
    let n = &mut [Wrapping(0)][..];
    n[0] += n[0];
    // std::ops::AddAssign::add_assign(&mut n[0], n[0]);
}

(See also.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants