Compiler suggests non-nonsensical `#[derive(<Trait>.into())]`

[ntc2]

Code

Seeing this in a private codebase, with

$ rustc --version
rustc 1.85.0-nightly (dff3e7ccd 2024-11-26)

Sorry about the low-effort bug report, but I'm hopeful this is easy to track down ...

Current output

error[E0308]: mismatched types
  --> <file>:83:17
   |
83 | #[derive(Debug, Storable, Clone, PartialEq, Arbitrary)]
   |                 ^^^^^^^^
    <big type error>
   = note: expected struct `Vec<ArenaKey<...>`
              found struct `Vec<VersionedArenaKey<...>`
   = note: the full type name has been written to '<path>'
   = note: consider using `--verbose` to print the full type name to the console
   = note: this error originates in the derive macro `Storable` (in Nightly builds, run with -Z macro-backtrace for more info)
help: call `Into::into` on this expression to convert `VersionedArenaKey<...>` into `ArenaKey<...>`
   |
83 | #[derive(Debug, Storable.into(), Clone, PartialEq, Arbitrary)]
   |                         +++++++

Desired output

Not sure, but it shouldn't be suggesting syntactically invalid nonsense :)

Rationale and extra context

No response

Other cases

Rust Version

rustc 1.85.0-nightly (dff3e7ccd 2024-11-26)
binary: rustc
commit-hash: dff3e7ccd4a18958c938136c4ccdc853fcc86194
commit-date: 2024-11-26
host: x86_64-unknown-linux-gnu
release: 1.85.0-nightly
LLVM version: 19.1.4

Anything else?

No response

[naskya]

Minimal example (I know this is very different from the actual code)

// src/lib.rs (main crate)
use sample_macro::Sample;

#[derive(Sample)]
struct Test;

// src/lib.rs (sample_macro crate)
use proc_macro::TokenStream;
use quote::quote;

#[proc_macro_derive(Sample)]
pub fn sample(_: TokenStream) -> TokenStream {
    quote! {
        fn bad<T: Into<U>, U>(a: T) -> U { a }
    }.into()
}

$ cargo check
    Checking sample v0.1.0 (/home/arch/workspace/sample)
error[E0308]: mismatched types
 --> src/lib.rs:3:10
  |
3 | #[derive(Sample)]
  |          ^^^^^^
  |          |
  |          expected type parameter `U`, found type parameter `T`
  |          expected `U` because of return type
  |
  = note: expected type parameter `U`
             found type parameter `T`
  = note: a type parameter was expected, but a different one was found; you might be missing a type parameter or trait bound
  = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters
  = note: the caller chooses a type for `U` which can be different from `T`
  = note: this error originates in the derive macro `Sample` (in Nightly builds, run with -Z macro-backtrace for more info)
help: call `Into::into` on this expression to convert `T` into `U`
  |
3 | #[derive(Sample.into())]
  |                +++++++

For more information about this error, try `rustc --explain E0308`.
error: could not compile `sample` (lib) due to 1 previous error

I doubt that this is a compiler problem. Rather, isn’t it a problem with the proc macro not being written well...? (e.g., it produces an invalid code, it doesn’t handle spans correctly)

[ntc2]

@naskya you are correct the type error is due to a bug in the proc_macro_derive. I don't know much about macros in Rust, but it still seems weird to me that "bug is in the macro" implies "compiler generating nonsense error message" is OK or expected?! Naively, I would expect the compiler to show me the generated code, with the error annotated on that generated code, along with telling me I'm looking at generated code.

I don't understand the "doesn't handle spans correctly" concern, but here's the broken macro code and the fix, before and after:

-                        quote! {child_hashes.push(Sp::hash(&self.#ident).clone())}
+                        quote! {child_hashes.push(Sp::hash(&self.#ident).into())}

So this is similar to your minimal example, except here I needed to add a missing into() on the inside of the quote, vs your example having an into() outside the quote. I guess it would be like your example with the into() removed?

[naskya]

@ntc2 Let’s say we have this code:

// Oops, I forgot to impl Debug for Foo
struct Foo;

#[derive(Debug)]
struct Bar {
    foo: Foo,
}

the #[derive(Debug)] expands to something like

struct Foo;

struct Bar {
    foo: Foo,
}

impl Debug for Bar {
    fn fmt(...) -> ... {
        /* Oh no, we can’t use the debug formatting for `foo` */
    }
}

so the compilation error (the root cause) is inside fn fmt(...). However, cargo check underlines foo: Foo (instead of the root cause) and tells us more helpful message:

$ cargo check
    Checking sample v0.1.0 (/home/arch/workspace/sample)
error[E0277]: `Foo` doesn't implement `Debug`
 --> src/lib.rs:5:5
  |
3 | #[derive(Debug)]
  |          ----- in this derive macro expansion
4 | struct Bar {
5 |     foo: Foo,
  |     ^^^^^^^^ `Foo` cannot be formatted using `{:?}`
  |
  = help: the trait `Debug` is not implemented for `Foo`
  = note: add `#[derive(Debug)]` to `Foo` or manually `impl Debug for Foo`
  = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `Foo` with `#[derive(Debug)]`
  |
1 + #[derive(Debug)]
2 | struct Foo;
  |

Like this, users often want more ergonomic messages than root causes, and this is where the proc macro functionality comes in. The writer of a proc macro has full control over what errors the macro reports. So macros should report useful errors rather than implementation details.

For example, serde derive macro has a huge mechanism under the hood:

#[derive(serde::Deserialize)]
struct Foo {
    bar: u8,
}

Expanded

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
struct Foo {
    bar: u8,
}
#[doc(hidden)]
#[allow(non_upper_case_globals, unused_attributes, unused_qualifications)]
const _: () = {
    #[allow(unused_extern_crates, clippy::useless_attribute)]
    extern crate serde as _serde;
    #[automatically_derived]
    impl<'de> _serde::Deserialize<'de> for Foo {
        fn deserialize<__D>(
            __deserializer: __D,
        ) -> _serde::__private::Result<Self, __D::Error>
        where
            __D: _serde::Deserializer<'de>,
        {
            #[allow(non_camel_case_types)]
            #[doc(hidden)]
            enum __Field {
                __field0,
                __ignore,
            }
            #[doc(hidden)]
            struct __FieldVisitor;
            #[automatically_derived]
            impl<'de> _serde::de::Visitor<'de> for __FieldVisitor {
                type Value = __Field;
                fn expecting(
                    &self,
                    __formatter: &mut _serde::__private::Formatter,
                ) -> _serde::__private::fmt::Result {
                    _serde::__private::Formatter::write_str(
                        __formatter,
                        "field identifier",
                    )
                }
                fn visit_u64<__E>(
                    self,
                    __value: u64,
                ) -> _serde::__private::Result<Self::Value, __E>
                where
                    __E: _serde::de::Error,
                {
                    match __value {
                        0u64 => _serde::__private::Ok(__Field::__field0),
                        _ => _serde::__private::Ok(__Field::__ignore),
                    }
                }
                fn visit_str<__E>(
                    self,
                    __value: &str,
                ) -> _serde::__private::Result<Self::Value, __E>
                where
                    __E: _serde::de::Error,
                {
                    match __value {
                        "bar" => _serde::__private::Ok(__Field::__field0),
                        _ => _serde::__private::Ok(__Field::__ignore),
                    }
                }
                fn visit_bytes<__E>(
                    self,
                    __value: &[u8],
                ) -> _serde::__private::Result<Self::Value, __E>
                where
                    __E: _serde::de::Error,
                {
                    match __value {
                        b"bar" => _serde::__private::Ok(__Field::__field0),
                        _ => _serde::__private::Ok(__Field::__ignore),
                    }
                }
            }
            #[automatically_derived]
            impl<'de> _serde::Deserialize<'de> for __Field {
                #[inline]
                fn deserialize<__D>(
                    __deserializer: __D,
                ) -> _serde::__private::Result<Self, __D::Error>
                where
                    __D: _serde::Deserializer<'de>,
                {
                    _serde::Deserializer::deserialize_identifier(
                        __deserializer,
                        __FieldVisitor,
                    )
                }
            }
            #[doc(hidden)]
            struct __Visitor<'de> {
                marker: _serde::__private::PhantomData<Foo>,
                lifetime: _serde::__private::PhantomData<&'de ()>,
            }
            #[automatically_derived]
            impl<'de> _serde::de::Visitor<'de> for __Visitor<'de> {
                type Value = Foo;
                fn expecting(
                    &self,
                    __formatter: &mut _serde::__private::Formatter,
                ) -> _serde::__private::fmt::Result {
                    _serde::__private::Formatter::write_str(__formatter, "struct Foo")
                }
                #[inline]
                fn visit_seq<__A>(
                    self,
                    mut __seq: __A,
                ) -> _serde::__private::Result<Self::Value, __A::Error>
                where
                    __A: _serde::de::SeqAccess<'de>,
                {
                    let __field0 = match _serde::de::SeqAccess::next_element::<
                        u8,
                    >(&mut __seq)? {
                        _serde::__private::Some(__value) => __value,
                        _serde::__private::None => {
                            return _serde::__private::Err(
                                _serde::de::Error::invalid_length(
                                    0usize,
                                    &"struct Foo with 1 element",
                                ),
                            );
                        }
                    };
                    _serde::__private::Ok(Foo { bar: __field0 })
                }
                #[inline]
                fn visit_map<__A>(
                    self,
                    mut __map: __A,
                ) -> _serde::__private::Result<Self::Value, __A::Error>
                where
                    __A: _serde::de::MapAccess<'de>,
                {
                    let mut __field0: _serde::__private::Option<u8> = _serde::__private::None;
                    while let _serde::__private::Some(__key) = _serde::de::MapAccess::next_key::<
                        __Field,
                    >(&mut __map)? {
                        match __key {
                            __Field::__field0 => {
                                if _serde::__private::Option::is_some(&__field0) {
                                    return _serde::__private::Err(
                                        <__A::Error as _serde::de::Error>::duplicate_field("bar"),
                                    );
                                }
                                __field0 = _serde::__private::Some(
                                    _serde::de::MapAccess::next_value::<u8>(&mut __map)?,
                                );
                            }
                            _ => {
                                let _ = _serde::de::MapAccess::next_value::<
                                    _serde::de::IgnoredAny,
                                >(&mut __map)?;
                            }
                        }
                    }
                    let __field0 = match __field0 {
                        _serde::__private::Some(__field0) => __field0,
                        _serde::__private::None => {
                            _serde::__private::de::missing_field("bar")?
                        }
                    };
                    _serde::__private::Ok(Foo { bar: __field0 })
                }
            }
            #[doc(hidden)]
            const FIELDS: &'static [&'static str] = &["bar"];
            _serde::Deserializer::deserialize_struct(
                __deserializer,
                "Foo",
                FIELDS,
                __Visitor {
                    marker: _serde::__private::PhantomData::<Foo>,
                    lifetime: _serde::__private::PhantomData,
                },
            )
        }
    }
};

... but we can use #[derive(Deserialize)] without thinking about the details, which is great.

This is why I assume the responsibility for error messages (and of course, the fact that the macro generates a valid code in the first place) lies with the macro writer and not with the compiler.

it still seems weird to me that "bug is in the macro" implies "compiler generating nonsense error message" is OK or expected?!

So, while I understand your point, unfortunately I don’t think the compiler can help much here... humans can/should write better errors.

Naively, I would expect the compiler to show me the generated code, with the error annotated on that generated code, along with telling me I'm looking at generated code.

I don’t think that is very helpful, especially because we can’t fix the errors in the generated code and the generated code can be complicated. By the way, you can cargo expand to see the expansion result.

[ntc2]

@naskya

This is why I assume the responsibility for error messages (and of course, the fact that the macro generates a valid code in the first place) lies with the macro writer and not with the compiler.

I'm still not understanding why the compiler is reporting syntactically invalid code in the error message tho. Are you saying the proc macro is responsible for the compiler showing the syntactically invalid code in the error message?

it still seems weird to me that "bug is in the macro" implies "compiler generating nonsense error message" is OK or expected?!

So, while I understand your point, unfortunately I don’t think the compiler can help much here... humans can/should write better errors.

I'm curious, in your minimal example, how would you change the macro to not have the error message be nonsensical? I.e. how to avoid the compiler suggesting the meaningless #[derive(Sample.into())]?

Naively, I would expect the compiler to show me the generated code, with the error annotated on that generated code, along with telling me I'm looking at generated code.

I don’t think that is very helpful, especially because we can’t fix the errors in the generated code and the generated code can be complicated. By the way, you can cargo expand to see the expansion result.

So, you make me realize that we're each making different assumptions about where the macro came from and how complex it is! In your example, the the #[derive(serde::Deserialize)] is some big "scary" thing from an external library, and you are right that seeing the full dump of the generated code with the error highlighted would probably not be so helpful. But in my case, the buggy macro is part of the code base I'm working on, and the reason it's buggy is that I'm in the middle of a rebase, and the rebase introduced some changes that are incompatible with the macro (i.e. a type changed, and the resolution is to get the old type back with .into()). So in my case, seeing the macro expansion is very useful (also, the expansion in my case is simple, ~10 lines).