Adding crate to project breaks existing code build !?!?

I just added dioxus to my project and was surprised to find in causes an error in compiling an existing module in the project. The module in question contains:

           let mut buf = [0u8];
            match self.shell.read_exact(&mut buf) {
                Ok(()) => {
                    let char = buf[0] as char;
                    if char != 0u8.into() {
                    ...

After adding dioxus that fails to compile with error:

error[E0283]: type annotations needed
  --> src/itc_shell.rs:36:36
   |
36 |                     if char != 0u8.into() {
   |                             --     ^^^^
   |                             |
   |                             type must be known at this point
   |
   = note: multiple `impl`s satisfying `char: PartialEq<_>` found in the following crates: `core`, `proc_macro`:
           - impl PartialEq for char;
           - impl PartialEq<proc_macro::Punct> for char;
help: try using a fully qualified path to specify the expected types
   |
36 |                     if char != <u8 as Into<T>>::into(0u8) {
   |                                ++++++++++++++++++++++   ~

This is easily fixed of course:

if char != 0 as char {
                    ...

Ultimately not a problem but I'm some what disturbed that adding a crate can break the build of other modules that don't even use that crate or know anything about it. It seems some definition in dioxus has infected my module somehow.

The note and suggestion in the error message are not useful.

In what other situations can this kind of thing happen?

8 Likes

Interestingly, this is reproduced by this short snippet:

extern crate proc_macro;

fn foo(c: char) -> bool {
    c == 0u8.into()
}

If the extern crate line is removed, the code compiles. If it's present, we have the same error as in the OP. And I guess (can't check right now) that this extern crate can very well be anywhere in the dependency tree - the outcome would be the same in any case.

I think the error is caused by this impl in proc_macro:

#[stable(feature = "proc_macro_punct_eq_flipped", since = "1.52.0")]
impl PartialEq<Punct> for char {
    fn eq(&self, rhs: &Punct) -> bool {
        *self == rhs.as_char()
    }
}

The error is caused by a corner case of Rust's trait resolution. If there is a trait Trait<T> and a single impl impl Trait<Foo> for Bar, then trying to resolve Trait<T> for type Bar will default to this unique impl, even if the trait parameter T cannot be inferred. However, as soon as there is more than one impl of Trait for Bar, trait resolution will fail, unless all types can be uniquely inferred.

In your case there is normally a unique impl of PartialEq for char, namely impl PartialEq<char> for char. This means that in c == x with c: char, the type of x defaults to char. If x is y.into(), this means that the target type of the Into conversion is char, and the impl can be properly found.

As soon as a second impl PartialEq<T> for char exists anywhere in dependencies, the type of x in c == x cannot be inferred. This means that y.into() has an unknown target type, and so the impl Into<T> for u8 cannot be resolved (since T is unknown).

This is a common error when dealing with abstract integer types, i.e. 0 or 0.0 (note the absence of explicit typing). If the type of a generic integer can't be uniquely inferred, it defaults to i32. If you need to resolve a trait for a generic integer (e.g. Trait::foo(0)) and the trait is implemented for a unique integer type (e.g. impl Trait for u8), the default works and everything typechecks. As soon as you add an impl for a different integer type (e.g. impl Trait for u16), the compiler will confusingly complain that it can't find an impl Trait for i32. That is, of course, unless you add impl Trait for i32, in which case everything typechecks again, but the type of the value will be inferred to i32, which may be quite surprising.

11 Likes

Sounds reasonable.

But how or why is any new trait, presumably added by dioxus or it's dependencies, getting into a module that does not use it or know anything about it? It's like the global space has been infected for everyone.

2 Likes

I really dislike as-cast, because they can swallow errors. I'd rewrite this to one of those, in order from most to least preferable:

'\0'
char::from(0) // infallible

Using a char literal if you want a char is the most clear way to me.

1 Like

In my opinion, your code is not secure in itself, and 0u8.into() is not a good way to express it. It cannot immediately infer the specific type you need.

It's not "any new trait". The trait is PartialEq, which is standard. It's the new impl.

Trait implementations indeed live in a global scope. Your crate can use a trait implementation from any of its transitive dependencies, provided that the trait & parameter types are accessible. Normally this isn't a problem: crates rarely implement traits from other crates, the trait must be explicitly imported to be used, and the orphan rules heavily constrain the possible implementations. But in the case of PartialEq, it's a commonly implemented standard trait, it's imported implicitly in the prelude, and used implicitly via the == syntax, so you're more likely to hit inference issues. Operator traits in general are prone to such issues, but using Into conversions inside of operator expressions is a poor practice anyway, since it usually doesn't work.

That said, I'm a bit confused why an impl from proc_macro is in the scope. I'm reasonably sure it's proc_macro, because I have created a test project with dioxus in the dependencies, and a global search over the project for impl.*PartialEq.*for char or impl.*PartialEq<char> regular expressions turned up only two impls in proc_macro. However, proc_macro should be a dependency only for proc macro crates, which are compiled separately.


It seems like dioxus depends on dioxus-rsx, which is used to implement the rsx! macro, but isn't a macro crate itself. It parses Rust syntax within the fields of node children declared in rsx! calls. For this reason it depends on syn, and proc-macro2. However, proc-macro2 can depend on proc-macro:

#[cfg(feature = "proc-macro")]
extern crate proc_macro;

This dependency is gated behind a proc-macro feature, but it is enabled by default, and would probably be enabled by unification with proc macro dependencies anyway (N.B.: with an old Cargo feature resolver; resolver v2 doesn't unify features between normal and build dependencies, including proc macros). This means that dioxus pulls transitively proc-macro2, which pulls in proc-macro, which provides the offending impls. That's not an edge case which one is likely to hit, thankfully.

Actually, this may be a good PR for dioxus: disable default features from proc-macro2 dependencies, so that no-macro crates don't depend on proc-macro.

8 Likes

I had this exact same issue, but with the + operator and auto-deref.

Before adding the aws-sdk-s3 dependency, somewhere in my code I had something like this working totally fine:

some_string + &some_other_string

But bringing that dependency broke this code (even without bringing it into scope), and forced me to change it to this:

some_string + some_other_string.as_str()

I really didn't like having a dependency breaking my code "at a distance".

1 Like

Are there any clippy lints that warn if code is relying on type inference that could break in the presence of an additional trait implementation?

2 Likes

Doesn't seem so. It looks like a lint which would be quite difficult to implement (at least in a performant way), given that potentially any type and trait, including defined in transitive dependencies, could have a single impl and thus be a hazard. Open search in general is problematic.

Also, I think this lint could have too many false positives. Unique impls are actually not that uncommon.

By the way, this:

...doesn't work, because Into::into is not generic (the trait itself is). You can either use Into::<char>::into(0u8), or pull in tap and use 0u8.conv::<char>().

1 Like

This "inference-breaking at a distance" is a known issue. serde_json is the most notable offender. Of the several issues opened over the past years, this one has some good context: Adding serde_json as an unused external crate causes a compiler error in unrelated code · Issue #46257 · rust-lang/rust · GitHub

4 Likes

Then the opposite: Is there a lint targeted at library authors which warns them when they implement a common operator trait (e.g. PartialEq) for a common primitive type or struct (e.g. String), which could trigger this issue?

And if not (at least my sample code did not trigger any warnings), would it make sense to propose this?

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.