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

Rustc reports trait implementations as conflicting even if they are not conflicting #100712

Closed
weiznich opened this issue Aug 18, 2022 · 6 comments
Labels
C-bug Category: This is a bug.

Comments

@weiznich
Copy link
Contributor

weiznich commented Aug 18, 2022

I tried this code:

// Cargo.toml: diesel = { version = "2.0.0-rc.1", features = ["sqlite", "postgres"]}
use diesel::backend::Backend;
use diesel::pg::Pg;
use diesel::sqlite::Sqlite;
use diesel::{PgConnection, SqliteConnection, Connection};
use diesel::serialize::{ToSql, self};

#[derive(Debug)]
struct Foo;

// These impls conflict with each other
impl ToSql<diesel::sql_types::Text, <PgConnection as Connection>::Backend> for Foo {
    fn to_sql<'b>(&'b self, out: &mut serialize::Output<'b, '_, <PgConnection as Connection>::Backend>) -> serialize::Result {
        todo!()
    }
}

impl ToSql<diesel::sql_types::Text, <SqliteConnection as Connection>::Backend> for Foo {
    fn to_sql<'b>(&'b self, out: &mut serialize::Output<'b, '_, <SqliteConnection as Connection>::Backend>) -> serialize::Result {
        todo!()
    }
}

#[derive(Debug)]
struct Bar;

impl ToSql<diesel::sql_types::Text, Pg> for Bar {
    // See we even use the associated type lookup as function argument here
    fn to_sql<'b>(&'b self, out: &mut serialize::Output<'b, '_, <PgConnection as Connection>::Backend>) -> serialize::Result {
        todo!()
    }
}

impl ToSql<diesel::sql_types::Text, Sqlite> for Bar {
    fn to_sql<'b>(&'b self, out: &mut serialize::Output<'b, '_, <SqliteConnection as Connection>::Backend>) -> serialize::Result {
        todo!()
    }
}

fn assert_backend<C, DB>() where C: Connection<Backend = DB>, DB: Backend{}


fn main() {
    // Just make really sure that we know the backend type
    assert_backend::<PgConnection, Pg>();
    assert_backend::<SqliteConnection, Sqlite>();
}

I expected to see this happen: The code compiles without errors, as all trait implementations are not conflicting (as demonstrated by the impls on Bar)

Instead, this happened: Compilation fails with the following error message:

error[E0119]: conflicting implementations of trait `diesel::serialize::ToSql<diesel::sql_types::Text, _>` for type `Foo`
  --> src/main.rs:17:1
   |
11 | impl ToSql<diesel::sql_types::Text, <PgConnection as Connection>::Backend> for Foo {
   | ---------------------------------------------------------------------------------- first implementation here
...
17 | impl ToSql<diesel::sql_types::Text, <SqliteConnection as Connection>::Backend> for Foo {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Foo`

For more information about this error, try `rustc --explain E0119`.

Meta

rustc --version --verbose:

rustc 1.62.1 (e092d0b6b 2022-07-16)
binary: rustc
commit-hash: e092d0b6b43f2de967af0887873151bb1c0b18d3
commit-date: 2022-07-16
host: x86_64-unknown-linux-gnu
release: 1.62.1
LLVM version: 14.0.5

@weiznich weiznich added the C-bug Category: This is a bug. label Aug 18, 2022
@b-naber
Copy link
Contributor

b-naber commented Aug 18, 2022

@rustbot claim

@b-naber
Copy link
Contributor

b-naber commented Aug 19, 2022

As far as I understand, this is actually intended behaviour, though admittedly confusing. The reason that we can't normalize that projection during the coherence check is, afaict, that we have to be conservative with trait refs that don't obey the orphan rules (which <PgConnection as Connection> doesn't), since there might be conflicting impls for that trait ref in upstream crates we can't unambiguously determine the impl source.

Edit: Actually it doesn't make sense that we should be able to normalize the projection in contexts outside of the coherence check, but fail to do so during the check.

Might make sense to have a lint that warns about normalization failures in that context though?!

@rustbot release-assignment

@lcnr
Copy link
Contributor

lcnr commented Sep 5, 2022

probably a duplicate of #99940

@weiznich
Copy link
Contributor Author

@lcnr Yes that seems like a duplicate. Will close that one and subscribe to the other issue. Is it planed to fix that someday or not? I'm asking because I'm currently trying to design a proc marco derive where such impls would be nice to have, otherwise I need to find other ways to implement the required functionality.

@weiznich weiznich closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2022
@lcnr
Copy link
Contributor

lcnr commented Sep 21, 2022

it is planned to fix this sometimes, sadly the fix is somewhat difficult and this is also not too high of a priority, so it may take a while '^^

@weiznich
Copy link
Contributor Author

Thanks for the fast answer. I will try to find another solution then 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants