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

TotalEq shouldn't need to have the secret method just for #[deriving] #13101

Closed
huonw opened this issue Mar 23, 2014 · 8 comments
Closed

TotalEq shouldn't need to have the secret method just for #[deriving] #13101

huonw opened this issue Mar 23, 2014 · 8 comments
Labels
A-trait-system Area: Trait system rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Mar 23, 2014

The assert_receiver_is_total_eq method is purely so that #[deriving] can enforce that all the contained types are TotalEq. It currently exists because the #[deriving] infrastructure makes it much much much easier to work with a method on a trait than with anything else.

Preferably #[deriving] would be refactored to allow creating a non trait function along the lines of

#[deriving(TotalEq)]
struct Foo<T> {
     x: int,
     y: T
}

// expands to

impl<T: TotalEq> TotalEq for Foo<T> {}

impl<T: TotalEq> Foo<T> {
     #[allow(dead_code)]
     fn assert_Foo_is_total_eq(&self) {
         fn total_eq<T: TotalEq>(_: &T) {}

         total_eq(&self.x);
         total_eq(&self.y);
     }
}

which would allow TotalEq to become just trait TotalEq: Eq {}.

(Placeholder issue for now.)

@metajack
Copy link
Contributor

metajack commented Jun 6, 2014

This secret method escapes into the error message when someone tries to derive(Eq) on something that isn't TotalEq. It was pretty hard to figure out what was wrong and I had to ask in #rust-internals before I managed to fix it.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 2, 2015
I think that deriving must have made progress since it was originally written,
this was a pretty easy change!

Closes rust-lang#13101
@steveklabnik
Copy link
Member

It's now Eq but all the rest is the same.

@steveklabnik steveklabnik added the A-trait-system Area: Trait system label Jan 23, 2015
@steveklabnik
Copy link
Member

Triage: no change.

@petrochenkov
Copy link
Contributor

I'm not sure why the restriction exists in the first place:

For struct S(PatialEqNotEq);

#[derive(PartialEq, Eq)]
struct S(PatialEqNotEq);

is illegal, while

#[derive(PartialEq)]
struct S(PatialEqNotEq);

impl Eq for S {}

is legal.
Is it solely a fool-proofing measure?

@petrochenkov
Copy link
Contributor

This secret method escapes into the error message when someone tries to derive(Eq) on something that isn't TotalEq. It was pretty hard to figure out what was wrong and I had to ask in #rust-internals before I managed to fix it.

The error message is improved in #36384, but assert_receiver_is_total_eq still stays in place.

#[derive(PartialEq)]
struct NotEq;

#[derive(PartialEq, Eq)]
struct S {
    a: NotEq,
}
error[E0277]: the trait bound `NotEq: std::cmp::Eq` is not satisfied
 --> ../../test.rs:9:2
  |
9 |     a: NotEq,
  |     ^^^^^^^^ trait `NotEq: std::cmp::Eq` not satisfied
  |
  = note: required by `std::cmp::AssertParamIsEq`

@bluss bluss added the rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 label Sep 12, 2016
@bluss
Copy link
Member

bluss commented Sep 12, 2016

The method is stable, but relatively well hidden. Is there anything that can be done about it, except removing it in "Rust 2"? It could be marked deprecated..

@Mark-Simulacrum
Copy link
Member

Nominating for libs team discussion, perhaps we could just deprecate and eventually remove the method of it doesn't do anything? I doubt that anyone is using it in practice. Searching for it on GitHub leads me to believe that most matches are in copied versions of either libsyntax or libcore, which seems "hey, you're unstable anyway" to me.

@Mark-Simulacrum Mark-Simulacrum added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 13, 2017
@alexcrichton
Copy link
Member

Discussed at libs traige and the decision was to close, we aren't going to remove these stable methods.

nivkner added a commit to nivkner/rust that referenced this issue Sep 30, 2017
remove FIXME(rust-lang#13101) since `assert_receiver_is_total_eq` stays.
remove FIXME(rust-lang#19649) now that stability markers render.
remove FIXME(rust-lang#13642) now the benchmarks were moved.
remove FIXME(rust-lang#6220) now that floating points can be formatted.
remove FIXME(rust-lang#18248) and write tests for `Rc<str>` and `Rc<[u8]>`
remove reference to irelevent issues in FIXME(rust-lang#1697, rust-lang#2178...)
update FIXME(rust-lang#5516) to point to getopts issue 7
update FIXME(rust-lang#7771) to point to RFC 628
update FIXME(rust-lang#19839) to point to issue 26925
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Aug 30, 2022
internal: Re-export standard semantic token types and mods

Should help in preventing future occurences of rust-lang#13099 by having all token types and mods come through the same place
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 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.

7 participants