From: Matt Corallo <git@bluematt.me>
Date: Fri, 23 Sep 2022 16:12:46 +0000 (+0000)
Subject: Prefer `Option_` mappings for more types over transparent mappings
X-Git-Tag: v0.0.111.0~1^2~8
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=c9189547e9e06bec4303a5d23de8bf3387a8e9a0;p=ldk-c-bindings

Prefer `Option_` mappings for more types over transparent mappings

In general, the explicit `Option_` mappings are easier for
downstream bindings to match against, and clearer for users anyway.
Sadly, originally we were trying to avoid them where possible, so
some types default to transparent mappings. Here we swap the
default, at least for manually-mapped types, with a few explicit
exceptions (that probably should be changed as well).
---

diff --git a/c-bindings-gen/src/types.rs b/c-bindings-gen/src/types.rs
index c6d69cc..c35b7ae 100644
--- a/c-bindings-gen/src/types.rs
+++ b/c-bindings-gen/src/types.rs
@@ -1381,7 +1381,19 @@ impl<'a, 'c: 'a> TypeResolver<'a, 'c> {
 					if let Some(resolved) = self.maybe_resolve_path(&p.path, generics) {
 						if self.c_type_has_inner_from_path(&resolved) { return true; }
 						if self.is_primitive(&resolved) { return false; }
-						if self.c_type_from_path(&resolved, false, false).is_some() { true } else { false }
+						// We want to move to using `Option_` mappings where possible rather than
+						// manual mappings, as it makes downstream bindings simpler and is more
+						// clear for users. Thus, we default to false but override for a few
+						// types which had mappings defined when we were avoiding the `Option_`s.
+						match &resolved as &str {
+							"lightning::ln::PaymentSecret" => true,
+							"lightning::ln::PaymentHash" => true,
+							"lightning::ln::PaymentPreimage" => true,
+							"lightning::ln::channelmanager::PaymentId" => true,
+							"bitcoin::hash_types::BlockHash" => true,
+							"secp256k1::PublicKey"|"bitcoin::secp256k1::PublicKey" => true,
+							_ => false,
+						}
 					} else { unimplemented!(); }
 				},
 				syn::Type::Tuple(_) => false,
@@ -1456,7 +1468,7 @@ impl<'a, 'c: 'a> TypeResolver<'a, 'c> {
 								(".is_none() { core::ptr::null_mut() } else { ".to_owned(), format!("({}.unwrap())", var_access))
 								], " }", ContainerPrefixLocation::OutsideConv));
 						}
-					} else if self.is_primitive(&inner_path) || self.c_type_from_path(&inner_path, false, false).is_none() {
+					} else if !self.is_transparent_container("Option", is_ref, [single_contained.unwrap()].iter().map(|a| *a), generics) {
 						if self.is_primitive(&inner_path) || (!is_contained_ref && !is_ref) || only_contained_has_inner {
 							let inner_name = self.get_c_mangled_container_type(vec![single_contained.unwrap()], generics, "Option").unwrap();
 							return Some(("if ", vec![