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

Tracking issue for RFC 2235, "Implement Debug, Eq, PartialEq, and Hash for libc structs" #57715

Closed
3 tasks
Centril opened this issue Jan 17, 2019 · 5 comments
Closed
3 tasks
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Jan 17, 2019

This is a tracking issue for the RFC "Implement Debug, Eq, PartialEq, and Hash for libc structs" (rust-lang/rfcs#2235).

Steps:

Unresolved questions:

  • Is it possible to just add #[derive(Debug, Eq, Hash, PartialEq)] since the Copy and Clone traits are directly implemented and not derived?
@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Jan 17, 2019
@Susurrus
Copy link
Contributor

Susurrus commented Feb 5, 2019

A problem that came up when implementing this is for implementing Hash for types that have floats or doubles in them. For the fpreg_t struct I did the following:

impl std::hash::Hash for fpreg_t {
    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
        self.d.to_bits().hash(state);
    }
}

We should decide if we want to handle this: avoid it entirely for types that have floats or doubles, implement them like I did above, or sort out a "better" way to deal with hashing them.

bors added a commit to rust-lang/libc that referenced this issue Feb 5, 2019
RFC 2235 - Implement PartialEq,Eq,Hash,Debug for all types

First pass at implementing [RFC2235](https://github.com/rust-lang/rfcs/blob/master/text/2235-libc-struct-traits.md). I just started with `PartialEq`/`Eq` and tested locally on my x64 Linux system. Definitely going to run into other types for other platforms that need this treatment, but thought I'd start small.

Open question is whether this should also rely on the `align` feature, which should improve which types can be auto-derived versus manually implemented (as it removed a lot of odd-sized padding arrays). My first pass here doesn't do that, but I might test it out to see if it does simplify quite a few structs. I think it might also be nice to have as it makes it easier for contributors to add new structs.

Part of rust-lang/rust#57715
@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 21, 2019

Many types still don't implement all of the traits, e.g., grep on libc for missing_debug_implementations, for example.

  • Some types can't easily derive(Debug) because they are repr(packed).
  • Some types can't easily derive most of the traits because they contain arrays with more than 32 elements.

bors added a commit to rust-lang/libc that referenced this issue Feb 24, 2019
Implement more extra_traits

Finishing the implementation of rust-lang/rust#57715 now that all platforms are tested in CI. I plan to expand this CI to all platforms that need this treatment (solarish, empscripten, freebsd, and netbsd), but thought I'd get the part I've already started running in CI since I can't test this changes locally.
@jonas-schievink
Copy link
Contributor

@Susurrus is this fully implemented now? rust-lang/libc#1271 seems to indicate so.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 9, 2019 via email

@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 31, 2020
@joshtriplett
Copy link
Member

This is mostly implemented, and to the extent it isn't, it's a libc issue rather than a rust-lang/rust issue. Closing; please feel free to open an issue on libc for any remaining aspects of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants