Ipv6Addr <-> u128 #39324

clarfonthey · 2017-01-26T21:12:22Z

Because we have u128 now, it makes sense to add a conversion between Ipv6Addr and u128 in addition to the existing one between Ipv4Addr and u32.

This shouldn't violate the existing feature gate on u128 because you can't use the type without the feature gate, but if i have to add something, I can.

rust-highfive · 2017-01-26T21:12:35Z

r? @alexcrichton

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

alexcrichton · 2017-01-26T22:36:52Z

Sounds reasonable to me!

alexcrichton · 2017-01-26T22:37:42Z

I think to fix the errors on Travis we'll want to tag these with #[cfg(not(stage0))] perhaps?

@rfcbot fcp merge

rfcbot · 2017-01-26T22:37:43Z

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

No concerns currently listed.

Once these reviewers reach consensus, 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.

brson · 2017-01-27T00:31:20Z

Aren't 128-bit ints cfg'd in some target-specific way? These are defined unconditionally.

clarfonthey · 2017-01-27T01:30:49Z

@brson are there any tier 1 platforms where this happens? Because if that's the case, bors will catch that

EDIT: I tried rg 'cfg.*128' and couldn't find anything that would indicate that this is compiled per-platform, but if that's wrong please let me know.

alexcrichton · 2017-01-27T04:16:04Z

@brson that's a good point, actually. I'm not sure what we should consider the status of u128/i128 in terms of portability. I believe that all current platforms Rust targets supports the types but that doesn't necessarily mean that all future platforms will.

clarfonthey · 2017-01-27T04:29:58Z

@alexcrichton I personally think that a catch-all cfg(has_i128) or something along those lines would be a better fix than the current cfg(not(stage0))

sfackler · 2017-01-27T04:36:06Z

I'm not sure I understand the portability issues with u/i128. How are those different than e.g. u/i64?

clarfonthey · 2017-01-27T05:13:12Z

@brson I'd check @nagisa's comment on the tracking issue for 128-bit issue regarding that. @sfackler is right

ollie27 · 2017-01-27T11:35:00Z

src/libstd/net/ip.rs

@@ -1153,6 +1153,28 @@ impl FromInner<c::in6_addr> for Ipv6Addr {
    }
 }

+#[cfg(not(stage0))]
+#[stable(feature = "ipv6_u128", since = "1.17.0")]


These are still unstable so should be #[unstable(feature = "i128", issue="35118")].

Impls can't be unstable so I made it stable.

You're correct but rustdoc will display the version number on impls that are marked stable and 1.17.0 is clearly wrong for these.

ollie27 · 2017-01-27T11:38:25Z

src/libstd/net/ip.rs

+impl From<Ipv6Addr> for u128 {
+    fn from(ip: Ipv6Addr) -> u128 {
+        let ip = ip.segments();
+        ((ip[0] as u128) << 104) + ((ip[1] as u128) << 96) + ((ip[2] as u128) << 80) +


104 -> 112.

Personally I would use u128::from() for these rather than as u128.

I mostly copy-pasted the one from IPv4; is the difference worthwhile?

Not really, either way works fine.

ollie27 · 2017-01-27T11:43:30Z

src/libstd/net/ip.rs

+impl From<u128> for Ipv6Addr {
+    fn from(ip: u128) -> Ipv6Addr {
+        Ipv6Addr::new(
+            (ip >> 104) as u16, (ip >> 96) as u16, (ip >> 80) as u16,


104 ->112.

ollie27 · 2017-01-27T11:45:34Z

src/libstd/net/ip.rs

@@ -1515,6 +1537,18 @@ mod tests {
    }

    #[test]
+    fn test_ipv6_to_int() {
+        let a = Ipv4Addr::new(0xffff, 0xffff, 0xffff, 0xffff, 127, 0, 0, 1);


Ipv4Addr -> Ipv6Addr for here and the two below.

ollie27 · 2017-01-27T11:46:26Z

src/libstd/net/ip.rs

@@ -1515,6 +1537,18 @@ mod tests {
    }

    #[test]
+    fn test_ipv6_to_int() {
+        let a = Ipv4Addr::new(0xffff, 0xffff, 0xffff, 0xffff, 127, 0, 0, 1);
+        assert_eq!(u128::from(a), 0xffffffffffffffff0080000000000001u128);


0xffffffffffffffff0080000000000001u128 -> 0xffffffffffffffff007f000000000001u128 for here and below.

Although it might be better to use examples like the ipv6_from_segments test where getting a shift wrong (like using 104 instead of 112) will definitely cause these tests to fail.

I agree with you there. I'll update the IPv4 test too.

clarfonthey · 2017-01-27T18:14:58Z

The latest travis errors seem to be an error in parsing the integer literals.

nagisa · 2017-01-27T19:42:48Z

The latest travis errors seem to be an error in parsing the integer literals.

Most likely a staging issue of some sort; these literals work just fine on a proper nightly compiler.

brson · 2017-01-27T23:59:27Z

Further comments here.

alexcrichton · 2017-01-31T01:16:20Z

ping @brson @BurntSushi (checkboxes)

alexcrichton · 2017-01-31T01:16:29Z

Also @clarcharr it looks like there's still failing tests on travis?

nagisa · 2017-01-31T01:20:10Z

The test should get fixed automatically when we release the new beta, so I do not see any point in trying to fix it now only to undo it a few days later.

clarfonthey · 2017-01-31T01:23:12Z

Yeah, I'm not particularly sure how to fix it myself. It seems to be accepting u128 as a type but not the integer literal, which indicates that it's probably not a staging issue.

alexcrichton · 2017-02-04T22:19:49Z

This was discussed at the libs triage a little while ago, and the conclusion was that we'll merge this for now but consider the portability of i128/u128 in the tracking issue (and before stabilizing).

@clarcharr want to rebase and remove the stage0 annotations? With a new stage0 I think they may not be necessary any more.

clarfonthey · 2017-02-06T01:28:24Z

@alexcrichton I removed the stage0 cfgs and Travis is passing now.

alexcrichton · 2017-02-06T01:50:55Z

Thanks!

ping @brson (checkbox)

alexcrichton · 2017-02-07T19:09:55Z

@bors: r+

bors · 2017-02-07T19:09:55Z

📌 Commit 8b2e334 has been approved by alexcrichton

bors · 2017-02-07T20:59:26Z

⌛ Testing commit 8b2e334 with merge 1807904...

bors · 2017-02-07T22:12:06Z

💔 Test failed - status-travis

alexcrichton · 2017-02-07T22:13:21Z

@bors: retry * sccache wedged #39003

…

On Tue, Feb 7, 2017 at 2:12 PM, bors ***@***.***> wrote: 💔 Test failed - status-travis <https://travis-ci.org/rust-lang/rust/builds/199377916> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#39324 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95FiodMpcLi8XL5TIQB4olH2iXSpHks5raOw4gaJpZM4LvJRu> .

bors · 2017-02-07T22:13:29Z

⌛ Testing commit 8b2e334 with merge afe9881...

Ipv6Addr <-> u128 Because we have `u128` now, it makes sense to add a conversion between `Ipv6Addr` and `u128` in addition to the existing one between `Ipv4Addr` and `u32`. This shouldn't violate the existing feature gate on `u128` because you can't use the type without the feature gate, but if i have to add something, I can.

bors · 2017-02-07T23:56:05Z

💔 Test failed - status-travis

alexcrichton · 2017-02-08T00:03:38Z

@bors: retry * osx linker segfault #38878

…

On Tue, Feb 7, 2017 at 3:56 PM, bors ***@***.***> wrote: 💔 Test failed - status-travis <https://travis-ci.org/rust-lang/rust/builds/199403284> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#39324 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95Nqayxvk1y82HmWmN9IIxQY8Q428ks5raQSWgaJpZM4LvJRu> .

bors · 2017-02-08T00:03:45Z

⌛ Testing commit 8b2e334 with merge c14f87e...

Ipv6Addr <-> u128 Because we have `u128` now, it makes sense to add a conversion between `Ipv6Addr` and `u128` in addition to the existing one between `Ipv4Addr` and `u32`. This shouldn't violate the existing feature gate on `u128` because you can't use the type without the feature gate, but if i have to add something, I can.

bors · 2017-02-08T02:49:21Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing c14f87e to master...

rust-highfive assigned alexcrichton Jan 26, 2017

alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 26, 2017

clarfonthey force-pushed the ipv6_u128 branch 3 times, most recently from 1e954f3 to c978631 Compare January 27, 2017 03:55

clarfonthey mentioned this pull request Jan 27, 2017

Tracking issue for 128-bit integer support (RFC 1504) #35118

Closed

5 tasks

clarfonthey force-pushed the ipv6_u128 branch from c978631 to 5a94c5b Compare January 27, 2017 05:18

ollie27 suggested changes Jan 27, 2017

View reviewed changes

clarfonthey mentioned this pull request Jan 29, 2017

A few ergonomic From impls for SocketAddr/IpAddr #39372

Merged

clarfonthey force-pushed the ipv6_u128 branch from 9fd1fba to 4659884 Compare February 4, 2017 18:24

clarfonthey force-pushed the ipv6_u128 branch 2 times, most recently from c186a4f to 3ef7428 Compare February 5, 2017 23:28

Ipv6Addr <-> u128

8b2e334

clarfonthey force-pushed the ipv6_u128 branch from 3ef7428 to 8b2e334 Compare February 5, 2017 23:55

bors merged commit 8b2e334 into rust-lang:master Feb 8, 2017

clarfonthey deleted the ipv6_u128 branch February 12, 2017 22:02

Ipv6Addr <-> u128 #39324

Ipv6Addr <-> u128 #39324

Conversation

clarfonthey commented Jan 26, 2017 • edited Loading

rust-highfive commented Jan 26, 2017

alexcrichton commented Jan 26, 2017

alexcrichton commented Jan 26, 2017

rfcbot commented Jan 26, 2017 • edited by BurntSushi Loading

brson commented Jan 27, 2017

clarfonthey commented Jan 27, 2017 • edited Loading

alexcrichton commented Jan 27, 2017

clarfonthey commented Jan 27, 2017 • edited Loading

sfackler commented Jan 27, 2017

clarfonthey commented Jan 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarfonthey commented Jan 27, 2017

nagisa commented Jan 27, 2017

brson commented Jan 27, 2017

alexcrichton commented Jan 31, 2017

alexcrichton commented Jan 31, 2017

nagisa commented Jan 31, 2017

clarfonthey commented Jan 31, 2017

alexcrichton commented Feb 4, 2017

clarfonthey commented Feb 6, 2017

alexcrichton commented Feb 6, 2017

alexcrichton commented Feb 7, 2017

bors commented Feb 7, 2017

bors commented Feb 7, 2017

bors commented Feb 7, 2017

alexcrichton commented Feb 7, 2017 via email

bors commented Feb 7, 2017

bors commented Feb 7, 2017

alexcrichton commented Feb 8, 2017 via email

bors commented Feb 8, 2017

bors commented Feb 8, 2017

clarfonthey commented Jan 26, 2017 •

edited

Loading

rfcbot commented Jan 26, 2017 •

edited by BurntSushi

Loading

clarfonthey commented Jan 27, 2017 •

edited

Loading

clarfonthey commented Jan 27, 2017 •

edited

Loading