Skip to content

Conversation

CDirkx
Copy link
Contributor

@CDirkx CDirkx commented May 29, 2021

Removes the unstable method Ipv6Addr::is_unicast_site_local, see also #85604 where I have tried to summarize related discussion so far.

Unicast site-local addresses (fec0::/10) were deprecated in IETF RFC #3879, see also RFC #4291 Section 2.5.7. Any new implementation must no longer support the special behaviour of site-local addresses. This is mentioned in the docs of is_unicast_site_local and already implemented in is_unicast_global, which considers addresses in fec0::/10 to have global scope, thus overlapping with is_unicast_site_local.

Given that RFC #3879 was published in 2004, long before Rust existed, and it is specified that any new implementation must no longer support the special behaviour of site-local addresses, I don't see how a user would ever have a need for is_unicast_site_local. It is also confusing that currently both is_unicast_site_local and is_unicast_global can be true for an address, but an address can actually only have a single scope. The deprecating RFC mentions that Site-Local scope was confusing to work with and that the classification of an address as either Link-Local or Global better matches the mental model of users.

There has been earlier discussion of removing is_unicast_site_local (#60145 (comment)) which decided against it, but that had the incorrect assumption that the method was already stable; it is not. (This confusion arose from the placement of the unstable attribute on the entire module, instead of on individual methods, resolved in #85672)

r? @joshtriplett as reviewer of all the related PRs

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2021
@joshtriplett joshtriplett added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 31, 2021
@joshtriplett
Copy link
Member

Seems reasonable and well-justified.

@bors r+

@bors
Copy link
Collaborator

bors commented May 31, 2021

📌 Commit 8417f483205b2d49029ca9822b04bf44514f6a05 has been approved by joshtriplett

@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 May 31, 2021
@bors
Copy link
Collaborator

bors commented May 31, 2021

☔ The latest upstream changes (presumably #85819) made this pull request unmergeable. Please resolve the merge conflicts.

@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 31, 2021
@CDirkx CDirkx force-pushed the is_unicast_site_local branch from 8417f48 to 7a07da2 Compare May 31, 2021 08:09
@CDirkx
Copy link
Contributor Author

CDirkx commented May 31, 2021

Resolved the merge conflict

@bors
Copy link
Collaborator

bors commented Jun 9, 2021

☔ The latest upstream changes (presumably #86160) made this pull request unmergeable. Please resolve the merge conflicts.

@CDirkx CDirkx force-pushed the is_unicast_site_local branch from 7a07da2 to ed0557e Compare June 9, 2021 07:42
@JohnTitor
Copy link
Member

@bors r=joshtriplett

@bors
Copy link
Collaborator

bors commented Jun 16, 2021

📌 Commit ed0557e has been approved by joshtriplett

@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 16, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 16, 2021
…htriplett

Remove `Ipv6Addr::is_unicast_site_local`

Removes the unstable method `Ipv6Addr::is_unicast_site_local`, see also rust-lang#85604 where I have tried to summarize related discussion so far.

Unicast site-local addresses (`fec0::/10`) were deprecated in [IETF RFC rust-lang#3879](https://datatracker.ietf.org/doc/html/rfc3879), see also [RFC rust-lang#4291 Section 2.5.7](https://datatracker.ietf.org/doc/html/rfc4291#section-2.5.7). Any new implementation must no longer support the special behaviour of site-local addresses. This is mentioned in the docs of `is_unicast_site_local` and already implemented in `is_unicast_global`, which considers addresses in `fec0::/10` to have global scope, thus overlapping with `is_unicast_site_local`.

Given that RFC rust-lang#3879 was published in 2004, long before Rust existed, and it is specified that any new implementation must no longer support the special behaviour of site-local addresses, I don't see how a user would ever have a need for `is_unicast_site_local`. It is also confusing that currently both `is_unicast_site_local` and `is_unicast_global` can be `true` for an address, but an address can actually only have a single scope. The deprecating RFC mentions that Site-Local scope was confusing to work with and that the classification of an address as either Link-Local or Global better matches the mental model of users.

There has been earlier discussion of removing `is_unicast_site_local` (rust-lang#60145 (comment)) which decided against it, but that had the incorrect assumption that the method was already stable; it is not. (This confusion arose from the placement of the unstable attribute on the entire module, instead of on individual methods, resolved in rust-lang#85672)

r? `@joshtriplett` as reviewer of all the related PRs
@bors
Copy link
Collaborator

bors commented Jun 16, 2021

⌛ Testing commit ed0557e with merge d192c80...

@bors
Copy link
Collaborator

bors commented Jun 16, 2021

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing d192c80 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 16, 2021
@bors bors merged commit d192c80 into rust-lang:master Jun 16, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 16, 2021
jstasiak added a commit to jstasiak/rust-libs-team that referenced this pull request Dec 7, 2023
The is_unicast_site_local() method has been gone since Rust 1.55[1] and
is_ietf_protocol_assignment() since 1.56[2].

I had to run this tool to test something, saw it wouldn't build with
modern Rust.

[1] rust-lang/rust#85820
[2] rust-lang/rust#86439
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
…riplett

Remove `Ipv6Addr::is_unicast_site_local`

Removes the unstable method `Ipv6Addr::is_unicast_site_local`, see also rust-lang#85604 where I have tried to summarize related discussion so far.

Unicast site-local addresses (`fec0::/10`) were deprecated in [IETF RFC rust-lang#3879](https://datatracker.ietf.org/doc/html/rfc3879), see also [RFC rust-lang#4291 Section 2.5.7](https://datatracker.ietf.org/doc/html/rfc4291#section-2.5.7). Any new implementation must no longer support the special behaviour of site-local addresses. This is mentioned in the docs of `is_unicast_site_local` and already implemented in `is_unicast_global`, which considers addresses in `fec0::/10` to have global scope, thus overlapping with `is_unicast_site_local`.

Given that RFC rust-lang#3879 was published in 2004, long before Rust existed, and it is specified that any new implementation must no longer support the special behaviour of site-local addresses, I don't see how a user would ever have a need for `is_unicast_site_local`. It is also confusing that currently both `is_unicast_site_local` and `is_unicast_global` can be `true` for an address, but an address can actually only have a single scope. The deprecating RFC mentions that Site-Local scope was confusing to work with and that the classification of an address as either Link-Local or Global better matches the mental model of users.

There has been earlier discussion of removing `is_unicast_site_local` (rust-lang#60145 (comment)) which decided against it, but that had the incorrect assumption that the method was already stable; it is not. (This confusion arose from the placement of the unstable attribute on the entire module, instead of on individual methods, resolved in rust-lang#85672)

r? `@joshtriplett` as reviewer of all the related PRs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants