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

PathAndQuery implements std::hash::Hash #582

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Jan 19, 2023

this provides an implementation of Hash for the PathAndQuery structure.

an example program is shown below, demonstrating why this would be a desirable improvement in ergonomics.

rust playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cb5e9eb3afc705d7760af0fe695fe3a5

✨ **See code snippet..** ✨
 //! A small example program demonstrating the ergonomics of [`Hash::hash`]ing
 // structures that contain a [`http::uri::PathAndQuery`] in an inner field.

 #![allow(dead_code)]

 use {
     http::uri::PathAndQuery,
     std::hash::{Hash, Hasher},
 };

 /// A structure that *can* be hashed.
 ///
 /// Note that it must convert the [`PathAndQuery`] to a string in order to be
 /// derivably [`Hash`]able.
 #[derive(Hash)]
 struct CanBeHashed {
     inner: String,
 }

 impl CanBeHashed {
     pub fn new(path_and_query: PathAndQuery) -> Self {
         let inner = path_and_query.as_str().to_owned();
         Self { inner }
     }

     pub fn path_and_query(&self) -> PathAndQuery {
         // We can derive a `Hash` implementation, but in order to access the
         // path and query, we have to parse the data again.
         self
             .inner
             .parse::<PathAndQuery>()
             .expect("inner data is a valid `PathAndQuery`")
     }
 }

 /// A structure that *cannot* be derivably hashed.
 ///
 /// If we uncomment the derivation below, and comment out the manual
 /// implementation provided later, we will see the following error:
 ///
 /// ```ignore
 /// error[E0277]: the trait bound `PathAndQuery: Hash` is not satisfied
 ///   --> src/main.rs:26:5
 ///    |
 /// 24 | #[derive(Hash)]
 ///    |          ---- in this derive macro expansion
 /// 25 | struct CannotBeHashed {
 /// 26 |     inner: PathAndQuery,
 ///    |     ^^^^^^^^^^^^^^^^^^^ the trait `Hash` is not implemented for `PathAndQuery`
 /// ```
 // #[derive(Hash)]
 struct CannotBeHashed {
     inner: PathAndQuery,
 }

 impl CannotBeHashed {
     fn new(inner: PathAndQuery) -> Self {
         Self { inner }
     }

     pub fn path_and_query(&self) -> &PathAndQuery {
         // The path and query can be cheaply accessed as such...
         &self.inner
     }
 }

 impl Hash for CannotBeHashed {
     fn hash<H: Hasher>(&self, state: &mut H) {
         // ...but we must manually implement `Hash`, casting the `PathAndQuery`
         // into a string slice.
         let Self { inner } = self;
         inner.as_str().hash(state);
     }
 }

 // NB: a clever reader may note `PathAndQuery::from_maybe_shared`, which could
 // reduce copying overhead! This still entails iterating through the buffer
 // to find the beginning of the query component, unfortunately.  :,(

 fn main() {}

this provides an implementation of `Hash` for the `PathAndQuery`
structure.

an example program is shown below, demonstrating why this would be a
desirable improvement in ergonomics.

rust playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cb5e9eb3afc705d7760af0fe695fe3a5

```rust
 //! A small example program demonstrating the ergonomics of [`Hash::hash`]ing
 // structures that contain a [`http::uri::PathAndQuery`] in an inner field.

 #![allow(dead_code)]

 use {
     http::uri::PathAndQuery,
     std::hash::{Hash, Hasher},
 };

 /// A structure that *can* be hashed.
 ///
 /// Note that it must convert the [`PathAndQuery`] to a string in order to be
 /// derivably [`Hash`]able.
 #[derive(Hash)]
 struct CanBeHashed {
     inner: String,
 }

 impl CanBeHashed {
     pub fn new(path_and_query: PathAndQuery) -> Self {
         let inner = path_and_query.as_str().to_owned();
         Self { inner }
     }

     pub fn path_and_query(&self) -> PathAndQuery {
         // We can derive a `Hash` implementation, but in order to access the
         // path and query, we have to parse the data again.
         self
             .inner
             .parse::<PathAndQuery>()
             .expect("inner data is a valid `PathAndQuery`")
     }
 }

 /// A structure that *cannot* be derivably hashed.
 ///
 /// If we uncomment the derivation below, and comment out the manual
 /// implementation provided later, we will see the following error:
 ///
 /// ```ignore
 /// error[E0277]: the trait bound `PathAndQuery: Hash` is not satisfied
 ///   --> src/main.rs:26:5
 ///    |
 /// 24 | #[derive(Hash)]
 ///    |          ---- in this derive macro expansion
 /// 25 | struct CannotBeHashed {
 /// 26 |     inner: PathAndQuery,
 ///    |     ^^^^^^^^^^^^^^^^^^^ the trait `Hash` is not implemented for `PathAndQuery`
 /// ```
 // #[derive(Hash)]
 struct CannotBeHashed {
     inner: PathAndQuery,
 }

 impl CannotBeHashed {
     fn new(inner: PathAndQuery) -> Self {
         Self { inner }
     }

     pub fn path_and_query(&self) -> &PathAndQuery {
         // The path and query can be cheaply accessed as such...
         &self.inner
     }
 }

 impl Hash for CannotBeHashed {
     fn hash<H: Hasher>(&self, state: &mut H) {
         // ...but we must manually implement `Hash`, casting the `PathAndQuery`
         // into a string slice.
         let Self { inner } = self;
         inner.as_str().hash(state);
     }
 }

 // NB: a clever reader may note `PathAndQuery::from_maybe_shared`, which could
 // reduce copying overhead! This still entails iterating through the buffer
 // to find the beginning of the query component, unfortunately.  :,(

 fn main() {}
```
Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and especially for the descriptive explanation! :)

@seanmonstar seanmonstar merged commit f0ba97f into hyperium:master Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants