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

Procedural macros design document #1109

Merged
merged 1 commit into from
Feb 27, 2024
Merged

Conversation

maciektr
Copy link
Contributor

@maciektr maciektr commented Feb 1, 2024

No description provided.

design/01-proc-macro.md Outdated Show resolved Hide resolved
design/01-proc-macro.md Show resolved Hide resolved
design/01-proc-macro.md Outdated Show resolved Hide resolved
design/01-proc-macro.md Outdated Show resolved Hide resolved
design/01-proc-macro.md Show resolved Hide resolved
design/01-proc-macro.md Show resolved Hide resolved
design/01-proc-macro.md Show resolved Hide resolved
design/01-proc-macro.md Outdated Show resolved Hide resolved
@maciektr maciektr force-pushed the maciektr/proc-macro-design branch 2 times, most recently from 48cbcb2 to ba702c8 Compare February 7, 2024 13:16
Copy link
Contributor

@MaksymilianDemitraszek MaksymilianDemitraszek left a comment

Choose a reason for hiding this comment

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

Don't see any major issues

design/01-proc-macro.md Outdated Show resolved Hide resolved
@maciektr maciektr force-pushed the maciektr/proc-macro-design branch 2 times, most recently from af1e183 to 2bb66a3 Compare February 8, 2024 11:40
Copy link
Contributor

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

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

LGTM, really good job 🎉

design/01-proc-macro.md Show resolved Hide resolved
Copy link
Contributor

@tomek0123456789 tomek0123456789 left a comment

Choose a reason for hiding this comment

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

Didn't understand anything beforehand, now I understand something, thanks and good job!

design/01-proc-macro.md Outdated Show resolved Hide resolved
design/01-proc-macro.md Outdated Show resolved Hide resolved
@maciektr maciektr force-pushed the maciektr/proc-macro-design branch from 59a1055 to 61f42ec Compare February 12, 2024 11:56
@maciektr maciektr requested a review from mkaput February 12, 2024 11:56
@glihm
Copy link
Contributor

glihm commented Feb 12, 2024

Looks great, amazing unlock for customization and abstraction in projects!

  1. I'm not sure having read something about a possible limitation of one macro by package. Will it be like in rust, we can expose several macros in the same package?
  2. In the rust code, the TokenStream a macro receives is exclusively the part of the tree that is concerned by the macro I guess:
#[derive(ToValue)]
struct Input {
    value: felt252,
}
// Let's say something similar to that.
#[derive_macro(ToValue)]
pub fn to_value(token_stream: TokenStream) -> ProcMacroResult {
    // only access the struct Input.
    ProcMacroResult::Leave
}

Is it considerable to give more context to the macro? I'm imagining some scenario where for instance the full path of the module in which the macro is executed, the crate id or even the file name could be useful. Or is this something not available at the time the macro is being called?

Awesome job on that guys, keen to contribute on that and see it evolve! 🚀

@maciektr maciektr force-pushed the maciektr/proc-macro-design branch from 61f42ec to c0c8a81 Compare February 12, 2024 17:14
design/01-proc-macro.md Outdated Show resolved Hide resolved
@maciektr maciektr force-pushed the maciektr/proc-macro-design branch from c0c8a81 to 8022889 Compare February 14, 2024 17:01
@maciektr
Copy link
Contributor Author

Thanks for feedback @glihm !

Looks great, amazing unlock for customization and abstraction in projects!

😍

  1. I'm not sure having read something about a possible limitation of one macro by package. Will it be like in rust, we can expose several macros in the same package?

Yes, it will be possible! Possibly not from the day one, but not long after it.

Is it considerable to give more context to the macro? I'm imagining some scenario where for instance the full path of the module in which the macro is executed, the crate id or even the file name could be useful. Or is this something not available at the time the macro is being called?

As of the design, the ProcMacroHost that would be responsible for execution of the procedural macros, is in fact just a Cairo plugin. Consequently, it has access to the Cairo compiler database, and some additional metadata. So it would be technically possible to get the full module path, crate id etc. at the time of procedural macro execution.

On the other hand, the current design proposes running procedural macro's only on simple and explicit request from the user made in the Cairo code, for instance by adding the #[derive(MyMacro)] attribute to their struct. So my concern is, that additional, custom logic based on some module metadata hard-coded into the procedural macro would obfuscate the compilation process quite a bit 😟. Although, if the community decides otherwise - supporting additional metadata in the future should not be complicated.

@maciektr maciektr requested a review from mkaput February 14, 2024 17:52
@maciektr maciektr force-pushed the maciektr/proc-macro-design branch from 8022889 to 20bc7a3 Compare February 16, 2024 09:31
@maciektr maciektr force-pushed the maciektr/proc-macro-design branch from 20bc7a3 to d04abd4 Compare February 19, 2024 11:00
@maciektr
Copy link
Contributor Author

Hi everyone!
We will be merging this design document this week, so make sure to submit all your comments!

@milancermak
Copy link

I've read through the doc. The whole apparatus seems very well though through as far as I can tell. Sadly, I haven't written a Rust macro in my life, but I have a couple of (potentially dumb, please bear with me) questions based on what I'd like to do with macros.

  1. Will this be possible?
#[derive(Packing(v0))]
struct Values {
    a: u64,
    b: u8,
    c: u16,
    d: u128
}

The idea is that this macro would expand into a full implementation of StorePacking for the Values struct. I'm particularly interested in the v0 part, because in the future, there can be a v1 and v2 that offer a smarter but backwards incompatible packing algorithm under the hood.

  1. I'd like to have autogenerated getters (and maybe setters). Imagine having
struct Storage {
    #[getter]
    foo: u64,
    #[getter]
    bar: LegacyMap<ContractAddress, u128>
    #[getter]
    #[setter(guard=only_admin)]
    baz: ContractAddress
}

that would expand into sth like

fn get_foo(self: @ContractState) -> u64 {
    self.foo.read()
}

fn get_bar(self: @ContractState, value: ContractAddress) -> u128 {
    self.bar.read(value)
}

fn get_baz(self: @ContractState) -> ContractAddress {
    self.baz.read()
}

fn set_baz(ref self: ContractState, new_value: ContractAddress) {
    assert_only_admin();
    self.baz.write(new_value);
}

Will it be possible to have macros "stacked" like the #[getter] + #[setter] combo?

Also note that while this example looks cool, it complicates writing the #[starknet::interface] for the contract because the autogenerated fns would have to either be declared inside the #[starknet::interface] trait manually (which just complicates dev-ex) or be placed inside the contract but then available under a different interface. Maybe there's a more elegant solution to it, I haven't thought about it deeply.

  1. This is probably too much on the crazy scale, but would it be possible to "hook into" the already present stuff like #[starknet::interface] and run some more macro expansion when that happens?

  2. Let's say I want to mark a fn as non reentrant:

#[starknet::contract]
mod C {
    /// ....
    
    #[abi(embed_v0)]
    impl Foo of super::IFoo<ContractState> {
        /// ...
        
        #[non_reentrant]
        fn do_stuff_with_callback(ref self: ContractState) {
            /// ...
        }
    }
}

Now in this case, say the macro has a dependency on OZ's cairo-contracts, so it pulls in the reentrancy guard component. Can the non_reentrant macro place the component! code in the mod C scope (that is, two levels "above" it)? Will the code compile correctly, since component! is another macro, albeit a "native" one? What if I want to have multiple #[non_reentrant] fns in my impl? Will I be able to write the macro's Rust code in a way that it can detect there's already a component! in scope from a previous expansion? I hope this all makes sense... 😇

  1. Will it be possible to write a macro that looks sth like tag! "foo", that is, without enclosing parentheses? Similar to the html! macro mentioned in the linked posts.

  2. As mentioned, I have 0 experience writing Rust macros, but I imagine it would be great to have some kind of tool to help debug macro expansion. Are you thinking of offering sth like that, to make writing macros easier?

@maciektr
Copy link
Contributor Author

Hi @milancermak !

This are all really great questions! Thanks for your input.

I don’t think I will exhaust all of these topics with my response, but I will try to give my thoughts right away. Please note, that the design doc in review is the first attempt at bringing said functionality to Cairo, so we intentionally want to keep things simple. Some topics mentioned here should ultimately become separate issues and be discussed as such.

By question number:

  1. Why not just PackingV0 then? How would this be different? 🤔
  2. I would imagine this to be implemented as a derive macro on struct, rather than attributes on struct fields. This way the macro implementation can modify struct implementation directly. As far as I know, that would also be the way to implement this in Rust (see getset or derive_getters libs as an example). As for “stacking” - yes, it will be possible to call multiple attribute macros on a single item.
  3. If I understand correctly what you mean, I don’t think so. We believe that the execution of macro implementation should be independent from other macros and plugins. They can be executed sequentially though.
  4. “Can the (...) macro place the component! code in mod C scope (…) two levels "above" it?”
    No, the current design implies that the attribute can only change implementation of the item it is defined on (in this case the do_stuff_with_callback ). This makes the consequences of the proc macro execution more natural for Cairo code reader, by reducing the number of possible side effects.
    ”Will the code compile correctly, since component! is another macro”
    Code generated by macros can call another macros. Unfortunately, as I said in previous point, attribute defined for a function cannot change the “parent” module definition.
  5. Good point! Ultimately, I would love to support impls as macro params (i.e. macro! { smth }) in Cairo, like it is supported in Rust. Quite frankly, I am not yet sure if we would need to make some changes to Cairo parser to implement this, or not. Sopossibly not from day one.
  6. We would like to support the expansion in the Cairo language server after it is available in Scarb, but this will require additional work on the LS side.

@maciektr
Copy link
Contributor Author

I would like to extend my thanks to everyone engaged in review and comments of this document. 😍 😍 😍
I will merge this now, but please feel welcomed to continue the discussion in issues, feature requests, other PRs and our support channels.

@maciektr maciektr added this pull request to the merge queue Feb 26, 2024
Merged via the queue into main with commit 0f7c071 Feb 27, 2024
21 checks passed
@maciektr maciektr deleted the maciektr/proc-macro-design branch February 27, 2024 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants