Skip to content

Commit

Permalink
Expand traits implemented by structs within libc
Browse files Browse the repository at this point in the history
  • Loading branch information
Bryant Mairs committed Mar 3, 2018
1 parent 9cd6ae9 commit 7567306
Showing 1 changed file with 67 additions and 0 deletions.
67 changes: 67 additions & 0 deletions text/0000-libc-struct-traits.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
- Feature Name: (libc_struct_traits)
- Start Date: (2017-12-05)
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Provide `Debug`, `Eq`, `Hash`, and `PartialEq` implementations for all structs within the `libc` crate.

# Motivation
[motivation]: #motivation

This will allow downstream crates to easily support similar operations with any types they
provide that contain `libc` structs. Additionally [The Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/checklist.html) specify that it is
considered useful to expose as many traits as possible from the standard library. In order to facilitate the
following of these guidelines, official Rust libraries should lead by example.

For many of these traits, it is trivial for downstream crates to implement them for these types by using
newtype wrappers. As a specific example, the `nix` crate offers the `TimeSpec` wrapper type around the `timespec` struct. This
wrapper could easily implement `Eq` through comparing both fields in the struct.

Unfortunately there are a great many structs that are large and vary widely between platforms. Some of these in use by `nix`
are `dqblk`, `utsname`, and `statvfs`. These structs have fields and field types that vary across platforms. As `nix` aims to
support as many platforms as `libc` does, this variation makes implementing these traits manually on wrapper types time consuming and
error prone.

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation

The `Debug`, `Eq`, `Hash`, and `PartialEq` traits will be implemented for all structs within `libc` if the user opts-in to the `all_traits` feature.

# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

The `all_traits` feature will be added to `libc` that will enable the `Debug`, `Eq`, `Hash`, and `PartialEq` traits for all types (technically `Debug`
was already allowed as part of the original `libc` RFC). Most types will use automatic derives within the `s!` macro in `src/macros.rs`, but this won't
work for structs containing arrays larger than 32 elements. For these types the traits will be implemented manually. As a simple test, on
`x86_64-unknown-linux-gnu`, I derived all the traits independently to show how many structs would need manual implementations:

* `Debug` - 17
* `Eq` and `PartialEq` - 46
* `Hash` - 17

# Drawbacks
[drawbacks]: #drawbacks

For users that opt-in to the `all_traits` feature, this will impact compilation times, especially first build times.

For the maintainers of `libc`, all new types added will require these implementations raising the barrier to entry for new contributors. This could be
mitigated by working to improve the auto-derive situation such that it applies to more types.

# Rationale and alternatives
[alternatives]: #alternatives

Alternatives considered were to not have a feature flag, but that raises the compilation time by 3x on the example test case I performed, which was
considered to be too drastic of an increase in compilation time for a crate so low in the crate hierarchy for projects.

There was consideration between opt-in or an opt-out flag. The opt-in flag is more conservative: it won't bring these new traits to users without their
explicit permission, but it won't increase their compilation time either. Additionally, this can be later changed to opt-out or removed entirely as
compilation times improve.

The maintenance burden and barrier to entry increases for `libc` with manual implementations of these traits for the types that require it. Instead these
traits could only be added for types that can auto-derive them. This was rejected as being too confusing for the users of `libc`.

# Unresolved questions
[unresolved]: #unresolved-questions

0 comments on commit 7567306

Please sign in to comment.