-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[contracts] Add docs generator for the contracts API to the #[define_env] macro
#13032
Changes from 4 commits
04769ea
e7f6b78
606d58d
6cf881d
c558c5e
cd2007b
ce9031b
dced64c
98a6c63
ab36942
f8def8c
f118f74
fff947e
1af74cc
1272108
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,9 +30,12 @@ use alloc::{ | |
| vec::Vec, | ||
| }; | ||
| use proc_macro::TokenStream; | ||
| use proc_macro2::TokenStream as TokenStream2; | ||
| use proc_macro2::{Span, TokenStream as TokenStream2}; | ||
| use quote::{quote, quote_spanned, ToTokens}; | ||
| use syn::{parse_macro_input, spanned::Spanned, Data, DeriveInput, FnArg, Ident}; | ||
| use syn::{ | ||
| parse_macro_input, punctuated::Punctuated, spanned::Spanned, token::Comma, Data, DeriveInput, | ||
| FnArg, Ident, | ||
| }; | ||
|
|
||
| /// This derives `Debug` for a struct where each field must be of some numeric type. | ||
| /// It interprets each field as its represents some weight and formats it as times so that | ||
|
|
@@ -383,16 +386,69 @@ fn is_valid_special_arg(idx: usize, arg: &FnArg) -> bool { | |
| matches!(*pat.ty, syn::Type::Infer(_)) | ||
| } | ||
|
|
||
| /// Expands documentation for host functions. | ||
| fn expand_docs(def: &mut EnvDef) -> TokenStream2 { | ||
| let mut modules = def.host_funcs.iter().map(|f| f.module.clone()).collect::<Vec<_>>(); | ||
| modules.sort(); | ||
| modules.dedup(); | ||
|
|
||
| let doc_selector = |a: &syn::Attribute| a.path.is_ident("doc"); | ||
| let docs = modules.iter().map(|m| { | ||
| let funcs = def.host_funcs.iter_mut().map(|f| { | ||
| if *m == f.module { | ||
| // Remove auxiliary args: `ctx: _` and `memory: _` | ||
| f.item.sig.inputs = f | ||
| .item | ||
| .sig | ||
| .inputs | ||
| .iter() | ||
| .skip(2) | ||
| .map(|p| p.clone()) | ||
| .collect::<Punctuated<FnArg, Comma>>(); | ||
| let func_decl = f.item.sig.to_token_stream(); | ||
| let func_docs = f.item.attrs.iter().filter(|a| doc_selector(a)).map(|d| { | ||
| let docs = d.to_token_stream(); | ||
| quote! { #docs } | ||
| }); | ||
| quote! { | ||
| #( #func_docs )* | ||
| #func_decl; | ||
| } | ||
| } else { | ||
| quote! {} | ||
| } | ||
| }); | ||
|
|
||
| let module = Ident::new(m, Span::call_site()); | ||
|
|
||
| quote! { | ||
| /// Documentation for the seal module host functions. | ||
| mod #module { | ||
| use crate::wasm::runtime::{TrapReason, ReturnCode}; | ||
| /// Dumb trait for generating host functions docs. | ||
| trait Docs { | ||
agryaznov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| #( #funcs )* | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| quote! { | ||
| #( #docs )* | ||
| } | ||
| } | ||
|
|
||
| /// Expands environment definiton. | ||
| /// Should generate source code for: | ||
| /// - implementations of the host functions to be added to the wasm runtime environment (see | ||
| /// `expand_impls()`). | ||
| fn expand_env(def: &mut EnvDef) -> TokenStream2 { | ||
| fn expand_env(def: &mut EnvDef, docs: bool) -> TokenStream2 { | ||
| let impls = expand_impls(def); | ||
| let docs = docs.then_some(expand_docs(def)).unwrap_or(TokenStream2::new()); | ||
|
|
||
| quote! { | ||
| pub struct Env; | ||
| #impls | ||
| #docs | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -579,18 +635,33 @@ fn expand_functions( | |
| /// | ||
| /// The implementation on `()` can be used in places where no `Ext` exists, yet. This is useful | ||
| /// when only checking whether a code can be instantiated without actually executing any code. | ||
| /// | ||
| /// # Generating Documentation | ||
| /// | ||
| /// Passing `doc` attribute to the macro (like `#[define_env(doc)]`) will make it also expand | ||
| /// additional `pallet_contracts::wasm::runtime::seal0`, `pallet_contracts::wasm::runtime::seal1`, | ||
| /// `...` modules each having its `Doc` trait containing methods holding documentation for every | ||
| /// defined host function. | ||
| /// | ||
| /// To build up these docs, run: | ||
| /// | ||
| /// ```nocompile | ||
| /// cargo doc --no-deps --document-private-items | ||
| /// ``` | ||
| #[proc_macro_attribute] | ||
| pub fn define_env(attr: TokenStream, item: TokenStream) -> TokenStream { | ||
| if !attr.is_empty() { | ||
| let msg = "Invalid `define_env` attribute macro: expected no attributes: `#[define_env]`."; | ||
| if !attr.is_empty() && !(attr.to_string() == "doc".to_string()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should rename
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to me, concise variant is more elegant and could hardly be misinterpreted
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But isn't it the same attribute name used for rust doc comments itself? To be that seems confusing. Also, why doesn't this PR add this attribute to the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, that's exactly the point. The idea is to make it short, appropriate and familiar to the user: at the end of the day, it is for generating the rustdoc. I don't see it is confusing, as here
I was thinking to leave it off by default. But could be done vice versa, why not. Added.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fair enough.
Most important point for me is that the standard docs.rs builds of this crate contain this documentation. Therefore it should be on.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Gotcha. The only objection comes to my mind is that it probably should be turned off for production use not to bloat runtime size in vain. Should we add a note to the macro docs at least?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should not bloat the runtime size as it is dead code and hence removed by the compiler. At least when using lto. But you have a point there. I think you should just add |
||
| let msg = r#"Invalid `define_env` attribute macro: expected either no attributes or a single `doc` attibute: | ||
| - `#[define_env]` | ||
| - `#[define_env(doc)]`"#; | ||
| let span = TokenStream2::from(attr).span(); | ||
| return syn::Error::new(span, msg).to_compile_error().into() | ||
| } | ||
|
|
||
| let item = syn::parse_macro_input!(item as syn::ItemMod); | ||
|
|
||
| match EnvDef::try_from(item) { | ||
| Ok(mut def) => expand_env(&mut def).into(), | ||
| Ok(mut def) => expand_env(&mut def, !attr.is_empty()).into(), | ||
| Err(e) => e.to_compile_error().into(), | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.