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

Documentation of PartialEq has example that violates transitivity #66476

Closed
robamler opened this issue Nov 16, 2019 · 1 comment · Fixed by #66566
Closed

Documentation of PartialEq has example that violates transitivity #66476

robamler opened this issue Nov 16, 2019 · 1 comment · Fixed by #66566
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@robamler
Copy link
Contributor

The Documentation of PartialEq states that implementations must be transitive: for all for all a, b and c, a == b and b == c implies a == c. This is followed by a note indicating that the transitivity must also hold if a, b, and c are of different types.

However, the last example implementation in the documentation is not transitive:

let b1 = Book { isbn: 1, format: BookFormat::Paperback };
let b2 = Book { isbn: 2, format: BookFormat::Paperback };

assert!(b1 == BookFormat::Paperback);
assert!(BookFormat::Paperback == b2);

// The following should hold by transitivity but doesn't.
assert!(b1 == b2); // <-- PANICS

I can think of three possible ways to fix this:

  • Remove the last example.
  • Keep the last example as an explicit counterexample to warn how easy it is to accidentally write implementations that violate the contract.
  • Modify the last example in a way that fixes the issue (I'm not sure what that would look like).

I can submit a pull request if I get some guidance which of the above solutions to choose (I personally have a weak preference for the second one).

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 16, 2019
@Centril
Copy link
Contributor

Centril commented Nov 16, 2019

Keep the last example as an explicit counterexample to warn how easy it is to accidentally write implementations that violate the contract.

This option seems quite helpful.

lo48576 added a commit to lo48576/json-ld that referenced this issue Nov 18, 2019
This is not transitive because it completely ignores `prefer_array`
field.
See <rust-lang/rust#66476>.
pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 22, 2019
Document pitfall with `impl PartialEq<B> for A`

Fixes rust-lang#66476 by turning the violating example into an explicit
counterexample.
Centril added a commit to Centril/rust that referenced this issue Nov 22, 2019
Document pitfall with `impl PartialEq<B> for A`

Fixes rust-lang#66476 by turning the violating example into an explicit
counterexample.
@bors bors closed this as completed in 5028fd8 Nov 23, 2019
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 C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants