rust-lang-nursery / lazy-static.rs

A small macro for defining lazy evaluated static variables in Rust.
Apache License 2.0
1.9k stars 108 forks source link

Incorrect usage of cargo features causes obscure compilation bugs #204

Open xStrom opened 2 years ago

xStrom commented 2 years ago

Trouble with features

Currently lazy_static has a feature called spin_no_std which violates the additive nature of Cargo featues and causes issues.

This is explicitly covered in the Cargo reference manual:

For example, if you want to optionally support no_std environments, do not use a no_std feature. Instead, use a std feature that enables std.

Thus the correct way to do it would be to depend on spin for the no_std behavior when no features are enabled, and then have a std feature, which could still be chosen to be enabled by default.

Effects in practice

Usage of lazy_static requires the type to implement Sync. As also mentioned in the docs: Any type in them needs to fulfill the Sync trait.

However spin::once::Once also requires Send, as seen in their docs.

This means that library A can implement something with lazy_static on a type that only implements Sync, and everything works fine.

Library B can implement something with lazy_static using spin_no_std on a type that implements both Send and Sync.

Application X then depends on both libraries A & B, but won't be able to compile. That's because Cargo features are additive and the spin_no_std feature also gets applied for library A. However the library A's type does not implement Send.

The application author is faced with a compilation error caused deep in some library that they don't even have control over.

This has happened now a few times with people building GUI applications using Druid, as Druid does not use the spin_no_std feature and has types which only implement Sync. Latest example in druid#2211.

Path forward

Fixing the Cargo feature usage would go a long way for both fixing this specific issue and helping prevent other future issues.

It would also be possible to address this specific issue by demanding Send just in general for using lazy_static. Then this specific issue wouldn't surface. Although we would still have a situation where one crate's choice of spin_no_std is king, even though the greater application has std anyway.