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
Show file tree
Hide file tree
Changes from 12 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
153 changes: 136 additions & 17 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 @@ -158,6 +161,7 @@ struct HostFn {
name: String,
returns: HostFnReturn,
is_stable: bool,
alias_to: Option<String>,
}

enum HostFnReturn {
Expand Down Expand Up @@ -187,7 +191,7 @@ impl ToTokens for HostFn {
}

impl HostFn {
pub fn try_from(item: syn::ItemFn) -> syn::Result<Self> {
pub fn try_from(mut item: syn::ItemFn) -> syn::Result<Self> {
let err = |span, msg| {
let msg = format!("Invalid host function definition. {}", msg);
syn::Error::new(span, msg)
Expand All @@ -198,10 +202,10 @@ impl HostFn {
"only #[version(<u8>)], #[unstable] and #[prefixed_alias] attributes are allowed.";
let span = item.span();
let mut attrs = item.attrs.clone();
attrs.retain(|a| !(a.path.is_ident("doc") || a.path.is_ident("prefixed_alias")));
let name = item.sig.ident.to_string();
attrs.retain(|a| !a.path.is_ident("doc"));
let mut maybe_module = None;
let mut is_stable = true;
let mut alias_to = None;
while let Some(attr) = attrs.pop() {
let ident = attr.path.get_ident().ok_or(err(span, msg))?.to_string();
match ident.as_str() {
Expand All @@ -219,9 +223,17 @@ impl HostFn {
}
is_stable = false;
},
"prefixed_alias" => {
alias_to = Some(item.sig.ident.to_string());
item.sig.ident = syn::Ident::new(
&format!("seal_{}", &item.sig.ident.to_string()),
item.sig.ident.span(),
);
},
_ => return Err(err(span, msg)),
}
}
let name = item.sig.ident.to_string();

// process arguments: The first and second arg are treated differently (ctx, memory)
// they must exist and be `ctx: _` and `memory: _`.
Expand Down Expand Up @@ -317,6 +329,7 @@ impl HostFn {
name,
returns,
is_stable,
alias_to,
})
},
_ => Err(err(span, &msg)),
Expand Down Expand Up @@ -348,19 +361,15 @@ impl EnvDef {
.iter()
.filter_map(extract_fn)
.filter(|i| i.attrs.iter().any(selector))
.map(|mut i| {
i.attrs.retain(|i| !selector(i));
i.sig.ident = syn::Ident::new(
&format!("seal_{}", &i.sig.ident.to_string()),
i.sig.ident.span(),
);
i
})
.map(|i| HostFn::try_from(i));

let host_funcs = items
.iter()
.filter_map(extract_fn)
.map(|mut i| {
i.attrs.retain(|i| !selector(i));
i
})
.map(|i| HostFn::try_from(i))
.chain(aliases)
.collect::<Result<Vec<_>, _>>()?;
Expand All @@ -383,16 +392,111 @@ 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_doc = if let Some(origin_fn) = &f.alias_to {
let alias_doc = format!(
"This is just an alias function to [`{0}()`][`Self::{0}`] with backwards-compatible prefixed identifier.",
origin_fn,
);
quote! { #[doc = #alias_doc] }

} else {
let func_docs = f.item.attrs.iter().filter(|a| doc_selector(a)).map(|d| {
let docs = d.to_token_stream();
quote! { #docs }
});
let unstable_notice = if !f.is_stable {
let warning = "\n # Unstable\n\n \
This function is unstable and it is a subject to change (or removal) in the future.\n \
Do not deploy a contract using it to a production chain.";
quote! { #[doc = #warning] }
} else {
quote! {}
};
quote! {
#( #func_docs )*
#unstable_notice
}
};
quote! {
#func_doc
#func_decl;
}
} else {
quote! {}
}
});

let module = Ident::new(m, Span::call_site());
let module_doc = format!(
"Documentation of the API available to contracts by importing `{}` WASM module.",
module
);

quote! {
#[doc = #module_doc]
pub mod #module {
use crate::wasm::runtime::{TrapReason, ReturnCode};
/// Every function in this trait represents (at least) one function that can be imported by a contract.
///
/// The function's identifier is to be set as the name in the import definition.
/// Where it is specifically indicated, an _alias_ function having `seal_`-prefixed identifier and
/// just the same signature and body, is also available (for backwards-compatibility purposes).
pub trait Api {
#( #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
pub use docs as api_doc;
athei marked this conversation as resolved.
Show resolved Hide resolved
/// Contains the documentation of the API available to contracts.
///
/// In order to generate this documentation, pass `doc` attribute to the [`#[define_env]`][`macro@define_env`] macro:
/// `#[define_env(doc)]`, and then run `cargo doc`.
///
/// This module is not meant to be used by any code. Rather, it is meant to be consumed by humans through rustdoc.
///
/// Every function described in this module's sub module's traits uses this sub module's identifier
/// as its imported module name. The identifier of the function is the function's imported name.
/// According to the [WASM spec of imports](https://webassembly.github.io/spec/core/text/modules.html#text-import).
pub mod docs {
#docs
}
}
}

Expand Down Expand Up @@ -579,18 +683,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::api_doc::seal0`, `pallet_contracts::api_doc::seal1`,
/// `...` modules each having its `Api` trait containing functions holding documentation for every
/// host function defined by the macro.
///
/// To build up these docs, run:
///
/// ```nocompile
/// cargo doc
/// ```
#[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(),
}
}
2 changes: 1 addition & 1 deletion frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ enum FrameArgs<'a, T: Config, E> {
/// If `None` the contract info needs to be reloaded from storage.
cached_info: Option<ContractInfo<T>>,
/// This frame was created by `seal_delegate_call` and hence uses different code than
/// what is stored at [`Self::dest`]. Its caller ([`Frame::delegated_caller`]) is the
/// what is stored at [`Self::Call::dest`]. Its caller ([`DelegatedCall::caller`]) is the
/// account which called the caller contract
delegated_call: Option<DelegatedCall<T, E>>,
},
Expand Down
1 change: 1 addition & 0 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ mod schedule;
mod storage;
mod wasm;

pub use wasm::api_doc;
athei marked this conversation as resolved.
Show resolved Hide resolved
pub mod chain_extension;
pub mod weights;

Expand Down
8 changes: 5 additions & 3 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ mod runtime;
pub use crate::wasm::code_cache::reinstrument;
pub use crate::wasm::{
prepare::TryInstantiate,
runtime::{CallFlags, Environment, ReturnCode, Runtime, RuntimeCosts},
runtime::{api_doc, CallFlags, Environment, ReturnCode, Runtime, RuntimeCosts},
};

use crate::{
exec::{ExecResult, Executable, ExportedFunction, Ext},
gas::GasMeter,
Expand Down Expand Up @@ -144,7 +145,7 @@ impl<T: Config> PrefabWasmModule<T> {
/// Create the module by checking and instrumenting `original_code`.
///
/// This does **not** store the module. For this one need to either call [`Self::store`]
/// or [`<Self as Executable>::execute`].
/// or [`<Self as Executable>::execute`][`Executable::execute`].
pub fn from_code(
original_code: Vec<u8>,
schedule: &Schedule<T>,
Expand All @@ -164,7 +165,8 @@ impl<T: Config> PrefabWasmModule<T> {

/// Store the code without instantiating it.
///
/// Otherwise the code is stored when [`<Self as Executable>::execute`] is called.
/// Otherwise the code is stored when [`<Self as Executable>::execute`][`Executable::execute`]
/// is called.
pub fn store(self) -> DispatchResult {
code_cache::store(self, false)
}
Expand Down
Loading