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

Decouple parsing from the Buffer type. #257

Merged
merged 1 commit into from
Mar 7, 2020
Merged

Decouple parsing from the Buffer type. #257

merged 1 commit into from
Mar 7, 2020

Conversation

eduardosm
Copy link
Collaborator

This is a step towards #251.

Now, the TryFrom<Buffer> and TryFrom<&Buffer> are no longer implemented. All the remaining code now uses TryFrom<&[u8]>.

This is a step towards #251.

Now, the `TryFrom<Buffer>` and `TryFrom<&Buffer>` are no longer implemented. All the remaining code now uses `TryFrom<&[u8]>`.
@@ -88,7 +88,7 @@ pub trait RequestConnection {
fds: Vec<RawFdContainer>,
) -> Result<Cookie<'_, Self, R>, ConnectionError>
where
R: TryFrom<Buffer, Error = ParseError>;
R: for<'a> TryFrom<&'a [u8], Error = ParseError>;
Copy link
Owner

Choose a reason for hiding this comment

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

Okay, this is something that I would never have come up with. Today I learned about what for<'a> is good for... (I saw this before, but this use here makes a lot of sense to me).

Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, I could not get around the single_use_lifetimes lint, so I disabled it.

@@ -68,7 +68,7 @@
//missing_docs,
private_doc_tests,
rust_2018_idioms,
single_use_lifetimes,
//single_use_lifetimes,
Copy link
Owner

Choose a reason for hiding this comment

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

Sigh. I had hoped this can be done without it.

Do you think deny(single_use_lifetimes) is useful in general? If not, we can just remove this. Otherwise, some #[!allow... is needed instead of just removing it globally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm afraid these allow would be needed on each implementation of the RequestConnection trait. I think we can go without single_use_lifetimes. I believe it won't be that bad if an avoidable single-use lifetime steps in.

@psychon
Copy link
Owner

psychon commented Mar 7, 2020

Thanks, this definitely looks like a step in the right direction. The whole Buffer thing is a hack that should be removed.

Do you want to continue working on #251 after this is merged?

@eduardosm
Copy link
Collaborator Author

Do you want to continue working on #251 after this is merged?

I'll be glad to 😄

@psychon psychon merged commit 2a40d52 into psychon:master Mar 7, 2020
@psychon
Copy link
Owner

psychon commented Mar 7, 2020

Okay, thanks!

@eduardosm eduardosm deleted the buffer branch March 7, 2020 16:31
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