From 787d5d9810dfc71006f3ed0c8a21290be5ea6c54 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 5 Aug 2021 01:53:48 +0000 Subject: [PATCH] Note which parameters or return values are (secretly) Options Because `Option` is mapped the same as `OpaqueType` its not immediately clear which values in the API are actually `Option<>`al. Thus, we should at least have documentation noting this. --- c-bindings-gen/src/blocks.rs | 71 ++++++++++++++++++++++++++++++++++++ c-bindings-gen/src/main.rs | 22 +++++------ c-bindings-gen/src/types.rs | 2 +- 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/c-bindings-gen/src/blocks.rs b/c-bindings-gen/src/blocks.rs index 643a8be..9c40441 100644 --- a/c-bindings-gen/src/blocks.rs +++ b/c-bindings-gen/src/blocks.rs @@ -377,8 +377,34 @@ pub fn write_option_block(w: &mut W, mangled_container: &str, } } +/// Prints the docs from a given attribute list unless its tagged no export +pub fn writeln_fn_docs<'a, W: std::io::Write, I>(w: &mut W, attrs: &[syn::Attribute], prefix: &str, types: &mut TypeResolver, generics: Option<&GenericTypes>, args: I, ret: &syn::ReturnType) where I: Iterator { + writeln_docs_impl(w, attrs, prefix, Some((types, generics, + args.filter_map(|arg| if let syn::FnArg::Typed(ty) = arg { + if let syn::Pat::Ident(id) = &*ty.pat { + Some((id.ident.to_string(), &*ty.ty)) + } else { unimplemented!() } + } else { None }), + if let syn::ReturnType::Type(_, ty) = ret { Some(&**ty) } else { None }, + None + ))); +} + /// Prints the docs from a given attribute list unless its tagged no export pub fn writeln_docs(w: &mut W, attrs: &[syn::Attribute], prefix: &str) { + writeln_docs_impl(w, attrs, prefix, None::<(_, _, std::vec::Drain<'_, (String, &syn::Type)>, _, _)>); +} + +pub fn writeln_arg_docs<'a, W: std::io::Write, I>(w: &mut W, attrs: &[syn::Attribute], prefix: &str, types: &mut TypeResolver, generics: Option<&GenericTypes>, args: I, ret: Option<&syn::Type>) where I: Iterator { + writeln_docs_impl(w, attrs, prefix, Some((types, generics, args, ret, None))) +} + +pub fn writeln_field_docs(w: &mut W, attrs: &[syn::Attribute], prefix: &str, types: &mut TypeResolver, generics: Option<&GenericTypes>, field: &syn::Type) { + writeln_docs_impl(w, attrs, prefix, Some((types, generics, vec![].drain(..), None, Some(field)))) +} + +/// Prints the docs from a given attribute list unless its tagged no export +fn writeln_docs_impl<'a, W: std::io::Write, I>(w: &mut W, attrs: &[syn::Attribute], prefix: &str, method_args_ret: Option<(&mut TypeResolver, Option<&GenericTypes>, I, Option<&syn::Type>, Option<&syn::Type>)>) where I: Iterator { for attr in attrs.iter() { let tokens_clone = attr.tokens.clone(); let mut token_iter = tokens_clone.into_iter(); @@ -414,6 +440,51 @@ pub fn writeln_docs(w: &mut W, attrs: &[syn::Attribute], pref }, } } + if let Some((types, generics, inp, outp, field)) = method_args_ret { + let mut nullable_found = false; + for (name, inp) in inp { + if types.skip_arg(inp, generics) { continue; } + if if let syn::Type::Reference(syn::TypeReference { elem, .. }) = inp { + if let syn::Type::Path(syn::TypePath { ref path, .. }) = &**elem { + types.is_path_transparent_container(path, generics, true) + } else { false } + } else if let syn::Type::Path(syn::TypePath { ref path, .. }) = inp { + types.is_path_transparent_container(path, generics, true) + } else { false } { + // Note downstream clients match this text exactly so any changes may require + // changes in the Java and Swift bindings, at least. + if !nullable_found { writeln!(w, "{}///", prefix).unwrap(); } + nullable_found = true; + writeln!(w, "{}/// Note that {} (or a relevant inner pointer) may be NULL or all-0s to represent None", prefix, name).unwrap(); + } + } + if if let Some(syn::Type::Reference(syn::TypeReference { elem, .. })) = outp { + if let syn::Type::Path(syn::TypePath { ref path, .. }) = &**elem { + types.is_path_transparent_container(path, generics, true) + } else { false } + } else if let Some(syn::Type::Path(syn::TypePath { ref path, .. })) = outp { + types.is_path_transparent_container(path, generics, true) + } else { false } { + // Note downstream clients match this text exactly so any changes may require + // changes in the Java and Swift bindings, at least. + if !nullable_found { writeln!(w, "{}///", prefix).unwrap(); } + nullable_found = true; + writeln!(w, "{}/// Note that the return value (or a relevant inner pointer) may be NULL or all-0s to represent None", prefix).unwrap(); + } + if if let Some(syn::Type::Reference(syn::TypeReference { elem, .. })) = field { + if let syn::Type::Path(syn::TypePath { ref path, .. }) = &**elem { + types.is_path_transparent_container(path, generics, true) + } else { false } + } else if let Some(syn::Type::Path(syn::TypePath { ref path, .. })) = field { + types.is_path_transparent_container(path, generics, true) + } else { false } { + // Note downstream clients match this text exactly so any changes may require + // changes in the Java and Swift bindings, at least. + if !nullable_found { writeln!(w, "{}///", prefix).unwrap(); } + writeln!(w, "{}/// Note that this (or a relevant inner pointer) may be NULL or all-0s to represent None", prefix).unwrap(); + } + } + } /// Print the parameters in a method declaration, starting after the open parenthesis, through and diff --git a/c-bindings-gen/src/main.rs b/c-bindings-gen/src/main.rs index c8adf7d..cb7305b 100644 --- a/c-bindings-gen/src/main.rs +++ b/c-bindings-gen/src/main.rs @@ -258,7 +258,7 @@ fn writeln_trait<'a, 'b, W: std::io::Write>(w: &mut W, t: &'a syn::ItemTrait, ty let mut meth_gen_types = gen_types.push_ctx(); assert!(meth_gen_types.learn_generics(&m.sig.generics, types)); - writeln_docs(w, &m.attrs, "\t"); + writeln_fn_docs(w, &m.attrs, "\t", types, Some(&meth_gen_types), m.sig.inputs.iter(), &m.sig.output); if let syn::ReturnType::Type(_, rtype) = &m.sig.output { if let syn::Type::Reference(r) = &**rtype { @@ -293,7 +293,7 @@ fn writeln_trait<'a, 'b, W: std::io::Write>(w: &mut W, t: &'a syn::ItemTrait, ty } let mut cpp_docs = Vec::new(); - writeln_docs(&mut cpp_docs, &m.attrs, "\t * "); + writeln_fn_docs(&mut cpp_docs, &m.attrs, "\t * ", types, Some(&meth_gen_types), m.sig.inputs.iter(), &m.sig.output); let docs_string = "\t/**\n".to_owned() + &String::from_utf8(cpp_docs).unwrap().replace("///", "") + "\t */\n"; write!(w, "\tpub {}: extern \"C\" fn (", m.sig.ident).unwrap(); @@ -617,7 +617,7 @@ fn writeln_struct<'a, 'b, W: std::io::Write>(w: &mut W, s: &'a syn::ItemStruct, and_token: syn::Token!(&)(Span::call_site()), lifetime: None, mutability: None, elem: Box::new(field.ty.clone()) }); if types.understood_c_type(&ref_type, Some(&gen_types)) { - writeln_docs(w, &field.attrs, ""); + writeln_arg_docs(w, &field.attrs, "", types, Some(&gen_types), vec![].drain(..), Some(&ref_type)); write!(w, "#[no_mangle]\npub extern \"C\" fn {}_get_{}(this_ptr: &{}) -> ", struct_name, ident, struct_name).unwrap(); types.write_c_type(w, &ref_type, Some(&gen_types), true); write!(w, " {{\n\tlet mut inner_val = &mut unsafe {{ &mut *this_ptr.inner }}.{};\n\t", ident).unwrap(); @@ -630,7 +630,7 @@ fn writeln_struct<'a, 'b, W: std::io::Write>(w: &mut W, s: &'a syn::ItemStruct, } if types.understood_c_type(&field.ty, Some(&gen_types)) { - writeln_docs(w, &field.attrs, ""); + writeln_arg_docs(w, &field.attrs, "", types, Some(&gen_types), vec![("val".to_owned(), &field.ty)].drain(..), None); write!(w, "#[no_mangle]\npub extern \"C\" fn {}_set_{}(this_ptr: &mut {}, mut val: ", struct_name, ident, struct_name).unwrap(); types.write_c_type(w, &field.ty, Some(&gen_types), false); write!(w, ") {{\n\t").unwrap(); @@ -1058,8 +1058,10 @@ fn writeln_impl(w: &mut W, i: &syn::ItemImpl, types: &mut Typ ExportStatus::NoExport|ExportStatus::TestOnly => continue, ExportStatus::NotImplementable => panic!("(C-not implementable) must only appear on traits"), } + let mut meth_gen_types = gen_types.push_ctx(); + assert!(meth_gen_types.learn_generics(&m.sig.generics, types)); if m.defaultness.is_some() { unimplemented!(); } - writeln_docs(w, &m.attrs, ""); + writeln_fn_docs(w, &m.attrs, "", types, Some(&meth_gen_types), m.sig.inputs.iter(), &m.sig.output); if let syn::ReturnType::Type(_, _) = &m.sig.output { writeln!(w, "#[must_use]").unwrap(); } @@ -1069,8 +1071,6 @@ fn writeln_impl(w: &mut W, i: &syn::ItemImpl, types: &mut Typ DeclType::StructImported => format!("{}", ident), _ => unimplemented!(), }; - let mut meth_gen_types = gen_types.push_ctx(); - assert!(meth_gen_types.learn_generics(&m.sig.generics, types)); write_method_params(w, &m.sig, &ret_type, types, Some(&meth_gen_types), false, true); write!(w, " {{\n\t").unwrap(); write_method_var_decl_body(w, &m.sig, "", types, Some(&meth_gen_types), false); @@ -1204,7 +1204,7 @@ fn writeln_enum<'a, 'b, W: std::io::Write>(w: &mut W, e: &'a syn::ItemEnum, type writeln!(w, " {{").unwrap(); for field in fields.named.iter() { if export_status(&field.attrs) == ExportStatus::TestOnly { continue; } - writeln_docs(w, &field.attrs, "\t\t"); + writeln_field_docs(w, &field.attrs, "\t\t", types, Some(&gen_types), &field.ty); write!(w, "\t\t{}: ", field.ident.as_ref().unwrap()).unwrap(); types.write_c_type(w, &field.ty, Some(&gen_types), false); writeln!(w, ",").unwrap(); @@ -1382,11 +1382,11 @@ fn writeln_fn<'a, 'b, W: std::io::Write>(w: &mut W, f: &'a syn::ItemFn, types: & ExportStatus::NoExport|ExportStatus::TestOnly => return, ExportStatus::NotImplementable => panic!("(C-not implementable) must only appear on traits"), } - writeln_docs(w, &f.attrs, ""); - let mut gen_types = GenericTypes::new(None); if !gen_types.learn_generics(&f.sig.generics, types) { return; } + writeln_fn_docs(w, &f.attrs, "", types, Some(&gen_types), f.sig.inputs.iter(), &f.sig.output); + write!(w, "#[no_mangle]\npub extern \"C\" fn {}(", f.sig.ident).unwrap(); write_method_params(w, &f.sig, "", types, Some(&gen_types), false, true); write!(w, " {{\n\t").unwrap(); @@ -1522,7 +1522,7 @@ fn convert_file<'a, 'b>(libast: &'a FullLibraryAST, crate_types: &CrateTypes<'a> if let syn::Type::Path(p) = &*c.ty { let resolved_path = type_resolver.resolve_path(&p.path, None); if type_resolver.is_primitive(&resolved_path) { - writeln_docs(&mut out, &c.attrs, ""); + writeln_field_docs(&mut out, &c.attrs, "", &mut type_resolver, None, &*c.ty); writeln!(out, "\n#[no_mangle]").unwrap(); writeln!(out, "pub static {}: {} = {}::{};", c.ident, resolved_path, module, c.ident).unwrap(); } diff --git a/c-bindings-gen/src/types.rs b/c-bindings-gen/src/types.rs index ff1d4b8..13c3b0e 100644 --- a/c-bindings-gen/src/types.rs +++ b/c-bindings-gen/src/types.rs @@ -1263,7 +1263,7 @@ impl<'a, 'c: 'a> TypeResolver<'a, 'c> { } /// Returns true if the path is a "transparent" container, ie an Option or a container which does /// not require a generated continer class. - fn is_path_transparent_container(&self, full_path: &syn::Path, generics: Option<&GenericTypes>, is_ref: bool) -> bool { + pub fn is_path_transparent_container(&self, full_path: &syn::Path, generics: Option<&GenericTypes>, is_ref: bool) -> bool { let inner_iter = match &full_path.segments.last().unwrap().arguments { syn::PathArguments::None => return false, syn::PathArguments::AngleBracketed(args) => args.args.iter().map(|arg| { -- 2.39.5