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

refactor: use macros to derive merge_right #1700

Closed
tusharmath opened this issue Apr 11, 2024 · 12 comments · Fixed by #1723
Closed

refactor: use macros to derive merge_right #1700

tusharmath opened this issue Apr 11, 2024 · 12 comments · Fixed by #1723

Comments

@tusharmath
Copy link
Contributor

Currently MergeRight implementation is implemented by hand and is quite verbose. Let's use macro to reduce verbosity.

Technical Requirements

  • Create a new project tailcall-macros to implement this procedural macro.
  • Move trait MergeRight into tailcall-macros.
  • Drop existing hand written implementation of merge_right.

Additional Information
You could start with something like:

[lib]
proc-macro = true

[dependencies]
syn = { version = "1.0", features = ["derive", "full"] }
quote = "1.0"
proc-macro2 = "1.0"

Then, in lib.rs, you can start implementing your macro:

extern crate proc_macro;

use proc_macro::TokenStream;
use quote::quote;
use syn::{parse_macro_input, Data, DeriveInput, Fields};

#[proc_macro_derive(MergeRight)]
pub fn merge_right_derive(input: TokenStream) -> TokenStream {
    let input = parse_macro_input!(input as DeriveInput);

    let name = input.ident;
    let gen = match input.data {
        // Implement for structs
        Data::Struct(data) => {
            let fields = if let Fields::Named(fields) = data.fields {
                fields.named
            } else {
                // Adjust this match arm to handle other kinds of struct fields (unnamed/tuple structs, unit structs)
                unimplemented!()
            };

            let merge_logic = fields.iter().map(|f| {
                let name = &f.ident;
                quote! {
                    #name: self.#name.merge_right(other.#name),
                }
            });

            quote! {
                impl MergeRight for #name {
                    fn merge_right(self, other: Self) -> Self {
                        Self {
                            #(#merge_logic)*
                        }
                    }
                }
            }
        },
        // Implement for enums
        Data::Enum(_) => quote! {
            impl MergeRight for #name {
                fn merge_right(self, other: Self) -> Self {
                    other
                }
            }
        },
        // Optionally handle or disallow unions
        Data::Union(_) => unimplemented!(),
    };

    gen.into()
}
@tusharmath tusharmath changed the title Use macros to derive merge_right refactor: use macros to derive merge_right Apr 11, 2024
@tusharmath
Copy link
Contributor Author

/bounty 100$

Copy link

algora-pbc bot commented Apr 11, 2024

💎 $100 bounty created by tailcallhq
🙋 If you start working on this, comment /attempt #1700 along with your implementation plan
👉 To claim this bounty, submit a pull request that includes the text /claim #1700 somewhere in its body

🙏 Thank you for contributing to tailcallhq/tailcall!.
🧐 Checkout our guidelines before you get started.

Attempt Started (GMT+0) Solution
🔴 @ssddOnTop Apr 11, 2024, 8:47:13 AM WIP
🟢 @Shylock-Hg Apr 13, 2024, 12:43:45 PM #1723

@ssddOnTop
Copy link
Member

ssddOnTop commented Apr 11, 2024

/attempt

Algora profile Completed bounties Tech Active attempts Options
@ssddOnTop 36 tailcallhq bounties
Rust, Java,
C & more
Cancel attempt

Copy link

algora-pbc bot commented Apr 12, 2024

@ssddOnTop: Reminder that in 1 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@ologbonowiwi
Copy link
Contributor

  • Drop existing hand written implementation of merge_right.

From my findings, we'll need to keep the manual implementations for external types (HashSet<T>, Vec<T>, Option<T>, etc) and use the macro on our internal types. Makes sense @tusharmath?

@tusharmath
Copy link
Contributor Author

Yes.

Copy link

algora-pbc bot commented Apr 13, 2024

The bounty is up for grabs! Everyone is welcome to /attempt #1700 🙌

@Shylock-Hg
Copy link
Contributor

Shylock-Hg commented Apr 13, 2024

/attempt

Algora profile Completed bounties Tech Active attempts Options
@Shylock-Hg    3 tailcallhq bounties
+ 3 bounties from 3 projects
C++, C,
Shell & more
Cancel attempt

Copy link

algora-pbc bot commented Apr 13, 2024

💡 @Shylock-Hg submitted a pull request that claims the bounty. You can visit your bounty board to reward.

@ologbonowiwi
Copy link
Contributor

I have a draft locally but forgot to declare my attempt.

In the queue 🤚🏽

Copy link

algora-pbc bot commented Apr 16, 2024

@Shylock-Hg: You've been awarded a $100 bounty by tailcallhq! We'll notify you once it is processed.

Copy link

algora-pbc bot commented Apr 24, 2024

🎉🎈 @Shylock-Hg has been awarded $100! 🎈🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants