Skip to content

Commit 63eada3

Browse files
committed
Revise libc_struct_traits
1 parent c5a82cf commit 63eada3

File tree

1 file changed

+32
-25
lines changed

1 file changed

+32
-25
lines changed

text/0000-libc-struct-traits.md

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,58 +28,65 @@ error prone.
2828
# Guide-level explanation
2929
[guide-level-explanation]: #guide-level-explanation
3030

31-
The feature flag `derive_all` is added to the `libc` library that when enabled adds `Debug`, `Eq`, `Hash`, and `PartialEq` traits for all structs. It will default to off.
31+
Add an `extra_traits` feature to the `libc` library that enables `Debug`, `Eq`, `Hash`, and `PartialEq` implementations for all structs.
3232

3333
# Reference-level explanation
3434
[reference-level-explanation]: #reference-level-explanation
3535

36-
The `Debug`, `Eq`, `Hash`, and `PartialEq` traits will be added as automatic derives within the `s!` macro in `src/macros.rs` if the `derive_all` feature
36+
The `Debug`, `Eq`/`PartialEq`, and `Hash` traits will be added as automatic derives within the `s!` macro in `src/macros.rs` if the corresponding feature
3737
flag is enabled. This won't work for some types because auto-derive doesn't work for arrays larger than 32 elements, so for these they'll be implemented manually. For `libc`
3838
as of `bbda50d20937e570df5ec857eea0e2a098e76b2d` on `x86_64-unknown-linux-gnu` these many structs will need manual implementations:
3939

4040
* `Debug` - 17
41-
* `Eq` and `PartialEq` - 46
41+
* `Eq`/`PartialEq` - 46
4242
* `Hash` - 17
4343

4444
# Drawbacks
4545
[drawbacks]: #drawbacks
4646

47-
The addition of this behind a feature flag does not have a significant effect on build times, but the burden of adding these implementations for new types that
48-
require manual implementations will be high, possibly hindering new contributors.
47+
While most structs will be able to derive these implementations automatically, some will not (for example arrays larger than 32 elements). This will make it harder to add
48+
some structs to `libc`.
49+
50+
This extra trait will increase the testing requirements for `libc`.
4951

5052
# Rationale and alternatives
5153
[alternatives]: #alternatives
5254

5355
Adding these trait implementations behind a singular feature flag has the best compination of utility and ergonomics out of the possible alternatives listed below:
5456

55-
## Always enabled
57+
## Always enabled with no feature flags
5658

57-
This was regarded as unsuitable because it doubles to triples compilation time. Compilation times of `libc` was tested at commit `bbda50d20937e570df5ec857eea0e2a098e76b2d`
58-
with modifications to add derives for the traits discussed here. Some types failed to have these traits derived because of specific fields, so these were removed from the
59-
struct declaration. The table below shows the results:
59+
This was regarded as unsuitable because it increases compilation times by 100-200%. Compilation times of `libc` was tested at commit `bbda50d20937e570df5ec857eea0e2a098e76b2d`
60+
with modifications to add derives for the traits discussed here under the `extra_traits` feature (with no other features). Some types failed to have these traits
61+
derived because of specific fields, so these were removed from the struct declaration. The table below shows the compilation times:
6062

61-
| Build arguments | Time |
62-
|--------------------------------------------------------------------------------------------|-------|
63-
| `cargo clean && cargo build --no-default-features` | 0.84s |
64-
| `cargo clean && cargo build --no-default-features --features derive_all` | 2.17s |
65-
| `cargo clean && cargo build --no-default-features --release` | 0.64s |
66-
| `cargo clean && cargo build --no-default-features --release --features derive_all` | 1.80s |
67-
| `cargo clean && cargo build --no-default-features --features use_std` | 1.14s |
68-
| `cargo clean && cargo build --no-default-features --features use_std,derive_all` | 2.34s |
69-
| `cargo clean && cargo build --no-default-features --release --features use_std` | 0.66s |
70-
| `cargo clean && cargo build --no-default-features --release --features use_std,derive_all` | 1.94s |
63+
| Build arguments | Time |
64+
|----------------------------------------------------------------------------------------------|-------|
65+
| `cargo clean && cargo build --no-default-features` | 0.84s |
66+
| `cargo clean && cargo build --no-default-features --features extra_traits` | 2.17s |
67+
| `cargo clean && cargo build --no-default-features --release` | 0.64s |
68+
| `cargo clean && cargo build --no-default-features --release --features extra_traits` | 1.80s |
69+
| `cargo clean && cargo build --no-default-features --features use_std` | 1.14s |
70+
| `cargo clean && cargo build --no-default-features --features use_std,extra_traits` | 2.34s |
71+
| `cargo clean && cargo build --no-default-features --release --features use_std` | 0.66s |
72+
| `cargo clean && cargo build --no-default-features --release --features use_std,extra_traits` | 1.94s |
7173

72-
## Default-on feature flag
74+
## Default-on feature
7375

7476
For crates that are more than one level above `libc` in the dependency chain it will be impossible for them to opt out. This could also happen with a default-off
7577
feature flag, but it's more likely the library authors will expose it as a flag as well.
7678

77-
## Independent feature flags
79+
## Multiple feature flags
80+
81+
Instead of having a single `extra_traits` feature, have it and feature flags for each trait individually like:
7882

79-
It wasn't tested how much compilation times increased per-trait, but further mitigation of slow compilation times could done by exposing all traits mentioned here
80-
behind individual feature flags. By doing this it becomes harder for downstream crates to pass-through these feature flags, so it's likely not a worthwhile tradeoff.
83+
* `trait_debug` - Enables `Debug` for all structs
84+
* `trait_eg` - Enables `Eq` and `PartialEq` for all structs
85+
* `trait_hash` - Enables `Hash` for all structs
86+
* `extra_traits` - Enables all of the above through dependent features
87+
88+
This change should reduce compilation times when not all traits are desired. The downsides are that it complicates CI. It can be added in a backwards-compatible
89+
manner later should compilation times or consumer demand changes.
8190

8291
# Unresolved questions
8392
[unresolved]: #unresolved-questions
84-
85-
Is `derive_all` a suitable name for this feature flag?

0 commit comments

Comments
 (0)