Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/either: Remove EitherName in favor of Either #2798

Closed
wants to merge 10 commits into from
3 changes: 3 additions & 0 deletions core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@
- Change `StreamMuxer` interface to be entirely poll-based. All functions on `StreamMuxer` now
require a `Context` and return `Poll`. This gives callers fine-grained control over what they
would like to make progress on. See [PR 2724] and [PR 2797].
- Remove `EitherName` in favor of `Either` from the `either` crate. See [PR 2798].
- Replace blanket `ProtocolName` impl with specific ones on `&'static [u8]` and `Cow<'static, [u8]>`. See [PR 2798].
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could split this out into a different PR if you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with a single pull request only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not implement ProtocolName for String and str?


[PR 2724]: https://github.com/libp2p/rust-libp2p/pull/2724
[PR 2762]: https://github.com/libp2p/rust-libp2p/pull/2762
[PR 2775]: https://github.com/libp2p/rust-libp2p/pull/2775
[PR 2776]: https://github.com/libp2p/rust-libp2p/pull/2776
[PR 2765]: https://github.com/libp2p/rust-libp2p/pull/2765
[PR 2797]: https://github.com/libp2p/rust-libp2p/pull/2797
[PR 2798]: https://github.com/libp2p/rust-libp2p/pull/2798

# 0.34.0

Expand Down
2 changes: 1 addition & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ categories = ["network-programming", "asynchronous"]
asn1_der = "0.7.4"
bs58 = "0.4.0"
ed25519-dalek = "1.0.1"
either = "1.5"
either = "1.7"
fnv = "1.0"
futures = { version = "0.3.1", features = ["executor", "thread-pool"] }
futures-timer = "3"
Expand Down
16 changes: 1 addition & 15 deletions core/src/either.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::muxing::StreamMuxerEvent;
use crate::{
muxing::StreamMuxer,
transport::{ListenerId, Transport, TransportError, TransportEvent},
Multiaddr, ProtocolName,
Multiaddr,
};
use futures::{
io::{IoSlice, IoSliceMut},
Expand Down Expand Up @@ -310,20 +310,6 @@ where
}
}

#[derive(Debug, Clone)]
pub enum EitherName<A, B> {
A(A),
B(B),
}

impl<A: ProtocolName, B: ProtocolName> ProtocolName for EitherName<A, B> {
fn protocol_name(&self) -> &[u8] {
match self {
EitherName::A(a) => a.protocol_name(),
EitherName::B(b) => b.protocol_name(),
}
}
}
#[pin_project(project = EitherTransportProj)]
#[derive(Debug)]
#[must_use = "transports do nothing unless polled"]
Expand Down
20 changes: 19 additions & 1 deletion core/src/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ mod pending;
mod select;
mod transfer;

use ::either::Either;
use futures::future::Future;
use std::borrow::Cow;

pub use self::{
apply::{apply, apply_inbound, apply_outbound, InboundUpgradeApply, OutboundUpgradeApply},
Expand Down Expand Up @@ -126,12 +128,28 @@ pub trait ProtocolName {
fn protocol_name(&self) -> &[u8];
}

impl<T: AsRef<[u8]>> ProtocolName for T {
impl ProtocolName for &'static [u8] {
fn protocol_name(&self) -> &[u8] {
self
}
}

impl ProtocolName for Cow<'static, [u8]> {
fn protocol_name(&self) -> &[u8] {
self.as_ref()
}
}

impl<A, B> ProtocolName for Either<A, B>
where
A: ProtocolName,
B: ProtocolName,
{
fn protocol_name(&self) -> &[u8] {
::either::for_both!(self, inner => inner.protocol_name())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

}
}

/// Common trait for upgrades that can be applied on inbound substreams, outbound substreams,
/// or both.
pub trait UpgradeInfo {
Expand Down
19 changes: 10 additions & 9 deletions core/src/upgrade/either.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
// DEALINGS IN THE SOFTWARE.

use crate::{
either::{EitherError, EitherFuture2, EitherName, EitherOutput},
either::{EitherError, EitherFuture2, EitherOutput},
upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo},
};
use either::Either;

/// A type to represent two possible upgrade types (inbound or outbound).
#[derive(Debug, Clone)]
Expand All @@ -35,7 +36,7 @@ where
A: UpgradeInfo,
B: UpgradeInfo,
{
type Info = EitherName<A::Info, B::Info>;
type Info = Either<A::Info, B::Info>;
type InfoIter = EitherIter<
<A::InfoIter as IntoIterator>::IntoIter,
<B::InfoIter as IntoIterator>::IntoIter,
Expand All @@ -60,10 +61,10 @@ where

fn upgrade_inbound(self, sock: C, info: Self::Info) -> Self::Future {
match (self, info) {
(EitherUpgrade::A(a), EitherName::A(info)) => {
(EitherUpgrade::A(a), Either::Left(info)) => {
EitherFuture2::A(a.upgrade_inbound(sock, info))
}
(EitherUpgrade::B(b), EitherName::B(info)) => {
(EitherUpgrade::B(b), Either::Right(info)) => {
EitherFuture2::B(b.upgrade_inbound(sock, info))
}
_ => panic!("Invalid invocation of EitherUpgrade::upgrade_inbound"),
Expand All @@ -82,10 +83,10 @@ where

fn upgrade_outbound(self, sock: C, info: Self::Info) -> Self::Future {
match (self, info) {
(EitherUpgrade::A(a), EitherName::A(info)) => {
(EitherUpgrade::A(a), Either::Left(info)) => {
EitherFuture2::A(a.upgrade_outbound(sock, info))
}
(EitherUpgrade::B(b), EitherName::B(info)) => {
(EitherUpgrade::B(b), Either::Right(info)) => {
EitherFuture2::B(b.upgrade_outbound(sock, info))
}
_ => panic!("Invalid invocation of EitherUpgrade::upgrade_outbound"),
Expand All @@ -105,12 +106,12 @@ where
A: Iterator,
B: Iterator,
{
type Item = EitherName<A::Item, B::Item>;
type Item = Either<A::Item, B::Item>;

fn next(&mut self) -> Option<Self::Item> {
match self {
EitherIter::A(a) => a.next().map(EitherName::A),
EitherIter::B(b) => b.next().map(EitherName::B),
EitherIter::A(a) => a.next().map(Either::Left),
EitherIter::B(b) => b.next().map(Either::Right),
}
}

Expand Down
19 changes: 10 additions & 9 deletions core/src/upgrade/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
// DEALINGS IN THE SOFTWARE.

use crate::{
either::{EitherError, EitherFuture2, EitherName, EitherOutput},
either::{EitherError, EitherFuture2, EitherOutput},
upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo},
};
use either::Either;

/// Upgrade that combines two upgrades into one. Supports all the protocols supported by either
/// sub-upgrade.
Expand All @@ -44,7 +45,7 @@ where
A: UpgradeInfo,
B: UpgradeInfo,
{
type Info = EitherName<A::Info, B::Info>;
type Info = Either<A::Info, B::Info>;
type InfoIter = InfoIterChain<
<A::InfoIter as IntoIterator>::IntoIter,
<B::InfoIter as IntoIterator>::IntoIter,
Expand All @@ -69,8 +70,8 @@ where

fn upgrade_inbound(self, sock: C, info: Self::Info) -> Self::Future {
match info {
EitherName::A(info) => EitherFuture2::A(self.0.upgrade_inbound(sock, info)),
EitherName::B(info) => EitherFuture2::B(self.1.upgrade_inbound(sock, info)),
Either::Left(info) => EitherFuture2::A(self.0.upgrade_inbound(sock, info)),
Either::Right(info) => EitherFuture2::B(self.1.upgrade_inbound(sock, info)),
}
}
}
Expand All @@ -86,8 +87,8 @@ where

fn upgrade_outbound(self, sock: C, info: Self::Info) -> Self::Future {
match info {
EitherName::A(info) => EitherFuture2::A(self.0.upgrade_outbound(sock, info)),
EitherName::B(info) => EitherFuture2::B(self.1.upgrade_outbound(sock, info)),
Either::Left(info) => EitherFuture2::A(self.0.upgrade_outbound(sock, info)),
Either::Right(info) => EitherFuture2::B(self.1.upgrade_outbound(sock, info)),
}
}
}
Expand All @@ -101,14 +102,14 @@ where
A: Iterator,
B: Iterator,
{
type Item = EitherName<A::Item, B::Item>;
type Item = Either<A::Item, B::Item>;

fn next(&mut self) -> Option<Self::Item> {
if let Some(info) = self.0.next() {
return Some(EitherName::A(info));
return Some(Either::Left(info));
}
if let Some(info) = self.1.next() {
return Some(EitherName::B(info));
return Some(Either::Right(info));
}
None
}
Expand Down
4 changes: 2 additions & 2 deletions core/tests/transport_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ use std::{io, pin::Pin};
struct HelloUpgrade {}

impl UpgradeInfo for HelloUpgrade {
type Info = &'static str;
type Info = &'static [u8];
type InfoIter = std::iter::Once<Self::Info>;

fn protocol_info(&self) -> Self::InfoIter {
std::iter::once("/hello/1")
std::iter::once(b"/hello/1")
}
}

Expand Down
12 changes: 4 additions & 8 deletions src/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,23 @@
// DEALINGS IN THE SOFTWARE.

use crate::core::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo};
use bytes::Bytes;
use futures::prelude::*;
use std::{iter, sync::Arc};

/// Implementation of `ConnectionUpgrade`. Convenient to use with small protocols.
#[derive(Debug)]
pub struct SimpleProtocol<F> {
info: Bytes,
info: &'static [u8],
// Note: we put the closure `F` in an `Arc` because Rust closures aren't automatically clonable
// yet.
upgrade: Arc<F>,
}

impl<F> SimpleProtocol<F> {
/// Builds a `SimpleProtocol`.
pub fn new<N>(info: N, upgrade: F) -> SimpleProtocol<F>
where
N: Into<Bytes>,
{
pub fn new<N>(info: &'static [u8], upgrade: F) -> SimpleProtocol<F> {
SimpleProtocol {
info: info.into(),
info,
upgrade: Arc::new(upgrade),
}
}
Expand All @@ -55,7 +51,7 @@ impl<F> Clone for SimpleProtocol<F> {
}

impl<F> UpgradeInfo for SimpleProtocol<F> {
type Info = Bytes;
type Info = &'static [u8];
type InfoIter = iter::Once<Self::Info>;

fn protocol_info(&self) -> Self::InfoIter {
Expand Down