The "std" feature isn't additive

[oli-obk]

We have a crate a depending on serde with default-features = false set. Crate b also depends on serde, but doesn't enable that feature. Now if crate c depends on both a and b, a doesn't compile anymore.

cc @whitequark

[Manishearth]

I believe the correct way to do no_std is an std feature, not a no_std feature, because that isn't additive?

This is the other crate's fault. Wouldn't it be broken without it's own no_std anyway?

[oli-obk]

Well serde has an std feature. A feature adding a new public type works without problems, even if the crate using serde does not have its own std feature. The issue is a feature which adds methods to a trait. There's no way to specify it correctly without having your own std feature.

We need to document this at the minimum.

[Manishearth]

Yes.

A feature adding a new public type vs one adding methods should behave the same? Features aren't aware of semantics. Could you elaborate?

[oli-obk]

Well... adding new methods to a trait will break anyone implementing said trait for a type. Adding a new public type breaks glob imports in rare cases.

[dtolnay]

I agree that this is implemented correctly and not something we need to fix. I opened serde-rs/serde-rs.github.io#59 to follow up on docs.

[whitequark]

Is this really implemented correctly? Can't there be two traints, one with common methods and another, deriving from it, that has the std-dependent ones?

[dtolnay]

Yes really. Of the 800+ dependencies on serde in crates.io, there are 6 that depend on no_std:

• generic-array
• oauth2-lib
• bincode_core
• ssmarshal
• corepack
• serde-tuple-vec-map

The current approach is dramatically simpler in terms of total engineering effort, compared to having two Visitor traits and four Error traits and requiring practically everybody to keep them straight and implement them separately.

[Manishearth]

Features are additive. Having a no_std feature means that if you have other crates depending on serde in your deptree they will break because you pulled in no_std. Having a std feature doesn't do that. #[cfg(not(feature = foo))] is not something you should be writing without a corresponding #[cfg(feature = foo)] that exposes the same API.

In general, if you want to use no_std, all the crates in your deptree have to be no_std compatible. It's not too much to ask for reverse dependencies to propagate no_std down because they will be doing the work anyway to be no_std compat.

[whitequark]

It's not too much to ask for reverse dependencies to propagate no_std down because they will be doing the work anyway to be no_std compat.

Or rather they could have never used std in the first place...

[Manishearth]

Fair, but in that case again it's up to them to depend on the no_std version of serde if they intend the crate to be no_std.

[whitequark]

Yeah, I misunderstood this as "a no_std feature that removes methods from traits", now that I see what it is correctly, there is not really anything wrong with this implementation. Again, sorry for the distraction.

[le-jzr]

Features are supposed to be additive. That means, in other words, enabling a feature should not, in general, break public API. In this case, the crate a must have its own feature std, not to gate any feature of its own, but to declare which of the two mutually incompatible versions of serde it's being linked with. Which is pretty weird.

[dtolnay]

This is working as intended. Please send a PR if you disagree.

[whitequark]

If I'm understanding serde's case correctly (am I?) the core of the issue here is that code generated by serde differs depending on whether liballoc and libcollections are available. I.e. with just bare no_std, with no unstable features, it is impossible to have a trait that refers to Vec. The public API from the point of view of the downstream user of the crate depending on serde does not really change, since the purely no_std crate cannot use liballoc or libcollections by definition.

Since serde normally works on stable Rust, there are essentially two choices here:

• Either make liballoc/libcollections and nightly Rust a hard dependency for all no_std crates depending on serde,
• Or remove the methods dependent on Box, Vec, etc, from the traits.

Both of these are unsatisfying. First, while most no_std use cases today will involve nightly Rust in some form of another to generate the final binary, this is not true of the libraries, and in fact all of my no_std libraries build on stable Rust by default.

Second, many no_std use cases explicitly and specifically avoid the use of heap, and so a dependency on liballoc or libcollections would make serde unusable, or at least strongly undesirable, in those environments.

Note that it does not matter whether the traits are split into two (say, Serializer + SerializerStd) or not, because to derive a SerializerStd trait there would still have to exist a dependency on liballoc/libcollections in order to import the types, even if the traits are never actually used. This is in addition to the significant burden of correctly updating the trait bounds everywhere, which I'm not even sure if possible to elegantly do in the first place for all cases.

@dtolnay Can you confirm that my understanding is correct?

[dtolnay]

Nope, the generated code is identical with or without std. You can tell because serde_derive does not have a std feature.

I'm not sure how this issue came up initially (maybe @oli-obk could clarify) but I suspect it may be the de::Error trait which has std::error::Error as a supertrait if building with std. This is important for code that is generic over the Deserializer and wants to be able to use Box<Error> or error-chain or anything else that requires std's Error trait.

[clarfonthey]

Perhaps as a longer-term solution upstream, the Error trait could be moved to core and the Box implementations could be moved to Alloc?

[le-jzr]

Since Error depends on Box, that's not an option. In fact, moving Error into core has been considered and rejected due to how much more difficult error handling would have become. I'm sure a newtype wrapper of some sort could sort it out, but it might make things more awkward for the usual case (maybe). In any case, the impact is almost nonexistent, so it's probably best to just accept it.

[oli-obk]

I just followed a conversation on Irc. But it didn't have much detail.

[bluss]

I was curious, just to clarify. Here's the snippet for the serde::de::Error trait (serde 1.0.2 docs.rs source link)

#[cfg(feature = "std")]
declare_error_trait!(Error: Sized + error::Error);

#[cfg(not(feature = "std"))]
declare_error_trait!(Error: Sized + Debug + Display);

This makes the crate feature "std" technically non-additive (the requirements for implementing the trait changes if one uses the feature). I understand that this is as intended, and there aren't great choices either way here.