impl ToSocketAddrs for (String, u16) by yoshuawuyts · Pull Request #73007 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation29 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

yoshuawuyts

This adds a convenience impl of ToSocketAddrs for (String, u16). When authoring HTTP services it's common to take command line options for host and port and parse them into String and u16 respectively. Consider the following program:

#[derive(Debug, StructOpt)] struct Config { host: String, port: u16, }

async fn main() -> io::Result<()> { let config = Config::from_args(); let stream = TcpStream::connect((&config.host, config.port))?; // & is not ideal // ... }

Networking is a pretty common starting point for people new to Rust, and seeing &* in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that String can't be passed directly there. Instead with this patch we can omit the &* conversion and pass host directly:

#[derive(Debug, StructOpt)] struct Config { host: String, port: u16, }

async fn main() -> io::Result<()> { let config = Config::from_args(); let stream = TcpStream::connect((config.host, config.port))?; // no more conversions! // ... }

I think should be an easy and small ergonomics improvement for networking. Thanks!

@rust-highfive

r? @hanna-kruppe

(rust_highfive has picked a reviewer for you, use r? to override)

sfackler

@@ -1000,6 +1000,14 @@ impl ToSocketAddrs for (&str, u16) {
}
}
#[unstable(feature = "string_u16_to_socket_addrs", issue = "73006")]

Choose a reason for hiding this comment

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

Trait implementations are always stable.

Choose a reason for hiding this comment

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

Oh I didn't realize! — what should this be marked with instead? #[stable()] with the next major?

Choose a reason for hiding this comment

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

Yep - 1.46.0 specifically.

Choose a reason for hiding this comment

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

Does these kind of change even needs a FCP? It looks minor but worthy enough to have a release note for it.

@sfackler sfackler added the T-libs-api

Relevant to the library API team, which will review and decide on the PR/issue.

label

Jun 5, 2020

@sfackler

@rfcbot

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot

🔔 This is now entering its final comment period, as per the review above. 🔔

@yoshuawuyts

@yoshuawuyts

@sfackler

@bors

📌 Commit 2764e54 has been approved by sfackler

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jun 5, 2020

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request

Jun 5, 2020

@Dylan-DPC

…u16, r=sfackler

impl ToSocketAddrs for (String, u16)

This adds a convenience impl of ToSocketAddrs for (String, u16). When authoring HTTP services it's common to take command line options for host and port and parse them into String and u16 respectively. Consider the following program:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
    // ...
}

Networking is a pretty common starting point for people new to Rust, and seeing &* in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that String can't be passed directly there. Instead with this patch we can omit the &* conversion and pass host directly:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
    // ...
}

I think should be an easy and small ergonomics improvement for networking. Thanks!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request

Jun 5, 2020

@Dylan-DPC

…u16, r=sfackler

impl ToSocketAddrs for (String, u16)

This adds a convenience impl of ToSocketAddrs for (String, u16). When authoring HTTP services it's common to take command line options for host and port and parse them into String and u16 respectively. Consider the following program:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
    // ...
}

Networking is a pretty common starting point for people new to Rust, and seeing &* in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that String can't be passed directly there. Instead with this patch we can omit the &* conversion and pass host directly:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
    // ...
}

I think should be an easy and small ergonomics improvement for networking. Thanks!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request

Jun 6, 2020

@Dylan-DPC

…u16, r=sfackler

impl ToSocketAddrs for (String, u16)

This adds a convenience impl of ToSocketAddrs for (String, u16). When authoring HTTP services it's common to take command line options for host and port and parse them into String and u16 respectively. Consider the following program:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
    // ...
}

Networking is a pretty common starting point for people new to Rust, and seeing &* in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that String can't be passed directly there. Instead with this patch we can omit the &* conversion and pass host directly:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
    // ...
}

I think should be an easy and small ergonomics improvement for networking. Thanks!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request

Jun 6, 2020

@Dylan-DPC

…u16, r=sfackler

impl ToSocketAddrs for (String, u16)

This adds a convenience impl of ToSocketAddrs for (String, u16). When authoring HTTP services it's common to take command line options for host and port and parse them into String and u16 respectively. Consider the following program:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
    // ...
}

Networking is a pretty common starting point for people new to Rust, and seeing &* in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that String can't be passed directly there. Instead with this patch we can omit the &* conversion and pass host directly:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
    // ...
}

I think should be an easy and small ergonomics improvement for networking. Thanks!

RalfJung added a commit to RalfJung/rust that referenced this pull request

Jun 6, 2020

@RalfJung

…u16, r=sfackler

impl ToSocketAddrs for (String, u16)

This adds a convenience impl of ToSocketAddrs for (String, u16). When authoring HTTP services it's common to take command line options for host and port and parse them into String and u16 respectively. Consider the following program:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
    // ...
}

Networking is a pretty common starting point for people new to Rust, and seeing &* in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that String can't be passed directly there. Instead with this patch we can omit the &* conversion and pass host directly:

#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
    // ...
}

I think should be an easy and small ergonomics improvement for networking. Thanks!

@bors bors added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Jun 8, 2020

@RalfJung

So it really was this one. I have no idea how that is possible...

@yoshuawuyts

I also have no idea how to fix or even proceed here. Does anyone have a clue what the right steps are to take? 😅

@withoutboats

My guess is this is changing the instantiation of something somewhere which is somehow revealing a bug in the windows implementation of something in rustc, logic which only works if a usize is 64 bits. Searching the error reveals that this assertion only appears in the align method of rustc's DroplessArena.

Someone needs to debug this failure more, probably the first step would be having a 32 bit Windows system to reproduce it on.

@rfcbot

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot removed the final-comment-period

In the final comment period and will be merged soon unless new substantive objections are raised.

label

Jun 15, 2020

Manishearth added a commit to Manishearth/rust that referenced this pull request

Jun 16, 2020

@Manishearth

Check for overflow in DroplessArena and align returned memory

Helps with rust-lang#73007, rust-lang#72624.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request

Jun 16, 2020

@Dylan-DPC

Check for overflow in DroplessArena and align returned memory

Helps with rust-lang#73007, rust-lang#72624.

@Manishearth

@bors r-

This failed CI since it was r+'d, unsure why homu thinks it's still in the queue. Feel free to r+ again once it's expected to pass.

@bors bors added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jun 18, 2020

@tmiasko

This should be fine to r+ again.

@Manishearth

@tmiasko i'm confused, what has changed that would fix the failure that was occurring earlier?

@mati865

@Manishearth

@bors

📌 Commit 2764e54 has been approved by sfackler

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Jun 22, 2020

@bors

@bors

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Aug 30, 2020

@alarixnia

according to various people on tech-pkg@, there are no problems with the Firefox build

Version 1.46.0 (2020-08-27)

Language

Compiler

Libraries

Stabilized APIs

Cargo

Added a number of new environment variables that are now available when compiling your crate.

Compatibility Notes

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Oct 30, 2020

@he32

Pkgsrc changes:

Upstream changes:

Version 1.46.0 (2020-08-27)

Language

Compiler

Libraries

Stabilized APIs

Cargo

Added a number of new environment variables that are now available when compiling your crate.

Compatibility Notes