Skip to content

Conversation

joboet
Copy link
Member

@joboet joboet commented Sep 14, 2025

The logic for splitting up a string into a hostname and port is currently duplicated across (nearly) all of the networking implementations in sys. Since it does not actually rely on any system internals, this PR moves it to the ToSocketAddr implementation for &str, making it easier to discover and maintain.

On the other hand, the ToSocketAddr implementation (or rather the resolve_socket_addr function) contained logic to overwrite the port on the socket addresses returned by LookupHost, even though LookupHost is already aware of the port and sets the port already on Xous. This PR thus removes this logic by moving the responsibility of setting the port to the system-specific LookupHost implementation.

As a consequence of these changes, there remains only one way of creating LookupHost, hence I've removed the TryFrom implementations in favour of a lookup_host function, mirroring other, public iterator-based features.

And finally, I've simplified the parsing logic responsible for recognising IP addresses passed to <(&str, u16)>::to_socket_addrs() by using the FromStr impl of IpAddr rather than duplicating the parsing for both IP versions.

@rustbot 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 Sep 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

@joboet joboet force-pushed the simplify-lookup-host branch from cd0ffd9 to 3f42540 Compare September 14, 2025 18:43
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [ui] tests/ui/inference/issue-72616.rs stdout ----
Saved the actual stderr to `/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/inference/issue-72616/issue-72616.stderr`
diff of stderr:

16 LL +         if String::from("a") == <&str as TryInto<T>>::try_into("a").unwrap() {}
17    |
18 
- error[E0283]: type annotations needed
-   --> $DIR/issue-72616.rs:22:37
-    |
- LL |         if String::from("a") == "a".try_into().unwrap() {}
-    |                                     ^^^^^^^^
-    |
-    = note: multiple `impl`s satisfying `_: TryFrom<&str>` found in the following crates: `core`, `std`:
-            - impl TryFrom<&str> for std::sys::net::connection::socket::LookupHost;
-            - impl<T, U> TryFrom<U> for T
-              where U: Into<T>;
-    = note: required for `&str` to implement `TryInto<_>`
- help: try using a fully qualified path to specify the expected types
-    |
- LL -         if String::from("a") == "a".try_into().unwrap() {}
- LL +         if String::from("a") == <&str as TryInto<T>>::try_into("a").unwrap() {}
-    |
- 
- error: aborting due to 2 previous errors
+ error: aborting due to 1 previous error
37 
---
To only update this specific test, also pass `--test-args inference/issue-72616.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/inference/issue-72616.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/inference/issue-72616" "-A" "unused" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
error[E0283]: type annotations needed
##[error]  --> /checkout/tests/ui/inference/issue-72616.rs:22:37
   |
LL |         if String::from("a") == "a".try_into().unwrap() {}
   |                              --     ^^^^^^^^
   |                              |
   |                              type must be known at this point
   |
   = note: multiple `impl`s satisfying `String: PartialEq<_>` found in the following crates: `alloc`, `std`:
           - impl PartialEq for String;
           - impl PartialEq<Path> for String;
           - impl PartialEq<PathBuf> for String;
help: try using a fully qualified path to specify the expected types
   |
LL -         if String::from("a") == "a".try_into().unwrap() {}
LL +         if String::from("a") == <&str as TryInto<T>>::try_into("a").unwrap() {}
   |

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0283`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants