Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[contracts] Add docs generator for the contracts API to the #[define_env] macro #13032

Merged
merged 15 commits into from
Jan 5, 2023
Merged
Changes from 4 commits
Commits
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
83 changes: 77 additions & 6 deletions frame/contracts/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
agryaznov marked this conversation as resolved.
Show resolved Hide resolved
mod #module {
use crate::wasm::runtime::{TrapReason, ReturnCode};
/// Dumb trait for generating host functions docs.
trait Docs {
agryaznov marked this conversation as resolved.
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
}
}

Expand Down Expand Up @@ -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()) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should rename doc to emit_doc. I think this makes it clearer what is happening. I won't die on that hill if you disagree, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to me, concise variant is more elegant and could hardly be misinterpreted

Copy link
Member

@athei athei Jan 4, 2023

Choose a reason for hiding this comment

The 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 define_env invocation in runtime.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

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 doc attr is used as input to define_env(doc) rather than solo (as when it's for a single doc comment)

Also, why doesn't this PR add this attribute to the define_env invocation in runtime.rs?

I was thinking to leave it off by default. But could be done vice versa, why not. Added.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see it is confusing, as here doc attr is used as input to define_env(doc) rather than solo (as when it's for a single doc comment)

Fair enough.

I was thinking to leave it off by default. But could be done vice versa, why not. Added.

Most important point for me is that the standard docs.rs builds of this crate contain this documentation. Therefore it should be on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most important point for me is that the standard docs.rs builds of this crate contain this documentation. Therefore it should be on.

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?

Copy link
Member

@athei athei Jan 4, 2023

Choose a reason for hiding this comment

The 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 #[cfg(doc)] to the emitted api_doc module (and its re-rexports). Then this code will be compiled out for every target but rustdoc.

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(),
}
}