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

Fix no_std support #84

Closed
wants to merge 1 commit into from
Closed

Fix no_std support #84

wants to merge 1 commit into from

Conversation

CryZe
Copy link

@CryZe CryZe commented Aug 12, 2019

Features are additive, so a no_std feature doesn't actually work. This replaces it with a std feature instead.

Features are additive, so a `no_std` feature doesn't actually work. This
replaces it with a `std` feature instead.
@CryZe
Copy link
Author

CryZe commented Aug 12, 2019

Uhm, is this just fundamentally broken?

@JelteF
Copy link
Owner

JelteF commented Aug 12, 2019

This hasn't been changed since it was added. I doubt it's suddenly not working anymore.
Did you set default-features to false like described in the README?:

derive_more = {version = "0.13.0", default-features = false, features=["no_std"]}

If so can you explain what is not working?

@CryZe
Copy link
Author

CryZe commented Aug 12, 2019

It was „never working“. Features are additive, you can‘t make a no_std feature. Let‘s say there‘s two completely separate crates that both depend on a library, one doesn‘t activate the no_std feature and thus makes use of all the std capabilities of the library and then there‘s another crate that depends on the same library, but it activates the no_std feature, because it doesn‘t need std. It‘s a lightweight crate that only needs a few core things. However by activating the no_std feature, the other crate now has that feature activated too and it stops compiling. Thus features always have to be additive and can never remove functionality.

Now interestingly derive_more is a macro crate, which means that either way you break some crates. Either you break the crates that don‘t import core or those that don‘t import std.

I honestly have no solution for this. This might be a fundamental issue in Rust. We may need to raise an issue in the Rust repo.

@CryZe
Copy link
Author

CryZe commented Aug 12, 2019

Alright so the solution seems to be to get rid of the std feature altogether for proc-macro crates (that seems to be the consensus when I talked about it in the Rust Discord). However that would be a slight breaking change for everyone still on Rust 2015.

@JelteF
Copy link
Owner

JelteF commented Aug 12, 2019

I think I understand what you mean now, but I don't see the problem for this crate exactly because it's a macro crate. It seems to me that for this crate the only person that would want to decide to have std or not is the author of the final binary. If he want's a binary without std he can simply set the no_std feature in Cargo.toml for this crate. Because the features are additive all his other dependencies that use derive_more will automatically also use the no_std version, thus generating only no_std code.

I'm probably missing some use case in this reasoning (or I'm wrong in general), because otherwise you would not say it's not working. Could you explain the exact use case that isn't covered like this? i.e. how do you want to use derive_more that is not possible with the current std/no_std approach

@JelteF
Copy link
Owner

JelteF commented Aug 12, 2019

Also, could you tell me what the difference is between rust 2018 and 2015 regarding this behaviour. If rust 2018 makes everything simpler regarding this I would be fine dropping support for rust 2015 in the next release.

@CryZe
Copy link
Author

CryZe commented Aug 13, 2019

Ideally only the final binary author would specify that derive_more should use no_std. However crates that the binary depends on fall into three categories: crates that support std only, crates that can support either no_std or std (via a feature), and crates that don't make use of std and thus can also support both std and no_std use cases. The latter two are problematic here.

If you have a crate that doesn't need std at all, which, with stable core and alloc, is fairly likely nowadays, it is recommend you make your crate no_std, because std crates can still depend on your crate, but you also support no_std use cases. So if one of these crates depends on derive_more, it will likely always activate the no_std feature of derive_more, which can then break other crates when a final std using binary uses that crate.

The crate I'm working on falls in the hybrid category where it supports no_std if no features are activated and with a std feature you can activate additional functionality. However I don't believe it's possible to even activate a no_std feature if my std is not activated and vice versa. You'd need a way to negate features. However since features are additive only, that's likely never going to be a thing, unless features in general are being overhauled completely. So in my case I would have (and currently do so in my WIP branch) to always activate the no_std feature of derive_more, such that I can support no_std for my crate at all. That will very likely break derive_more for someone else in a final binary's dependency graph.

The problematic part is that derive_more is a macro crate, so it needs to generate code for both Rust 2015 and Rust 2018 crates. The problem is that some crates only have std available and some only have core available - in the same dependency graph. This is horrible to work with. When discussing this problem with others they linked me some futures macro that has the same problem. They solve it by reexporting (with doc(hidden)) core out of their non-macro crate such that the macro has a guaranteed path to core. However derive_more does not have a base non-macro crate. So that solution is not feasible.

Here is the matrix that we work with. The column is the Rust edition that a crate uses. The row is the std / no_std mode a crate chose. And for the intersection it shows what std / core crates are available.

Rust 2015 Rust 2018
Is no_std core core
Is std std core + std

As you can see the Rust 2015 + std combination only has std available. The no_std row only has core available. So having derive_more's proc macro target std, you break all no_std crates (despite no_std crates usually working just fine as a dependency of a std crate). If you have it target core instead, you break Rust 2015 crates that use std. So no matter what the state of the std / no_std feature of derive_more is, a combination of these crates as dependencies in your final std targeting binary is always problematic.

However there is a solution to this, and that is derive_more's macro always targeting core and Rust 2015 + std crates having to explicitly specify extern crate core;. It's not great, but it allows for derive_more targeting all 4 combinations within the same dependency graph.

TL;DR: There are crates that don't have a std / no_std feature that always just target core. They work just fine as a dependency of a std crate. However derive_more would generate code for std even for the no_std crate then, which breaks it, as it doesn't use std. derive_more always generating code for core mostly solves the problem.

@JelteF
Copy link
Owner

JelteF commented Aug 13, 2019

Thanks for the clear and extensive explanation. If you could make a PR for this:

However there is a solution to this, and that is derive_more's macro always targeting core and Rust 2015 + std crates having to explicitly specify extern crate core;. It's not great, but it allows for derive_more targeting all 4 combinations within the same dependency graph.

Then I'll merge it. Please update the README to note that that is needed for rust 2015 crates. And make a test for rust 2018 and one for 2015.

@JelteF
Copy link
Owner

JelteF commented Aug 14, 2019

This is fixed on master now. Could you confirm that this indeed solves your issue?

@JelteF JelteF closed this Aug 14, 2019
@JelteF
Copy link
Owner

JelteF commented Nov 11, 2019

This is now released as version 0.99.0. Announcement link: https://reddit.com/r/rust/comments/duptc0/derive_more_0990_released_biggest_release_ever

@CryZe
Copy link
Author

CryZe commented Nov 11, 2019

Thank you so much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants