From 94abd445a82ac3fc61bbc1954c6ba22c499a21d4 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 21 Jul 2014 14:39:52 +0400 Subject: [PATCH 1/4] RFC: Add SocketAddr conversion trait to std::io::net and generalize socket constructors --- active/0000-socket-addr-conversion-trait.md | 175 ++++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100644 active/0000-socket-addr-conversion-trait.md diff --git a/active/0000-socket-addr-conversion-trait.md b/active/0000-socket-addr-conversion-trait.md new file mode 100644 index 00000000000..d0beadcd7ee --- /dev/null +++ b/active/0000-socket-addr-conversion-trait.md @@ -0,0 +1,175 @@ +- Start Date: (2014-07-21) +- RFC PR #: (leave this empty) +- Rust Issue #: (leave this empty) + +# Summary + +Make `std::io::net` socket-wrapping structs (`TcpStream`, `UdpStream` etc.) accept bounded generic +type for address. + +# Motivation + +Currently there is an inconsistency in our networking API when an address to connect to/listen +on should be provided. `TcpStream::connect()` method accepts an `&str` address and `u16` port; +`TcpStream::connect_timeout()` accepts `SocketAddr`; `TcpListener::bind()` accepts `&str` and `u16`; +`UdpSocket::bind()` and `UdpSocket::connect()` accept `SocketAddr`. + +This inconsistency itself is disturbing; however, there is more than this. Even if we changed +everything to use `&str/u16` pair or back to `SocketAddr`, there still will be usability problems. +`SocketAddr` is inconvenient to construct manually; `&str/u16`, on the other hand, requires needless +conversion from `SocketAddr` and back when `SocketAddr` is *the* original value of the address (for +example, it can be retrieved from `getifaddrs()` system call). + +Ideally, the user should be able to provide anything resembling IP address/port pair, and the +library should convert it internally to `SocketAddr` to use in low-level socket calls. And Rust +has all necessary facilities to allow so. + +# Detailed design + +A new trait is introduced to the standard library: + +```rust +pub trait AsSocketAddr { + fn as_socket_addr(&self) -> IoResult; +} +``` + +Then all functions/methods which need an address accept a generic parameter bounded by this trait: + +```rust +impl TcpStream { + pub fn connect(addr: A) -> IoResult { ... } + pub fn connect_timeout(addr: A, timeout_ms: u64) -> IoResult { ... } +} + +impl TcpListener { + pub fn bind(addr: A) -> IoResult { ... } +} + +impl UdpSocket { + pub fn bind(addr: A) -> IoResult { ... } + pub fn connect(self, other: A) -> UdpStream { ... } +} +``` + +This trait is implemented for a number of types: + +```rust +impl AsSocketAddr for SocketAddr { + #[inline] + fn as_socket_addr(&self) -> IoResult { Ok(*self) } +} + +impl AsSocketAddr for (IpAddr, u16) { + #[inline] + fn as_socket_addr(&self) -> IoResult { + let (ip, port) = *self; + Ok(SocketAddr { ip: ip, port: port }); + } +} + +impl AsSocketAddr for (S, u16) { + fn as_socket_addr(&self) -> IoResult { + let (ref host, ref port) = *self; + match get_host_addresses(host.as_slice()).map(|v| v.move_iter().next()) { + Ok(Some(addr)) => (addr, *port).as_socket_addr(), + Ok(None) => { /* whatever error indicating that no addresses are available */ } + Err(e) => Err(e) + } + } +} + +// better be `impl AsSocketAddr for S`, but it will conflict with other implementations +// in current trait matching system +impl<'a> AsSocketAddr for &'a str { + // accepts strings like 'localhost:12345' + fn as_socket_addr(&self) -> IoResult { + // split the string by ':' and convert the second part to u16 + } +} +``` + +This will allow calling aforementioned methods with different types of arguments: +```rust +let mut stream = TcpStream::connect("localhost:12345").unwrap(); +// or +let mut stream = TcpStream::connect(("localhost", 12345)).unwrap(); +// or +let mut stream = TcpStream::connect((Ipv4Addr(127, 0, 0, 1), 12345)).unwrap(); +// or +let addr: SocketAddr = first_address_for_interface("eth0").unwrap(); +let mut stream = TcpStream::connect_timeout(addr, 10_000).unwrap(); +``` + +This provides great flexibility, does not hamper performance at all (due to static dispatch) and +still gives nice and clean interface. + +Note that this pattern is already used in `std`, namely, in `std::path` module. There is a trait, +[`BytesContainer`](http://doc.rust-lang.org/std/path/trait.BytesContainer.html), which represents +generic string of bytes, and `Path` constructors accept values of types implementing this trait. +This allow calling `Path::new()` with string slices, byte vectors or even other paths. + +# Drawbacks + +Adding such "overloading" trait increases complexity of implementation slightly. It also somewhat increases +cognitive load on the programmer when he or she reads API documentation because it won't be immediately +obvious which arguments constructor methods can accept. However, this can be mitigated easily by +emphasizing polymorphic behavior of such methods in the same documentation. Apparently, there seem +to be no problems with `Path` API now. + +Another drawback is esthetical: currently connecting to an address represented as a `&str/u16` looks +like this: +```rust +let mut stream = TcpStream::connect("localhost", 12345).unwrap(); +``` +Under this proposal it will look like this: +```rust +let mut stream = TcpStream::connect(("localhost", 12345)).unwrap(); +``` +Note an additional set of parentheses. This may be somewhat confusing. However, implementing +`AsSocketAddr` for `&str` greatly reduces this problem: when a fixed address is needed, it can be +written as a string directly (see example above). + +# Alternatives + +Leave everything as it is and be stuck with inconsistent and sometimes inconvenient API forever. + +One thing that really should be done regardless of whether this proposal is accepted or not is +that all aforementioned methods have to be unified, for example, they all should accept `&str/u16` +arguments, without variations. This counts as an alternative. + +Another alternative is to provide several methods, for example, one accepting `&str/u16` and another +`SocketAddr`. However, this will increase amount of available methods, and they will have different +names as Rust does not have methods overloading, cluttering socket interfaces. + +Even another possibility is to introduce new enum for this purpose: +```rust +enum SocketAddrWrapper<'a> { + SocketAddr(SocketAddr), + HostPort(&'a str, u16), + Address(&'a str) +} + +impl<'a> SocketAddrWrapper<'a> { + pub fn as_socket_addr(&self) -> SocketAddr { + match *self { + // whatever + } + } +} +``` +This will require calling constructor methods like this: +```rust +let mut stream = TcpStream::connect(Address("localhost:12345")).unwrap(); +// or +let addr: SocketAddr = first_address_for_interface("eth0").unwrap(); +let mut stream = TcpStream::connect(SocketAddr(addr)).unwrap(); +``` + +However, this alternative is less flexible (since enums are closed) and more syntactically heavy at +the call site, which is undesirable. + +# Unresolved questions + +Exact name of the trait and its method is an open question. + From 03dce3498909ae5b6afd278045fd6c0fc3c4f8b6 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 21 Jul 2014 14:48:13 +0400 Subject: [PATCH 2/4] Fixed impl signatures --- active/0000-socket-addr-conversion-trait.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/active/0000-socket-addr-conversion-trait.md b/active/0000-socket-addr-conversion-trait.md index d0beadcd7ee..f15d205fafc 100644 --- a/active/0000-socket-addr-conversion-trait.md +++ b/active/0000-socket-addr-conversion-trait.md @@ -68,19 +68,20 @@ impl AsSocketAddr for (IpAddr, u16) { } } -impl AsSocketAddr for (S, u16) { +// Better use `` instead of `&'a str`, but it will conflict with other implementations +// in the current trait matching system + +impl<'a> AsSocketAddr for (&'a str, u16) { fn as_socket_addr(&self) -> IoResult { - let (ref host, ref port) = *self; - match get_host_addresses(host.as_slice()).map(|v| v.move_iter().next()) { - Ok(Some(addr)) => (addr, *port).as_socket_addr(), + let (host, port) = *self; + match get_host_addresses(host).map(|v| v.move_iter().next()) { + Ok(Some(addr)) => (addr, port).as_socket_addr(), Ok(None) => { /* whatever error indicating that no addresses are available */ } Err(e) => Err(e) } } } -// better be `impl AsSocketAddr for S`, but it will conflict with other implementations -// in current trait matching system impl<'a> AsSocketAddr for &'a str { // accepts strings like 'localhost:12345' fn as_socket_addr(&self) -> IoResult { From 6832cbe8280854845a783af83021e9685fb1c7cd Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 21 Jul 2014 20:07:50 +0400 Subject: [PATCH 3/4] Changed trait definition Added `as_socket_addr_all()` method and explained its function and relationship with its counterpart. --- active/0000-socket-addr-conversion-trait.md | 42 +++++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/active/0000-socket-addr-conversion-trait.md b/active/0000-socket-addr-conversion-trait.md index f15d205fafc..dc1a02df0d8 100644 --- a/active/0000-socket-addr-conversion-trait.md +++ b/active/0000-socket-addr-conversion-trait.md @@ -30,10 +30,31 @@ A new trait is introduced to the standard library: ```rust pub trait AsSocketAddr { - fn as_socket_addr(&self) -> IoResult; + fn as_socket_addr(&self) -> IoResult { + match self.as_socket_addr_all().map(|v| v.move_iter().next()) { + Ok(Some(addr)) => addr, + Ok(None) => Err(/* some error on that no addresses are available */), + Err(e) => Err(e) + } + } + + #[inline] + fn as_socket_addr_all(&self) -> IoResult> { + self.as_socket_addr().map(|a| vec![a]) + } } ``` +This trait contains two methods which are defined in terms of each other. This is done to simplify +implementations; for those types which can be represented with multiple `SocketAddr`esses, +`as_socket_addr_all()` method should be implemented, and its counterpart will be automatically +there; for those types which can only be represented by one `SocketAddr`ess, `as_socket_addr()` +method should be implemented, and the trivial vector implementation will be readily available. + +The method returning a vector is required because some values correspond to potentially multiple IP +addresses, for example, host names. All these addresses are sometimes needed in the underlying +libraries to choose the most appropriate one. + Then all functions/methods which need an address accept a generic parameter bounded by this trait: ```rust @@ -72,13 +93,14 @@ impl AsSocketAddr for (IpAddr, u16) { // in the current trait matching system impl<'a> AsSocketAddr for (&'a str, u16) { - fn as_socket_addr(&self) -> IoResult { + fn as_socket_addr_all(&self) -> IoResult> { let (host, port) = *self; - match get_host_addresses(host).map(|v| v.move_iter().next()) { - Ok(Some(addr)) => (addr, port).as_socket_addr(), - Ok(None) => { /* whatever error indicating that no addresses are available */ } - Err(e) => Err(e) - } + get_host_addresses(host).map(|v| + v.move_iter().map(|a| SocketAddr { + ip: a, + port: port + }).collect() + ) } } @@ -105,6 +127,9 @@ let mut stream = TcpStream::connect_timeout(addr, 10_000).unwrap(); This provides great flexibility, does not hamper performance at all (due to static dispatch) and still gives nice and clean interface. +Underlying socket API may use `as_socket_addr_all()` method to obtain all available addresses and +make a decision on which to use, if it is required. + Note that this pattern is already used in `std`, namely, in `std::path` module. There is a trait, [`BytesContainer`](http://doc.rust-lang.org/std/path/trait.BytesContainer.html), which represents generic string of bytes, and `Path` constructors accept values of types implementing this trait. @@ -174,3 +199,6 @@ the call site, which is undesirable. Exact name of the trait and its method is an open question. +Is it OK that the trait contains mutually recursive default methods? This is in spirit of Haskell +`Eq` trait with its `(==)` and `(/=)` methods, but it is debatable whether such approach is +appropriate in Rust. From b03d058efa4aec7db0408bcb583227b209fa15e6 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 22 Jul 2014 20:47:32 +0400 Subject: [PATCH 4/4] Renamed the trait and methods according to the guidelines --- active/0000-socket-addr-conversion-trait.md | 46 ++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/active/0000-socket-addr-conversion-trait.md b/active/0000-socket-addr-conversion-trait.md index dc1a02df0d8..4d57d301008 100644 --- a/active/0000-socket-addr-conversion-trait.md +++ b/active/0000-socket-addr-conversion-trait.md @@ -29,9 +29,9 @@ has all necessary facilities to allow so. A new trait is introduced to the standard library: ```rust -pub trait AsSocketAddr { - fn as_socket_addr(&self) -> IoResult { - match self.as_socket_addr_all().map(|v| v.move_iter().next()) { +pub trait ToSocketAddr { + fn to_socket_addr(&self) -> IoResult { + match self.to_socket_addr_all().map(|v| v.move_iter().next()) { Ok(Some(addr)) => addr, Ok(None) => Err(/* some error on that no addresses are available */), Err(e) => Err(e) @@ -39,16 +39,16 @@ pub trait AsSocketAddr { } #[inline] - fn as_socket_addr_all(&self) -> IoResult> { - self.as_socket_addr().map(|a| vec![a]) + fn to_socket_addr_all(&self) -> IoResult> { + self.to_socket_addr().map(|a| vec![a]) } } ``` This trait contains two methods which are defined in terms of each other. This is done to simplify implementations; for those types which can be represented with multiple `SocketAddr`esses, -`as_socket_addr_all()` method should be implemented, and its counterpart will be automatically -there; for those types which can only be represented by one `SocketAddr`ess, `as_socket_addr()` +`to_socket_addr_all()` method should be implemented, and its counterpart will be automatically +there; for those types which can only be represented by one `SocketAddr`ess, `to_socket_addr()` method should be implemented, and the trivial vector implementation will be readily available. The method returning a vector is required because some values correspond to potentially multiple IP @@ -59,31 +59,31 @@ Then all functions/methods which need an address accept a generic parameter boun ```rust impl TcpStream { - pub fn connect(addr: A) -> IoResult { ... } - pub fn connect_timeout(addr: A, timeout_ms: u64) -> IoResult { ... } + pub fn connect(addr: A) -> IoResult { ... } + pub fn connect_timeout(addr: A, timeout_ms: u64) -> IoResult { ... } } impl TcpListener { - pub fn bind(addr: A) -> IoResult { ... } + pub fn bind(addr: A) -> IoResult { ... } } impl UdpSocket { - pub fn bind(addr: A) -> IoResult { ... } - pub fn connect(self, other: A) -> UdpStream { ... } + pub fn bind(addr: A) -> IoResult { ... } + pub fn connect(self, other: A) -> UdpStream { ... } } ``` This trait is implemented for a number of types: ```rust -impl AsSocketAddr for SocketAddr { +impl ToSocketAddr for SocketAddr { #[inline] - fn as_socket_addr(&self) -> IoResult { Ok(*self) } + fn to_socket_addr(&self) -> IoResult { Ok(*self) } } -impl AsSocketAddr for (IpAddr, u16) { +impl ToSocketAddr for (IpAddr, u16) { #[inline] - fn as_socket_addr(&self) -> IoResult { + fn to_socket_addr(&self) -> IoResult { let (ip, port) = *self; Ok(SocketAddr { ip: ip, port: port }); } @@ -92,8 +92,8 @@ impl AsSocketAddr for (IpAddr, u16) { // Better use `` instead of `&'a str`, but it will conflict with other implementations // in the current trait matching system -impl<'a> AsSocketAddr for (&'a str, u16) { - fn as_socket_addr_all(&self) -> IoResult> { +impl<'a> ToSocketAddr for (&'a str, u16) { + fn to_socket_addr_all(&self) -> IoResult> { let (host, port) = *self; get_host_addresses(host).map(|v| v.move_iter().map(|a| SocketAddr { @@ -104,9 +104,9 @@ impl<'a> AsSocketAddr for (&'a str, u16) { } } -impl<'a> AsSocketAddr for &'a str { +impl<'a> ToSocketAddr for &'a str { // accepts strings like 'localhost:12345' - fn as_socket_addr(&self) -> IoResult { + fn to_socket_addr(&self) -> IoResult { // split the string by ':' and convert the second part to u16 } } @@ -127,7 +127,7 @@ let mut stream = TcpStream::connect_timeout(addr, 10_000).unwrap(); This provides great flexibility, does not hamper performance at all (due to static dispatch) and still gives nice and clean interface. -Underlying socket API may use `as_socket_addr_all()` method to obtain all available addresses and +Underlying socket API may use `to_socket_addr_all()` method to obtain all available addresses and make a decision on which to use, if it is required. Note that this pattern is already used in `std`, namely, in `std::path` module. There is a trait, @@ -153,7 +153,7 @@ Under this proposal it will look like this: let mut stream = TcpStream::connect(("localhost", 12345)).unwrap(); ``` Note an additional set of parentheses. This may be somewhat confusing. However, implementing -`AsSocketAddr` for `&str` greatly reduces this problem: when a fixed address is needed, it can be +`ToSocketAddr` for `&str` greatly reduces this problem: when a fixed address is needed, it can be written as a string directly (see example above). # Alternatives @@ -177,7 +177,7 @@ enum SocketAddrWrapper<'a> { } impl<'a> SocketAddrWrapper<'a> { - pub fn as_socket_addr(&self) -> SocketAddr { + pub fn to_socket_addr(&self) -> SocketAddr { match *self { // whatever }