Stabilize Ipv6Addr::is_unique_local and Ipv6Addr::is_unicast_link_local - #129238
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
|
r? dtolnay Since @dtolnay reviewed the last stabilisation PR in this domain. |
|
Failed to set assignee to
|
dtolnay
removed
the
T-libs
This one isn't entirely true. (I didn't try any of the other languages.) #![feature(ip)]
fn main() {
let ip = std::net::Ipv6Addr::from([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 169, 254, 0, 0]);
println!("{} {}", ip, ip.is_unicast_link_local());
}::ffff:169.254.0.0 falseGo: https://go.dev/play/p/b4E92eet5Yg package main
import (
"fmt"
"net/netip"
)
func main() {
ip := netip.AddrFrom16([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 169, 254, 0, 0})
fmt.Println(ip, ip.IsLinkLocalUnicast())
}::ffff:169.254.0.0 trueThe difference comes down to this "not assigned any special meaning", which is not the case in Go: rust/library/core/src/net/ip_addr.rs Lines 98 to 104 in 804be74 |
|
@rust-lang/libs-api: |
|
Team member @dtolnay 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
added
proposed-final-comment-period
|
Is These methods, along with similar unstabilized ones, have unfortunately had a pretty rough time getting finalized. I added unresolved questions to the tracking issue at #27709 to list some of these concerns. Most relevant here:
There is a tool designed to check our IP methods against those from other languages, could you try to run it to see where we stand? https://github.com/rust-lang/libs-team/tree/main/tools/ipcheck?
It predates rust 1 :) #22015 also typo unqiue/unique if you were searching for that exact string. |
|
I ran the ipcheck tool, some of my changes I applied can be seen in my PR: rust-lang/libs-team#454 Complete output of a runFound Rust implementation.
Operations with only a Rust implementation:
SummaryAs far as I can see the difference in output in However since Rust went a different way anyway in handling these cases, I would recon that we can assume Rust is compatible with other languages, however there exists a pitfall. |
I'm open to discussion here. If you want to see how it looks, I could include it in this. |
|
I agree with @tgross35 that adding this would make it more consistent with |
|
What I get from https://www.rfc-editor.org/rfc/rfc4291#section-2.5: there are only two scope types for unicast addresses: global unicast and link-local unicast. However, I think we could also add |
|
☔ The latest upstream changes (presumably #131672) made this pull request unmergeable. Please resolve the merge conflicts. |
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
@joshtriplett (or others who checked for FCP) could you ack that the |
rfcbot
added
the
finished-final-comment-period
|
@bors r+ |
|
@bors ping |
|
@bors ping |
|
😪 I'm awake I'm awake |
|
@bors r+ |
bors
added
S-waiting-on-bors
|
Yield to rollup @bors retry |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (2cf7908): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -1.3%, secondary 3.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -2.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 796.222s -> 796.948s (0.09%) |
11 participants
Make
Ipv6Addr::is_unique_localandIpv6Addr::is_unicast_link_localstable (+const).Newly stable API:
These stabilise a subset of the following tracking issue:
I have looked and could not find any issues with
is_unique_localandis_unicast_link_local. There is a well received comment calling for stabilisation of the latter function.Both functions are well defined and consistent with implementations in other languages:
cc implementor @little-dude
(I can't find the original PR for
is_unqiue_local)r? libs-api
@rustbot label +T-libs-api +needs-fcp