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.

Sign up for GitHub

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

swarm*/: Remove NetworkBehaviourEventProcess and generate OutEvent #2751

Closed
wants to merge 14 commits into from
Closed

swarm*/: Remove NetworkBehaviourEventProcess and generate OutEvent #2751

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0da8707
swarm-derive/: Remove NetworkBehaviourEventProcess and generate OutEvent
mxinden Jul 13, 2022
6c87078
swarm-derive/src/lib: Support both generated and provided event
mxinden Jul 25, 2022
5b1bfcc
swarm-derive/src/lib: Provide trait parameters to OutEvent
mxinden Jul 25, 2022
b4cdbff
swarm-derive/src/lib: Use heck to upper case enum variant identifier
mxinden Jul 25, 2022
1179827
swarm-derive: Support mismatching field names
mxinden Jul 25, 2022
1619970
swarm-derive/src/lib: Use `split_for_impl` on Generic
mxinden Jul 25, 2022
ec86293
swarm-derive/src/lib: Refactor out_event
mxinden Jul 25, 2022
f5c7b02
swarm-derive/src/lib: Clean up where clauses
mxinden Jul 25, 2022
7e61cd0
Merge branch 'master' into no-event-process
mxinden Jul 25, 2022
060b789
swarm-derive: Bump version and add changelog entry
mxinden Jul 25, 2022
4c01c31
swarm/src/lib: Remove NetworkBehaviourEventProcess
mxinden Jul 26, 2022
77bfafe
swarm/CHANGELOG: Add entry
mxinden Jul 26, 2022
60adbaa
swarm/CHANGELOG: Add `out_event` macro parameter
mxinden Jul 27, 2022
3e1b866
Merge branch 'master' into no-event-process
mxinden Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 51 additions & 71 deletions swarm-derive/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

#![recursion_limit = "256"]

use proc_macro::TokenStream;
use proc_macro::{Span, TokenStream};
use quote::quote;
use syn::{parse_macro_input, Data, DataStruct, DeriveInput, Ident};

Expand All @@ -44,10 +44,10 @@ fn build(ast: &DeriveInput) -> TokenStream {
/// The version for structs
fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
let name = &ast.ident;
let event_name = Ident::new(&(ast.ident.to_string() + "Event"), Span::call_site().into());
let (_, ty_generics, where_clause) = ast.generics.split_for_impl();
let multiaddr = quote! {::libp2p::core::Multiaddr};
let trait_to_impl = quote! {::libp2p::swarm::NetworkBehaviour};
let net_behv_event_proc = quote! {::libp2p::swarm::NetworkBehaviourEventProcess};
let either_ident = quote! {::libp2p::core::either::EitherOutput};
let network_behaviour_action = quote! {::libp2p::swarm::NetworkBehaviourAction};
let into_connection_handler = quote! {::libp2p::swarm::IntoConnectionHandler};
Expand All @@ -70,72 +70,42 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
quote! {<#(#lf,)* #(#tp,)* #(#cst,)*>}
};

// Whether or not we require the `NetworkBehaviourEventProcess` trait to be implemented.
let event_process = {
let mut event_process = false;

for meta_items in ast.attrs.iter().filter_map(get_meta_items) {
for meta_item in meta_items {
match meta_item {
syn::NestedMeta::Meta(syn::Meta::NameValue(ref m))
if m.path.is_ident("event_process") =>
{
if let syn::Lit::Bool(ref b) = m.lit {
event_process = b.value
}
}
_ => (),
}
}
}

event_process
};

// 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<_>>();

// The final out event.
// If we find a `#[behaviour(out_event = "Foo")]` attribute on the struct, we set `Foo` as
// the out event. Otherwise we use `()`.
let out_event = {
let mut out = quote! {()};
for meta_items in ast.attrs.iter().filter_map(get_meta_items) {
for meta_item in meta_items {
match meta_item {
syn::NestedMeta::Meta(syn::Meta::NameValue(ref m))
if m.path.is_ident("out_event") =>
{
if let syn::Lit::Str(ref s) = m.lit {
let ident: syn::Type = syn::parse_str(&s.value()).unwrap();
out = quote! {#ident};
}
}
_ => (),
}
let event = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not at least give the user the option to set their own OutEvent? Per default it could still be generated. This would also avoid breaking the implementation of current users that already use a custom OutEvent.

Copy link
Member Author

Choose a reason for hiding this comment

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

In which use-case would a user need their own OutEvent? Asked the other way around, which use-case would not be supported with a generated OutEvent?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think they strictly need their own OutEvent. But it may be more convenient. Especially because of the point that @thomaseizinger raised above, that is the interaction with a generated enum. As a user I'd always prefer to have my own type rather than one generated by a macro. Most likely I would parse the OutEvent into my custom type anyway, e.g. when bubbling the event up to a higher layer. So still having to deal with the generated one would probably just be extra work (understand how the enum looks like, where the variant names come from, handling changes in the enum if this variant names change because a field was renames, etc).

let fields = data_struct_fields
.iter()
.map(|field| {
let variant = &capitalize(
field
.ident
.clone()
.expect("Fields of NetworkBehaviour implementation to be named."),
);
let ty = &field.ty;
quote! {#variant(<#ty as NetworkBehaviour>::OutEvent)}
})
.collect::<Vec<_>>();
let visibility = &ast.vis;
quote! {
#visibility enum #event_name {
#(#fields),*
}
}
out
};

// Build the `where ...` clause of the trait implementation.
let where_clause = {
let additional = data_struct_fields
.iter()
.flat_map(|field| {
.map(|field| {
let ty = &field.ty;
vec![
quote! {#ty: #trait_to_impl},
if event_process {
quote! {Self: #net_behv_event_proc<<#ty as #trait_to_impl>::OutEvent>}
} else {
quote! {#out_event: From< <#ty as #trait_to_impl>::OutEvent >}
},
]
quote! {#ty: #trait_to_impl}
})
.collect::<Vec<_>>();

Expand Down Expand Up @@ -437,18 +407,20 @@ 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().enumerate().map(|(enum_n, (field_n, field))| {
let field_name = match field.ident {
Some(ref i) => quote!{ self.#i },
None => quote!{ self.#field_n },
};
let poll_stmts = data_struct_fields.iter().enumerate().map(|(field_n, field)| {
let field = field
.ident
.clone()
.expect("Fields of NetworkBehaviour implementation to be named.");

let event_variant = capitalize(field.clone());

let mut wrapped_event = if enum_n != 0 {
let mut wrapped_event = if field_n != 0 {
quote!{ #either_ident::Second(event) }
} else {
quote!{ event }
};
for _ in 0 .. data_struct_fields.len() - 1 - enum_n {
for _ in 0 .. data_struct_fields.len() - 1 - field_n {
wrapped_event = quote!{ #either_ident::First(#wrapped_event) };
}

Expand Down Expand Up @@ -485,23 +457,16 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
out_handler.unwrap_or(quote! {()}) // TODO: See test `empty`.
};

let generate_event_match_arm = if event_process {
quote! {
std::task::Poll::Ready(#network_behaviour_action::GenerateEvent(event)) => {
#net_behv_event_proc::inject_event(self, event)
}
}
} else {
let generate_event_match_arm =
quote! {
std::task::Poll::Ready(#network_behaviour_action::GenerateEvent(event)) => {
return std::task::Poll::Ready(#network_behaviour_action::GenerateEvent(event.into()))
return std::task::Poll::Ready(#network_behaviour_action::GenerateEvent(#event_name::#event_variant(event)))
}
}
};
};

Some(quote!{
loop {
match #trait_to_impl::poll(&mut #field_name, cx, poll_params) {
match #trait_to_impl::poll(&mut self.#field, cx, poll_params) {
#generate_event_match_arm
std::task::Poll::Ready(#network_behaviour_action::Dial { opts, handler: provided_handler }) => {
return std::task::Poll::Ready(#network_behaviour_action::Dial { opts, handler: #provided_handler_and_new_handlers });
Expand All @@ -527,11 +492,14 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {

// Now the magic happens.
let final_quote = quote! {
#event


impl #impl_generics #trait_to_impl for #name #ty_generics
#where_clause
{
type ConnectionHandler = #connection_handler_ty;
type OutEvent = #out_event;
type OutEvent = #event_name;

fn new_handler(&mut self) -> Self::ConnectionHandler {
use #into_connection_handler;
Expand Down Expand Up @@ -645,3 +613,15 @@ fn is_ignored(field: &syn::Field) -> bool {

false
}

fn capitalize(ident: Ident) -> Ident {
let name = ident.to_string();
let mut chars = name.chars();
let capitalized = chars
.next()
.expect("Identifier to have at least one character.")
.to_uppercase()
.collect::<String>()
+ chars.as_str();
Ident::new(&capitalized, Span::call_site().into())
}
Copy link
Contributor

@elenaf9 elenaf9 Jul 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We use capitalize for the Idents of our data struct fields, right? Since those are written in snake_case, shouldn't we iter through all chars, filter out "_" and capitalize the subsequent char? Otherwise we might end up with something like Gossipsub_behaviourEvent.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦 Oh, good catch. Silly me.

Loading