Skip to content

Commit

Permalink
swarm-derive/: Remove support for ignoring fields on struct (#2842)
Browse files Browse the repository at this point in the history
With the removal of `NetworkBehaviourEventProcess` there is no more need for
ignoring fields.
  • Loading branch information
mxinden committed Aug 29, 2022
1 parent 247b553 commit 6855ab9
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 135 deletions.
6 changes: 0 additions & 6 deletions examples/chat.rs
Expand Up @@ -85,11 +85,6 @@ async fn main() -> Result<(), Box<dyn Error>> {
struct MyBehaviour {
floodsub: Floodsub,
mdns: Mdns,

// Struct fields which do not implement NetworkBehaviour need to be ignored
#[behaviour(ignore)]
#[allow(dead_code)]
ignored_member: bool,
}

#[allow(clippy::large_enum_variant)]
Expand Down Expand Up @@ -117,7 +112,6 @@ async fn main() -> Result<(), Box<dyn Error>> {
let mut behaviour = MyBehaviour {
floodsub: Floodsub::new(local_peer_id),
mdns,
ignored_member: false,
};

behaviour.floodsub.subscribe(floodsub_topic.clone());
Expand Down
5 changes: 5 additions & 0 deletions swarm-derive/CHANGELOG.md
Expand Up @@ -8,6 +8,11 @@
[PR 2840]: https://github.com/libp2p/rust-libp2p/pull/2840
[PR 2841]: https://github.com/libp2p/rust-libp2p/pull/2841

- Remove support for non-`NetworkBehaviour` fields on main `struct` via `#[behaviour(ignore)]`. See
[PR 2842].

[PR 2842]: https://github.com/libp2p/rust-libp2p/pull/2842

# 0.29.0

- Generate `NetworkBehaviour::OutEvent` if not provided through `#[behaviour(out_event =
Expand Down
81 changes: 35 additions & 46 deletions swarm-derive/src/lib.rs
Expand Up @@ -70,13 +70,6 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
quote! {<#(#lf,)* #(#tp,)* #(#cst,)*>}
};

// The fields of the struct we are interested in (no ignored fields).
let data_struct_fields = data_struct
.fields
.iter()
.filter(|f| !is_ignored(f))
.collect::<Vec<_>>();

let (out_event_name, out_event_definition, out_event_from_clauses) = {
// If we find a `#[behaviour(out_event = "Foo")]` attribute on the
// struct, we set `Foo` as the out event. If not, the `OutEvent` is
Expand All @@ -102,7 +95,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
// User provided `OutEvent`.
Some(name) => {
let definition = None;
let from_clauses = data_struct_fields
let from_clauses = data_struct
.fields
.iter()
.map(|field| {
let ty = &field.ty;
Expand All @@ -115,7 +109,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
None => {
let name: syn::Type = syn::parse_str(&(ast.ident.to_string() + "Event")).unwrap();
let definition = {
let fields = data_struct_fields
let fields = data_struct
.fields
.iter()
.map(|field| {
let variant: syn::Variant = syn::parse_str(
Expand Down Expand Up @@ -152,7 +147,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the `where ...` clause of the trait implementation.
let where_clause = {
let additional = data_struct_fields
let additional = data_struct
.fields
.iter()
.map(|field| {
let ty = &field.ty;
Expand All @@ -174,7 +170,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `addresses_of_peer()`.
let addresses_of_peer_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -185,7 +182,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_connection_established()`.
let inject_connection_established_stmts = {
data_struct_fields.iter().enumerate().map(move |(field_n, field)| {
data_struct.fields.iter().enumerate().map(move |(field_n, field)| {
match field.ident {
Some(ref i) => quote!{ self.#i.inject_connection_established(peer_id, connection_id, endpoint, errors, other_established); },
None => quote!{ self.#field_n.inject_connection_established(peer_id, connection_id, endpoint, errors, other_established); },
Expand All @@ -195,7 +192,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_address_change()`.
let inject_address_change_stmts = {
data_struct_fields.iter().enumerate().map(move |(field_n, field)| {
data_struct.fields.iter().enumerate().map(move |(field_n, field)| {
match field.ident {
Some(ref i) => quote!{ self.#i.inject_address_change(peer_id, connection_id, old, new); },
None => quote!{ self.#field_n.inject_address_change(peer_id, connection_id, old, new); },
Expand All @@ -205,7 +202,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_connection_closed()`.
let inject_connection_closed_stmts = {
data_struct_fields
data_struct.fields
.iter()
.enumerate()
// The outmost handler belongs to the last behaviour.
Expand Down Expand Up @@ -234,7 +231,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_dial_failure()`.
let inject_dial_failure_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
// The outmost handler belongs to the last behaviour.
Expand Down Expand Up @@ -268,7 +266,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_listen_failure()`.
let inject_listen_failure_stmts = {
data_struct_fields
data_struct.fields
.iter()
.enumerate()
.rev()
Expand Down Expand Up @@ -296,7 +294,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_new_listener()`.
let inject_new_listener_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -307,7 +306,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_new_listen_addr()`.
let inject_new_listen_addr_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -318,7 +318,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_expired_listen_addr()`.
let inject_expired_listen_addr_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -329,7 +330,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_new_external_addr()`.
let inject_new_external_addr_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -340,7 +342,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_expired_external_addr()`.
let inject_expired_external_addr_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -351,7 +354,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_listener_error()`.
let inject_listener_error_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -362,7 +366,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Build the list of statements to put in the body of `inject_listener_closed()`.
let inject_listener_closed_stmts = {
data_struct_fields
data_struct
.fields
.iter()
.enumerate()
.map(move |(field_n, field)| match field.ident {
Expand All @@ -375,14 +380,14 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
//
// The event type is a construction of nested `#either_ident`s of the events of the children.
// We call `inject_event` on the corresponding child.
let inject_node_event_stmts = data_struct_fields.iter().enumerate().enumerate().map(|(enum_n, (field_n, field))| {
let inject_node_event_stmts = data_struct.fields.iter().enumerate().enumerate().map(|(enum_n, (field_n, field))| {
let mut elem = if enum_n != 0 {
quote!{ #either_ident::Second(ev) }
} else {
quote!{ ev }
};

for _ in 0 .. data_struct_fields.len() - 1 - enum_n {
for _ in 0 .. data_struct.fields.len() - 1 - enum_n {
elem = quote!{ #either_ident::First(#elem) };
}

Expand All @@ -395,7 +400,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
// The [`ConnectionHandler`] associated type.
let connection_handler_ty = {
let mut ph_ty = None;
for field in data_struct_fields.iter() {
for field in data_struct.fields.iter() {
let ty = &field.ty;
let field_info = quote! { <#ty as #trait_to_impl>::ConnectionHandler };
match ph_ty {
Expand All @@ -412,7 +417,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
let new_handler = {
let mut out_handler = None;

for (field_n, field) in data_struct_fields.iter().enumerate() {
for (field_n, field) in data_struct.fields.iter().enumerate() {
let field_name = match field.ident {
Some(ref i) => quote! { self.#i },
None => quote! { self.#field_n },
Expand All @@ -436,7 +441,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
// List of statements to put in `poll()`.
//
// We poll each child one by one and wrap around the output.
let poll_stmts = data_struct_fields.iter().enumerate().map(|(field_n, field)| {
let poll_stmts = data_struct.fields.iter().enumerate().map(|(field_n, field)| {
let field = field
.ident
.clone()
Expand All @@ -447,7 +452,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
} else {
quote!{ event }
};
for _ in 0 .. data_struct_fields.len() - 1 - field_n {
for _ in 0 .. data_struct.fields.len() - 1 - field_n {
wrapped_event = quote!{ #either_ident::First(#wrapped_event) };
}

Expand All @@ -458,7 +463,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
let provided_handler_and_new_handlers = {
let mut out_handler = None;

for (f_n, f) in data_struct_fields.iter().enumerate() {
for (f_n, f) in data_struct.fields.iter().enumerate() {
let f_name = match f.ident {
Some(ref i) => quote! { self.#i },
None => quote! { self.#f_n },
Expand Down Expand Up @@ -637,19 +642,3 @@ fn get_meta_items(attr: &syn::Attribute) -> Option<Vec<syn::NestedMeta>> {
None
}
}

/// Returns true if a field is marked as ignored by the user.
fn is_ignored(field: &syn::Field) -> bool {
for meta_items in field.attrs.iter().filter_map(get_meta_items) {
for meta_item in meta_items {
match meta_item {
syn::NestedMeta::Meta(syn::Meta::Path(ref m)) if m.is_ident("ignore") => {
return true;
}
_ => (),
}
}
}

false
}
48 changes: 0 additions & 48 deletions swarm-derive/tests/test.rs
Expand Up @@ -83,8 +83,6 @@ fn three_fields() {
ping: libp2p::ping::Ping,
identify: libp2p::identify::Identify,
kad: libp2p::kad::Kademlia<libp2p::kad::record::store::MemoryStore>,
#[behaviour(ignore)]
foo: String,
}

#[allow(dead_code)]
Expand All @@ -103,30 +101,6 @@ fn three_fields() {
}
}

#[test]
fn three_fields_non_last_ignored() {
#[allow(dead_code)]
#[derive(NetworkBehaviour)]
struct Foo {
ping: libp2p::ping::Ping,
#[behaviour(ignore)]
identify: String,
kad: libp2p::kad::Kademlia<libp2p::kad::record::store::MemoryStore>,
}

#[allow(dead_code)]
#[allow(unreachable_code)]
fn foo() {
let _out_event: <Foo as NetworkBehaviour>::OutEvent = unimplemented!();
match _out_event {
FooEvent::Ping(libp2p::ping::Event { .. }) => {}
FooEvent::Kad(event) => {
let _: libp2p::kad::KademliaEvent = event;
}
}
}
}

#[test]
fn custom_event() {
#[allow(dead_code)]
Expand Down Expand Up @@ -359,28 +333,6 @@ fn custom_event_with_either() {
}
}

#[test]
fn mixed_field_order() {
struct Foo {}

#[derive(NetworkBehaviour)]
pub struct Behaviour {
#[behaviour(ignore)]
_foo: Foo,
_ping: libp2p::ping::Ping,
#[behaviour(ignore)]
_foo2: Foo,
_identify: libp2p::identify::Identify,
#[behaviour(ignore)]
_foo3: Foo,
}

#[allow(dead_code)]
fn behaviour() {
require_net_behaviour::<Behaviour>();
}
}

#[test]
fn generated_out_event_derive_debug() {
#[allow(dead_code)]
Expand Down
35 changes: 0 additions & 35 deletions swarm/src/behaviour.rs
Expand Up @@ -117,41 +117,6 @@ pub(crate) type THandlerOutEvent<THandler> =
/// }
/// }
/// ```
///
/// Struct members that don't implement [`NetworkBehaviour`] must be annotated with
/// `#[behaviour(ignore)]`.
///
/// ``` rust
/// # use libp2p::identify::{Identify, IdentifyEvent};
/// # use libp2p::ping::{Ping, PingEvent};
/// # use libp2p::NetworkBehaviour;
/// #[derive(NetworkBehaviour)]
/// #[behaviour(out_event = "Event")]
/// struct MyBehaviour {
/// identify: Identify,
/// ping: Ping,
///
/// #[behaviour(ignore)]
/// some_string: String,
/// }
/// #
/// # enum Event {
/// # Identify(IdentifyEvent),
/// # Ping(PingEvent),
/// # }
/// #
/// # impl From<IdentifyEvent> for Event {
/// # fn from(event: IdentifyEvent) -> Self {
/// # Self::Identify(event)
/// # }
/// # }
/// #
/// # impl From<PingEvent> for Event {
/// # fn from(event: PingEvent) -> Self {
/// # Self::Ping(event)
/// # }
/// # }
/// ```
pub trait NetworkBehaviour: 'static {
/// Handler for all the protocols the network behaviour supports.
type ConnectionHandler: IntoConnectionHandler;
Expand Down

0 comments on commit 6855ab9

Please sign in to comment.