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

Error ‘could not find derive_more in the list of imported crates’ when re-exporting macros #338

Closed
YuliaProkopovych opened this issue Mar 8, 2024 · 13 comments · Fixed by #344
Assignees
Milestone

Comments

@YuliaProkopovych
Copy link

Hi,

I have local lib crate that uses derive_more and re-exports its macros.
Then I import my lib with re-exported macros into another crate that uses them:

use my_derive::{ Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign };
#[ derive( Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign ) ]
pub struct Factor( pub f64 );

But I get following error :

29 | #[ derive( Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign ) ]
   |            ^^^ could not find `derive_more` in the list of imported crates
   |
   = note: this error originates in the derive macro `Add` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this crate

As I see, currently Add, Sub, Mul, Div macros use path to trait that starts with ::derive_more::
which doesn’t resolve after re-export.

let trait_path = quote! { ::derive_more::#trait_ident };

It would be great if macros used a path that resolves after re-export.

@JelteF
Copy link
Owner

JelteF commented Mar 9, 2024

Why are you re-exporting the macros like this? I'm inclined to think that this is not really a supported usecase.

I think you can solve the error by adding extern crate derive_more; to the crate that uses your re-exported macros.

To be clear, this was originally introduced to fix: #180

In theory we could reference the traits from core again, instead of from derive_more. But that would only work for including the traits. So the generated code would still fail to compile for derives that create any of the error structs: https://docs.rs/derive_more/1.0.0-beta.6/derive_more/index.html#structs

@YuliaProkopovych
Copy link
Author

YuliaProkopovych commented Mar 11, 2024

Why are you re-exporting the macros like this? I'm inclined to think that this is not really a supported usecase.

It is really convenient in my case, and it worked fine with previous version 0.99.17, so I didn't expect I'll have problems after update :(
By the way, AddAssign derive still works, and as I see in the source of derive_more-impl 1.0.0-beta.6 its implementation still uses path with ::core::ops::, what is the reason for this?

I think you can solve the error by adding extern crate derive_more; to the crate that uses your re-exported macros.

Unfortunately, it didn't work.

But that would only work for including the traits. So the generated code would still fail to compile for derives that create any of the error structs: https://docs.rs/derive_more/1.0.0-beta.6/derive_more/index.html#structs

Didn't encounter it so far, probably because I use only numeric derives for regular structs.

@tyranron
Copy link
Collaborator

@JelteF

In theory we could reference the traits from core again, instead of from derive_more. But that would only work for including the traits. So the generated code would still fail to compile for derives that create any of the error struct

We could also use derive_more::some::path instead of ::derive_more::some::path. This way, if someone re-exports our macros, he could also re-export the top-level name:

use my_derive::{derive_more, Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign};

#[derive(Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign)]
pub struct Factor(pub f64);
// expanding to `derive_more::core::ops::Add` will work now

This way is not ideal, but at least we provide an option for such re-exports at all.

On the other hand, I don't see any practical downsides for us if we'd do derive_more::some::path instead of ::derive_more::some::path.


@YuliaProkopovych

By the way, AddAssign derive still works, and as I see in the source of derive_more-impl 1.0.0-beta.6 its implementation still uses path with ::core::ops::, what is the reason for this?

Some old, not yet refactored, code. Eventually, will be remade too.

@JelteF
Copy link
Owner

JelteF commented Mar 11, 2024

This way is not ideal, but at least we provide an option for such re-exports at all.

Does adding extern crate derive_more; not work currently to solve this issue? I feel like that's a cleaner way of solving this (assuming it works).

@tyranron
Copy link
Collaborator

@JelteF no, unfortunately, it doesn't work if the derive_more is a transitive dependency.

I was thinking about this too, and wondered why Rust doesn't allow this. Then I realized that due to version requirements and resolution, there can be multiple derive_more crates of different versions in the dependency tree, which makes extern crate derive_more; extremely ambiguous if it's not specified as a direct dependency.

@JelteF
Copy link
Owner

JelteF commented Mar 11, 2024

On the other hand, I don't see any practical downsides for us if we'd do derive_more::some::path instead of ::derive_more::some::path.

@tyranron I don't really like doing this. It makes our generated code less hygenic.

It is really convenient in my case,

@YuliaProkopovych could you clarify why it is convenient? I'm currently leaning towards the position: "we cannot reasonbly support this usecase with the new design where we need to import error types from the derive_more crate". If you can clarify why you are are trying to do this in the first place, maybe I change my mind.

@tyranron
Copy link
Collaborator

@JelteF

I don't really like doing this. It makes our generated code less hygenic.

My point is that it's very unlikely for people to have derive_more modules in their code. The name is special enough.
I haven't figured out the practical case where the hygiene becomes worse. Have you some in mind to share?

@JelteF
Copy link
Owner

JelteF commented Mar 11, 2024

Yeah, maybe it's uncommon enough for people to have that not to care. But honestly I thought the same about people using "core" as their package name or re-exporting the derives from derive_more. I don't think handling all is possible, so I'm trying to understand which usecases actually make sense to support and which don't.

@YuliaProkopovych
Copy link
Author

@YuliaProkopovych could you clarify why it is convenient?

I have a crate where I put together commonly used derive macros, like the ones I mentioned, some derives from strum, some custom implementations of derive macros, so when I use them in other projects they are all in one place.

Unfortunately using extern crate derive_more; doesn't help. It would be great if there was some way to make it work.

@JelteF
Copy link
Owner

JelteF commented Mar 12, 2024

We could also use derive_more::some::path instead of ::derive_more::some::path. This way, if someone re-exports our macros, he could also re-export the top-level name:

use my_derive::{derive_more, Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign};

#[derive(Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign)]
pub struct Factor(pub f64);
// expanding to `derive_more::core::ops::Add` will work now

@YuliaProkopovych Would this be an acceptable workaround for you? i.e. Requiring to re-export and then also use the derive_more module?

@Wandalen
Copy link

Would be wonderful ❤️

@YuliaProkopovych
Copy link
Author

@YuliaProkopovych Would this be an acceptable workaround for you? i.e. Requiring to re-export and then also use the derive_more module?

Yes, absolutely. Thank you!

@JelteF JelteF added this to the 1.0.0 milestone Mar 16, 2024
@JelteF
Copy link
Owner

JelteF commented Mar 16, 2024

@tyranron okay let's do your suggestion then. And we should also update the path of AddAssign to include from derive_more.

@tyranron tyranron self-assigned this Mar 26, 2024
tyranron added a commit that referenced this issue Mar 27, 2024
…338)

## Synopsis

At the moment, the `derive_more` macros cannot be re-exported or the
`derive_more` crate cannot be renamed as the dependency, because
expansions require `::derive_more` module be present in the scope. See
#338 for details.




## Solution

Use `derive_more::*` paths instead of `::derive_more::*` in expansions,
allowing to provide this module in the scope on the call site even when
`derive_more` is transitive dependency of the crate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants