From ed769a7a3b5421e2fc7d8cea7b2a96774bde681e Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 May 2022 17:03:16 -0400 Subject: [PATCH] Simplify bindgen for into_ methods in tag unions Thanks for the improvement @folkertdev! --- bindgen/src/bindgen_rs.rs | 62 ++++++++++++++++----------------------- bindgen/tests/gen_rs.rs | 13 +------- 2 files changed, 26 insertions(+), 49 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index a212a6c855..87a2567a4a 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -191,43 +191,31 @@ fn write_tag_union( let payload_type = types.get(*payload_id); let payload_type_name = type_name(*payload_id, types); - let (init_payload, get_payload, deref_for_as, self_for_into) = if payload_type - .has_pointer(types) - { - ( - "core::mem::ManuallyDrop::new(payload)", - format!( - r#"// We know our Drop impl was only ever going to run the payload's Drop impl, - // so returning the payload directly doesn't leak resources. - let variant = core::mem::replace( - &mut self.variant, - core::mem::transmute::, {}>( - core::mem::MaybeUninit::uninit(), - ), - ); - - core::mem::forget(self); - - core::mem::ManuallyDrop::<{}>::into_inner(variant.{})"#, - variant_name, variant_name, payload_type_name, tag_name - ), - // Since this is a ManuallyDrop, our `as_` method will need - // to dereference the variant (e.g. `&self.variant.Foo`) - "&", - // we need `mut self` for the argument because of ManuallyDrop - "mut self", - ) - } else { - ( - "payload", - format!("self.variant.{}", tag_name), - // Since this is not a ManuallyDrop, our `as_` method will not - // want to dereference the variant (e.g. `self.variant.Foo` with no '&') - "", - // we don't need `mut self` unless we need ManuallyDrop - "self", - ) - }; + let (init_payload, get_payload, deref_for_as, self_for_into) = + if payload_type.has_pointer(types) { + ( + "core::mem::ManuallyDrop::new(payload)", + format!( + "core::mem::ManuallyDrop::take(&mut self.variant.{})", + tag_name, + ), + // Since this is a ManuallyDrop, our `as_` method will need + // to dereference the variant (e.g. `&self.variant.Foo`) + "&", + // we need `mut self` for the argument because of ManuallyDrop + "mut self", + ) + } else { + ( + "payload", + format!("self.variant.{}", tag_name), + // Since this is not a ManuallyDrop, our `as_` method will not + // want to dereference the variant (e.g. `self.variant.Foo` with no '&') + "", + // we don't need `mut self` unless we need ManuallyDrop + "self", + ) + }; writeln!( buf, diff --git a/bindgen/tests/gen_rs.rs b/bindgen/tests/gen_rs.rs index 2e32558b70..6475d4435c 100644 --- a/bindgen/tests/gen_rs.rs +++ b/bindgen/tests/gen_rs.rs @@ -295,18 +295,7 @@ fn tag_union_aliased() { /// Unsafely assume the given MyTagUnion has a .tag() of Foo and convert it to Foo's payload. /// (always examine .tag() first to make sure this is the correct variant!) pub unsafe fn into_Foo(mut self) -> roc_std::RocStr { - // We know our Drop impl was only ever going to run the payload's Drop impl, - // so returning the payload directly doesn't leak resources. - let variant = core::mem::replace( - &mut self.variant, - core::mem::transmute::, union_MyTagUnion>( - core::mem::MaybeUninit::uninit(), - ), - ); - - core::mem::forget(self); - - core::mem::ManuallyDrop::::into_inner(variant.Foo) + core::mem::ManuallyDrop::take(&mut self.variant.Foo) } /// Unsafely assume the given MyTagUnion has a .tag() of Foo and return its payload.