Skip to content

/rust

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

Ipv4Addr and Ipv6Addr convenience constructors. #44395

Merged
merged 4 commits into from Sep 17, 2017

Conversation

Reviewers

@estebank estebank

@BurntSushi BurntSushi

Assignees

@BurntSushi BurntSushi

Labels
S-waiting-on-author
Projects
None yet
Milestone
No milestone
6 participants
@jcdyer @rust-highfive @alexcrichton @bors @BurntSushi @estebank
@jcdyer
Copy link
Contributor

jcdyer commented Sep 7, 2017 •
edited

Introduce convenience constructors for common types.

This introduces the following constructors:

  • Ipv6Addr::localhost()
  • Ipv6Addr::unspecified()
  • Ipv4Addr::localhost()
  • Ipv4Addr::unspecified()

The recently added From implementations were nice for avoiding the fallibility of conversions from strings like "127.0.0.1".parse().unwrap(), and "::1".parse().unwrap(), but while the Ipv4 version is roughly comparable in verbosity, the Ipv6 version lacks zero-segment elision, which makes it significantly more awkward: [0, 0, 0, 0, 0, 0, 0, 0].into(). While there isn't a clear way to introduce zero elision to type that can infallibly be converted into Ipv6 addresses, this PR resolves the problem for the two most commonly used addresses, which, incidentally, are the ones that suffer the most from the lack of zero-segment elision.

This change is dead simple, and introduces no backwards incompatibility.

See also, this topic on the inernals board

@rust-highfive rust-highfive assigned BurntSushi Sep 7, 2017

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Sep 7, 2017

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @BurntSushi (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.
@estebank

estebank requested changes Sep 7, 2017
View changes

src/libstd/net/ip.rs Outdated
assert_eq!(Ipv6Addr::unspecified(), Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0));
assert!(Ipv6Addr::unspecified().is_unspecified());
}

This comment has been minimized.

Sign in to view
Copy link
@estebank

estebank Sep 7, 2017

Contributor

Trim the trailing whitespace.
src/libstd/net/ip.rs Outdated
/// use std::net::Ipv4Addr;
///
/// let addr = Ipv4Addr::localhost();
/// assert_eq!(addr, Ipv4Addr::new(127, 0, 0, 1));

This comment has been minimized.

Sign in to view
Copy link
@estebank

estebank Sep 7, 2017

Contributor

missing line closing the code example:

/// ```
src/libstd/net/ip.rs Outdated
/// use std::net::Ipv4Addr;
///
/// let addr = Ipv4Addr::unspecified();
/// assert_eq!(addr, Ipv4Addr::new(0, 0, 0, 0));

This comment has been minimized.

Sign in to view
Copy link
@estebank

estebank Sep 7, 2017

Contributor

missing line closing the code example:

/// ```
src/libstd/net/ip.rs Outdated
/// use std::net::Ipv6Addr;
///
/// let addr = Ipv6Addr::localhost();
/// assert_eq!(addr, Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1));

This comment has been minimized.

Sign in to view
Copy link
@estebank

estebank Sep 7, 2017

Contributor

missing line closing the code example:

/// ```
src/libstd/net/ip.rs Outdated
/// use std::net::Ipv6Addr;
///
/// let addr = Ipv6Addr::unspecified();
/// assert_eq!(addr, Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0));

This comment has been minimized.

Sign in to view
Copy link
@estebank

estebank Sep 7, 2017

Contributor

missing line closing the code example:

/// ```

This comment has been minimized.

Sign in to view
Copy link
@jcdyer

jcdyer Sep 8, 2017

Author Contributor

All resolved. Thanks.

@alexcrichton alexcrichton added the S-waiting-on-review label Sep 7, 2017

@estebank

estebank reviewed Sep 8, 2017
View changes

src/libstd/net/ip.rs
///
/// # Examples
///
/// ```

This comment has been minimized.

Sign in to view
Copy link
@estebank

estebank Sep 8, 2017

Contributor

I believe you need to add # #![feature(ip)] to this and the other new examples in order for the doc tests to work. The first # hides the line from the docs (which you might not want to do to make it clear that it is needed for this usage).
@jcdyer
Add feature gate to doctests.
Loading status checks…
8b6122f

@jcdyer jcdyer force-pushed the jcdyer:ip-constructors branch from 3572665 to 8b6122f Sep 9, 2017

@BurntSushi

BurntSushi requested changes Sep 11, 2017
View changes

Copy link
Member

BurntSushi left a comment

This looks reasonable to me, but I think these need stability attributes, and should be unstable for now.

cc @rust-lang/libs

@BurntSushi BurntSushi added S-waiting-on-author and removed S-waiting-on-review labels Sep 11, 2017

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented Sep 14, 2017

Looks reasonable to me as well! @jcdyer mind adding the stability attributes and filing an issue to track the stability here?

@jcdyer jcdyer referenced this pull request Sep 15, 2017

Closed

Tracking issue for IP constructors #44582

@jcdyer

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

jcdyer commented Sep 15, 2017

@estebank @BurntSushi I added the unstable attributes, and created a tracking issue. I'm not entirely sure what needs to happen, but I did my best based on a quick search of other similar examples. Let me know if anything needs to be different.
@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented Sep 15, 2017

Looks great, thanks @jcdyer!

@bors: r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 15, 2017

📌 Commit 9d5b0e1 has been approved by alexcrichton
@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented Sep 16, 2017

@bors: rollup

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 16, 2017

@alexcrichton
Rollup merge of rust-lang#44395 - jcdyer:ip-constructors, r=alexcrichton 
Ipv4Addr and Ipv6Addr convenience constructors.

Introduce convenience constructors for common types.

This introduces the following constructors:

* Ipv6Addr::localhost()
* Ipv6Addr::unspecified()
* Ipv4Addr::localhost()
* Ipv4Addr::unspecified()

The recently added `From` implementations were nice for avoiding the fallibility of conversions from strings like `"127.0.0.1".parse().unwrap()`, and `"::1".parse().unwrap()`, but while the Ipv4 version is roughly comparable in verbosity, the Ipv6 version lacks zero-segment elision, which makes it significantly more awkward: `[0, 0, 0, 0, 0, 0, 0, 0].into()`.  While there isn't a clear way to introduce zero elision to type that can infallibly be converted into Ipv6 addresses, this PR resolves the problem for the two most commonly used addresses, which, incidentally, are the ones that suffer the most from the lack of zero-segment elision.

This change is dead simple, and introduces no backwards incompatibility.

See also, [this topic on the inernals board](https://internals.rust-lang.org/t/pre-rfc-convenience-ip-address-constructors/5878)
Loading status checks…
900707e

@alexcrichton alexcrichton referenced this pull request Sep 16, 2017

Closed

Rollup of 27 pull requests #44621

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 16, 2017

@frewsxcv
Rollup merge of rust-lang#44395 - jcdyer:ip-constructors, r=alexcrichton 
Ipv4Addr and Ipv6Addr convenience constructors.

Introduce convenience constructors for common types.

This introduces the following constructors:

* Ipv6Addr::localhost()
* Ipv6Addr::unspecified()
* Ipv4Addr::localhost()
* Ipv4Addr::unspecified()

The recently added `From` implementations were nice for avoiding the fallibility of conversions from strings like `"127.0.0.1".parse().unwrap()`, and `"::1".parse().unwrap()`, but while the Ipv4 version is roughly comparable in verbosity, the Ipv6 version lacks zero-segment elision, which makes it significantly more awkward: `[0, 0, 0, 0, 0, 0, 0, 0].into()`.  While there isn't a clear way to introduce zero elision to type that can infallibly be converted into Ipv6 addresses, this PR resolves the problem for the two most commonly used addresses, which, incidentally, are the ones that suffer the most from the lack of zero-segment elision.

This change is dead simple, and introduces no backwards incompatibility.

See also, [this topic on the inernals board](https://internals.rust-lang.org/t/pre-rfc-convenience-ip-address-constructors/5878)
9ac81ba

@frewsxcv frewsxcv referenced this pull request Sep 16, 2017

Closed

Rollup of 25 pull requests #44623

bors added a commit that referenced this pull request Sep 16, 2017

@bors
Auto merge of #44621 - alexcrichton:rollup, r=alexcrichton 
Rollup of 27 pull requests

- Successful merges: #43964, #43989, #44273, #44356, #44395, #44397, #44515, #44531, #44542, #44549, #44560, #44562, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44603, #44604, #44605, #44606, #44609, #44610, #44616
- Failed merges: #44537
Loading status checks…
1f3526e

bors added a commit that referenced this pull request Sep 16, 2017

@bors
Auto merge of #44621 - alexcrichton:rollup, r=alexcrichton 
Rollup of 27 pull requests

- Successful merges: #43964, #43989, #44273, #44356, #44395, #44397, #44515, #44531, #44542, #44549, #44560, #44562, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44603, #44604, #44605, #44606, #44609, #44610, #44616
- Failed merges: #44537
Loading status checks…
fd30475

bors added a commit that referenced this pull request Sep 16, 2017

@bors
Auto merge of #44621 - alexcrichton:rollup, r=alexcrichton 
Rollup of 27 pull requests

- Successful merges: #43964, #43989, #44273, #44356, #44395, #44397, #44515, #44531, #44542, #44549, #44560, #44562, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44603, #44604, #44605, #44606, #44609, #44610, #44616
- Failed merges: #44537
Loading status checks…
5cd8902

bors added a commit that referenced this pull request Sep 16, 2017

@bors
Auto merge of #44623 - frewsxcv:rollup, r=frewsxcv 
Rollup of 25 pull requests

- Successful merges: #44273, #44356, #44395, #44397, #44515, #44531, #44537, #44542, #44549, #44560, #44562, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44603, #44604, #44606, #44609, #44616, #44622
- Failed merges:
Loading status checks…
d4dfec1

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 16, 2017

@alexcrichton
Rollup merge of rust-lang#44395 - jcdyer:ip-constructors, r=alexcrichton 
Ipv4Addr and Ipv6Addr convenience constructors.

Introduce convenience constructors for common types.

This introduces the following constructors:

* Ipv6Addr::localhost()
* Ipv6Addr::unspecified()
* Ipv4Addr::localhost()
* Ipv4Addr::unspecified()

The recently added `From` implementations were nice for avoiding the fallibility of conversions from strings like `"127.0.0.1".parse().unwrap()`, and `"::1".parse().unwrap()`, but while the Ipv4 version is roughly comparable in verbosity, the Ipv6 version lacks zero-segment elision, which makes it significantly more awkward: `[0, 0, 0, 0, 0, 0, 0, 0].into()`.  While there isn't a clear way to introduce zero elision to type that can infallibly be converted into Ipv6 addresses, this PR resolves the problem for the two most commonly used addresses, which, incidentally, are the ones that suffer the most from the lack of zero-segment elision.

This change is dead simple, and introduces no backwards incompatibility.

See also, [this topic on the inernals board](https://internals.rust-lang.org/t/pre-rfc-convenience-ip-address-constructors/5878)
Loading status checks…
1b9443a

@alexcrichton alexcrichton referenced this pull request Sep 16, 2017

Merged

Rollup of 19 pull requests #44634

bors added a commit that referenced this pull request Sep 16, 2017

@bors
Auto merge of #44634 - alexcrichton:rollup, r=alexcrichton 
Rollup of 19 pull requests

- Successful merges: #44273, #44356, #44395, #44531, #44537, #44542, #44560, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44606, #44609, #44616, #44631
- Failed merges:
Loading status checks…
05ad88b

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 16, 2017

@alexcrichton
Rollup merge of rust-lang#44395 - jcdyer:ip-constructors, r=alexcrichton 
Ipv4Addr and Ipv6Addr convenience constructors.

Introduce convenience constructors for common types.

This introduces the following constructors:

* Ipv6Addr::localhost()
* Ipv6Addr::unspecified()
* Ipv4Addr::localhost()
* Ipv4Addr::unspecified()

The recently added `From` implementations were nice for avoiding the fallibility of conversions from strings like `"127.0.0.1".parse().unwrap()`, and `"::1".parse().unwrap()`, but while the Ipv4 version is roughly comparable in verbosity, the Ipv6 version lacks zero-segment elision, which makes it significantly more awkward: `[0, 0, 0, 0, 0, 0, 0, 0].into()`.  While there isn't a clear way to introduce zero elision to type that can infallibly be converted into Ipv6 addresses, this PR resolves the problem for the two most commonly used addresses, which, incidentally, are the ones that suffer the most from the lack of zero-segment elision.

This change is dead simple, and introduces no backwards incompatibility.

See also, [this topic on the inernals board](https://internals.rust-lang.org/t/pre-rfc-convenience-ip-address-constructors/5878)
878047c

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 17, 2017

@alexcrichton
Rollup merge of rust-lang#44395 - jcdyer:ip-constructors, r=alexcrichton 
Ipv4Addr and Ipv6Addr convenience constructors.

Introduce convenience constructors for common types.

This introduces the following constructors:

* Ipv6Addr::localhost()
* Ipv6Addr::unspecified()
* Ipv4Addr::localhost()
* Ipv4Addr::unspecified()

The recently added `From` implementations were nice for avoiding the fallibility of conversions from strings like `"127.0.0.1".parse().unwrap()`, and `"::1".parse().unwrap()`, but while the Ipv4 version is roughly comparable in verbosity, the Ipv6 version lacks zero-segment elision, which makes it significantly more awkward: `[0, 0, 0, 0, 0, 0, 0, 0].into()`.  While there isn't a clear way to introduce zero elision to type that can infallibly be converted into Ipv6 addresses, this PR resolves the problem for the two most commonly used addresses, which, incidentally, are the ones that suffer the most from the lack of zero-segment elision.

This change is dead simple, and introduces no backwards incompatibility.

See also, [this topic on the inernals board](https://internals.rust-lang.org/t/pre-rfc-convenience-ip-address-constructors/5878)
256b887

bors added a commit that referenced this pull request Sep 17, 2017

@bors
Auto merge of #44634 - alexcrichton:rollup, r=alexcrichton 
Rollup of 19 pull requests

- Successful merges: #44273, #44356, #44395, #44531, #44537, #44542, #44560, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44606, #44609, #44616, #44631
- Failed merges:
Loading status checks…
71e3767

@bors bors merged commit 9d5b0e1 into rust-lang:master Sep 17, 2017
1 check failed

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.