search

seanmonstar / httparse

A push parser for the HTTP 1.x protocol in Rust.
https://docs.rs/httparse
Apache License 2.0
569 stars 110 forks source link

Add missing Eq derivations #77

Closed Hawk777 closed 4 years ago

Hawk777 commented 4 years ago

The derivation on Header is correct because comparisons of string slices and byte slices meet the requirements. The derivations on Request and Response are correct because they contain only string slices, byte slices, slices of Header objects (which are now Eq), primitives, and options (which are Eq if their contents are). Finally the derivation on Status is correct because #[derive(Eq)] on a generic only actually implements Eq if all generic parameters are Eq, which for Status is sufficient.

seanmonstar commented 4 years ago

I agree they're correct, I just have one question: why would we want these implementations? What case does this help solve?

Hawk777 commented 4 years ago

Given that Eq is an empty trait, AFAICT there is no downside to adding it. Rust API Guidelines recommends eagerly implementing Eq where it is appropriate. It means that other people will then be able to #[derive(Eq)] on their own types that contain e.g. a Request object, which would not otherwise work; without this, those other people would have to impl Eq for MyType {} explicitly, which is significantly more verbose and also a little bit error-prone given that it leaves open the possibility that Request might stop being effectively Eq sometime and the other person not notice.

seanmonstar commented 4 years ago

Yea, thats true. Thanks, I'll merge!