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
This comment has been minimized.
This comment has been minimized.
I hope this comment wasn't overlooked. Personally I don't think Ipv6UnicastScope is not a good idea since there are only two things that could be seen as a scope. I hope you are ok with going ahead @tgross35. |
umgefahren
force-pushed
the
stabilize-ipv6-unique-local
branch
from
33d251e
to
9cca296
Compare
rustbot
removed
has-merge-commits
There was a problem hiding this comment.
Thank you!
I think stabilizing these as-is is good. I am open to a separate Ipv6UnicastScope API in addition if someone can express use cases for it beyond just replacing if ip.is_unicast_link_local() with if let Some(Ipv6UnicastScope::LinkLocal) = ip.unicast_scope().
For me the API of the IP types is governed by exposing the behaviors that people want in real-world usage, that are well-specified, and using naming that aligns with term-of-art in a specification where possible. This is a different set of factors than the API for something like Result, where the possible behaviors people want is boundless, and the naming space is cramped, and the design needs to be driven by using a small number of naming schemes/fragments to describe orthogonal and composable behaviors.
|
@bors r+ |
|
@bors ping |
|
@bors ping |
|
😪 I'm awake I'm awake |
|
@bors r+ |
bors
added
S-waiting-on-bors
…ocal, r=dtolnay
Stabilize `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local`
Make `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local` stable (+const).
Newly stable API:
```rust
impl Ipv6Addr {
// Newly stable under `ipv6_is_unique_local`
const fn is_unique_local(&self) -> bool;
// Newly stable under `ipv6_is_unique_local`
const fn is_unicast_link_local(&self) -> bool;
}
```
These stabilise a subset of the following tracking issue:
- rust-lang#27709
I have looked and could not find any issues with `is_unique_local` and `is_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:
- [Go](https://cs.opensource.google/go/go/+/refs/tags/go1.23.0:src/net/netip/netip.go;l=518)
- [Python](https://github.com/python/cpython/blob/e9d1bf353c3ccafc0d9b61b1b3688051bc976604/Lib/ipaddress.py#L2319-L2321)
- [Ruby (unique local)](https://ruby-doc.org/stdlib-2.5.1/libdoc/ipaddr/rdoc/IPAddr.html#private-3F-source)
- [Ruby (unicast link local)](https://ruby-doc.org/stdlib-2.5.1/libdoc/ipaddr/rdoc/IPAddr.html#link_local-3F-source)
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
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#129238 (Stabilize `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local`) - rust-lang#130867 (distinguish overflow and unimplemented in Step::steps_between) - rust-lang#131505 (use `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)` as a `TMPDIR` fallback on Darwin) - rust-lang#132090 (Stop being so bail-y in candidate assembly) - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value ) - rust-lang#133215 (Fix missing submodule in `./x vendor`) - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns) - rust-lang#133301 (Add code example for `wrapping_neg` method for signed integers) - rust-lang#133313 (Use arc4random of libc for RTEMS target) r? `@ghost` `@rustbot` modify labels: rollup
…al, r=dtolnay
Stabilize `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local`
Make `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local` stable (+const).
Newly stable API:
```rust
impl Ipv6Addr {
// Newly stable under `ipv6_is_unique_local`
const fn is_unique_local(&self) -> bool;
// Newly stable under `ipv6_is_unique_local`
const fn is_unicast_link_local(&self) -> bool;
}
```
These stabilise a subset of the following tracking issue:
- rust-lang#27709
I have looked and could not find any issues with `is_unique_local` and `is_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:
- [Go](https://cs.opensource.google/go/go/+/refs/tags/go1.23.0:src/net/netip/netip.go;l=518)
- [Python](https://github.com/python/cpython/blob/e9d1bf353c3ccafc0d9b61b1b3688051bc976604/Lib/ipaddress.py#L2319-L2321)
- [Ruby (unique local)](https://ruby-doc.org/stdlib-2.5.1/libdoc/ipaddr/rdoc/IPAddr.html#private-3F-source)
- [Ruby (unicast link local)](https://ruby-doc.org/stdlib-2.5.1/libdoc/ipaddr/rdoc/IPAddr.html#link_local-3F-source)
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
|
⌛ Testing commit 9cca296 with merge 6b5b52663ee637f1ea29efdaa7348871c9241101... |
|
Yield to rollup @bors retry |
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#129238 (Stabilize `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local`) - rust-lang#130867 (distinguish overflow and unimplemented in Step::steps_between) - rust-lang#131505 (use `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)` as a `TMPDIR` fallback on Darwin) - rust-lang#132090 (Stop being so bail-y in candidate assembly) - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value ) - rust-lang#133215 (Fix missing submodule in `./x vendor`) - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns) - rust-lang#133301 (Add code example for `wrapping_neg` method for signed integers) - rust-lang#133313 (Use arc4random of libc for RTEMS target) r? `@ghost` `@rustbot` modify labels: rollup
|
☀️ Test successful - checks-actions |
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