Move IpAddr, SocketAddr and V4+V6 related types to `core`#104265

faern · 2022-11-10T23:32:46Z

Implements RFC rust-lang/rfcs#2832. The RFC has completed FCP with disposition merge, but is not yet merged.

Moves IP types to core as specified in the RFC.

The full list of moved types is: IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6, Ipv6MulticastScope and AddrParseError.

Doing this move was one of the main driving arguments behind #78802.

rustbot · 2022-11-10T23:32:53Z

r? @thomcc

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

rustbot · 2022-11-10T23:32:55Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

faern · 2022-11-10T23:34:02Z

@rustbot label +T-libs-api -T-libs

faern · 2022-11-11T08:36:43Z

Should moves like this mark the new types in core::net an unstable? I found no way of doing that. Maybe I made a mistake.

Also, what to do with all the existing stability attributes, such as:

#[stable(feature = "ip_shared", since = "1.12.0")]
pub const fn is_loopback(&self) -> bool {

Do we leave them as is in moves like this? I have not yet found another PR doing a similar move.

slanterns · 2022-11-11T15:18:23Z

Should moves like this mark the new types in core::net an unstable? I found no way of doing that. Maybe I made a mistake.

I recall a similar attempt which moves Error into libcore. Maybe you can refer to #95956 and #99917.

thomcc · 2022-11-12T02:06:00Z

I think this is blocked on the RFC at the moment.

bors · 2022-11-15T09:38:34Z

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

pinkforest · 2023-01-04T13:32:48Z

Has anyone done testing about code sizes in microcontroller / wasm32-u-u context / between code size sensitive targets ? I think tests should be essential to "bench" / measure these kind of things if core is to be added with this stuff that realistically should be standard library stuff - this would alleviate concerns that binary sizes are kept in check.

Use-cases are - measured in x86_64 host:
1/ std wasm32-unknown-unknown ~30 kB std
2/ std wasm32-wasi ~70 kB std
3/ no_std + detached standard alloc (dlmalloc crate) ~4 kB
4/ no_std + lol_alloc 0.3 - 2kB allocator depending on type (wee_alloc leaks)
5/ no_std + heapless (crate)
6/ no_std + no heap at all (e.g. no String/Vec)

e.g. w/ cdylib context

[profile.release]
codegen-units = 1
strip = "symbols"
debug = false
lto = true
opt-level = "z"
panic = 'abort'

Another no_std target that should be tested could be thumbv7em-none-eabi for code sizes.

newAM · 2023-01-04T20:16:52Z

I have internet connected Cortex-M micros and can help with benchmarking, but I am confused about what is being measured. Embedded no_std binaries are entirely statically linked, and unused symbols are stripped. Is there something to measure or an assumption to check?

pinkforest · 2023-01-04T20:24:04Z

what is being measured.

Binary sizes - There was some other related effort out there somewhere else and might be worthwhile to check if anything was done re: in general and if this could be incorporated somehow there.

Embedded no_std binaries are entirely statically linked, and unused symbols are stripped.

Yes that is true and correct. However it could be helpful to come up with a test that stays around and ensures this is correct in case of core::net e.g. for wasm32-u-u that the symbol was stripped for completeness.

Question may / probably is how to test it if this is done and what are the downsides of such test if any ?

newAM · 2023-01-05T06:29:56Z

Binary sizes - There was some other related effort out there somewhere else and might be worthwhile to check if anything was done re: in general and if this could be incorporated somehow there.

I checked binary sizes of the cortex-m-quickstart built for the thumbv7em-none-eabi target with and without this patch (with slight modifications to make the patch work with the 1.65.0 release), section sizes and symbol sizes are identical for both binaries. Rough work is here: core-net-benchmark.

Yes that is true and correct. However it could be helpful to come up with a test that stays around and ensures this is correct in case of core::net e.g. for wasm32-u-u that the symbol was stripped for completeness.

Is this not covered by existing dead code tests, such as src/test/codegen/link-dead-code.rs?

faern · 2023-01-07T16:31:13Z

Rebased and updated. Since the RFC seems to be close to being merged.

Changed the std re-exports into type aliases so they can remain stable while the core versions are unstable (under the new feature ip_in_core). However, for Ipv6MulticastScope I kept it a pub use. Otherwise I ran into this error. And I don't think it should matter, because it's still an unstable type and I suppose it will be stabilized in core and std at the same time anyway. So no need to be able to have them separate?

Not going to touch anything regarding testing/benchmarking the size of core in this PR unless someone from the libs team tell me to. That concern seems unrelated to this PR and seems to be a concern regarding linker optimizations in general, not anything specific to IP types at all.

The CI failure is something I can reproduce locally but have not yet figured out how to solve 🤔

bors · 2023-01-22T17:13:11Z

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

faern · 2023-01-25T22:33:57Z

I fixed the merge conflict and the CI error that existed. The stderr of a compiler error changed slightly, so I just needed to adapt the test's expected output.

faern · 2023-01-25T23:04:37Z

To fix this error it looks like we need to update the book. But that's a separate git module. And it's also temporary, while the types are unstable in core... Will the book changes be merged before this PR is approved? I submitted the required change in rust-lang/book#3519

bors · 2023-02-26T14:51:07Z

@faern: 🔑 Insufficient privileges: Not in reviewers

albertlarsan68 · 2023-02-26T14:53:51Z

@bors r=joshtriplett
There you go!

bors · 2023-02-26T14:53:53Z

📌 Commit 1291216 has been approved by joshtriplett

It is now in the queue for this repository.

…riplett Move IpAddr, SocketAddr and V4+V6 related types to `core` Implements RFC rust-lang/rfcs#2832. The RFC has completed FCP with disposition merge, but is not yet merged. Moves IP types to `core` as specified in the RFC. The full list of moved types is: `IpAddr`, `Ipv4Addr`, `Ipv6Addr`, `SocketAddr`, `SocketAddrV4`, `SocketAddrV6`, `Ipv6MulticastScope` and `AddrParseError`. Doing this move was one of the main driving arguments behind rust-lang#78802.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#104265 (Move IpAddr, SocketAddr and V4+V6 related types to `core`) - rust-lang#107110 ([stdio][windows] Use MBTWC and WCTMB) - rust-lang#108308 (Allow building serde and serde_derive in parallel) - rust-lang#108363 (Move the unused extern crate check back to the resolver.) - rust-lang#108519 (Bages for easy access links to Rust community) - rust-lang#108522 (Commit some new solver tests) - rust-lang#108523 (Avoid `&str` to `String` conversions) - rust-lang#108533 (diagnostics: avoid querying `associated_item` in the resolver) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

RalfJung · 2023-02-28T08:53:49Z

library/core/tests/lib.rs

 #![feature(iterator_try_reduce)]
+#![feature(const_ip)]
+#![feature(const_ipv4)]
+#![feature(const_ipv6)]


I am very surprised that this does not need the ip_in_core feature? miri-test-libstd is failing due to that feature missing somewhere.

I guess because core itself does not use anything from core::net (which is the unstable module). Submodules of core::net uses parts of itself, but I guess if you are inside an unstable module you can freely use the stuff in it?

This is a different crate though, it is the coretest crate, not the core crate.

Ah, sorry. I misread the path! Yeah, that's very strange. But I see you have a PR up for fixing it 👍

Yeah it fixes miri-test-libstd. But still I am worried that the fact that it builds here without the feature flag might be indicative of a more fundamental problem -- as mentioned elsewhere, historically reexport stability did not work properly.

library/core/tests/net/parser.rs

RalfJung · 2023-02-28T09:16:26Z

library/std/src/net/ip_addr.rs

-pub struct Ipv4Addr {
-    octets: [u8; 4],
-}
+pub use core::net::{Ipv4Addr, Ipv6Addr};


FWIW, I was under the impression that pub use with different stability doesn't actually work -- that's why intrinsics::transmute has to be stable, so that it can be pub used in mem. IOW, all reexports of the same type/function must have the same stability. Has that been fixed?

See #18517, which is closed as a duplicate of an issue that is still open.

Quick testing indicates that the feature gate works as expected (except for the strange behavior in coretest).

Cc @petrochenkov your statement here sounds like reexport stability doesn't work properly, has that changed since last year?

add missing feature in core/tests rust-lang#104265 introduced the `ip_in_core` feature. For some reason core tests seem to still build without that feature -- no idea how that is possible. Might be related to rust-lang#15702? I was under the impression that `pub use` with different stability doesn't actually work. That's why `intrinsics::transmute` is stable, for example. Either way, core tests fail to build in miri-test-libstd, and adding the feature fixes that. r? `@thomcc`

klensy · 2023-02-28T10:36:16Z

This reduced std crate size by 200kb (by reducing linked .rustc section) under x86_64-pc-windows-msvc, but i don't understand why.

add missing feature in core/tests rust-lang#104265 introduced the `ip_in_core` feature. For some reason core tests seem to still build without that feature -- no idea how that is possible. Might be related to rust-lang#15702? I was under the impression that `pub use` with different stability doesn't actually work. That's why `intrinsics::transmute` is stable, for example. Either way, core tests fail to build in miri-test-libstd, and adding the feature fixes that. r? ``@thomcc``

add missing feature in core/tests rust-lang#104265 introduced the `ip_in_core` feature. For some reason core tests seem to still build without that feature -- no idea how that is possible. Might be related to rust-lang#15702? I was under the impression that `pub use` with different stability doesn't actually work. That's why `intrinsics::transmute` is stable, for example. Either way, core tests fail to build in miri-test-libstd, and adding the feature fixes that. r? ```@thomcc```

add missing feature in core/tests rust-lang/rust#104265 introduced the `ip_in_core` feature. For some reason core tests seem to still build without that feature -- no idea how that is possible. Might be related to rust-lang/rust#15702? I was under the impression that `pub use` with different stability doesn't actually work. That's why `intrinsics::transmute` is stable, for example. Either way, core tests fail to build in miri-test-libstd, and adding the feature fixes that. r? ```@thomcc```

rustbot assigned thomcc Nov 10, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 10, 2022

rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 10, 2022

faern force-pushed the move-ipaddr-to-core branch from bffa4ec to ea37750 Compare November 11, 2022 08:41

faern mentioned this pull request Nov 12, 2022

RFC: Move std::net::IpAddr types into core::net. rust-lang/rfcs#2832

Merged

chrysn mentioned this pull request Dec 15, 2022

Possibility of implementing this as a facade dunmatt/no-std-net#5

Open

faern force-pushed the move-ipaddr-to-core branch from ea37750 to 125dc44 Compare January 7, 2023 14:55

This comment has been minimized.

Sign in to view

faern force-pushed the move-ipaddr-to-core branch from 880a99f to 2066822 Compare January 25, 2023 21:59

faern mentioned this pull request Jan 25, 2023

Fix IpAddr link when IpAddr moves to core rust-lang/book#3519

Closed

compiler-errors mentioned this pull request Feb 26, 2023

Rollup of 6 pull requests #108510

Closed

Dylan-DPC mentioned this pull request Feb 27, 2023

Rollup of 5 pull requests #108530

Closed

matthiaskrgr mentioned this pull request Feb 27, 2023

Rollup of 8 pull requests #108538

Merged

bors merged commit cf04603 into rust-lang:master Feb 27, 2023

rustbot added this to the 1.69.0 milestone Feb 27, 2023

RalfJung reviewed Feb 28, 2023

View reviewed changes

library/core/tests/net/parser.rs Show resolved Hide resolved

RalfJung mentioned this pull request Feb 28, 2023

add missing feature in core/tests #108558

Merged

RalfJung reviewed Feb 28, 2023

View reviewed changes

yescallop mentioned this pull request Mar 3, 2023

no_std support yescallop/fluent-uri-rs#7

Merged

gwilymk mentioned this pull request Mar 16, 2023

Latest nightly are failing to build non-toy projects at linking stage when using LTO optimization #108853

Closed

ryoqun mentioned this pull request May 4, 2023

Update to nightly rustc to 2023-04-19 solana-labs/solana#31381

Merged

brooksprumo mentioned this pull request May 4, 2023

Upgrades nightly Rust to 2023-03-04 solana-labs/solana#31487

Merged

Uh oh!

Conversation

faern commented Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Nov 10, 2022

Uh oh!

rustbot commented Nov 10, 2022

Uh oh!

faern commented Nov 10, 2022

Uh oh!

faern commented Nov 11, 2022

Uh oh!

slanterns commented Nov 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomcc commented Nov 12, 2022

Uh oh!

bors commented Nov 15, 2022

Uh oh!

pinkforest commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

newAM commented Jan 4, 2023

Uh oh!

pinkforest commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

newAM commented Jan 5, 2023

Uh oh!

faern commented Jan 7, 2023

Uh oh!

This comment has been minimized.

bors commented Jan 22, 2023

Uh oh!

faern commented Jan 25, 2023

Uh oh!

faern commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Feb 26, 2023

Uh oh!

albertlarsan68 commented Feb 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Feb 26, 2023

Uh oh!

RalfJung Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

faern Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

RalfJung Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

faern Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

RalfJung Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RalfJung Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

klensy commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

faern commented Nov 10, 2022 •

edited

Loading

slanterns commented Nov 11, 2022 •

edited

Loading

pinkforest commented Jan 4, 2023 •

edited

Loading

pinkforest commented Jan 4, 2023 •

edited

Loading

faern commented Jan 25, 2023 •

edited

Loading

albertlarsan68 commented Feb 26, 2023 •

edited

Loading

RalfJung Feb 28, 2023 •

edited

Loading

klensy commented Feb 28, 2023 •

edited

Loading