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

== fails to match types on associated type with two PartialEq<A / B> bounds #34792

Closed
SimonSapin opened this issue Jul 12, 2016 · 14 comments · Fixed by #81081
Closed

== fails to match types on associated type with two PartialEq<A / B> bounds #34792

SimonSapin opened this issue Jul 12, 2016 · 14 comments · Fixed by #81081
Assignees
Labels
A-associated-items Area: Associated items (types, constants & functions) A-typesystem Area: The type system C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@SimonSapin
Copy link
Contributor

SimonSapin commented Jul 12, 2016

rustc 1.12.0-nightly (47b3a98 2016-07-10)

pub struct A;
pub struct B;

pub trait Foo {
    type T: PartialEq<A> + PartialEq<B>;
}

pub fn generic<F: Foo>(t: F::T, a: A, b: B) -> bool {
    t == a && t == b
}
error: mismatched types [--explain E0308]
  --> a.rs:12:10
   |>
12 |>     t == a && t == b
   |>          ^ expected struct `B`, found struct `A`
note: expected type `B`
note:    found type `A`

error: aborting due to previous error

Changing the order of bounds in the associated type declaration changes which == causes a spurious "mismatched types" error.

Using UFCS works around the issue:

    PartialEq::<A>::eq(&t, &a) && t == b

The same issue exists with bounds like type T: PartialEq + PartialEq<B> to compare T with itself as well as another type.

@eddyb eddyb added A-typesystem Area: The type system I-nominated A-associated-items Area: Associated items (types, constants & functions) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 12, 2016
@hanna-kruppe
Copy link
Contributor

cf. #24066

@eddyb
Copy link
Member

eddyb commented Jul 16, 2016

Looks like this is caused by using find on the set of bounds from the trait, but that is just find the first one (in the order they are internally stored), this needs to filter and produce a vector of bounds or something similar.

cc @rust-lang/compiler

@rozbb
Copy link

rozbb commented Jul 18, 2016

Update: using filter and then calling candidates.extend on all matching bounds does not fix this issue.

@eddyb
Copy link
Member

eddyb commented Jul 18, 2016

@doomrobo I believe your problem is this logic.
Right now it works fine because the if victim.candidate == other.candidate above would trigger for ObjectCandidate vs ObjectCandidate or ParamCandidate vs ParamCandidate.

However, it's incorrect with your changes as ParamCandidate(bound_a) is dropped in favor of ParamCandidate(bound_b) even if bound_a and bound_b are not equal.
The correct form would be more like this:

                // Arbitrarily give param candidates priority
                // over projection and object candidates.
                ObjectCandidate => true,
                ProjectionCandidate(_) => {
                    if let ProjectionCandidate(_) = other.candidate {
                        false
                    } else {
                        true
                    }
                },

@nikomatsakis
Copy link
Contributor

I guess that you didn't just call filter, but are also packaging up the specific bound that was matched? Anyway, not having seen the full diff, the general direction of these changes make sense to me.

rozbb pushed a commit to rozbb/rust that referenced this issue Jul 20, 2016
@nikomatsakis
Copy link
Contributor

Related: #34912

@pnkfelix
Copy link
Member

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Jul 21, 2016
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 25, 2017
@estebank
Copy link
Contributor

Triage: no change:

error[E0308]: mismatched types
 --> src/lib.rs:9:10
  |
9 |     t == a && t == b
  |          ^ expected struct `B`, found struct `A`
  |
  = note: expected type `B`
             found type `A`

@ldm0
Copy link
Contributor

ldm0 commented Dec 18, 2020

This is fixed on current master. Should be closed.

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. labels Dec 18, 2020
@akshatagarwl
Copy link

@estebank I would like to work on this issue if its still relevant?

@SimonSapin
Copy link
Contributor Author

As @ldm0 mentioned the original code now compiles: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=106d275bc9d5ae878cc0c28c038dcec7

Closing as there doesn’t seem to be anything left to do here.

@estebank
Copy link
Contributor

estebank commented Jan 8, 2021

@SimonSapin do we have any // check-pass regression test? I couldn't see any.

@SimonSapin
Copy link
Contributor Author

Oh good point. Sorry I missed the label.

@humancalico Yes, feel free to work on adding a regression test!

@SimonSapin SimonSapin reopened this Jan 8, 2021
@bugadani
Copy link
Contributor

@rustbot claim

bugadani added a commit to bugadani/rust that referenced this issue Jan 16, 2021
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 16, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 16, 2021
Rollup of 17 pull requests

Successful merges:

 - rust-lang#78455 (Introduce {Ref, RefMut}::try_map for optional projections in RefCell)
 - rust-lang#80144 (Remove giant badge in README)
 - rust-lang#80614 (Explain why borrows can't be held across yield point in async blocks)
 - rust-lang#80670 (TrustedRandomAaccess specialization composes incorrectly for nested iter::Zips)
 - rust-lang#80681 (Clarify what the effects of a 'logic error' are)
 - rust-lang#80764 (Re-stabilize Weak::as_ptr and friends for unsized T)
 - rust-lang#80901 (Make `x.py --color always` apply to logging too)
 - rust-lang#80902 (Add a regression test for rust-lang#76281)
 - rust-lang#80941 (Do not suggest invalid code in pattern with loop)
 - rust-lang#80968 (Stabilize the poll_map feature)
 - rust-lang#80971 (Put all feature gate tests under `feature-gates/`)
 - rust-lang#81021 (Remove doctree::Import)
 - rust-lang#81040 (doctest: Reset errors before dropping the parse session)
 - rust-lang#81060 (Add a regression test for rust-lang#50041)
 - rust-lang#81065 (codegen_cranelift: Fix redundant semicolon warn)
 - rust-lang#81069 (Add sample code for Rc::new_cyclic)
 - rust-lang#81081 (Add test for rust-lang#34792)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in a6b2e1f Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-typesystem Area: The type system C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.