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.

Sign up for GitHub

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

Jump to bottom

Replace usage of mem::uninitialized with MaybeUninit #2532

Closed
memoryruins opened this issue May 1, 2021 · 4 comments · Fixed by #2545
Closed

Replace usage of mem::uninitialized with MaybeUninit #2532

memoryruins opened this issue May 1, 2021 · 4 comments · Fixed by #2545
Labels
A-http1 Area: HTTP/1 specific. C-refactor Category: refactor. This would improve the clarity of internal code. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@memoryruins
Copy link
Contributor

When the following comment was added in #1845, hyper's minimum supported rust version was 1.27.0

hyper/src/proto/h1/role.rs

Lines 1 to 3 in 4e9a006

// `mem::uninitialized` replaced with `mem::MaybeUninit`,
// can't upgrade yet
#![allow(deprecated)]

which I assume is why std::mem::MaybeUninit, stabilized in 1.36.0, could not be used yet. Today, hyper's MSRV is 1.46.0, so it should now be possible to investigate removing the #![allow(deprecated)].
@seanmonstar
Copy link
Member

Good call!

@seanmonstar seanmonstar added A-http1 Area: HTTP/1 specific. E-easy Effort: easy. A task that would be a great starting point for a new contributor. C-refactor Category: refactor. This would improve the clarity of internal code. labels May 4, 2021
@mohsen-alizadeh mohsen-alizadeh mentioned this issue May 4, 2021
Closed
@mohsen-alizadeh
Copy link
Contributor

Refactored the code.

@Soveu
Copy link
Contributor

Soveu commented May 4, 2021 •
edited
Loading

One problem with refactoring the code is that httparse::Request::new accepts &mut [Header], so it is assumed that they are initialized. It would need to accept &mut [MaybeUninit<Header>]

Otherwise MaybeUninit::uninit().assume_init() (like it is done here) gives the same problems as mem::uninitialized

This was referenced May 4, 2021
Closed
Merged
@RalfJung
Copy link
Contributor

FYI, this seems to be one of the major sources of regressions in rust-lang/rust#87041.

@RalfJung RalfJung mentioned this issue Jul 21, 2021
Closed
seanmonstar pushed a commit that referenced this issue Aug 19, 2021
@Soveu
refactor(http1): use MaybeUninit for parsing with uninitialized heade…
95a9783 
…rs (#2545)

Closes #2532
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-http1 Area: HTTP/1 specific. C-refactor Category: refactor. This would improve the clarity of internal code. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use new methods for parsing with uninitialized headers from httparse Soveu/hyper
5 participants
@seanmonstar @RalfJung @mohsen-alizadeh @memoryruins @Soveu