From b719e2a89bf9556f86029ed6a44074b44d4b4748 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 14:51:48 -0400 Subject: [PATCH 01/65] Obtain target_info from architecture --- bindgen/src/bindgen_rs.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 90623924b2..6cf657bfb7 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -233,9 +233,7 @@ fn add_tag_union( let tag_names = tags.iter().map(|(name, _)| name).cloned().collect(); let discriminant_name = add_discriminant(name, architecture, tag_names, types, impls); let typ = types.get(type_id); - // TODO also do this for other targets. Remember, these can change based on more - // than just pointer width; e.g. on wasm, the alignments of U16 and U8 are both 4! - let target_info = TargetInfo::from(&target_lexicon::Triple::host()); + let target_info = architecture.into(); let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); let size = typ.size(types, target_info); From 60a28cb857852d4dd6c9a9188a8a6b0a9f24bf6f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:38:33 -0400 Subject: [PATCH 02/65] Add preliminary recursive union test --- .../recursive-union/Package-Config.roc | 11 ++ .../tests/fixtures/recursive-union/app.roc | 6 + .../tests/fixtures/recursive-union/src/lib.rs | 106 ++++++++++++++++++ bindgen/tests/test_bindgen_cli.rs | 5 + 4 files changed, 128 insertions(+) create mode 100644 bindgen/tests/fixtures/recursive-union/Package-Config.roc create mode 100644 bindgen/tests/fixtures/recursive-union/app.roc create mode 100644 bindgen/tests/fixtures/recursive-union/src/lib.rs diff --git a/bindgen/tests/fixtures/recursive-union/Package-Config.roc b/bindgen/tests/fixtures/recursive-union/Package-Config.roc new file mode 100644 index 0000000000..730c022dfb --- /dev/null +++ b/bindgen/tests/fixtures/recursive-union/Package-Config.roc @@ -0,0 +1,11 @@ +platform "test-platform" + requires {} { main : _ } + exposes [] + packages {} + imports [] + provides [mainForHost] + +Expr : [String Str, Concat Expr Expr] + +mainForHost : Expr +mainForHost = main diff --git a/bindgen/tests/fixtures/recursive-union/app.roc b/bindgen/tests/fixtures/recursive-union/app.roc new file mode 100644 index 0000000000..98336ca8a8 --- /dev/null +++ b/bindgen/tests/fixtures/recursive-union/app.roc @@ -0,0 +1,6 @@ +app "app" + packages { pf: "." } + imports [] + provides [main] to pf + +main = Concat (String "Hello, ") (String "World!") diff --git a/bindgen/tests/fixtures/recursive-union/src/lib.rs b/bindgen/tests/fixtures/recursive-union/src/lib.rs new file mode 100644 index 0000000000..317d73ca55 --- /dev/null +++ b/bindgen/tests/fixtures/recursive-union/src/lib.rs @@ -0,0 +1,106 @@ +mod bindings; + +use bindings::{Expr, Expr_Concat}; +use indoc::indoc; + +extern "C" { + #[link_name = "roc__mainForHost_1_exposed_generic"] + fn roc_main(_: *mut Expr); +} + +#[no_mangle] +pub extern "C" fn rust_main() -> i32 { + use std::cmp::Ordering; + use std::collections::hash_set::HashSet; + + let tag_union = unsafe { + let mut ret: core::mem::MaybeUninit = core::mem::MaybeUninit::uninit(); + + roc_main(ret.as_mut_ptr()); + + ret.assume_init() + }; + + // Verify that it has all the expected traits. + + assert!(tag_union == tag_union); // PartialEq + assert!(tag_union.clone() == tag_union.clone()); // Clone + + assert!(tag_union.partial_cmp(&tag_union) == Some(Ordering::Equal)); // PartialOrd + assert!(tag_union.cmp(&tag_union) == Ordering::Equal); // Ord + + print!( + indoc!( + r#" + tag_union was: {:?} + `Concat (String "Hello, ") (String "World!")` is: {:?} + `String "this is a test"` is: {:?} + "# + ), + tag_union, + Expr::Concat(Expr_Concat { + f0: Expr::String("Hello, ".into()), + f1: Expr::String("World!".into()), + }), + Expr::String("this is a test".into()), + ); // Debug + + let mut set = HashSet::new(); + + set.insert(tag_union.clone()); // Eq, Hash + set.insert(tag_union); + + assert_eq!(set.len(), 1); + + // Exit code + 0 +} + +// Externs required by roc_std and by the Roc app + +use core::ffi::c_void; +use std::ffi::CStr; +use std::os::raw::c_char; + +#[no_mangle] +pub unsafe extern "C" fn roc_alloc(size: usize, _alignment: u32) -> *mut c_void { + return libc::malloc(size); +} + +#[no_mangle] +pub unsafe extern "C" fn roc_realloc( + c_ptr: *mut c_void, + new_size: usize, + _old_size: usize, + _alignment: u32, +) -> *mut c_void { + return libc::realloc(c_ptr, new_size); +} + +#[no_mangle] +pub unsafe extern "C" fn roc_dealloc(c_ptr: *mut c_void, _alignment: u32) { + return libc::free(c_ptr); +} + +#[no_mangle] +pub unsafe extern "C" fn roc_panic(c_ptr: *mut c_void, tag_id: u32) { + match tag_id { + 0 => { + let slice = CStr::from_ptr(c_ptr as *const c_char); + let string = slice.to_str().unwrap(); + eprintln!("Roc hit a panic: {}", string); + std::process::exit(1); + } + _ => todo!(), + } +} + +#[no_mangle] +pub unsafe extern "C" fn roc_memcpy(dst: *mut c_void, src: *mut c_void, n: usize) -> *mut c_void { + libc::memcpy(dst, src, n) +} + +#[no_mangle] +pub unsafe extern "C" fn roc_memset(dst: *mut c_void, c: i32, n: usize) -> *mut c_void { + libc::memset(dst, c, n) +} diff --git a/bindgen/tests/test_bindgen_cli.rs b/bindgen/tests/test_bindgen_cli.rs index 1dfe86ffe5..0b7687630d 100644 --- a/bindgen/tests/test_bindgen_cli.rs +++ b/bindgen/tests/test_bindgen_cli.rs @@ -121,6 +121,11 @@ mod bindgen_cli_run { `Cons "small str" Nil` is: StrConsList::Cons(StrConsList_Cons { f0: "small str", f1: StrConsList::Nil }) `Nil` is: StrConsList::Nil "#), + recursive_union:"recursive-union" => indoc!(r#" + tag_union was: StrConsList::Cons(StrConsList_Cons { f0: "World!", f1: StrConsList::Cons(StrConsList_Cons { f0: "Hello ", f1: StrConsList::Nil }) }) + `Cons "small str" Nil` is: StrConsList::Cons(StrConsList_Cons { f0: "small str", f1: StrConsList::Nil }) + `Nil` is: StrConsList::Nil + "#), } fn check_for_tests(all_fixtures: &mut roc_collections::VecSet) { From eeec0f8990307348384d33cd7053dda2a5d71bcf Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:38:53 -0400 Subject: [PATCH 03/65] Bindgen RoctagUnion::Recursive --- bindgen/src/bindgen.rs | 4 +--- bindgen/src/types.rs | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 9f84d68088..04fd58dd69 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -403,9 +403,7 @@ fn add_tag_union( NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), // A recursive tag union (general case) // e.g. `Expr : [Sym Str, Add Expr Expr]` - Recursive(_) => { - todo!() - } + Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), // A recursive tag union with just one constructor // Optimization: No need to store a tag ID (the payload is "unwrapped") // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 2e0ccce7ff..8ff3309f25 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -321,7 +321,7 @@ impl RocType { RocType::RocBox(_) => target_info.ptr_size(), RocType::TagUnion(tag_union) => match tag_union { RocTagUnion::Enumeration { tags, .. } => size_for_tag_count(tags.len()), - RocTagUnion::NonRecursive { tags, .. } => { + RocTagUnion::NonRecursive { tags, .. } | RocTagUnion::Recursive { tags, .. } => { // The "unpadded" size (without taking alignment into account) // is the sum of all the sizes of the fields. let size_unpadded = tags.iter().fold(0, |total, (_, opt_payload_id)| { @@ -355,7 +355,6 @@ impl RocType { size_padded } } - RocTagUnion::Recursive { .. } => todo!(), RocTagUnion::NonNullableUnwrapped { .. } => todo!(), RocTagUnion::NullableWrapped { .. } => todo!(), RocTagUnion::NullableUnwrapped { .. } => todo!(), From 9725ea62127eca2b3757962d524d0bb1b4fe2e67 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:39:21 -0400 Subject: [PATCH 04/65] Fix syntax error in generated Rust code --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 6cf657bfb7..86d5f5de66 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -59,7 +59,7 @@ pub fn emit(types_by_architecture: &[(Architecture, Types)]) -> String { 1 => { let arch = arch_to_str(architectures.get(0).unwrap()); - buf.push_str(&format!("r#[cfg(target_arch = \"{arch}\")]")); + buf.push_str(&format!("#[cfg(target_arch = \"{arch}\")]")); } _ => { // We should never have a decl recorded with 0 architectures! From d20954a2d1fb43a89a8002b73f9224446abd3b14 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:40:10 -0400 Subject: [PATCH 05/65] Initial bindgen for recursive unions --- bindgen/src/bindgen_rs.rs | 189 +++++++++++++++++++++++++++++++------- 1 file changed, 157 insertions(+), 32 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 86d5f5de66..fa7be599e2 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -125,11 +125,31 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I // Empty tag unions can never come up at runtime, // and so don't need declared types. if !tags.is_empty() { - add_tag_union(name, architecture, id, tags, types, impls); + add_tag_union( + Recursiveness::NonRecursive, + name, + architecture, + id, + tags, + types, + impls, + ); } } - RocTagUnion::Recursive { .. } => { - todo!(); + RocTagUnion::Recursive { tags, name } => { + // Empty tag unions can never come up at runtime, + // and so don't need declared types. + if !tags.is_empty() { + add_tag_union( + Recursiveness::Recursive, + name, + architecture, + id, + tags, + types, + impls, + ); + } } RocTagUnion::NullableWrapped { .. } => { todo!(); @@ -222,7 +242,13 @@ fn add_discriminant( discriminant_name } +enum Recursiveness { + Recursive, + NonRecursive, +} + fn add_tag_union( + recursiveness: Recursiveness, name: &str, architecture: Architecture, type_id: TypeId, @@ -237,6 +263,40 @@ fn add_tag_union( let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); let size = typ.size(types, target_info); + // Find the first recursive pointer field in the tags' payloads. + // TODO: what if there's more than one? Is it safe to assume the first + // one is it? What if it's another one? + let recursive_pointer_field = match recursiveness { + Recursiveness::Recursive => { + let opt_tag_and_field = tags.iter().find_map(|(tag_name, opt_payload_id)| { + if let Some(payload_id) = opt_payload_id { + match types.get(*payload_id) { + RocType::Struct { fields, .. } => { + fields.iter().find_map(|field| match field { + Field::NonRecursive(_, _) => None, + Field::Recursive(label, field_id) => { + debug_assert_eq!(*field_id, type_id); + + Some(label) + } + }) + } + _ => None, + } + .map(|label| (tag_name, label)) + } else { + None + } + }); + + match opt_tag_and_field { + Some((tag_name, label)) => format!("{tag_name}.{label}"), + None => String::new(), + } + } + Recursiveness::NonRecursive => String::new(), + }; + { // No #[derive(...)] for unions; we have to generate each impl ourselves! let mut buf = format!("#[repr(C)]\npub union {name} {{\n"); @@ -280,25 +340,69 @@ fn add_tag_union( { let opt_impl = Some(format!("impl {name}")); - // An old design, which ended up not working out, was that the tag union - // was a struct containing two fields: one for the `union`, and another - // for the discriminant. - // - // The problem with this was alignment; e.g. if you have one variant with a - // RocStr in it and another with an I128, then the `union` has a size of 32B - // and the discriminant is right after it - making the size of the whole struct - // round up to 48B total, since it has an alignment of 16 from the I128. - // - // However, Roc will generate the more efficient thing here: the whole thing will - // be 32B, and the discriminant will appear at offset 24 - right after the end of - // the RocStr. The current design recognizes this and works with it, by representing - // the entire structure as a union and manually setting the tag at the appropriate offset. - add_decl( - impls, - opt_impl.clone(), - architecture, - format!( - r#"{VARIANT_DOC_COMMENT} + match recursiveness { + Recursiveness::Recursive => { + if tags.len() <= max_pointer_tagged_variants(architecture) { + let bitmask = format!("{:#b}", tagged_pointer_bitmask(architecture)); + + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"{VARIANT_DOC_COMMENT} + pub fn variant(&self) -> {discriminant_name} {{ + // The discriminant is stored in the unused bytes at the end of the recursive pointer + unsafe {{ core::mem::transmute::((self.{recursive_pointer_field} as u8) & {bitmask}) }} + }}"# + ), + ); + + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"/// Internal helper + fn set_discriminant(&mut self, discriminant: {discriminant_name}) {{ + // The discriminant is stored in the unused bytes at the end of the recursive pointer + unsafe {{ + let untagged = (self.{recursive_pointer_field} as usize) & (!{bitmask} as usize); + let tagged = pointer | (self.variant() as usize); + + *self.{recursive_pointer_field} = tagged as _; + }} + }}"# + ), + ); + } else { + todo!( + "Support {} tags in a recursive tag union on architecture {:?}. (This is too many tags for pointer tagging to work, so we need to bindgen something different.)", + tags.len(), + architecture + ); + } + } + Recursiveness::NonRecursive => { + // An old design, which ended up not working out, was that the tag union + // was a struct containing two fields: one for the `union`, and another + // for the discriminant. + // + // The problem with this was alignment; e.g. if you have one variant with a + // RocStr in it and another with an I128, then the `union` has a size of 32B + // and the discriminant is right after it - making the size of the whole struct + // round up to 48B total, since it has an alignment of 16 from the I128. + // + // However, Roc will generate the more efficient thing here: the whole thing will + // be 32B, and the discriminant will appear at offset 24 - right after the end of + // the RocStr. The current design recognizes this and works with it, by representing + // the entire structure as a union and manually setting the tag at the appropriate offset. + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"{VARIANT_DOC_COMMENT} pub fn variant(&self) -> {discriminant_name} {{ unsafe {{ let bytes = core::mem::transmute::<&Self, &[u8; core::mem::size_of::()]>(self); @@ -306,15 +410,15 @@ fn add_tag_union( core::mem::transmute::(*bytes.as_ptr().add({discriminant_offset})) }} }}"# - ), - ); + ), + ); - add_decl( - impls, - opt_impl.clone(), - architecture, - format!( - r#"/// Internal helper + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"/// Internal helper fn set_discriminant(&mut self, discriminant: {discriminant_name}) {{ let discriminant_ptr: *mut {discriminant_name} = (self as *mut {name}).cast(); @@ -322,8 +426,10 @@ fn add_tag_union( *(discriminant_ptr.add({discriminant_offset})) = discriminant; }} }}"# - ), - ); + ), + ); + } + } for (tag_name, opt_payload_id) in tags { // Add a convenience constructor function to the impl, e.g. @@ -1272,3 +1378,22 @@ fn write_indents(indentations: usize, buf: &mut String) { buf.push_str(INDENT); } } + +fn max_pointer_tagged_variants(architecture: Architecture) -> usize { + match architecture { + // On a 64-bit system, pointers have 3 bits that are unused, so return 2^3 = 8 + Architecture::X86_64 | Architecture::Aarch64 => 8, + // On a 32-bit system, pointers have 2 bits that are unused, so return 2^4 = 4 + Architecture::X86_32 | Architecture::Aarch32 | Architecture::Wasm32 => 4, + } +} + +#[inline(always)] +fn tagged_pointer_bitmask(architecture: Architecture) -> u8 { + match architecture { + // On a 64-bit system, pointers have 3 bits that are unused + Architecture::X86_64 | Architecture::Aarch64 => 0b0000_0111, + // On a 32-bit system, pointers have 2 bits that are unused + Architecture::X86_32 | Architecture::Aarch32 | Architecture::Wasm32 => 0b0000_0011, + } +} From 1e97a450e343afd6fffc57c1d3a0676fd2381fb5 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:40:30 -0400 Subject: [PATCH 06/65] Drop unused import --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index fa7be599e2..359f41f53f 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1,7 +1,7 @@ use crate::types::{Field, RocTagUnion, RocType, TypeId, Types}; use indexmap::IndexMap; use roc_mono::layout::UnionLayout; -use roc_target::{Architecture, TargetInfo}; +use roc_target::Architecture; use std::convert::TryInto; use std::fmt::Display; From 19c2396623e526db32c7787173155f35fcdd316d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:52:19 -0400 Subject: [PATCH 07/65] Unwrap transparent wrappers --- bindgen/src/bindgen_rs.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 359f41f53f..587773f910 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -196,17 +196,8 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I | RocType::RocDict(_, _) | RocType::RocSet(_) | RocType::RocList(_) - | RocType::RocBox(_) => {} - RocType::TransparentWrapper { name, content } => { - let typ = types.get(id); - let derive = derive_str(typ, types, !typ.has_enumeration(types)); - let body = format!( - "{derive}\n#[repr(transparent)]\npub struct {name}({});", - type_name(*content, types) - ); - - add_decl(impls, None, architecture, body); - } + | RocType::RocBox(_) + | RocType::TransparentWrapper { .. } => {} } } @@ -1015,13 +1006,13 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } - | RocType::TransparentWrapper { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) | RocType::TagUnion(RocTagUnion::Enumeration { name, .. }) | RocType::TagUnion(RocTagUnion::NullableWrapped { name, .. }) | RocType::TagUnion(RocTagUnion::NullableUnwrapped { name, .. }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { name, .. }) => name.clone(), + RocType::TransparentWrapper { content, .. } => type_name(*content, types), } } From 6d3e354b38899227793819295d38f95db6ed4f66 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:53:01 -0400 Subject: [PATCH 08/65] Revert "Unwrap transparent wrappers" This reverts commit a2aa0a4d46c1f6742942d36f16392f4e71542964. --- bindgen/src/bindgen_rs.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 587773f910..359f41f53f 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -196,8 +196,17 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I | RocType::RocDict(_, _) | RocType::RocSet(_) | RocType::RocList(_) - | RocType::RocBox(_) - | RocType::TransparentWrapper { .. } => {} + | RocType::RocBox(_) => {} + RocType::TransparentWrapper { name, content } => { + let typ = types.get(id); + let derive = derive_str(typ, types, !typ.has_enumeration(types)); + let body = format!( + "{derive}\n#[repr(transparent)]\npub struct {name}({});", + type_name(*content, types) + ); + + add_decl(impls, None, architecture, body); + } } } @@ -1006,13 +1015,13 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } + | RocType::TransparentWrapper { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) | RocType::TagUnion(RocTagUnion::Enumeration { name, .. }) | RocType::TagUnion(RocTagUnion::NullableWrapped { name, .. }) | RocType::TagUnion(RocTagUnion::NullableUnwrapped { name, .. }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { name, .. }) => name.clone(), - RocType::TransparentWrapper { content, .. } => type_name(*content, types), } } From bceb787806b6043701f71282319ec925c8e31b17 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:57:17 -0400 Subject: [PATCH 09/65] Have transparent wrappers expose their field --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 359f41f53f..97211b554c 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -201,7 +201,7 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I let typ = types.get(id); let derive = derive_str(typ, types, !typ.has_enumeration(types)); let body = format!( - "{derive}\n#[repr(transparent)]\npub struct {name}({});", + "{derive}\n#[repr(transparent)]\npub struct {name}(pub {});", type_name(*content, types) ); From 93f8261ed005877be032dc334023a6c8adb63e46 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 22:57:45 -0400 Subject: [PATCH 10/65] Fix recursive union bindgen fixture type mismatches --- bindgen/tests/fixtures/recursive-union/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bindgen/tests/fixtures/recursive-union/src/lib.rs b/bindgen/tests/fixtures/recursive-union/src/lib.rs index 317d73ca55..188a6e605d 100644 --- a/bindgen/tests/fixtures/recursive-union/src/lib.rs +++ b/bindgen/tests/fixtures/recursive-union/src/lib.rs @@ -1,6 +1,6 @@ mod bindings; -use bindings::{Expr, Expr_Concat}; +use bindings::{Expr, Expr_Concat, Expr_String}; use indoc::indoc; extern "C" { @@ -39,10 +39,10 @@ pub extern "C" fn rust_main() -> i32 { ), tag_union, Expr::Concat(Expr_Concat { - f0: Expr::String("Hello, ".into()), - f1: Expr::String("World!".into()), + f0: Box::into_raw(Box::new(Expr::String(Expr_String("Hello, ".into())))), + f1: Box::into_raw(Box::new(Expr::String(Expr_String("World!".into())))), }), - Expr::String("this is a test".into()), + Expr::String(Expr_String("this is a test".into())), ); // Debug let mut set = HashSet::new(); From 786ef564167ca50a2a50e851a2200401d27d9a29 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 23:00:52 -0400 Subject: [PATCH 11/65] Fix naming error --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 97211b554c..24f116c482 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -368,7 +368,7 @@ fn add_tag_union( // The discriminant is stored in the unused bytes at the end of the recursive pointer unsafe {{ let untagged = (self.{recursive_pointer_field} as usize) & (!{bitmask} as usize); - let tagged = pointer | (self.variant() as usize); + let tagged = untagged | (self.variant() as usize); *self.{recursive_pointer_field} = tagged as _; }} From aadce61238739e06546b697618fad28ab608e2ca Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 23:03:33 -0400 Subject: [PATCH 12/65] Fix tagged pointer type mismatch --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 24f116c482..063400892c 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -370,7 +370,7 @@ fn add_tag_union( let untagged = (self.{recursive_pointer_field} as usize) & (!{bitmask} as usize); let tagged = untagged | (self.variant() as usize); - *self.{recursive_pointer_field} = tagged as _; + self.{recursive_pointer_field} = tagged as *mut Self; }} }}"# ), From 705de474ee6ba93c174f2f5d8438998411779cfb Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 23:03:48 -0400 Subject: [PATCH 13/65] bindgen recursive fields as pointers --- bindgen/src/bindgen_rs.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 063400892c..2374b723d3 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -974,11 +974,13 @@ fn add_struct( let mut buf = format!("{derive}\n#[repr(C)]\npub struct {name} {{\n"); for field in fields { - buf.push_str(&format!( - "{INDENT}pub {}: {},\n", - field.label(), - type_name(field.type_id(), types) - )); + let (label, type_str) = match field { + Field::NonRecursive(label, field_id) => (label, type_name(*field_id, types)), + Field::Recursive(label, field_id) => { + (label, format!("*mut {}", type_name(*field_id, types))) + } + }; + buf.push_str(&format!("{INDENT}pub {label}: {type_str},\n",)); } buf.push('}'); From 06349a867063042bc72a8fc2a2fe05d768191d43 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 26 May 2022 23:55:04 -0400 Subject: [PATCH 14/65] Hoist bitmask definition --- bindgen/src/bindgen_rs.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 2374b723d3..73422819ba 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -339,11 +339,12 @@ fn add_tag_union( // The impl for the tag union { let opt_impl = Some(format!("impl {name}")); + let bitmask; match recursiveness { Recursiveness::Recursive => { if tags.len() <= max_pointer_tagged_variants(architecture) { - let bitmask = format!("{:#b}", tagged_pointer_bitmask(architecture)); + bitmask = format!("{:#b}", tagged_pointer_bitmask(architecture)); add_decl( impls, @@ -384,6 +385,9 @@ fn add_tag_union( } } Recursiveness::NonRecursive => { + // The bitmask doesn't come up in a nonrecursive tag union. + bitmask = String::new(); + // An old design, which ended up not working out, was that the tag union // was a struct containing two fields: one for the `union`, and another // for the discriminant. From 8e937333fc7bec9ff5ce2d9dfb098b4c31d3b4c4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 00:04:19 -0400 Subject: [PATCH 15/65] Bindgen into_Foo to return a tuple --- bindgen/src/bindgen_rs.rs | 62 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 73422819ba..0208f7616f 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -492,7 +492,10 @@ fn add_tag_union( ), ); - let (ret_type_str, ret) = match payload_type { + let ret_type_str; + let ret; + + match payload_type { RocType::RocStr | RocType::Bool | RocType::I8 @@ -515,10 +518,58 @@ fn add_tag_union( | RocType::RocBox(_) | RocType::TagUnion(_) | RocType::TransparentWrapper { .. } => { - (payload_type_name.clone(), get_payload) + ret_type_str = type_name(*payload_id, types); + ret = "payload".to_string(); } - RocType::Struct { .. } => { - todo!(); + RocType::Struct { fields, .. } => { + let mut sorted_fields = fields.iter().collect::>(); + + sorted_fields.sort_by(|field1, field2| { + // Convert from e.g. "f12" to 12u64 + // This is necessary because the string "f10" sorts + // to earlier than "f2", whereas the number 10 + // sorts after the number 2. + let num1 = field1.label()[1..].parse::().unwrap(); + let num2 = field2.label()[1..].parse::().unwrap(); + + num1.partial_cmp(&num2).unwrap() + }); + + let mut ret_types = Vec::new(); + let mut ret_buf = "(\n".to_string(); + + for field in fields { + for _ in 0..3 { + ret_buf.push_str(INDENT); + } + + let field_type_name = type_name(field.type_id(), types); + + ret_types.push(field_type_name.clone()); + + match field { + Field::NonRecursive(label, _) => { + ret_buf.push_str("payload."); + ret_buf.push_str(label); + } + Field::Recursive(label, _) => { + ret_buf.push_str(&format!( + "*((payload.{label} as usize & !{bitmask}) as *mut {field_type_name})" + )); + } + } + + ret_buf.push_str(",\n"); + } + + for _ in 0..2 { + ret_buf.push_str(INDENT); + } + + ret_buf.push_str(")"); + + ret = ret_buf; + ret_type_str = format!("({})", ret_types.join(", ")); } }; @@ -532,6 +583,9 @@ fn add_tag_union( /// Panics in debug builds if the .variant() doesn't return {tag_name}. pub unsafe fn into_{tag_name}({self_for_into}) -> {ret_type_str} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{tag_name}); + + let payload = {get_payload}; + {ret} }}"#, ), From e6d96a2d9be4ece20439de605c32b70f1e4ae19f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 00:14:03 -0400 Subject: [PATCH 16/65] Bindgen as_Foo to return a tuple --- bindgen/src/bindgen_rs.rs | 67 +++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 0208f7616f..ae72ed98a0 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -492,8 +492,10 @@ fn add_tag_union( ), ); - let ret_type_str; - let ret; + let owned_ret_type; + let borrowed_ret_type; + let owned_ret; + let borrowed_ret; match payload_type { RocType::RocStr @@ -518,8 +520,10 @@ fn add_tag_union( | RocType::RocBox(_) | RocType::TagUnion(_) | RocType::TransparentWrapper { .. } => { - ret_type_str = type_name(*payload_id, types); - ret = "payload".to_string(); + owned_ret_type = type_name(*payload_id, types); + borrowed_ret_type = format!("&{}", owned_ret_type); + owned_ret = "payload".to_string(); + borrowed_ret = "&payload".to_string(); } RocType::Struct { fields, .. } => { let mut sorted_fields = fields.iter().collect::>(); @@ -536,40 +540,52 @@ fn add_tag_union( }); let mut ret_types = Vec::new(); - let mut ret_buf = "(\n".to_string(); + let mut ret_values = Vec::new(); for field in fields { - for _ in 0..3 { - ret_buf.push_str(INDENT); - } - let field_type_name = type_name(field.type_id(), types); ret_types.push(field_type_name.clone()); match field { Field::NonRecursive(label, _) => { - ret_buf.push_str("payload."); - ret_buf.push_str(label); + ret_values.push(format!("payload.{label}")); } Field::Recursive(label, _) => { - ret_buf.push_str(&format!( + ret_values.push(format!( "*((payload.{label} as usize & !{bitmask}) as *mut {field_type_name})" )); } } - - ret_buf.push_str(",\n"); } - for _ in 0..2 { - ret_buf.push_str(INDENT); - } + owned_ret = { + let lines = ret_values + .iter() + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) + .collect::>() + .join(", "); - ret_buf.push_str(")"); + format!("({lines}\n{INDENT}{INDENT})") + }; + borrowed_ret = { + let lines = ret_values + .iter() + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) + .collect::>() + .join(", "); - ret = ret_buf; - ret_type_str = format!("({})", ret_types.join(", ")); + format!("({lines}\n{INDENT}{INDENT})") + }; + owned_ret_type = format!("({})", ret_types.join(", ")); + borrowed_ret_type = format!( + "({})", + ret_types + .iter() + .map(|ret_type| { format!("&{ret_type}") }) + .collect::>() + .join(", ") + ); } }; @@ -581,12 +597,12 @@ fn add_tag_union( r#"/// Unsafely assume the given {name} has a .variant() of {tag_name} and convert it to {tag_name}'s payload. /// (Always examine .variant() first to make sure this is the correct variant!) /// Panics in debug builds if the .variant() doesn't return {tag_name}. - pub unsafe fn into_{tag_name}({self_for_into}) -> {ret_type_str} {{ + pub unsafe fn into_{tag_name}({self_for_into}) -> {owned_ret_type} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{tag_name}); let payload = {get_payload}; - {ret} + {owned_ret} }}"#, ), ); @@ -599,9 +615,12 @@ fn add_tag_union( r#"/// Unsafely assume the given {name} has a .variant() of {tag_name} and return its payload. /// (Always examine .variant() first to make sure this is the correct variant!) /// Panics in debug builds if the .variant() doesn't return {tag_name}. - pub unsafe fn as_{tag_name}(&self) -> {ref_if_needed}{payload_type_name} {{ + pub unsafe fn as_{tag_name}(&self) -> {borrowed_ret_type} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{tag_name}); - {ref_if_needed}self.{tag_name} + + let payload = {get_payload}; + + {borrowed_ret} }}"#, ), ); From 6b2605aebd66c754f6c587d00687252104c8bfa4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 00:14:28 -0400 Subject: [PATCH 17/65] Drop unused variable --- bindgen/src/bindgen_rs.rs | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index ae72ed98a0..462db1efb8 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -451,28 +451,23 @@ fn add_tag_union( let payload_type = types.get(*payload_id); let payload_type_name = type_name(*payload_id, types); - let (init_payload, get_payload, ref_if_needed, self_for_into) = - if payload_type.has_pointer(types) { - ( - "core::mem::ManuallyDrop::new(payload)", - format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",), - // Since this is a ManuallyDrop, our `as_` method will need - // to dereference the variant (e.g. `&self.Foo`) - "&", - // we need `mut self` for the argument because of ManuallyDrop - "mut self", - ) - } else { - ( - "payload", - format!("self.{tag_name}"), - // Since this is not a ManuallyDrop, our `as_` method will not - // want to dereference the variant (e.g. `self.Foo` with no '&') - "", - // we don't need `mut self` unless we need ManuallyDrop - "self", - ) - }; + + let (init_payload, get_payload, self_for_into) = if payload_type.has_pointer(types) + { + ( + "core::mem::ManuallyDrop::new(payload)", + format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",), + // we need `mut self` for the argument because of ManuallyDrop + "mut self", + ) + } else { + ( + "payload", + format!("self.{tag_name}"), + // we don't need `mut self` unless we need ManuallyDrop + "self", + ) + }; add_decl( impls, From 08a93d0d94ddb8500a08a7f018c791c97b511a8b Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 00:37:17 -0400 Subject: [PATCH 18/65] Construct tag payloads from tupled args --- bindgen/src/bindgen_rs.rs | 128 ++++++++++++------ .../tests/fixtures/recursive-union/src/lib.rs | 12 +- 2 files changed, 95 insertions(+), 45 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 462db1efb8..553e51f1cc 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -449,49 +449,27 @@ fn add_tag_union( // } if let Some(payload_id) = opt_payload_id { let payload_type = types.get(*payload_id); - let payload_type_name = type_name(*payload_id, types); - - - let (init_payload, get_payload, self_for_into) = if payload_type.has_pointer(types) - { - ( - "core::mem::ManuallyDrop::new(payload)", - format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",), - // we need `mut self` for the argument because of ManuallyDrop - "mut self", - ) - } else { - ( - "payload", - format!("self.{tag_name}"), - // we don't need `mut self` unless we need ManuallyDrop - "self", - ) - }; - - add_decl( - impls, - opt_impl.clone(), - architecture, - format!( - r#"/// Construct a tag named {tag_name}, with the appropriate payload - pub fn {tag_name}(payload: {payload_type_name}) -> Self {{ - let mut answer = Self {{ - {tag_name}: {init_payload} - }}; - - answer.set_discriminant({discriminant_name}::{tag_name}); - - answer - }}"# - ), - ); + let init_payload; + let get_payload; + let self_for_into; + let payload_args; + let args_to_payload; let owned_ret_type; let borrowed_ret_type; let owned_ret; let borrowed_ret; + if payload_type.has_pointer(types) { + get_payload = format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",); + // we need `mut self` for the argument because of ManuallyDrop + self_for_into = "mut self"; + } else { + get_payload = format!("self.{tag_name}"); + // we don't need `mut self` unless we need ManuallyDrop + self_for_into = "self"; + }; + match payload_type { RocType::RocStr | RocType::Bool @@ -513,14 +491,47 @@ fn add_tag_union( | RocType::RocDict(_, _) | RocType::RocSet(_) | RocType::RocBox(_) - | RocType::TagUnion(_) - | RocType::TransparentWrapper { .. } => { + | RocType::TagUnion(_) => { + if payload_type.has_pointer(types) { + init_payload = "core::mem::ManuallyDrop::new(payload)".to_string(); + } else { + init_payload = "payload".to_string(); + } + owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); owned_ret = "payload".to_string(); borrowed_ret = "&payload".to_string(); + payload_args = format!("arg: {owned_ret_type}"); + args_to_payload = "arg".to_string(); + } + RocType::TransparentWrapper { content, .. } => { + let wrapper_type_name = type_name(*payload_id, types); + + if payload_type.has_pointer(types) { + init_payload = format!( + "core::mem::ManuallyDrop::new({wrapper_type_name}(payload))" + ); + } else { + init_payload = format!("{wrapper_type_name}(payload)"); + } + + // This is a payload with 1 value, so we want to hide the wrapper + // from the public API. + owned_ret_type = type_name(*content, types); + borrowed_ret_type = format!("&{}", owned_ret_type); + owned_ret = "payload.0".to_string(); + borrowed_ret = format!("&{owned_ret}"); + payload_args = format!("arg: {owned_ret_type}"); + args_to_payload = "arg".to_string(); } RocType::Struct { fields, .. } => { + if payload_type.has_pointer(types) { + init_payload = "core::mem::ManuallyDrop::new(payload)".to_string(); + } else { + init_payload = "payload".to_string(); + } + let mut sorted_fields = fields.iter().collect::>(); sorted_fields.sort_by(|field1, field2| { @@ -554,6 +565,26 @@ fn add_tag_union( } } + let payload_type_name = type_name(*payload_id, types); + + payload_args = ret_types + .iter() + .enumerate() + .map(|(index, typ)| format!("arg{index}: {typ}")) + .collect::>() + .join(", "); + args_to_payload = format!( + "{payload_type_name} {{\n{}\n{INDENT}{INDENT}}}", + fields + .iter() + .enumerate() + .map(|(index, field)| format!( + "{INDENT}{INDENT}{INDENT}{}: arg{index},", + field.label() + )) + .collect::>() + .join("\n") + ); owned_ret = { let lines = ret_values .iter() @@ -584,6 +615,25 @@ fn add_tag_union( } }; + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"/// Construct a tag named {tag_name}, with the appropriate payload + pub fn {tag_name}({payload_args}) -> Self {{ + let payload = {args_to_payload}; + let mut answer = Self {{ + {tag_name}: {init_payload} + }}; + + answer.set_discriminant({discriminant_name}::{tag_name}); + + answer + }}"# + ), + ); + add_decl( impls, opt_impl.clone(), diff --git a/bindgen/tests/fixtures/recursive-union/src/lib.rs b/bindgen/tests/fixtures/recursive-union/src/lib.rs index 188a6e605d..1cc2d76e9d 100644 --- a/bindgen/tests/fixtures/recursive-union/src/lib.rs +++ b/bindgen/tests/fixtures/recursive-union/src/lib.rs @@ -1,6 +1,6 @@ mod bindings; -use bindings::{Expr, Expr_Concat, Expr_String}; +use bindings::Expr; use indoc::indoc; extern "C" { @@ -38,11 +38,11 @@ pub extern "C" fn rust_main() -> i32 { "# ), tag_union, - Expr::Concat(Expr_Concat { - f0: Box::into_raw(Box::new(Expr::String(Expr_String("Hello, ".into())))), - f1: Box::into_raw(Box::new(Expr::String(Expr_String("World!".into())))), - }), - Expr::String(Expr_String("this is a test".into())), + Expr::Concat( + Expr::String("Hello, ".into()), + Expr::String("World!".into()), + ), + Expr::String("this is a test".into()), ); // Debug let mut set = HashSet::new(); From 5edb4fd26d2692b835c3d4ddd27987b323ab6bce Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 09:11:02 -0400 Subject: [PATCH 19/65] Return owned and borrowed payloads differently --- bindgen/src/bindgen_rs.rs | 56 ++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 553e51f1cc..24b0934061 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -451,21 +451,25 @@ fn add_tag_union( let payload_type = types.get(*payload_id); let init_payload; - let get_payload; let self_for_into; let payload_args; let args_to_payload; let owned_ret_type; let borrowed_ret_type; + let owned_get_payload; + let borrowed_get_payload; let owned_ret; let borrowed_ret; if payload_type.has_pointer(types) { - get_payload = format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",); + owned_get_payload = + format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",); + borrowed_get_payload = format!("&self.{tag_name}",); // we need `mut self` for the argument because of ManuallyDrop self_for_into = "mut self"; } else { - get_payload = format!("self.{tag_name}"); + owned_get_payload = format!("self.{tag_name}"); + borrowed_get_payload = format!("&{owned_get_payload}"); // we don't need `mut self` unless we need ManuallyDrop self_for_into = "self"; }; @@ -500,10 +504,10 @@ fn add_tag_union( owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); - owned_ret = "payload".to_string(); - borrowed_ret = "&payload".to_string(); payload_args = format!("arg: {owned_ret_type}"); args_to_payload = "arg".to_string(); + owned_ret = "payload".to_string(); + borrowed_ret = format!("&{owned_ret}"); } RocType::TransparentWrapper { content, .. } => { let wrapper_type_name = type_name(*payload_id, types); @@ -520,10 +524,10 @@ fn add_tag_union( // from the public API. owned_ret_type = type_name(*content, types); borrowed_ret_type = format!("&{}", owned_ret_type); - owned_ret = "payload.0".to_string(); - borrowed_ret = format!("&{owned_ret}"); payload_args = format!("arg: {owned_ret_type}"); args_to_payload = "arg".to_string(); + owned_ret = "payload.0".to_string(); + borrowed_ret = format!("&{owned_ret}"); } RocType::Struct { fields, .. } => { if payload_type.has_pointer(types) { @@ -578,10 +582,36 @@ fn add_tag_union( fields .iter() .enumerate() - .map(|(index, field)| format!( - "{INDENT}{INDENT}{INDENT}{}: arg{index},", - field.label() - )) + .map(|(index, field)| { + let (label, arg_val) = match field { + Field::NonRecursive(label, _) => { + (label, format!("arg{index}")) + } + Field::Recursive(label, field_id) => { + let field_type = type_name(*field_id, types); + + // recursive pointers need to be heap-allocated + ( + label, + format!( + r#"{{ + let size = core::mem::size_of::<{field_type}>(); + let align = core::mem::size_of::<{field_type}>() as u32; + + unsafe {{ + let ptr = crate::roc_alloc(size, align) as *mut {field_type}; + + *ptr = arg{index}; + + ptr + }} + }}"# + ), + ) + } + }; + format!("{INDENT}{INDENT}{INDENT}{label}: {arg_val},") + }) .collect::>() .join("\n") ); @@ -645,7 +675,7 @@ fn add_tag_union( pub unsafe fn into_{tag_name}({self_for_into}) -> {owned_ret_type} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{tag_name}); - let payload = {get_payload}; + let payload = {owned_get_payload}; {owned_ret} }}"#, @@ -663,7 +693,7 @@ fn add_tag_union( pub unsafe fn as_{tag_name}(&self) -> {borrowed_ret_type} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{tag_name}); - let payload = {get_payload}; + let payload = {borrowed_get_payload}; {borrowed_ret} }}"#, From 014a9deaf718f58ae95b8e11e39535319b655e69 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 09:13:16 -0400 Subject: [PATCH 20/65] Actually use discriminant argument --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 24b0934061..02d90df294 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -369,7 +369,7 @@ fn add_tag_union( // The discriminant is stored in the unused bytes at the end of the recursive pointer unsafe {{ let untagged = (self.{recursive_pointer_field} as usize) & (!{bitmask} as usize); - let tagged = untagged | (self.variant() as usize); + let tagged = untagged | (discriminant as usize); self.{recursive_pointer_field} = tagged as *mut Self; }} From 38969cde6886f2ca56e9dfc522daf78a1080a4e3 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 09:20:22 -0400 Subject: [PATCH 21/65] Use ptr::read over deref, to avoid move error --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 02d90df294..5222cae1c1 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -563,7 +563,7 @@ fn add_tag_union( } Field::Recursive(label, _) => { ret_values.push(format!( - "*((payload.{label} as usize & !{bitmask}) as *mut {field_type_name})" + "core::mem::read((payload.{label} as usize & !{bitmask}) as *const {field_type_name})" )); } } From 22b849bd82c7c4bac98061edfa48b212f0fe0a32 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 09:25:05 -0400 Subject: [PATCH 22/65] Don't use ptr::read for borrowed payloads --- bindgen/src/bindgen_rs.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 5222cae1c1..ae5d6f5169 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -550,7 +550,8 @@ fn add_tag_union( }); let mut ret_types = Vec::new(); - let mut ret_values = Vec::new(); + let mut borrowed_ret_values = Vec::new(); + let mut owned_ret_values = Vec::new(); for field in fields { let field_type_name = type_name(field.type_id(), types); @@ -559,12 +560,17 @@ fn add_tag_union( match field { Field::NonRecursive(label, _) => { - ret_values.push(format!("payload.{label}")); + let line = format!("payload.{label}"); + + owned_ret_values.push(line.clone()); + borrowed_ret_values.push(line); } Field::Recursive(label, _) => { - ret_values.push(format!( - "core::mem::read((payload.{label} as usize & !{bitmask}) as *const {field_type_name})" - )); + let line = format!( + "(payload.{label} as usize & !{bitmask}) as *const {field_type_name}" + ); + borrowed_ret_values.push(format!("&*({line})")); + owned_ret_values.push(format!("core::ptr::read({line})")); } } } @@ -616,7 +622,7 @@ fn add_tag_union( .join("\n") ); owned_ret = { - let lines = ret_values + let lines = owned_ret_values .iter() .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) .collect::>() @@ -625,9 +631,9 @@ fn add_tag_union( format!("({lines}\n{INDENT}{INDENT})") }; borrowed_ret = { - let lines = ret_values + let lines = borrowed_ret_values .iter() - .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) .collect::>() .join(", "); From 448e1ca717c5da2e1f9103dfd65d5dadbc62b6f3 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 11:20:49 -0400 Subject: [PATCH 23/65] Generate a separate `union` for recursive tag unions --- bindgen/src/bindgen_rs.rs | 177 ++++++++++++++++++++------------------ 1 file changed, 92 insertions(+), 85 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index ae5d6f5169..1abaf4a607 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -262,44 +262,39 @@ fn add_tag_union( let target_info = architecture.into(); let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); let size = typ.size(types, target_info); - - // Find the first recursive pointer field in the tags' payloads. - // TODO: what if there's more than one? Is it safe to assume the first - // one is it? What if it's another one? - let recursive_pointer_field = match recursiveness { - Recursiveness::Recursive => { - let opt_tag_and_field = tags.iter().find_map(|(tag_name, opt_payload_id)| { - if let Some(payload_id) = opt_payload_id { - match types.get(*payload_id) { - RocType::Struct { fields, .. } => { - fields.iter().find_map(|field| match field { - Field::NonRecursive(_, _) => None, - Field::Recursive(label, field_id) => { - debug_assert_eq!(*field_id, type_id); - - Some(label) - } - }) - } - _ => None, - } - .map(|label| (tag_name, label)) - } else { - None - } - }); - - match opt_tag_and_field { - Some((tag_name, label)) => format!("{tag_name}.{label}"), - None => String::new(), - } - } - Recursiveness::NonRecursive => String::new(), - }; + let union_name = format!("union_{name}"); { + // Recursive tag unions have a public struct, not a public union + let pub_str; + let decl_union_name: &str; + + match recursiveness { + Recursiveness::Recursive => { + add_decl( + impls, + None, + architecture, + format!( + r#" +pub struct {name} {{ + pointer: *mut {union_name}, +}} +"# + ), + ); + + pub_str = ""; + decl_union_name = &union_name; + } + Recursiveness::NonRecursive => { + pub_str = "pub "; + decl_union_name = name; + } + }; + // No #[derive(...)] for unions; we have to generate each impl ourselves! - let mut buf = format!("#[repr(C)]\npub union {name} {{\n"); + let mut buf = format!("#[repr(C)]\n{pub_str}union {decl_union_name} {{\n"); for (tag_name, opt_payload_id) in tags { // If there's no payload, we don't need a variant for it. @@ -354,7 +349,7 @@ fn add_tag_union( r#"{VARIANT_DOC_COMMENT} pub fn variant(&self) -> {discriminant_name} {{ // The discriminant is stored in the unused bytes at the end of the recursive pointer - unsafe {{ core::mem::transmute::((self.{recursive_pointer_field} as u8) & {bitmask}) }} + unsafe {{ core::mem::transmute::((self.pointer as u8) & {bitmask}) }} }}"# ), ); @@ -365,13 +360,13 @@ fn add_tag_union( architecture, format!( r#"/// Internal helper - fn set_discriminant(&mut self, discriminant: {discriminant_name}) {{ - // The discriminant is stored in the unused bytes at the end of the recursive pointer + fn set_discriminant(pointer: *mut {union_name}, discriminant: {discriminant_name}) -> *mut {union_name} {{ + // The discriminant is stored in the unused bytes at the end of the union pointer unsafe {{ - let untagged = (self.{recursive_pointer_field} as usize) & (!{bitmask} as usize); + let untagged = (pointer as usize) & (!{bitmask} as usize); let tagged = untagged | (discriminant as usize); - self.{recursive_pointer_field} = tagged as *mut Self; + tagged as *mut {union_name} }} }}"# ), @@ -449,8 +444,6 @@ fn add_tag_union( // } if let Some(payload_id) = opt_payload_id { let payload_type = types.get(*payload_id); - - let init_payload; let self_for_into; let payload_args; let args_to_payload; @@ -462,14 +455,37 @@ fn add_tag_union( let borrowed_ret; if payload_type.has_pointer(types) { - owned_get_payload = - format!("core::mem::ManuallyDrop::take(&mut self.{tag_name})",); - borrowed_get_payload = format!("&self.{tag_name}",); + owned_get_payload = format!( + r#"unsafe {{ + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + + core::mem::ManuallyDrop::take(&mut (*ptr).{tag_name}) + }}"# + ); + borrowed_get_payload = format!( + r#"unsafe {{ + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + + &(*ptr).{tag_name} + }}"# + ); // we need `mut self` for the argument because of ManuallyDrop self_for_into = "mut self"; } else { - owned_get_payload = format!("self.{tag_name}"); - borrowed_get_payload = format!("&{owned_get_payload}"); + owned_get_payload = format!( + r#"unsafe {{ + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + + core::ptr::read(ptr).{tag_name} + }}"# + ); + borrowed_get_payload = format!( + r#"unsafe {{ + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + + (&ptr).{tag_name} + }}"# + ); // we don't need `mut self` unless we need ManuallyDrop self_for_into = "self"; }; @@ -496,12 +512,6 @@ fn add_tag_union( | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) => { - if payload_type.has_pointer(types) { - init_payload = "core::mem::ManuallyDrop::new(payload)".to_string(); - } else { - init_payload = "payload".to_string(); - } - owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); payload_args = format!("arg: {owned_ret_type}"); @@ -510,16 +520,6 @@ fn add_tag_union( borrowed_ret = format!("&{owned_ret}"); } RocType::TransparentWrapper { content, .. } => { - let wrapper_type_name = type_name(*payload_id, types); - - if payload_type.has_pointer(types) { - init_payload = format!( - "core::mem::ManuallyDrop::new({wrapper_type_name}(payload))" - ); - } else { - init_payload = format!("{wrapper_type_name}(payload)"); - } - // This is a payload with 1 value, so we want to hide the wrapper // from the public API. owned_ret_type = type_name(*content, types); @@ -530,12 +530,6 @@ fn add_tag_union( borrowed_ret = format!("&{owned_ret}"); } RocType::Struct { fields, .. } => { - if payload_type.has_pointer(types) { - init_payload = "core::mem::ManuallyDrop::new(payload)".to_string(); - } else { - init_payload = "payload".to_string(); - } - let mut sorted_fields = fields.iter().collect::>(); sorted_fields.sort_by(|field1, field2| { @@ -584,7 +578,7 @@ fn add_tag_union( .collect::>() .join(", "); args_to_payload = format!( - "{payload_type_name} {{\n{}\n{INDENT}{INDENT}}}", + "core::mem::ManuallyDrop::new({payload_type_name} {{\n{}\n{INDENT}{INDENT}{INDENT}{INDENT}}})", fields .iter() .enumerate() @@ -601,22 +595,29 @@ fn add_tag_union( label, format!( r#"{{ - let size = core::mem::size_of::<{field_type}>(); - let align = core::mem::size_of::<{field_type}>() as u32; + let size = core::mem::size_of::<{field_type}>(); + let align = core::mem::size_of::<{field_type}>() as u32; - unsafe {{ - let ptr = crate::roc_alloc(size, align) as *mut {field_type}; + unsafe {{ + let ptr = crate::roc_alloc(size, align) as *mut {field_type}; - *ptr = arg{index}; + *ptr = arg{index}; - ptr - }} - }}"# + ptr + }} + }}"# ), ) } }; - format!("{INDENT}{INDENT}{INDENT}{label}: {arg_val},") + + let mut indents = String::new(); + + for _ in 0..5 { + indents.push_str(INDENT); + } + + format!("{indents}{label}: {arg_val},") }) .collect::>() .join("\n") @@ -658,14 +659,20 @@ fn add_tag_union( format!( r#"/// Construct a tag named {tag_name}, with the appropriate payload pub fn {tag_name}({payload_args}) -> Self {{ - let payload = {args_to_payload}; - let mut answer = Self {{ - {tag_name}: {init_payload} - }}; + let size = core::mem::size_of::<{union_name}>(); + let align = core::mem::size_of::<{union_name}>() as u32; - answer.set_discriminant({discriminant_name}::{tag_name}); + unsafe {{ + let ptr = crate::roc_alloc(size, align) as *mut {union_name}; - answer + *ptr = {union_name} {{ + {tag_name}: {args_to_payload} + }}; + + Self {{ + pointer: Self::set_discriminant(ptr, {discriminant_name}::{tag_name}), + }} + }} }}"# ), ); From fb5924ab611d4c324d7260766b192b32bdceeb17 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 12:37:51 -0400 Subject: [PATCH 24/65] Use self.pointer in more places, where appropriate --- bindgen/src/bindgen_rs.rs | 177 ++++++++++++++++++++++++-------------- 1 file changed, 111 insertions(+), 66 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 1abaf4a607..1b6673f774 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -14,6 +14,32 @@ const VARIANT_DOC_COMMENT: &str = type Impls = IndexMap>>; type Impl = Option; +/// Recursive tag unions need a custom Clone which bumps refcount. +const RECURSIVE_TAG_UNION_CLONE: &str = r#"fn clone(&self) -> Self { + if let Some(storage) = self.storage() { + let mut new_storage = storage.get(); + if !new_storage.is_readonly() { + new_storage.increment_reference_count(); + storage.set(new_storage); + } + } + + Self { + pointer: self.pointer + } +}"#; + +const RECURSIVE_TAG_UNION_STORAGE: &str = r#"#[inline(always)] + fn storage(&self) -> Option<&core::cell::Cell> { + if self.pointer.is_null() { + None + } else { + unsafe { + Some(&*self.pointer.cast::>().sub(1)) + } + } + }"#; + /// Add the given declaration body, along with the architecture, to the Impls. /// This can optionally be within an `impl`, or if no `impl` is specified, /// then it's added at the top level. @@ -263,6 +289,14 @@ fn add_tag_union( let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); let size = typ.size(types, target_info); let union_name = format!("union_{name}"); + let (actual_self, actual_self_mut, actual_other) = match recursiveness { + Recursiveness::Recursive => ( + "(&*self.pointer)", + "(&mut *self.pointer)", + "(&*other.pointer)", + ), + Recursiveness::NonRecursive => ("self", "self", "other"), + }; { // Recursive tag unions have a public struct, not a public union @@ -338,6 +372,13 @@ pub struct {name} {{ match recursiveness { Recursiveness::Recursive => { + add_decl( + impls, + opt_impl.clone(), + architecture, + RECURSIVE_TAG_UNION_STORAGE.to_string(), + ); + if tags.len() <= max_pointer_tagged_variants(architecture) { bitmask = format!("{:#b}", tagged_pointer_bitmask(architecture)); @@ -520,12 +561,18 @@ pub struct {name} {{ borrowed_ret = format!("&{owned_ret}"); } RocType::TransparentWrapper { content, .. } => { + let payload_type_name = type_name(*payload_id, types); + // This is a payload with 1 value, so we want to hide the wrapper // from the public API. owned_ret_type = type_name(*content, types); borrowed_ret_type = format!("&{}", owned_ret_type); payload_args = format!("arg: {owned_ret_type}"); - args_to_payload = "arg".to_string(); + args_to_payload = if types.get(*content).has_pointer(types) { + format!("core::mem::ManuallyDrop::new({payload_type_name}(arg))") + } else { + format!("{payload_type_name}(arg)") + }; owned_ret = "payload.0".to_string(); borrowed_ret = format!("&{owned_ret}"); } @@ -771,7 +818,7 @@ pub struct {name} {{ |tag_name, opt_payload_id| { match opt_payload_id { Some(payload_id) if types.get(payload_id).has_pointer(types) => { - format!("unsafe {{ core::mem::ManuallyDrop::drop(&mut self.{tag_name}) }},",) + format!("unsafe {{ core::mem::ManuallyDrop::drop(&mut {actual_self_mut}.{tag_name}) }},",) } _ => { // If it had no payload, or if the payload had no pointers, @@ -814,7 +861,7 @@ pub struct {name} {{ &mut buf, |tag_name, opt_payload_id| { if opt_payload_id.is_some() { - format!("self.{tag_name} == other.{tag_name},") + format!("{actual_self}.{tag_name} == {actual_other}.{tag_name},") } else { // if the tags themselves had been unequal, we already would have // early-returned with false, so this means the tags were equal @@ -837,12 +884,12 @@ pub struct {name} {{ { let opt_impl = Some(format!("impl PartialOrd for {name}")); let mut buf = r#"fn partial_cmp(&self, other: &Self) -> Option { - match self.variant().partial_cmp(&other.variant()) { - Some(core::cmp::Ordering::Equal) => {} - not_eq => return not_eq, - } + match self.variant().partial_cmp(&other.variant()) { + Some(core::cmp::Ordering::Equal) => {} + not_eq => return not_eq, + } - unsafe { + unsafe { "# .to_string(); @@ -853,7 +900,7 @@ pub struct {name} {{ &mut buf, |tag_name, opt_payload_id| { if opt_payload_id.is_some() { - format!("self.{tag_name}.partial_cmp(&other.{tag_name}),",) + format!("{actual_self}.{tag_name}.partial_cmp(&{actual_other}.{tag_name}),",) } else { // if the tags themselves had been unequal, we already would have // early-returned, so this means the tags were equal and there's @@ -892,7 +939,7 @@ pub struct {name} {{ &mut buf, |tag_name, opt_payload_id| { if opt_payload_id.is_some() { - format!("self.{tag_name}.cmp(&other.{tag_name}),",) + format!("{actual_self}.{tag_name}.cmp(&{actual_other}.{tag_name}),",) } else { // if the tags themselves had been unequal, we already would have // early-returned, so this means the tags were equal and there's @@ -920,46 +967,66 @@ pub struct {name} {{ }; let opt_impl = Some(format!("{opt_impl_prefix}impl Clone for {name}")); - let mut buf = r#"fn clone(&self) -> Self { + let body = match recursiveness { + Recursiveness::Recursive => RECURSIVE_TAG_UNION_CLONE.to_string(), + Recursiveness::NonRecursive => { + let mut buf = r#"fn clone(&self) -> Self { let mut answer = unsafe { "# - .to_string(); + .to_string(); - write_impl_tags( - 3, - tags.iter(), - &discriminant_name, - &mut buf, - |tag_name, opt_payload_id| { - if opt_payload_id.is_some() { - format!( - r#"Self {{ - {tag_name}: self.{tag_name}.clone(), + write_impl_tags( + 3, + tags.iter(), + &discriminant_name, + &mut buf, + |tag_name, opt_payload_id| { + if opt_payload_id.is_some() { + match recursiveness { + Recursiveness::Recursive => { + format!( + r#"Self {{ + {union_name} {{ + {tag_name}: self.pointer + }} }},"#, - ) - } else { - // when there's no payload, initialize to garbage memory. - format!( - r#"core::mem::transmute::< + ) + } + Recursiveness::NonRecursive => { + format!( + r#"Self {{ + {tag_name}: {actual_self}.{tag_name}.clone(), + }},"#, + ) + } + } + } else { + // when there's no payload, initialize to garbage memory. + format!( + r#"core::mem::transmute::< core::mem::MaybeUninit<{name}>, {name}, >(core::mem::MaybeUninit::uninit()),"#, - ) - } - }, - ); + ) + } + }, + ); - buf.push_str( - r#" + buf.push_str( + r#" }; answer.set_discriminant(self.variant()); answer }"#, - ); + ); - add_decl(impls, opt_impl, architecture, buf); + buf + } + }; + + add_decl(impls, opt_impl, architecture, body); } // The Hash impl for the tag union @@ -979,7 +1046,7 @@ pub struct {name} {{ format!( r#"unsafe {{ {hash_tag}; - self.{tag_name}.hash(state); + {actual_self}.{tag_name}.hash(state); }},"# ) } else { @@ -1022,7 +1089,7 @@ pub struct {name} {{ }; format!( - r#"f.debug_tuple("{tag_name}").field({deref_str}self.{tag_name}).finish(),"#, + r#"f.debug_tuple("{tag_name}").field({deref_str}{actual_self}.{tag_name}).finish(),"#, ) } None => format!(r#"f.write_str("{tag_name}"),"#), @@ -1268,17 +1335,7 @@ pub struct {name} {{ impls, opt_impl.clone(), architecture, - r#"#[inline(always)] - fn storage(&self) -> Option<&core::cell::Cell> { - if self.pointer.is_null() { - None - } else { - unsafe { - Some(&*self.pointer.cast::>().sub(1)) - } - } - }"# - .to_string(), + RECURSIVE_TAG_UNION_STORAGE.to_string(), ); add_decl( @@ -1437,24 +1494,12 @@ pub struct {name} {{ // Note that these never have Copy because they always contain a pointer. let opt_impl = Some(format!("impl Clone for {name}")); - // Recursive tag unions need a custom Clone which bumps refcount. - let body = r#"fn clone(&self) -> Self { - if let Some(storage) = self.storage() { - let mut new_storage = storage.get(); - if !new_storage.is_readonly() { - new_storage.increment_reference_count(); - storage.set(new_storage); - } - } - - Self { - pointer: self.pointer - } - } -"# - .to_string(); - - add_decl(impls, opt_impl, architecture, body); + add_decl( + impls, + opt_impl, + architecture, + RECURSIVE_TAG_UNION_CLONE.to_string(), + ); } // The Drop impl for the tag union From f30897c78d9578a88c38253cfebb700c4b5e9d01 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 13:32:27 -0400 Subject: [PATCH 25/65] Always untag the pointer in recursive unions --- bindgen/src/bindgen_rs.rs | 49 +++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 1b6673f774..e1345b7959 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -291,9 +291,9 @@ fn add_tag_union( let union_name = format!("union_{name}"); let (actual_self, actual_self_mut, actual_other) = match recursiveness { Recursiveness::Recursive => ( - "(&*self.pointer)", - "(&mut *self.pointer)", - "(&*other.pointer)", + "(&*self.union_pointer())", + "(&mut *self.union_pointer())", + "(&*other.union_pointer())", ), Recursiveness::NonRecursive => ("self", "self", "other"), }; @@ -401,7 +401,7 @@ pub struct {name} {{ architecture, format!( r#"/// Internal helper - fn set_discriminant(pointer: *mut {union_name}, discriminant: {discriminant_name}) -> *mut {union_name} {{ + fn tag_discriminant(pointer: *mut {union_name}, discriminant: {discriminant_name}) -> *mut {union_name} {{ // The discriminant is stored in the unused bytes at the end of the union pointer unsafe {{ let untagged = (pointer as usize) & (!{bitmask} as usize); @@ -409,6 +409,19 @@ pub struct {name} {{ tagged as *mut {union_name} }} + }}"# + ), + ); + + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"/// Internal helper + fn union_pointer(&self) -> *mut {union_name} {{ + // The discriminant is stored in the unused bytes at the end of the union pointer + ((self.pointer as usize) & (!{bitmask} as usize)) as *mut {union_name} }}"# ), ); @@ -498,34 +511,34 @@ pub struct {name} {{ if payload_type.has_pointer(types) { owned_get_payload = format!( r#"unsafe {{ - let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; - core::mem::ManuallyDrop::take(&mut (*ptr).{tag_name}) - }}"# + core::mem::ManuallyDrop::take(&mut (*ptr).{tag_name}) + }}"# ); borrowed_get_payload = format!( r#"unsafe {{ - let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; - &(*ptr).{tag_name} - }}"# + &(*ptr).{tag_name} + }}"# ); // we need `mut self` for the argument because of ManuallyDrop self_for_into = "mut self"; } else { owned_get_payload = format!( r#"unsafe {{ - let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; - core::ptr::read(ptr).{tag_name} - }}"# + core::ptr::read(ptr).{tag_name} + }}"# ); borrowed_get_payload = format!( r#"unsafe {{ - let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; + let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; - (&ptr).{tag_name} - }}"# + (&ptr).{tag_name} + }}"# ); // we don't need `mut self` unless we need ManuallyDrop self_for_into = "self"; @@ -643,7 +656,7 @@ pub struct {name} {{ format!( r#"{{ let size = core::mem::size_of::<{field_type}>(); - let align = core::mem::size_of::<{field_type}>() as u32; + let align = core::mem::align_of::<{field_type}>() as u32; unsafe {{ let ptr = crate::roc_alloc(size, align) as *mut {field_type}; @@ -717,7 +730,7 @@ pub struct {name} {{ }}; Self {{ - pointer: Self::set_discriminant(ptr, {discriminant_name}::{tag_name}), + pointer: Self::tag_discriminant(ptr, {discriminant_name}::{tag_name}), }} }} }}"# From 266e62820e76a9d9d0443fbf6d3e0beca198297f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 21:38:34 -0400 Subject: [PATCH 26/65] Make Recursiveness derive Copy --- bindgen/src/bindgen_rs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index e1345b7959..3379698caa 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -268,6 +268,7 @@ fn add_discriminant( discriminant_name } +#[derive(Copy, Clone)] enum Recursiveness { Recursive, NonRecursive, From 498bc9a88e6836bc88887933fd5258ff66531ead Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 21:39:17 -0400 Subject: [PATCH 27/65] Fix a Debug derivation --- bindgen/src/bindgen_rs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 3379698caa..62ad16df48 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -225,7 +225,7 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I | RocType::RocBox(_) => {} RocType::TransparentWrapper { name, content } => { let typ = types.get(id); - let derive = derive_str(typ, types, !typ.has_enumeration(types)); + let derive = derive_str(typ, types, true); let body = format!( "{derive}\n#[repr(transparent)]\npub struct {name}(pub {});", type_name(*content, types) @@ -1080,9 +1080,9 @@ pub struct {name} {{ let opt_impl = Some(format!("impl core::fmt::Debug for {name}")); let mut buf = format!( r#"fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {{ - f.write_str("{name}::")?; + f.write_str("{name}::")?; - unsafe {{ + unsafe {{ "# ); From 01ba5e8f36df116fdd04a430a8c9ab9d774a5364 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 21:39:31 -0400 Subject: [PATCH 28/65] Drop obsolete comment --- bindgen/src/load.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/bindgen/src/load.rs b/bindgen/src/load.rs index 7b0f4d3937..207e8e8ae0 100644 --- a/bindgen/src/load.rs +++ b/bindgen/src/load.rs @@ -19,8 +19,6 @@ pub fn load_types( dir: &Path, threading: Threading, ) -> Result, io::Error> { - // TODO: generate both 32-bit and 64-bit #[cfg] macros if structs are different - // depending on 32-bit vs 64-bit targets. let target_info = (&Triple::host()).into(); let arena = &Bump::new(); From f2a7ecb392916b3385faf724e8de1f69723837d6 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 22:45:05 -0400 Subject: [PATCH 29/65] Remove incorrect pointer indirection --- bindgen/src/bindgen_rs.rs | 70 +++++++-------------------------------- 1 file changed, 12 insertions(+), 58 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 62ad16df48..3448c37719 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -605,29 +605,13 @@ pub struct {name} {{ }); let mut ret_types = Vec::new(); - let mut borrowed_ret_values = Vec::new(); - let mut owned_ret_values = Vec::new(); + let mut ret_values = Vec::new(); for field in fields { - let field_type_name = type_name(field.type_id(), types); + let label = field.label(); - ret_types.push(field_type_name.clone()); - - match field { - Field::NonRecursive(label, _) => { - let line = format!("payload.{label}"); - - owned_ret_values.push(line.clone()); - borrowed_ret_values.push(line); - } - Field::Recursive(label, _) => { - let line = format!( - "(payload.{label} as usize & !{bitmask}) as *const {field_type_name}" - ); - borrowed_ret_values.push(format!("&*({line})")); - owned_ret_values.push(format!("core::ptr::read({line})")); - } - } + ret_values.push(format!("payload.{label}")); + ret_types.push(type_name(field.type_id(), types)); } let payload_type_name = type_name(*payload_id, types); @@ -644,47 +628,20 @@ pub struct {name} {{ .iter() .enumerate() .map(|(index, field)| { - let (label, arg_val) = match field { - Field::NonRecursive(label, _) => { - (label, format!("arg{index}")) - } - Field::Recursive(label, field_id) => { - let field_type = type_name(*field_id, types); - - // recursive pointers need to be heap-allocated - ( - label, - format!( - r#"{{ - let size = core::mem::size_of::<{field_type}>(); - let align = core::mem::align_of::<{field_type}>() as u32; - - unsafe {{ - let ptr = crate::roc_alloc(size, align) as *mut {field_type}; - - *ptr = arg{index}; - - ptr - }} - }}"# - ), - ) - } - }; - + let label = field.label(); let mut indents = String::new(); for _ in 0..5 { indents.push_str(INDENT); } - format!("{indents}{label}: {arg_val},") + format!("{indents}{label}: arg{index},") }) .collect::>() .join("\n") ); owned_ret = { - let lines = owned_ret_values + let lines = ret_values .iter() .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) .collect::>() @@ -693,9 +650,9 @@ pub struct {name} {{ format!("({lines}\n{INDENT}{INDENT})") }; borrowed_ret = { - let lines = borrowed_ret_values + let lines = ret_values .iter() - .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) .collect::>() .join(", "); @@ -1220,12 +1177,9 @@ fn add_struct( let mut buf = format!("{derive}\n#[repr(C)]\npub struct {name} {{\n"); for field in fields { - let (label, type_str) = match field { - Field::NonRecursive(label, field_id) => (label, type_name(*field_id, types)), - Field::Recursive(label, field_id) => { - (label, format!("*mut {}", type_name(*field_id, types))) - } - }; + let label = field.label(); + let type_str = type_name(field.type_id(), types); + buf.push_str(&format!("{INDENT}pub {label}: {type_str},\n",)); } From 29bc67a12f2aca2d9fc2c85c843912121d7ce730 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 23:01:07 -0400 Subject: [PATCH 30/65] Debug print tag union payloads as tuples --- bindgen/src/bindgen_rs.rs | 49 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 3448c37719..521fd55860 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1053,14 +1053,59 @@ pub struct {name} {{ // If it's a ManuallyDrop, we need a `*` prefix to dereference it // (because otherwise we're using ManuallyDrop's Debug instance // rather than the Debug instance of the value it wraps). - let deref_str = if types.get(payload_id).has_pointer(types) { + let payload_type = types.get(payload_id); + let deref_str = if payload_type.has_pointer(types) { "&*" } else { "&" }; + let fields_str = match payload_type { + RocType::RocStr + | RocType::Bool + | RocType::I8 + | RocType::U8 + | RocType::I16 + | RocType::U16 + | RocType::I32 + | RocType::U32 + | RocType::I64 + | RocType::U64 + | RocType::I128 + | RocType::U128 + | RocType::F32 + | RocType::F64 + | RocType::F128 + | RocType::RocDec + | RocType::RocList(_) + | RocType::RocDict(_, _) + | RocType::RocSet(_) + | RocType::RocBox(_) + | RocType::TagUnion(_) => { + format!(".field({deref_str}{actual_self}.{tag_name})") + } + RocType::TransparentWrapper { .. } => { + format!(".field(&({deref_str}{actual_self}.{tag_name}).0)") + } + RocType::Struct { fields, .. } => { + let mut buf = Vec::new(); + + for field in fields { + let label = field.label(); + + buf.push(format!( + ".field(&({deref_str}{actual_self}.{tag_name}).{label})" + )); + } + + buf.join("\n") + } + }; + format!( - r#"f.debug_tuple("{tag_name}").field({deref_str}{actual_self}.{tag_name}).finish(),"#, + r#"f.debug_tuple("{tag_name}") + {fields_str} + .finish(),"#, ) } None => format!(r#"f.write_str("{tag_name}"),"#), From 514595cca72dae5fdafe86e2b8a816c40ebb4dee Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 27 May 2022 23:08:21 -0400 Subject: [PATCH 31/65] Fix recursive tag union bindgen CLI test --- bindgen/tests/test_bindgen_cli.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bindgen/tests/test_bindgen_cli.rs b/bindgen/tests/test_bindgen_cli.rs index 0b7687630d..bd075840f4 100644 --- a/bindgen/tests/test_bindgen_cli.rs +++ b/bindgen/tests/test_bindgen_cli.rs @@ -122,9 +122,9 @@ mod bindgen_cli_run { `Nil` is: StrConsList::Nil "#), recursive_union:"recursive-union" => indoc!(r#" - tag_union was: StrConsList::Cons(StrConsList_Cons { f0: "World!", f1: StrConsList::Cons(StrConsList_Cons { f0: "Hello ", f1: StrConsList::Nil }) }) - `Cons "small str" Nil` is: StrConsList::Cons(StrConsList_Cons { f0: "small str", f1: StrConsList::Nil }) - `Nil` is: StrConsList::Nil + tag_union was: Expr::Concat(Expr::String("Hello, "), Expr::String("World!")) + `Concat (String "Hello, ") (String "World!")` is: Expr::Concat(Expr::String("Hello, "), Expr::String("World!")) + `String "this is a test"` is: Expr::String("this is a test") "#), } From 51e69e93c72866f8a3632938c253189f4544ba90 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 11:48:29 -0400 Subject: [PATCH 32/65] Attempt at making recursive tag unions better --- bindgen/src/bindgen_rs.rs | 51 +++++++++++++++++++++++++++++++++++++-- bindgen/src/types.rs | 38 ++++++++++++++++++++++------- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 521fd55860..e81cba38af 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -125,6 +125,9 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I RocType::Struct { name, fields } => { add_struct(name, architecture, fields, id, types, impls) } + RocType::TagUnionPayload { name, fields } => { + add_payload(name, architecture, fields, id, types, impls) + } RocType::TagUnion(tag_union) => { match tag_union { RocTagUnion::Enumeration { tags, name } => { @@ -591,7 +594,7 @@ pub struct {name} {{ borrowed_ret = format!("&{owned_ret}"); } RocType::Struct { fields, .. } => { - let mut sorted_fields = fields.iter().collect::>(); + let mut sorted_fields = fields.iter().collect::>>(); sorted_fields.sort_by(|field1, field2| { // Convert from e.g. "f12" to 12u64 @@ -1199,7 +1202,7 @@ fn add_enumeration, S: AsRef + Display>( fn add_struct( name: &str, architecture: Architecture, - fields: &[Field], + fields: &[Field], struct_id: TypeId, types: &Types, impls: &mut Impls, @@ -1235,6 +1238,49 @@ fn add_struct( } } +fn add_payload( + name: &str, + architecture: Architecture, + fields: &[Field], + payload_id: TypeId, + types: &Types, + impls: &mut Impls, +) { + match fields.len() { + 0 => { + // An empty payload is zero-sized and won't end up being passed to/from the host. + } + 1 => { + // Unwrap single-field payloads + add_type( + architecture, + fields.first().unwrap().type_id(), + types, + impls, + ) + } + _ => { + // these never get a Debug impl, because they should always be debug-printed + // using their tag union's custom Debug implementation. + let derive = derive_str(types.get(payload_id), types, false); + // payload structs are private + let mut buf = format!("{derive}\n#[repr(C)]\nstruct {name} {{\n"); + + for field in fields { + let label = field.label(); + let type_str = type_name(field.type_id(), types); + + // The labels are numeric, so print them as f0, f1, f2, etc. + buf.push_str(&format!("{INDENT}pub f{label}: {type_str},\n",)); + } + + buf.push('}'); + + add_decl(impls, None, architecture, buf); + } + } +} + fn type_name(id: TypeId, types: &Types) -> String { match types.get(id) { RocType::U8 => "u8".to_string(), @@ -1262,6 +1308,7 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } + | RocType::TagUnionPayload { name, .. } | RocType::TransparentWrapper { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 8ff3309f25..7902ec111c 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -4,7 +4,9 @@ use roc_collections::VecMap; use roc_mono::layout::UnionLayout; use roc_std::RocDec; use roc_target::TargetInfo; +use std::cmp::Ordering; use std::convert::TryInto; +use std::fmt::Display; #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct TypeId(usize); @@ -138,7 +140,7 @@ pub enum RocType { TagUnion(RocTagUnion), Struct { name: String, - fields: Vec, + fields: Vec>, }, /// Either a single-tag union or a single-field record TransparentWrapper { @@ -148,15 +150,15 @@ pub enum RocType { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum Field { - NonRecursive(String, TypeId), +pub enum Field { + NonRecursive(T, TypeId), /// A recursive field, e.g. in StrConsList : [Nil, Cons Str StrConsList], /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. - Recursive(String, TypeId), + Recursive(T, TypeId), } -impl Field { +impl Field { pub fn type_id(&self) -> TypeId { match self { Field::NonRecursive(_, type_id) => *type_id, @@ -164,7 +166,7 @@ impl Field { } } - pub fn label(&self) -> &str { + pub fn label(&self) -> &T { match self { Field::NonRecursive(label, _) => label, Field::Recursive(label, _) => label, @@ -172,6 +174,18 @@ impl Field { } } +impl Ord for Field { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.label().cmp(other.label()) + } +} + +impl PartialOrd for Field { + fn partial_cmp(&self, other: &Self) -> Option { + self.label().partial_cmp(other.label()) + } +} + impl RocType { /// Useful when determining whether to derive Copy in a Rust type. pub fn has_pointer(&self, types: &Types) -> bool { @@ -525,6 +539,12 @@ fn align_for_tag_count(num_tags: usize, target_info: TargetInfo) -> usize { } } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct TagUnionPayload { + name: String, + fields: Vec>, +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum RocTagUnion { Enumeration { @@ -535,13 +555,13 @@ pub enum RocTagUnion { /// e.g. `Result a e : [Ok a, Err e]` NonRecursive { name: String, - tags: Vec<(String, Option)>, + tags: Vec<(String, Option)>, }, /// A recursive tag union (general case) /// e.g. `Expr : [Sym Str, Add Expr Expr]` Recursive { name: String, - tags: Vec<(String, Option)>, + tags: Vec<(String, Option)>, }, /// A recursive tag union with just one constructor /// Optimization: No need to store a tag ID (the payload is "unwrapped") @@ -559,7 +579,7 @@ pub enum RocTagUnion { NullableWrapped { name: String, null_tag: String, - non_null_tags: Vec<(u16, String, Option)>, + non_null_tags: Vec<(u16, String, Option)>, }, /// A recursive tag union with only two variants, where one is empty. From 13acea47048c6c0927fd45e0f284d7c495c6193f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 11:48:42 -0400 Subject: [PATCH 33/65] Revert "Attempt at making recursive tag unions better" This reverts commit 51e69e93c72866f8a3632938c253189f4544ba90. --- bindgen/src/bindgen_rs.rs | 51 ++------------------------------------- bindgen/src/types.rs | 38 +++++++---------------------- 2 files changed, 11 insertions(+), 78 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index e81cba38af..521fd55860 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -125,9 +125,6 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I RocType::Struct { name, fields } => { add_struct(name, architecture, fields, id, types, impls) } - RocType::TagUnionPayload { name, fields } => { - add_payload(name, architecture, fields, id, types, impls) - } RocType::TagUnion(tag_union) => { match tag_union { RocTagUnion::Enumeration { tags, name } => { @@ -594,7 +591,7 @@ pub struct {name} {{ borrowed_ret = format!("&{owned_ret}"); } RocType::Struct { fields, .. } => { - let mut sorted_fields = fields.iter().collect::>>(); + let mut sorted_fields = fields.iter().collect::>(); sorted_fields.sort_by(|field1, field2| { // Convert from e.g. "f12" to 12u64 @@ -1202,7 +1199,7 @@ fn add_enumeration, S: AsRef + Display>( fn add_struct( name: &str, architecture: Architecture, - fields: &[Field], + fields: &[Field], struct_id: TypeId, types: &Types, impls: &mut Impls, @@ -1238,49 +1235,6 @@ fn add_struct( } } -fn add_payload( - name: &str, - architecture: Architecture, - fields: &[Field], - payload_id: TypeId, - types: &Types, - impls: &mut Impls, -) { - match fields.len() { - 0 => { - // An empty payload is zero-sized and won't end up being passed to/from the host. - } - 1 => { - // Unwrap single-field payloads - add_type( - architecture, - fields.first().unwrap().type_id(), - types, - impls, - ) - } - _ => { - // these never get a Debug impl, because they should always be debug-printed - // using their tag union's custom Debug implementation. - let derive = derive_str(types.get(payload_id), types, false); - // payload structs are private - let mut buf = format!("{derive}\n#[repr(C)]\nstruct {name} {{\n"); - - for field in fields { - let label = field.label(); - let type_str = type_name(field.type_id(), types); - - // The labels are numeric, so print them as f0, f1, f2, etc. - buf.push_str(&format!("{INDENT}pub f{label}: {type_str},\n",)); - } - - buf.push('}'); - - add_decl(impls, None, architecture, buf); - } - } -} - fn type_name(id: TypeId, types: &Types) -> String { match types.get(id) { RocType::U8 => "u8".to_string(), @@ -1308,7 +1262,6 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } - | RocType::TagUnionPayload { name, .. } | RocType::TransparentWrapper { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 7902ec111c..8ff3309f25 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -4,9 +4,7 @@ use roc_collections::VecMap; use roc_mono::layout::UnionLayout; use roc_std::RocDec; use roc_target::TargetInfo; -use std::cmp::Ordering; use std::convert::TryInto; -use std::fmt::Display; #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct TypeId(usize); @@ -140,7 +138,7 @@ pub enum RocType { TagUnion(RocTagUnion), Struct { name: String, - fields: Vec>, + fields: Vec, }, /// Either a single-tag union or a single-field record TransparentWrapper { @@ -150,15 +148,15 @@ pub enum RocType { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum Field { - NonRecursive(T, TypeId), +pub enum Field { + NonRecursive(String, TypeId), /// A recursive field, e.g. in StrConsList : [Nil, Cons Str StrConsList], /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. - Recursive(T, TypeId), + Recursive(String, TypeId), } -impl Field { +impl Field { pub fn type_id(&self) -> TypeId { match self { Field::NonRecursive(_, type_id) => *type_id, @@ -166,7 +164,7 @@ impl Field { } } - pub fn label(&self) -> &T { + pub fn label(&self) -> &str { match self { Field::NonRecursive(label, _) => label, Field::Recursive(label, _) => label, @@ -174,18 +172,6 @@ impl Field { } } -impl Ord for Field { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.label().cmp(other.label()) - } -} - -impl PartialOrd for Field { - fn partial_cmp(&self, other: &Self) -> Option { - self.label().partial_cmp(other.label()) - } -} - impl RocType { /// Useful when determining whether to derive Copy in a Rust type. pub fn has_pointer(&self, types: &Types) -> bool { @@ -539,12 +525,6 @@ fn align_for_tag_count(num_tags: usize, target_info: TargetInfo) -> usize { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct TagUnionPayload { - name: String, - fields: Vec>, -} - #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum RocTagUnion { Enumeration { @@ -555,13 +535,13 @@ pub enum RocTagUnion { /// e.g. `Result a e : [Ok a, Err e]` NonRecursive { name: String, - tags: Vec<(String, Option)>, + tags: Vec<(String, Option)>, }, /// A recursive tag union (general case) /// e.g. `Expr : [Sym Str, Add Expr Expr]` Recursive { name: String, - tags: Vec<(String, Option)>, + tags: Vec<(String, Option)>, }, /// A recursive tag union with just one constructor /// Optimization: No need to store a tag ID (the payload is "unwrapped") @@ -579,7 +559,7 @@ pub enum RocTagUnion { NullableWrapped { name: String, null_tag: String, - non_null_tags: Vec<(u16, String, Option)>, + non_null_tags: Vec<(u16, String, Option)>, }, /// A recursive tag union with only two variants, where one is empty. From 8866498469a9bec79de01283bd2b931c241c7edb Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 11:55:40 -0400 Subject: [PATCH 34/65] Add RocType::RecursivePointer --- bindgen/src/bindgen_rs.rs | 4 ++++ bindgen/src/types.rs | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 521fd55860..8f949bc705 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -233,6 +233,10 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I add_decl(impls, None, architecture, body); } + RocType::RecursivePointer(_) => { + // This is recursively pointing to a type that should already have been added, + // so no extra work needs to happen. + } } } diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 8ff3309f25..92c71e13b8 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -145,6 +145,10 @@ pub enum RocType { name: String, content: TypeId, }, + /// A recursive pointer, e.g. in StrConsList : [Nil, Cons Str StrConsList], + /// this would be the field of Cons containing the (recursive) StrConsList type, + /// and the TypeId is the TypeId of StrConsList itself. + RecursivePointer(TypeId), } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -200,7 +204,8 @@ impl RocType { | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { .. }) | RocType::TagUnion(RocTagUnion::NullableUnwrapped { .. }) | RocType::TagUnion(RocTagUnion::NullableWrapped { .. }) - | RocType::TagUnion(RocTagUnion::Recursive { .. }) => true, + | RocType::TagUnion(RocTagUnion::Recursive { .. }) + | RocType::RecursivePointer { .. } => true, RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => tags .iter() .any(|(_, payloads)| payloads.iter().any(|id| types.get(*id).has_pointer(types))), @@ -230,6 +235,7 @@ impl RocType { | RocType::U128 | RocType::RocDec | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, + RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { types.get(*id).has_float(types) } @@ -254,14 +260,15 @@ impl RocType { .. }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) - | RocType::TransparentWrapper { content, .. } => types.get(*content).has_float(types), + | RocType::TransparentWrapper { content, .. } + | RocType::RecursivePointer(content) => types.get(*content).has_float(types), } } /// Useful when determining whether to derive Default in a Rust type. pub fn has_enumeration(&self, types: &Types) -> bool { match self { - RocType::TagUnion { .. } => true, + RocType::TagUnion { .. } | RocType::RecursivePointer(_) => true, RocType::RocStr | RocType::Bool | RocType::I8 @@ -385,6 +392,7 @@ impl RocType { RocType::TransparentWrapper { content, .. } => { types.get(*content).size(types, target_info) } + RocType::RecursivePointer(_) => target_info.ptr_size(), } } @@ -479,6 +487,7 @@ impl RocType { .try_into() .unwrap() } + RocType::RecursivePointer(_) => target_info.ptr_alignment_bytes(), } } } From 96da12074d98293be3afabb3378a8204e80a7d04 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 12:27:20 -0400 Subject: [PATCH 35/65] Add RocType::PENDING_RECURSIVE_POINTER --- bindgen/src/types.rs | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 92c71e13b8..d6c855dc4e 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -151,32 +151,12 @@ pub enum RocType { RecursivePointer(TypeId), } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum Field { - NonRecursive(String, TypeId), - /// A recursive field, e.g. in StrConsList : [Nil, Cons Str StrConsList], - /// this would be the field of Cons containing the (recursive) StrConsList type, - /// and the TypeId is the TypeId of StrConsList itself. - Recursive(String, TypeId), -} - -impl Field { - pub fn type_id(&self) -> TypeId { - match self { - Field::NonRecursive(_, type_id) => *type_id, - Field::Recursive(_, type_id) => *type_id, - } - } - - pub fn label(&self) -> &str { - match self { - Field::NonRecursive(label, _) => label, - Field::Recursive(label, _) => label, - } - } -} - impl RocType { + /// Used when making recursive pointers, which need to temporarily + /// have *some* TypeId value until we later in the process determine + /// their real TypeId and can go back and fix them up. + pub(crate) const PENDING_RECURSIVE_POINTER: Self = Self::RecursivePointer(TypeId(usize::MAX)); + /// Useful when determining whether to derive Copy in a Rust type. pub fn has_pointer(&self, types: &Types) -> bool { match self { From d5ce6a39545eef516fc5f9bb2c14a383dd38d288 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 12:28:28 -0400 Subject: [PATCH 36/65] Drop types::Field in favor of RocType::RecursivePointer --- bindgen/src/types.rs | 56 +++++++++++++------------------------------- 1 file changed, 16 insertions(+), 40 deletions(-) diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index d6c855dc4e..042649fb86 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -138,7 +138,7 @@ pub enum RocType { TagUnion(RocTagUnion), Struct { name: String, - fields: Vec, + fields: Vec<(String, TypeId)>, }, /// Either a single-tag union or a single-field record TransparentWrapper { @@ -189,10 +189,9 @@ impl RocType { RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => tags .iter() .any(|(_, payloads)| payloads.iter().any(|id| types.get(*id).has_pointer(types))), - RocType::Struct { fields, .. } => fields.iter().any(|field| match field { - Field::NonRecursive(_, type_id) => types.get(*type_id).has_pointer(types), - Field::Recursive(_, _) => true, - }), + RocType::Struct { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_pointer(types)), RocType::TransparentWrapper { content, .. } => types.get(*content).has_pointer(types), } } @@ -222,12 +221,9 @@ impl RocType { RocType::RocDict(key_id, val_id) => { types.get(*key_id).has_float(types) || types.get(*val_id).has_float(types) } - RocType::Struct { fields, .. } => fields.iter().any(|field| match field { - Field::NonRecursive(_, type_id) => types.get(*type_id).has_float(types), - // This has a float iff there's a float somewhere else. - // We don't want to recurse here, because that would recurse forever! - Field::Recursive(_, _) => false, - }), + RocType::Struct { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_float(types)), RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => tags .iter() @@ -272,11 +268,9 @@ impl RocType { types.get(*key_id).has_enumeration(types) || types.get(*val_id).has_enumeration(types) } - RocType::Struct { fields, .. } => fields.iter().any(|field| match field { - Field::NonRecursive(_, type_id) => types.get(*type_id).has_enumeration(types), - // If this struct has a recursive field, that means we're inside an enumeration! - Field::Recursive(_, _) => true, - }), + RocType::Struct { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_enumeration(types)), RocType::TransparentWrapper { content, .. } => { types.get(*content).has_enumeration(types) } @@ -349,14 +343,8 @@ impl RocType { RocType::Struct { fields, .. } => { // The "unpadded" size (without taking alignment into account) // is the sum of all the sizes of the fields. - let size_unpadded = fields.iter().fold(0, |total, field| match field { - Field::NonRecursive(_, field_id) => { - total + types.get(*field_id).size(types, target_info) - } - Field::Recursive(_, _) => { - // The recursion var is a pointer. - total + target_info.ptr_size() - } + let size_unpadded = fields.iter().fold(0, |total, (_, field_id)| { + total + types.get(*field_id).size(types, target_info) }); // Round up to the next multiple of alignment, to incorporate @@ -431,14 +419,8 @@ impl RocType { align } - RocType::Struct { fields, .. } => fields.iter().fold(0, |align, field| match field { - Field::NonRecursive(_, field_id) => { - align.max(types.get(*field_id).alignment(types, target_info)) - } - Field::Recursive(_, _) => { - // The recursion var is a pointer. - align.max(target_info.ptr_alignment_bytes()) - } + RocType::Struct { fields, .. } => fields.iter().fold(0, |align, (_, field_id)| { + align.max(types.get(*field_id).alignment(types, target_info)) }), RocType::I8 => IntWidth::I8.alignment_bytes(target_info) as usize, RocType::U8 => IntWidth::U8.alignment_bytes(target_info) as usize, @@ -591,14 +573,8 @@ impl RocTagUnion { // into account, since the discriminant is // stored after those bytes. RocType::Struct { fields, .. } => { - fields.iter().fold(0, |total, field| match field { - Field::NonRecursive(_, field_id) => { - total + types.get(*field_id).size(types, target_info) - } - Field::Recursive(_, _) => { - // The recursion var is a pointer. - total + target_info.ptr_size() - } + fields.iter().fold(0, |total, (_, field_id)| { + total + types.get(*field_id).size(types, target_info) }) } typ => max_size.max(typ.size(types, target_info)), From 17a19818b68a1add2280bbad28f76149afb12969 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 12:29:07 -0400 Subject: [PATCH 37/65] Have Env::vars_to_types handle recursive pointers --- bindgen/src/bindgen.rs | 66 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 04fd58dd69..fc20895224 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -1,8 +1,9 @@ use crate::structs::Structs; -use crate::types::{Field, RocTagUnion, TypeId, Types}; +use crate::types::{RocTagUnion, TypeId, Types}; use crate::{enums::Enums, types::RocType}; use bumpalo::Bump; use roc_builtins::bitcode::{FloatWidth::*, IntWidth::*}; +use roc_collections::VecMap; use roc_module::ident::{Lowercase, TagName}; use roc_module::symbol::{Interns, Symbol}; use roc_mono::layout::{cmp_fields, ext_var_is_empty_tag_union, Builtin, Layout, LayoutCache}; @@ -19,10 +20,28 @@ pub struct Env<'a> { pub interns: &'a Interns, pub struct_names: Structs, pub enum_names: Enums, + pub pending_recursive_types: VecMap, + pub known_recursive_types: VecMap, } impl<'a> Env<'a> { - pub fn add_type(&mut self, var: Variable, types: &mut Types) -> TypeId { + pub fn vars_to_types(&mut self, variables: I) -> Types + where + I: IntoIterator, + V: AsRef, + { + let mut types = Types::default(); + + for var in variables { + self.add_type(*var.as_ref(), &mut types); + } + + self.resolve_pending_recursive_types(&mut types); + + types + } + + fn add_type(&mut self, var: Variable, types: &mut Types) -> TypeId { let layout = self .layout_cache .from_var(self.arena, var, self.subs) @@ -30,6 +49,31 @@ impl<'a> Env<'a> { add_type_help(self, layout, var, None, types, None) } + + fn add_pending_recursive_type(&self, type_id: TypeId, var: Variable) { + self.pending_recursive_types.insert(type_id, var); + } + + fn resolve_pending_recursive_types(&mut self, types: &mut Types) { + // TODO if VecMap gets a drain() method, use that instead of doing take() and into_iter + let pending = core::mem::take(&mut self.pending_recursive_types); + + for (type_id, var) in pending.into_iter() { + let actual_type_id = self.known_recursive_types.get(&var).unwrap_or_else(|| { + unreachable!( + "There was no known recursive TypeId for the pending recursive variable {:?}", + var + ); + }); + + debug_assert!(matches!( + types.get(type_id), + &RocType::PENDING_RECURSIVE_POINTER + )); + + types.replace(type_id, RocType::RecursivePointer(*actual_type_id)); + } + } } fn add_type_help<'a>( @@ -130,9 +174,12 @@ fn add_type_help<'a>( } Content::RangedNumber(_, _) => todo!(), Content::Error => todo!(), - Content::RecursionVar { .. } => { - // We should always skip over RecursionVars before we get here. - unreachable!() + Content::RecursionVar { structure, .. } => { + let type_id = types.add(RocType::PENDING_RECURSIVE_POINTER); + + env.add_pending_recursive_type(type_id, *structure); + + type_id } } } @@ -253,14 +300,9 @@ fn add_struct>( .into_iter() .map(|(label, field_var, field_layout)| { let content = subs.get_content_without_compacting(field_var); + let type_id = add_type_help(env, field_layout, field_var, None, types, None); - if matches!(content, Content::RecursionVar { .. }) { - Field::Recursive(label.to_string(), opt_recursion_id.unwrap()) - } else { - let type_id = add_type_help(env, field_layout, field_var, None, types, None); - - Field::NonRecursive(label.to_string(), type_id) - } + (label.to_string(), type_id) }) .collect(); From ce0422186deb3dc34008d85011282f3b3653ab67 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 12:43:39 -0400 Subject: [PATCH 38/65] Store type name in RocType::RecursivePointer --- bindgen/src/bindgen.rs | 26 ++++++++++++++++++++------ bindgen/src/types.rs | 25 +++++++++++++++---------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index fc20895224..5a06e37d8c 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -66,12 +66,23 @@ impl<'a> Env<'a> { ); }); - debug_assert!(matches!( - types.get(type_id), - &RocType::PENDING_RECURSIVE_POINTER - )); + let name = match types.get(type_id) { + RocType::RecursivePointer { + content: TypeId::PENDING, + name, + } => name.clone(), + _ => { + unreachable!("The TypeId {:?} was registered as a pending recursive pointer, but was not stored in Types as one.", type_id); + } + }; - types.replace(type_id, RocType::RecursivePointer(*actual_type_id)); + types.replace( + type_id, + RocType::RecursivePointer { + content: *actual_type_id, + name, + }, + ); } } } @@ -175,7 +186,10 @@ fn add_type_help<'a>( Content::RangedNumber(_, _) => todo!(), Content::Error => todo!(), Content::RecursionVar { structure, .. } => { - let type_id = types.add(RocType::PENDING_RECURSIVE_POINTER); + let type_id = types.add(RocType::RecursivePointer { + name: env.enum_names.get_name(*structure), + content: TypeId::PENDING, + }); env.add_pending_recursive_type(type_id, *structure); diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 042649fb86..434f87d8be 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -9,6 +9,13 @@ use std::convert::TryInto; #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct TypeId(usize); +impl TypeId { + /// Used when making recursive pointers, which need to temporarily + /// have *some* TypeId value until we later in the process determine + /// their real TypeId and can go back and fix them up. + pub(crate) const PENDING: Self = Self(usize::MAX); +} + #[derive(Default, Debug, Clone)] pub struct Types { by_id: Vec, @@ -148,15 +155,13 @@ pub enum RocType { /// A recursive pointer, e.g. in StrConsList : [Nil, Cons Str StrConsList], /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. - RecursivePointer(TypeId), + RecursivePointer { + name: String, + content: TypeId, + }, } impl RocType { - /// Used when making recursive pointers, which need to temporarily - /// have *some* TypeId value until we later in the process determine - /// their real TypeId and can go back and fix them up. - pub(crate) const PENDING_RECURSIVE_POINTER: Self = Self::RecursivePointer(TypeId(usize::MAX)); - /// Useful when determining whether to derive Copy in a Rust type. pub fn has_pointer(&self, types: &Types) -> bool { match self { @@ -237,14 +242,14 @@ impl RocType { }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) | RocType::TransparentWrapper { content, .. } - | RocType::RecursivePointer(content) => types.get(*content).has_float(types), + | RocType::RecursivePointer { content, .. } => types.get(*content).has_float(types), } } /// Useful when determining whether to derive Default in a Rust type. pub fn has_enumeration(&self, types: &Types) -> bool { match self { - RocType::TagUnion { .. } | RocType::RecursivePointer(_) => true, + RocType::TagUnion { .. } | RocType::RecursivePointer { .. } => true, RocType::RocStr | RocType::Bool | RocType::I8 @@ -360,7 +365,7 @@ impl RocType { RocType::TransparentWrapper { content, .. } => { types.get(*content).size(types, target_info) } - RocType::RecursivePointer(_) => target_info.ptr_size(), + RocType::RecursivePointer { .. } => target_info.ptr_size(), } } @@ -449,7 +454,7 @@ impl RocType { .try_into() .unwrap() } - RocType::RecursivePointer(_) => target_info.ptr_alignment_bytes(), + RocType::RecursivePointer { .. } => target_info.ptr_alignment_bytes(), } } } From 09fbfe621c288cc6c17915d12ef100a0347f16ef Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 12:56:25 -0400 Subject: [PATCH 39/65] Have `load` use env.vars_to_types --- bindgen/src/bindgen.rs | 9 +++-- bindgen/src/load.rs | 80 ++++++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 44 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 5a06e37d8c..9123cbe638 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -25,15 +25,14 @@ pub struct Env<'a> { } impl<'a> Env<'a> { - pub fn vars_to_types(&mut self, variables: I) -> Types + pub fn vars_to_types(&mut self, variables: I) -> Types where - I: IntoIterator, - V: AsRef, + I: IntoIterator, { let mut types = Types::default(); for var in variables { - self.add_type(*var.as_ref(), &mut types); + self.add_type(var, &mut types); } self.resolve_pending_recursive_types(&mut types); @@ -50,7 +49,7 @@ impl<'a> Env<'a> { add_type_help(self, layout, var, None, types, None) } - fn add_pending_recursive_type(&self, type_id: TypeId, var: Variable) { + fn add_pending_recursive_type(&mut self, type_id: TypeId, var: Variable) { self.pending_recursive_types.insert(type_id, var); } diff --git a/bindgen/src/load.rs b/bindgen/src/load.rs index 207e8e8ae0..3688ebfc83 100644 --- a/bindgen/src/load.rs +++ b/bindgen/src/load.rs @@ -59,53 +59,55 @@ pub fn load_types( let mut answer = Vec::with_capacity(Architecture::iter().size_hint().0); for architecture in Architecture::iter() { - let mut layout_cache = LayoutCache::new(architecture.into()); - let mut env = Env { - arena, - layout_cache: &mut layout_cache, - interns: &interns, - struct_names: Default::default(), - enum_names: Default::default(), - subs, - }; - let mut types = Types::default(); + let defs_iter = decls.iter().flat_map(|decl| match decl { + Declaration::Declare(def) => { + vec![def.clone()] + } + Declaration::DeclareRec(defs, cycle_mark) => { + if cycle_mark.is_illegal(subs) { + Vec::new() + } else { + defs.clone() + } + } + Declaration::Builtin(..) => { + unreachable!("Builtin decl in userspace module?") + } + Declaration::InvalidCycle(..) => Vec::new(), + }); - for decl in decls.iter() { - let defs = match decl { - Declaration::Declare(def) => { - vec![def.clone()] - } - Declaration::DeclareRec(defs, cycle_mark) => { - if cycle_mark.is_illegal(subs) { - Vec::new() - } else { - defs.clone() - } - } - Declaration::Builtin(..) => { - unreachable!("Builtin decl in userspace module?") - } - Declaration::InvalidCycle(..) => Vec::new(), - }; - - for Def { - loc_pattern, - pattern_vars, - .. - } in defs.into_iter() - { + let vars_iter = defs_iter.filter_map( + |Def { + loc_pattern, + pattern_vars, + .. + }| { if let Pattern::Identifier(sym) = loc_pattern.value { let var = pattern_vars .get(&sym) .expect("Indetifier known but it has no var?"); - env.add_type(*var, &mut types); + Some(*var) } else { - // figure out if we need to export non-identifier defs - when would that - // happen? + // figure out if we need to export non-identifier defs - when + // would that happen? + None } - } - } + }, + ); + + let mut layout_cache = LayoutCache::new(architecture.into()); + let mut env = Env { + arena, + layout_cache: &mut layout_cache, + interns: &interns, + subs, + struct_names: Default::default(), + enum_names: Default::default(), + pending_recursive_types: Default::default(), + known_recursive_types: Default::default(), + }; + let types = env.vars_to_types(vars_iter); answer.push((architecture, types)); } From 8a22cfbf3cf2cebdb560ac7193eccb249a174302 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 13:00:07 -0400 Subject: [PATCH 40/65] Drop some unused variables and arguments --- bindgen/src/bindgen.rs | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 9123cbe638..56da7a5077 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -46,7 +46,7 @@ impl<'a> Env<'a> { .from_var(self.arena, var, self.subs) .expect("Something weird ended up in the content"); - add_type_help(self, layout, var, None, types, None) + add_type_help(self, layout, var, None, types) } fn add_pending_recursive_type(&mut self, type_id: TypeId, var: Variable) { @@ -92,7 +92,6 @@ fn add_type_help<'a>( var: Variable, opt_name: Option, types: &mut Types, - opt_recursion_id: Option, ) -> TypeId { let subs = env.subs; @@ -124,7 +123,7 @@ fn add_type_help<'a>( None => env.struct_names.get_name(var), }; - add_struct(env, name, it, types, opt_recursion_id) + add_struct(env, name, it, types) } Content::Structure(FlatType::TagUnion(tags, ext_var)) => { debug_assert!(ext_var_is_empty_tag_union(subs, *ext_var)); @@ -179,7 +178,7 @@ fn add_type_help<'a>( } else { // If this was a non-builtin type alias, we can use that alias name // in the generated bindings. - add_type_help(env, layout, *real_var, Some(*name), types, opt_recursion_id) + add_type_help(env, layout, *real_var, Some(*name), types) } } Content::RangedNumber(_, _) => todo!(), @@ -227,8 +226,8 @@ fn add_builtin_type<'a>( Builtin::Str => types.add(RocType::RocStr), Builtin::Dict(key_layout, val_layout) => { // TODO FIXME this `var` is wrong - should have a different `var` for key and for val - let key_id = add_type_help(env, *key_layout, var, opt_name, types, None); - let val_id = add_type_help(env, *val_layout, var, opt_name, types, None); + let key_id = add_type_help(env, *key_layout, var, opt_name, types); + let val_id = add_type_help(env, *val_layout, var, opt_name, types); let dict_id = types.add(RocType::RocDict(key_id, val_id)); types.depends(dict_id, key_id); @@ -237,7 +236,7 @@ fn add_builtin_type<'a>( dict_id } Builtin::Set(elem_layout) => { - let elem_id = add_type_help(env, *elem_layout, var, opt_name, types, None); + let elem_id = add_type_help(env, *elem_layout, var, opt_name, types); let set_id = types.add(RocType::RocSet(elem_id)); types.depends(set_id, elem_id); @@ -245,7 +244,7 @@ fn add_builtin_type<'a>( set_id } Builtin::List(elem_layout) => { - let elem_id = add_type_help(env, *elem_layout, var, opt_name, types, None); + let elem_id = add_type_help(env, *elem_layout, var, opt_name, types); let list_id = types.add(RocType::RocList(elem_id)); types.depends(list_id, elem_id); @@ -260,7 +259,6 @@ fn add_struct>( name: String, fields: I, types: &mut Types, - opt_recursion_id: Option, ) -> TypeId { let subs = env.subs; let fields_iter = &mut fields.into_iter(); @@ -312,8 +310,7 @@ fn add_struct>( let fields = sortables .into_iter() .map(|(label, field_var, field_layout)| { - let content = subs.get_content_without_compacting(field_var); - let type_id = add_type_help(env, field_layout, field_var, None, types, None); + let type_id = add_type_help(env, field_layout, field_var, None, types); (label.to_string(), type_id) }) @@ -384,7 +381,7 @@ fn add_tag_union( // // ...then it's not even theoretically possible to instantiate one, so // bindgen won't be able to help you do that! - add_struct(env, name, fields, types, None) + add_struct(env, name, fields, types) } } } else { @@ -405,12 +402,6 @@ fn add_tag_union( // Sort tags alphabetically by tag name tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - let opt_recursion_id = if is_recursive_tag_union(&layout) { - Some(type_id) - } else { - None - }; - let mut tags: Vec<_> = tags .into_iter() .map(|(tag_name, payload_vars)| { @@ -419,7 +410,7 @@ fn add_tag_union( // no payload (tag_name, None) } - 1 if opt_recursion_id.is_none() => { + 1 if !is_recursive_tag_union(&layout) => { // this isn't recursive and there's 1 payload item, so it doesn't // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them // can have payloads of plain old Str, no struct wrapper needed. @@ -428,8 +419,7 @@ fn add_tag_union( .layout_cache .from_var(env.arena, *payload_var, env.subs) .expect("Something weird ended up in the content"); - let payload_id = - add_type_help(env, layout, *payload_var, None, types, opt_recursion_id); + let payload_id = add_type_help(env, layout, *payload_var, None, types); (tag_name, Some(payload_id)) } @@ -439,8 +429,7 @@ fn add_tag_union( let fields = payload_vars.iter().enumerate().map(|(index, payload_var)| { (format!("f{}", index).into(), *payload_var) }); - let struct_id = - add_struct(env, struct_name, fields, types, opt_recursion_id); + let struct_id = add_struct(env, struct_name, fields, types); (tag_name, Some(struct_id)) } From daef1b62eeb5032647dccae47efe79d93f209a44 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 13:00:27 -0400 Subject: [PATCH 41/65] Use RecursivePointer and new fields in bindgen_rs --- bindgen/src/bindgen_rs.rs | 46 ++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 8f949bc705..6b0629c806 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1,4 +1,4 @@ -use crate::types::{Field, RocTagUnion, RocType, TypeId, Types}; +use crate::types::{RocTagUnion, RocType, TypeId, Types}; use indexmap::IndexMap; use roc_mono::layout::UnionLayout; use roc_target::Architecture; @@ -233,7 +233,7 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I add_decl(impls, None, architecture, body); } - RocType::RecursivePointer(_) => { + RocType::RecursivePointer { .. } => { // This is recursively pointing to a type that should already have been added, // so no extra work needs to happen. } @@ -570,7 +570,8 @@ pub struct {name} {{ | RocType::RocDict(_, _) | RocType::RocSet(_) | RocType::RocBox(_) - | RocType::TagUnion(_) => { + | RocType::TagUnion(_) + | RocType::RecursivePointer { .. } => { owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); payload_args = format!("arg: {owned_ret_type}"); @@ -595,15 +596,15 @@ pub struct {name} {{ borrowed_ret = format!("&{owned_ret}"); } RocType::Struct { fields, .. } => { - let mut sorted_fields = fields.iter().collect::>(); + let mut sorted_fields = fields.iter().collect::>(); - sorted_fields.sort_by(|field1, field2| { + sorted_fields.sort_by(|(label1, _), (label2, _)| { // Convert from e.g. "f12" to 12u64 // This is necessary because the string "f10" sorts // to earlier than "f2", whereas the number 10 // sorts after the number 2. - let num1 = field1.label()[1..].parse::().unwrap(); - let num2 = field2.label()[1..].parse::().unwrap(); + let num1 = label1[1..].parse::().unwrap(); + let num2 = label2[1..].parse::().unwrap(); num1.partial_cmp(&num2).unwrap() }); @@ -611,11 +612,9 @@ pub struct {name} {{ let mut ret_types = Vec::new(); let mut ret_values = Vec::new(); - for field in fields { - let label = field.label(); - + for (label, type_id) in fields { ret_values.push(format!("payload.{label}")); - ret_types.push(type_name(field.type_id(), types)); + ret_types.push(type_name(*type_id, types)); } let payload_type_name = type_name(*payload_id, types); @@ -631,8 +630,7 @@ pub struct {name} {{ fields .iter() .enumerate() - .map(|(index, field)| { - let label = field.label(); + .map(|(index, (label, _))| { let mut indents = String::new(); for _ in 0..5 { @@ -1085,7 +1083,8 @@ pub struct {name} {{ | RocType::RocDict(_, _) | RocType::RocSet(_) | RocType::RocBox(_) - | RocType::TagUnion(_) => { + | RocType::TagUnion(_) + | RocType::RecursivePointer { .. } => { format!(".field({deref_str}{actual_self}.{tag_name})") } RocType::TransparentWrapper { .. } => { @@ -1094,9 +1093,7 @@ pub struct {name} {{ RocType::Struct { fields, .. } => { let mut buf = Vec::new(); - for field in fields { - let label = field.label(); - + for (label, _) in fields { buf.push(format!( ".field(&({deref_str}{actual_self}.{tag_name}).{label})" )); @@ -1203,7 +1200,7 @@ fn add_enumeration, S: AsRef + Display>( fn add_struct( name: &str, architecture: Architecture, - fields: &[Field], + fields: &[(String, TypeId)], struct_id: TypeId, types: &Types, impls: &mut Impls, @@ -1214,20 +1211,14 @@ fn add_struct( } 1 => { // Unwrap single-field records - add_type( - architecture, - fields.first().unwrap().type_id(), - types, - impls, - ) + add_type(architecture, fields.first().unwrap().1, types, impls) } _ => { let derive = derive_str(types.get(struct_id), types, true); let mut buf = format!("{derive}\n#[repr(C)]\npub struct {name} {{\n"); - for field in fields { - let label = field.label(); - let type_str = type_name(field.type_id(), types); + for (label, type_id) in fields { + let type_str = type_name(*type_id, types); buf.push_str(&format!("{INDENT}pub {label}: {type_str},\n",)); } @@ -1267,6 +1258,7 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } | RocType::TransparentWrapper { name, .. } + | RocType::RecursivePointer { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) | RocType::TagUnion(RocTagUnion::Enumeration { name, .. }) From 0de87593afb3ff4535f008970fff0bbf2979c1c9 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 15:32:34 -0400 Subject: [PATCH 42/65] Drop unnecessary function --- bindgen/src/bindgen.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 56da7a5077..4fffa5b75e 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -49,10 +49,6 @@ impl<'a> Env<'a> { add_type_help(self, layout, var, None, types) } - fn add_pending_recursive_type(&mut self, type_id: TypeId, var: Variable) { - self.pending_recursive_types.insert(type_id, var); - } - fn resolve_pending_recursive_types(&mut self, types: &mut Types) { // TODO if VecMap gets a drain() method, use that instead of doing take() and into_iter let pending = core::mem::take(&mut self.pending_recursive_types); @@ -189,7 +185,7 @@ fn add_type_help<'a>( content: TypeId::PENDING, }); - env.add_pending_recursive_type(type_id, *structure); + env.pending_recursive_types.insert(type_id, *structure); type_id } From ecfcdc12c314e9e44cbf9356943f584d2714010d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 15:32:47 -0400 Subject: [PATCH 43/65] Record known recursive types --- bindgen/src/bindgen.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 4fffa5b75e..0151938ab4 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -398,6 +398,8 @@ fn add_tag_union( // Sort tags alphabetically by tag name tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); + let is_recursive = is_recursive_tag_union(&layout); + let mut tags: Vec<_> = tags .into_iter() .map(|(tag_name, payload_vars)| { @@ -406,7 +408,7 @@ fn add_tag_union( // no payload (tag_name, None) } - 1 if !is_recursive_tag_union(&layout) => { + 1 if !is_recursive => { // this isn't recursive and there's 1 payload item, so it doesn't // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them // can have payloads of plain old Str, no struct wrapper needed. @@ -517,6 +519,10 @@ fn add_tag_union( types.replace(type_id, typ); + if is_recursive { + env.known_recursive_types.insert(var, type_id); + } + type_id } } From 2a8acb0f49fc1bb53a189e15afb85493975f35c1 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 15:33:56 -0400 Subject: [PATCH 44/65] Attempt to fix infinite recursion in has_float --- bindgen/src/bindgen_rs.rs | 59 ++++++++-------- bindgen/src/types.rs | 145 ++++++++++++++++++++++++++------------ 2 files changed, 130 insertions(+), 74 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 6b0629c806..895464f20d 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -131,20 +131,13 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I if tags.len() == 1 { // An enumeration with one tag is a zero-sized unit type, so // represent it as a zero-sized struct (e.g. "struct Foo()"). - let derive = derive_str(types.get(id), types, true); + let derive = derive_str(id, types, true); let struct_name = type_name(id, types); let body = format!("{derive}\nstruct {struct_name}();"); add_decl(impls, None, architecture, body); } else { - add_enumeration( - name, - architecture, - types.get(id), - tags.iter(), - types, - impls, - ) + add_enumeration(name, architecture, id, tags.iter(), types, impls) } } RocTagUnion::NonRecursive { tags, name } => { @@ -224,8 +217,7 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I | RocType::RocList(_) | RocType::RocBox(_) => {} RocType::TransparentWrapper { name, content } => { - let typ = types.get(id); - let derive = derive_str(typ, types, true); + let derive = derive_str(id, types, true); let body = format!( "{derive}\n#[repr(transparent)]\npub struct {name}(pub {});", type_name(*content, types) @@ -244,7 +236,6 @@ fn add_discriminant( name: &str, architecture: Architecture, tag_names: Vec, - types: &Types, impls: &mut Impls, ) -> String { // The tag union's discriminant, e.g. @@ -255,18 +246,17 @@ fn add_discriminant( // Foo, // } let discriminant_name = format!("variant_{name}"); - let discriminant_type = RocType::TagUnion(RocTagUnion::Enumeration { - name: discriminant_name.clone(), - tags: tag_names.clone(), - }); - add_enumeration( + // Don't call add_enumeration directly, because it needs a TypeId in order + // to correctly infer whether the enumeration contains floats...but it only + // does that in order to determine derive, and we already know exactly what to derive + // for a discriminant! + add_enumeration_with_derive( &discriminant_name, architecture, - &discriminant_type, tag_names.into_iter(), - types, impls, + "#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]", ); discriminant_name @@ -288,7 +278,7 @@ fn add_tag_union( impls: &mut Impls, ) { let tag_names = tags.iter().map(|(name, _)| name).cloned().collect(); - let discriminant_name = add_discriminant(name, architecture, tag_names, types, impls); + let discriminant_name = add_discriminant(name, architecture, tag_names, impls); let typ = types.get(type_id); let target_info = architecture.into(); let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); @@ -812,7 +802,7 @@ pub struct {name} {{ // The PartialEq impl for the tag union { - let opt_impl_prefix = if typ.has_float(types) { + let opt_impl_prefix = if types.has_float(type_id) { String::new() } else { format!("impl Eq for {name} {{}}\n\n") @@ -1156,17 +1146,27 @@ fn write_impl_tags< fn add_enumeration, S: AsRef + Display>( name: &str, architecture: Architecture, - typ: &RocType, + type_id: TypeId, tags: I, types: &Types, impls: &mut Impls, +) { + let derive = derive_str(type_id, types, false); + + add_enumeration_with_derive(name, architecture, tags, impls, &derive) +} + +fn add_enumeration_with_derive, S: AsRef + Display>( + name: &str, + architecture: Architecture, + tags: I, + impls: &mut Impls, + derive: &str, ) { let tag_bytes: usize = UnionLayout::discriminant_size(tags.len()) .stack_size() .try_into() .unwrap(); - - let derive = derive_str(typ, types, false); let repr_bytes = tag_bytes * 8; // e.g. "#[repr(u8)]\npub enum Foo {\n" @@ -1214,7 +1214,7 @@ fn add_struct( add_type(architecture, fields.first().unwrap().1, types, impls) } _ => { - let derive = derive_str(types.get(struct_id), types, true); + let derive = derive_str(struct_id, types, true); let mut buf = format!("{derive}\n#[repr(C)]\npub struct {name} {{\n"); for (label, type_id) in fields { @@ -1271,7 +1271,8 @@ fn type_name(id: TypeId, types: &Types) -> String { /// This explicitly asks for whether to include Debug because in the very specific /// case of a struct that's a payload for a recursive tag union, typ.has_enumeration() /// will return true, but actually we want to derive Debug here anyway. -fn derive_str(typ: &RocType, types: &Types, include_debug: bool) -> String { +fn derive_str(type_id: TypeId, types: &Types, include_debug: bool) -> String { + let typ = types.get(type_id); let mut buf = "#[derive(Clone, ".to_string(); if !typ.has_pointer(types) { @@ -1286,7 +1287,7 @@ fn derive_str(typ: &RocType, types: &Types, include_debug: bool) -> String { buf.push_str("Default, "); } - if !typ.has_float(types) { + if !types.has_float(type_id) { buf.push_str("Eq, Ord, Hash, "); } @@ -1311,7 +1312,7 @@ fn add_nullable_unwrapped( tag_names.sort(); - let discriminant_name = add_discriminant(name, architecture, tag_names, types, impls); + let discriminant_name = add_discriminant(name, architecture, tag_names, impls); let payload_type = types.get(non_null_payload); let payload_type_name = type_name(non_null_payload, types); let has_pointer = payload_type.has_pointer(types); @@ -1320,7 +1321,7 @@ fn add_nullable_unwrapped( { // This struct needs its own Clone impl because it has // a refcount to bump - let derive_extras = if types.get(id).has_float(types) { + let derive_extras = if types.has_float(id) { "" } else { ", Eq, Ord, Hash" diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 434f87d8be..3ad8d6ba8a 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -94,6 +94,106 @@ impl Types { len: self.by_id.len(), } } + + /// Useful when determining whether to derive Eq, Ord, and Hash in a Rust type. + pub fn has_float(&self, type_id: TypeId) -> bool { + self.has_float_help(type_id, &[]) + } + + fn has_float_help(&self, type_id: TypeId, do_not_recurse_on: &[TypeId]) -> bool { + match self.get(type_id) { + RocType::F32 | RocType::F64 | RocType::F128 => true, + RocType::RocStr + | RocType::Bool + | RocType::I8 + | RocType::U8 + | RocType::I16 + | RocType::U16 + | RocType::I32 + | RocType::U32 + | RocType::I64 + | RocType::U64 + | RocType::I128 + | RocType::U128 + | RocType::RocDec + | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, + RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { + if do_not_recurse_on.contains(id) { + false + } else { + self.has_float(*id) + } + } + RocType::RocDict(key_id, val_id) => { + if do_not_recurse_on.contains(key_id) || do_not_recurse_on.contains(val_id) { + false + } else { + self.has_float(*key_id) || self.has_float(*val_id) + } + } + RocType::Struct { fields, .. } => fields.iter().any(|(_, type_id)| { + if do_not_recurse_on.contains(type_id) { + false + } else { + self.has_float(*type_id) + } + }), + RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) + | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => { + let mut do_not_recurse_on: Vec = do_not_recurse_on.into(); + + do_not_recurse_on.push(type_id); + + tags.iter().any(|(_, payloads)| { + payloads.iter().any(|id| { + if do_not_recurse_on.contains(id) { + false + } else { + self.has_float_help(*id, &do_not_recurse_on) + } + }) + }) + } + RocType::TagUnion(RocTagUnion::NullableWrapped { non_null_tags, .. }) => { + let mut do_not_recurse_on: Vec = do_not_recurse_on.into(); + + do_not_recurse_on.push(type_id); + + non_null_tags.iter().any(|(_, _, payloads)| { + payloads.iter().any(|id| { + if do_not_recurse_on.contains(id) { + false + } else { + self.has_float_help(*id, &do_not_recurse_on) + } + }) + }) + } + RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) + | RocType::TagUnion(RocTagUnion::NullableUnwrapped { + non_null_payload: content, + .. + }) => { + let mut do_not_recurse_on: Vec = do_not_recurse_on.into(); + + do_not_recurse_on.push(type_id); + + if do_not_recurse_on.contains(&type_id) { + false + } else { + self.has_float_help(*content, &do_not_recurse_on) + } + } + RocType::TransparentWrapper { content, .. } + | RocType::RecursivePointer { content, .. } => { + if do_not_recurse_on.contains(&type_id) { + false + } else { + self.has_float(*content) + } + } + } + } } struct TypesIter<'a> { @@ -201,51 +301,6 @@ impl RocType { } } - /// Useful when determining whether to derive Eq, Ord, and Hash in a Rust type. - pub fn has_float(&self, types: &Types) -> bool { - match self { - RocType::F32 | RocType::F64 | RocType::F128 => true, - RocType::RocStr - | RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::RocDec - | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, - - RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { - types.get(*id).has_float(types) - } - RocType::RocDict(key_id, val_id) => { - types.get(*key_id).has_float(types) || types.get(*val_id).has_float(types) - } - RocType::Struct { fields, .. } => fields - .iter() - .any(|(_, type_id)| types.get(*type_id).has_float(types)), - RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) - | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => tags - .iter() - .any(|(_, payloads)| payloads.iter().any(|id| types.get(*id).has_float(types))), - RocType::TagUnion(RocTagUnion::NullableWrapped { non_null_tags, .. }) => non_null_tags - .iter() - .any(|(_, _, payloads)| payloads.iter().any(|id| types.get(*id).has_float(types))), - RocType::TagUnion(RocTagUnion::NullableUnwrapped { - non_null_payload: content, - .. - }) - | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) - | RocType::TransparentWrapper { content, .. } - | RocType::RecursivePointer { content, .. } => types.get(*content).has_float(types), - } - } - /// Useful when determining whether to derive Default in a Rust type. pub fn has_enumeration(&self, types: &Types) -> bool { match self { From 405c5a65e3814b109752e7e9afa1f313d1b63f08 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 15:34:13 -0400 Subject: [PATCH 45/65] Revert "Attempt to fix infinite recursion in has_float" This reverts commit 2a8acb0f49fc1bb53a189e15afb85493975f35c1. --- bindgen/src/bindgen_rs.rs | 59 ++++++++-------- bindgen/src/types.rs | 145 ++++++++++++-------------------------- 2 files changed, 74 insertions(+), 130 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 895464f20d..6b0629c806 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -131,13 +131,20 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I if tags.len() == 1 { // An enumeration with one tag is a zero-sized unit type, so // represent it as a zero-sized struct (e.g. "struct Foo()"). - let derive = derive_str(id, types, true); + let derive = derive_str(types.get(id), types, true); let struct_name = type_name(id, types); let body = format!("{derive}\nstruct {struct_name}();"); add_decl(impls, None, architecture, body); } else { - add_enumeration(name, architecture, id, tags.iter(), types, impls) + add_enumeration( + name, + architecture, + types.get(id), + tags.iter(), + types, + impls, + ) } } RocTagUnion::NonRecursive { tags, name } => { @@ -217,7 +224,8 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I | RocType::RocList(_) | RocType::RocBox(_) => {} RocType::TransparentWrapper { name, content } => { - let derive = derive_str(id, types, true); + let typ = types.get(id); + let derive = derive_str(typ, types, true); let body = format!( "{derive}\n#[repr(transparent)]\npub struct {name}(pub {});", type_name(*content, types) @@ -236,6 +244,7 @@ fn add_discriminant( name: &str, architecture: Architecture, tag_names: Vec, + types: &Types, impls: &mut Impls, ) -> String { // The tag union's discriminant, e.g. @@ -246,17 +255,18 @@ fn add_discriminant( // Foo, // } let discriminant_name = format!("variant_{name}"); + let discriminant_type = RocType::TagUnion(RocTagUnion::Enumeration { + name: discriminant_name.clone(), + tags: tag_names.clone(), + }); - // Don't call add_enumeration directly, because it needs a TypeId in order - // to correctly infer whether the enumeration contains floats...but it only - // does that in order to determine derive, and we already know exactly what to derive - // for a discriminant! - add_enumeration_with_derive( + add_enumeration( &discriminant_name, architecture, + &discriminant_type, tag_names.into_iter(), + types, impls, - "#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]", ); discriminant_name @@ -278,7 +288,7 @@ fn add_tag_union( impls: &mut Impls, ) { let tag_names = tags.iter().map(|(name, _)| name).cloned().collect(); - let discriminant_name = add_discriminant(name, architecture, tag_names, impls); + let discriminant_name = add_discriminant(name, architecture, tag_names, types, impls); let typ = types.get(type_id); let target_info = architecture.into(); let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); @@ -802,7 +812,7 @@ pub struct {name} {{ // The PartialEq impl for the tag union { - let opt_impl_prefix = if types.has_float(type_id) { + let opt_impl_prefix = if typ.has_float(types) { String::new() } else { format!("impl Eq for {name} {{}}\n\n") @@ -1146,27 +1156,17 @@ fn write_impl_tags< fn add_enumeration, S: AsRef + Display>( name: &str, architecture: Architecture, - type_id: TypeId, + typ: &RocType, tags: I, types: &Types, impls: &mut Impls, -) { - let derive = derive_str(type_id, types, false); - - add_enumeration_with_derive(name, architecture, tags, impls, &derive) -} - -fn add_enumeration_with_derive, S: AsRef + Display>( - name: &str, - architecture: Architecture, - tags: I, - impls: &mut Impls, - derive: &str, ) { let tag_bytes: usize = UnionLayout::discriminant_size(tags.len()) .stack_size() .try_into() .unwrap(); + + let derive = derive_str(typ, types, false); let repr_bytes = tag_bytes * 8; // e.g. "#[repr(u8)]\npub enum Foo {\n" @@ -1214,7 +1214,7 @@ fn add_struct( add_type(architecture, fields.first().unwrap().1, types, impls) } _ => { - let derive = derive_str(struct_id, types, true); + let derive = derive_str(types.get(struct_id), types, true); let mut buf = format!("{derive}\n#[repr(C)]\npub struct {name} {{\n"); for (label, type_id) in fields { @@ -1271,8 +1271,7 @@ fn type_name(id: TypeId, types: &Types) -> String { /// This explicitly asks for whether to include Debug because in the very specific /// case of a struct that's a payload for a recursive tag union, typ.has_enumeration() /// will return true, but actually we want to derive Debug here anyway. -fn derive_str(type_id: TypeId, types: &Types, include_debug: bool) -> String { - let typ = types.get(type_id); +fn derive_str(typ: &RocType, types: &Types, include_debug: bool) -> String { let mut buf = "#[derive(Clone, ".to_string(); if !typ.has_pointer(types) { @@ -1287,7 +1286,7 @@ fn derive_str(type_id: TypeId, types: &Types, include_debug: bool) -> String { buf.push_str("Default, "); } - if !types.has_float(type_id) { + if !typ.has_float(types) { buf.push_str("Eq, Ord, Hash, "); } @@ -1312,7 +1311,7 @@ fn add_nullable_unwrapped( tag_names.sort(); - let discriminant_name = add_discriminant(name, architecture, tag_names, impls); + let discriminant_name = add_discriminant(name, architecture, tag_names, types, impls); let payload_type = types.get(non_null_payload); let payload_type_name = type_name(non_null_payload, types); let has_pointer = payload_type.has_pointer(types); @@ -1321,7 +1320,7 @@ fn add_nullable_unwrapped( { // This struct needs its own Clone impl because it has // a refcount to bump - let derive_extras = if types.has_float(id) { + let derive_extras = if types.get(id).has_float(types) { "" } else { ", Eq, Ord, Hash" diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 3ad8d6ba8a..434f87d8be 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -94,106 +94,6 @@ impl Types { len: self.by_id.len(), } } - - /// Useful when determining whether to derive Eq, Ord, and Hash in a Rust type. - pub fn has_float(&self, type_id: TypeId) -> bool { - self.has_float_help(type_id, &[]) - } - - fn has_float_help(&self, type_id: TypeId, do_not_recurse_on: &[TypeId]) -> bool { - match self.get(type_id) { - RocType::F32 | RocType::F64 | RocType::F128 => true, - RocType::RocStr - | RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::RocDec - | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, - RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { - if do_not_recurse_on.contains(id) { - false - } else { - self.has_float(*id) - } - } - RocType::RocDict(key_id, val_id) => { - if do_not_recurse_on.contains(key_id) || do_not_recurse_on.contains(val_id) { - false - } else { - self.has_float(*key_id) || self.has_float(*val_id) - } - } - RocType::Struct { fields, .. } => fields.iter().any(|(_, type_id)| { - if do_not_recurse_on.contains(type_id) { - false - } else { - self.has_float(*type_id) - } - }), - RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) - | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => { - let mut do_not_recurse_on: Vec = do_not_recurse_on.into(); - - do_not_recurse_on.push(type_id); - - tags.iter().any(|(_, payloads)| { - payloads.iter().any(|id| { - if do_not_recurse_on.contains(id) { - false - } else { - self.has_float_help(*id, &do_not_recurse_on) - } - }) - }) - } - RocType::TagUnion(RocTagUnion::NullableWrapped { non_null_tags, .. }) => { - let mut do_not_recurse_on: Vec = do_not_recurse_on.into(); - - do_not_recurse_on.push(type_id); - - non_null_tags.iter().any(|(_, _, payloads)| { - payloads.iter().any(|id| { - if do_not_recurse_on.contains(id) { - false - } else { - self.has_float_help(*id, &do_not_recurse_on) - } - }) - }) - } - RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) - | RocType::TagUnion(RocTagUnion::NullableUnwrapped { - non_null_payload: content, - .. - }) => { - let mut do_not_recurse_on: Vec = do_not_recurse_on.into(); - - do_not_recurse_on.push(type_id); - - if do_not_recurse_on.contains(&type_id) { - false - } else { - self.has_float_help(*content, &do_not_recurse_on) - } - } - RocType::TransparentWrapper { content, .. } - | RocType::RecursivePointer { content, .. } => { - if do_not_recurse_on.contains(&type_id) { - false - } else { - self.has_float(*content) - } - } - } - } } struct TypesIter<'a> { @@ -301,6 +201,51 @@ impl RocType { } } + /// Useful when determining whether to derive Eq, Ord, and Hash in a Rust type. + pub fn has_float(&self, types: &Types) -> bool { + match self { + RocType::F32 | RocType::F64 | RocType::F128 => true, + RocType::RocStr + | RocType::Bool + | RocType::I8 + | RocType::U8 + | RocType::I16 + | RocType::U16 + | RocType::I32 + | RocType::U32 + | RocType::I64 + | RocType::U64 + | RocType::I128 + | RocType::U128 + | RocType::RocDec + | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, + + RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { + types.get(*id).has_float(types) + } + RocType::RocDict(key_id, val_id) => { + types.get(*key_id).has_float(types) || types.get(*val_id).has_float(types) + } + RocType::Struct { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_float(types)), + RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) + | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => tags + .iter() + .any(|(_, payloads)| payloads.iter().any(|id| types.get(*id).has_float(types))), + RocType::TagUnion(RocTagUnion::NullableWrapped { non_null_tags, .. }) => non_null_tags + .iter() + .any(|(_, _, payloads)| payloads.iter().any(|id| types.get(*id).has_float(types))), + RocType::TagUnion(RocTagUnion::NullableUnwrapped { + non_null_payload: content, + .. + }) + | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) + | RocType::TransparentWrapper { content, .. } + | RocType::RecursivePointer { content, .. } => types.get(*content).has_float(types), + } + } + /// Useful when determining whether to derive Default in a Rust type. pub fn has_enumeration(&self, types: &Types) -> bool { match self { From 5c7a5998ac35d9bbaec51077e2e299a8b6576d30 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 15:42:34 -0400 Subject: [PATCH 46/65] Remove name from RocType::RecursivePointer --- bindgen/src/bindgen.rs | 27 +++++++-------------------- bindgen/src/bindgen_rs.rs | 2 +- bindgen/src/types.rs | 7 ++----- 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 0151938ab4..7bd9b90903 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -61,23 +61,13 @@ impl<'a> Env<'a> { ); }); - let name = match types.get(type_id) { - RocType::RecursivePointer { - content: TypeId::PENDING, - name, - } => name.clone(), - _ => { - unreachable!("The TypeId {:?} was registered as a pending recursive pointer, but was not stored in Types as one.", type_id); - } - }; - - types.replace( - type_id, - RocType::RecursivePointer { - content: *actual_type_id, - name, - }, + debug_assert!( + matches!(types.get(type_id), RocType::RecursivePointer(TypeId::PENDING)), + "The TypeId {:?} was registered as a pending recursive pointer, but was not stored in Types as one.", + type_id ); + + types.replace(type_id, RocType::RecursivePointer(*actual_type_id)); } } } @@ -180,10 +170,7 @@ fn add_type_help<'a>( Content::RangedNumber(_, _) => todo!(), Content::Error => todo!(), Content::RecursionVar { structure, .. } => { - let type_id = types.add(RocType::RecursivePointer { - name: env.enum_names.get_name(*structure), - content: TypeId::PENDING, - }); + let type_id = types.add(RocType::RecursivePointer(TypeId::PENDING)); env.pending_recursive_types.insert(type_id, *structure); diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 6b0629c806..733e59de67 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1258,13 +1258,13 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } | RocType::TransparentWrapper { name, .. } - | RocType::RecursivePointer { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) | RocType::TagUnion(RocTagUnion::Enumeration { name, .. }) | RocType::TagUnion(RocTagUnion::NullableWrapped { name, .. }) | RocType::TagUnion(RocTagUnion::NullableUnwrapped { name, .. }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { name, .. }) => name.clone(), + RocType::RecursivePointer(content) => type_name(*content, types), } } diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 434f87d8be..602f2c74f2 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -155,10 +155,7 @@ pub enum RocType { /// A recursive pointer, e.g. in StrConsList : [Nil, Cons Str StrConsList], /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. - RecursivePointer { - name: String, - content: TypeId, - }, + RecursivePointer(TypeId), } impl RocType { @@ -242,7 +239,7 @@ impl RocType { }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) | RocType::TransparentWrapper { content, .. } - | RocType::RecursivePointer { content, .. } => types.get(*content).has_float(types), + | RocType::RecursivePointer(content) => types.get(*content).has_float(types), } } From 376198344f655293bf822c0daf322229bbf7fac4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 15:50:03 -0400 Subject: [PATCH 47/65] Fix infinite recursion in has_float --- bindgen/src/types.rs | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 602f2c74f2..f66f9dc309 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -200,6 +200,10 @@ impl RocType { /// Useful when determining whether to derive Eq, Ord, and Hash in a Rust type. pub fn has_float(&self, types: &Types) -> bool { + self.has_float_help(types, &[]) + } + + fn has_float_help(&self, types: &Types, do_not_recurse: &[TypeId]) -> bool { match self { RocType::F32 | RocType::F64 | RocType::F128 => true, RocType::RocStr @@ -216,30 +220,48 @@ impl RocType { | RocType::U128 | RocType::RocDec | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, - RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { - types.get(*id).has_float(types) + types.get(*id).has_float_help(types, do_not_recurse) } RocType::RocDict(key_id, val_id) => { - types.get(*key_id).has_float(types) || types.get(*val_id).has_float(types) + types.get(*key_id).has_float_help(types, do_not_recurse) + || types.get(*val_id).has_float_help(types, do_not_recurse) } RocType::Struct { fields, .. } => fields .iter() - .any(|(_, type_id)| types.get(*type_id).has_float(types)), + .any(|(_, type_id)| types.get(*type_id).has_float_help(types, do_not_recurse)), RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) - | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => tags - .iter() - .any(|(_, payloads)| payloads.iter().any(|id| types.get(*id).has_float(types))), - RocType::TagUnion(RocTagUnion::NullableWrapped { non_null_tags, .. }) => non_null_tags - .iter() - .any(|(_, _, payloads)| payloads.iter().any(|id| types.get(*id).has_float(types))), + | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => { + tags.iter().any(|(_, payloads)| { + payloads + .iter() + .any(|id| types.get(*id).has_float_help(types, do_not_recurse)) + }) + } + RocType::TagUnion(RocTagUnion::NullableWrapped { non_null_tags, .. }) => { + non_null_tags.iter().any(|(_, _, payloads)| { + payloads + .iter() + .any(|id| types.get(*id).has_float_help(types, do_not_recurse)) + }) + } RocType::TagUnion(RocTagUnion::NullableUnwrapped { non_null_payload: content, .. }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) | RocType::TransparentWrapper { content, .. } - | RocType::RecursivePointer(content) => types.get(*content).has_float(types), + | RocType::RecursivePointer(content) => { + if do_not_recurse.contains(content) { + false + } else { + let mut do_not_recurse: Vec = do_not_recurse.into(); + + do_not_recurse.push(*content); + + types.get(*content).has_float_help(types, &do_not_recurse) + } + } } } From 65a59711831ffb317fc37c9cb4e4a1b1f6b90cab Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 16:01:35 -0400 Subject: [PATCH 48/65] Consolidate numeric variants into RocType::Num --- bindgen/src/bindgen.rs | 33 ++++---- bindgen/src/bindgen_rs.rs | 77 +++++------------- bindgen/src/types.rs | 162 ++++++++++++++++++-------------------- 3 files changed, 114 insertions(+), 158 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 7bd9b90903..9803f74dec 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -1,6 +1,9 @@ use crate::structs::Structs; use crate::types::{RocTagUnion, TypeId, Types}; -use crate::{enums::Enums, types::RocType}; +use crate::{ + enums::Enums, + types::{RocNum, RocType}, +}; use bumpalo::Bump; use roc_builtins::bitcode::{FloatWidth::*, IntWidth::*}; use roc_collections::VecMap; @@ -188,24 +191,24 @@ fn add_builtin_type<'a>( ) -> TypeId { match builtin { Builtin::Int(width) => match width { - U8 => types.add(RocType::U8), - U16 => types.add(RocType::U16), - U32 => types.add(RocType::U32), - U64 => types.add(RocType::U64), - U128 => types.add(RocType::U128), - I8 => types.add(RocType::I8), - I16 => types.add(RocType::I16), - I32 => types.add(RocType::I32), - I64 => types.add(RocType::I64), - I128 => types.add(RocType::I128), + U8 => types.add(RocType::Num(RocNum::U8)), + U16 => types.add(RocType::Num(RocNum::U16)), + U32 => types.add(RocType::Num(RocNum::U32)), + U64 => types.add(RocType::Num(RocNum::U64)), + U128 => types.add(RocType::Num(RocNum::U128)), + I8 => types.add(RocType::Num(RocNum::I8)), + I16 => types.add(RocType::Num(RocNum::I16)), + I32 => types.add(RocType::Num(RocNum::I32)), + I64 => types.add(RocType::Num(RocNum::I64)), + I128 => types.add(RocType::Num(RocNum::I128)), }, Builtin::Float(width) => match width { - F32 => types.add(RocType::F32), - F64 => types.add(RocType::F64), - F128 => types.add(RocType::F128), + F32 => types.add(RocType::Num(RocNum::F32)), + F64 => types.add(RocType::Num(RocNum::F64)), + F128 => types.add(RocType::Num(RocNum::F128)), }, + Builtin::Decimal => types.add(RocType::Num(RocNum::Dec)), Builtin::Bool => types.add(RocType::Bool), - Builtin::Decimal => types.add(RocType::RocDec), Builtin::Str => types.add(RocType::RocStr), Builtin::Dict(key_layout, val_layout) => { // TODO FIXME this `var` is wrong - should have a different `var` for key and for val diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 733e59de67..11848bc50a 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1,4 +1,4 @@ -use crate::types::{RocTagUnion, RocType, TypeId, Types}; +use crate::types::{RocNum, RocTagUnion, RocType, TypeId, Types}; use indexmap::IndexMap; use roc_mono::layout::UnionLayout; use roc_target::Architecture; @@ -203,21 +203,8 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I } } // These types don't need to be declared in Rust. - RocType::U8 - | RocType::U16 - | RocType::U32 - | RocType::U64 - | RocType::U128 - | RocType::I8 - | RocType::I16 - | RocType::I32 - | RocType::I64 - | RocType::I128 - | RocType::F32 - | RocType::F64 - | RocType::F128 + RocType::Num(_) | RocType::Bool - | RocType::RocDec | RocType::RocStr | RocType::RocDict(_, _) | RocType::RocSet(_) @@ -552,20 +539,7 @@ pub struct {name} {{ match payload_type { RocType::RocStr | RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::F32 - | RocType::F64 - | RocType::F128 - | RocType::RocDec + | RocType::Num(_) | RocType::RocList(_) | RocType::RocDict(_, _) | RocType::RocSet(_) @@ -1065,20 +1039,7 @@ pub struct {name} {{ let fields_str = match payload_type { RocType::RocStr | RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::F32 - | RocType::F64 - | RocType::F128 - | RocType::RocDec + | RocType::Num(_) | RocType::RocList(_) | RocType::RocDict(_, _) | RocType::RocSet(_) @@ -1232,22 +1193,22 @@ fn add_struct( fn type_name(id: TypeId, types: &Types) -> String { match types.get(id) { - RocType::U8 => "u8".to_string(), - RocType::U16 => "u16".to_string(), - RocType::U32 => "u32".to_string(), - RocType::U64 => "u64".to_string(), - RocType::U128 => "roc_std::U128".to_string(), - RocType::I8 => "i8".to_string(), - RocType::I16 => "i16".to_string(), - RocType::I32 => "i32".to_string(), - RocType::I64 => "i64".to_string(), - RocType::I128 => "roc_std::I128".to_string(), - RocType::F32 => "f32".to_string(), - RocType::F64 => "f64".to_string(), - RocType::F128 => "roc_std::F128".to_string(), - RocType::Bool => "bool".to_string(), - RocType::RocDec => "roc_std::RocDec".to_string(), RocType::RocStr => "roc_std::RocStr".to_string(), + RocType::Bool => "bool".to_string(), + RocType::Num(RocNum::U8) => "u8".to_string(), + RocType::Num(RocNum::U16) => "u16".to_string(), + RocType::Num(RocNum::U32) => "u32".to_string(), + RocType::Num(RocNum::U64) => "u64".to_string(), + RocType::Num(RocNum::U128) => "roc_std::U128".to_string(), + RocType::Num(RocNum::I8) => "i8".to_string(), + RocType::Num(RocNum::I16) => "i16".to_string(), + RocType::Num(RocNum::I32) => "i32".to_string(), + RocType::Num(RocNum::I64) => "i64".to_string(), + RocType::Num(RocNum::I128) => "roc_std::I128".to_string(), + RocType::Num(RocNum::F32) => "f32".to_string(), + RocType::Num(RocNum::F64) => "f64".to_string(), + RocType::Num(RocNum::F128) => "roc_std::F128".to_string(), + RocType::Num(RocNum::Dec) => "roc_std::RocDec".to_string(), RocType::RocDict(key_id, val_id) => format!( "roc_std::RocDict<{}, {}>", type_name(*key_id, types), diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index f66f9dc309..47bf2ae6cd 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -124,20 +124,7 @@ impl<'a> Iterator for TypesIter<'a> { pub enum RocType { RocStr, Bool, - I8, - U8, - I16, - U16, - I32, - U32, - I64, - U64, - I128, - U128, - F32, - F64, - F128, - RocDec, + Num(RocNum), RocList(TypeId), RocDict(TypeId, TypeId), RocSet(TypeId), @@ -158,26 +145,76 @@ pub enum RocType { RecursivePointer(TypeId), } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum RocNum { + I8, + U8, + I16, + U16, + I32, + U32, + I64, + U64, + I128, + U128, + F32, + F64, + F128, + Dec, +} + +impl RocNum { + fn size(&self) -> usize { + use core::mem::size_of; + use RocNum::*; + + match self { + I8 => size_of::(), + U8 => size_of::(), + I16 => size_of::(), + U16 => size_of::(), + I32 => size_of::(), + U32 => size_of::(), + I64 => size_of::(), + U64 => size_of::(), + I128 => size_of::(), + U128 => size_of::(), + F32 => size_of::(), + F64 => size_of::(), + F128 => todo!(), + Dec => size_of::(), + } + } + + fn alignment(&self, target_info: TargetInfo) -> usize { + use RocNum::*; + + match self { + I8 => IntWidth::I8.alignment_bytes(target_info) as usize, + U8 => IntWidth::U8.alignment_bytes(target_info) as usize, + I16 => IntWidth::I16.alignment_bytes(target_info) as usize, + U16 => IntWidth::U16.alignment_bytes(target_info) as usize, + I32 => IntWidth::I32.alignment_bytes(target_info) as usize, + U32 => IntWidth::U32.alignment_bytes(target_info) as usize, + I64 => IntWidth::I64.alignment_bytes(target_info) as usize, + U64 => IntWidth::U64.alignment_bytes(target_info) as usize, + I128 => IntWidth::I128.alignment_bytes(target_info) as usize, + U128 => IntWidth::U128.alignment_bytes(target_info) as usize, + F32 => FloatWidth::F32.alignment_bytes(target_info) as usize, + F64 => FloatWidth::F64.alignment_bytes(target_info) as usize, + F128 => FloatWidth::F128.alignment_bytes(target_info) as usize, + Dec => align_of::(), + } + } +} + impl RocType { /// Useful when determining whether to derive Copy in a Rust type. pub fn has_pointer(&self, types: &Types) -> bool { match self { RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::F32 - | RocType::F64 - | RocType::F128 - | RocType::TagUnion(RocTagUnion::Enumeration { .. }) - | RocType::RocDec => false, + | RocType::Num(_) + | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, RocType::RocStr | RocType::RocList(_) | RocType::RocDict(_, _) @@ -205,20 +242,16 @@ impl RocType { fn has_float_help(&self, types: &Types, do_not_recurse: &[TypeId]) -> bool { match self { - RocType::F32 | RocType::F64 | RocType::F128 => true, + RocType::Num(num) => { + use RocNum::*; + + match num { + F32 | F64 | F128 => true, + I8 | U8 | I16 | U16 | I32 | U32 | I64 | U64 | I128 | U128 | Dec => false, + } + } RocType::RocStr | RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::RocDec | RocType::TagUnion(RocTagUnion::Enumeration { .. }) => false, RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { types.get(*id).has_float_help(types, do_not_recurse) @@ -269,22 +302,7 @@ impl RocType { pub fn has_enumeration(&self, types: &Types) -> bool { match self { RocType::TagUnion { .. } | RocType::RecursivePointer { .. } => true, - RocType::RocStr - | RocType::Bool - | RocType::I8 - | RocType::U8 - | RocType::I16 - | RocType::U16 - | RocType::I32 - | RocType::U32 - | RocType::I64 - | RocType::U64 - | RocType::I128 - | RocType::U128 - | RocType::F32 - | RocType::F64 - | RocType::F128 - | RocType::RocDec => false, + RocType::RocStr | RocType::Bool | RocType::Num(_) => false, RocType::RocList(id) | RocType::RocSet(id) | RocType::RocBox(id) => { types.get(*id).has_enumeration(types) } @@ -306,20 +324,7 @@ impl RocType { match self { RocType::Bool => size_of::(), - RocType::I8 => size_of::(), - RocType::U8 => size_of::(), - RocType::I16 => size_of::(), - RocType::U16 => size_of::(), - RocType::I32 => size_of::(), - RocType::U32 => size_of::(), - RocType::I64 => size_of::(), - RocType::U64 => size_of::(), - RocType::I128 => size_of::(), - RocType::U128 => size_of::(), - RocType::F32 => size_of::(), - RocType::F64 => size_of::(), - RocType::F128 => todo!(), - RocType::RocDec => size_of::(), + RocType::Num(num) => num.size(), RocType::RocStr | RocType::RocList(_) | RocType::RocDict(_, _) | RocType::RocSet(_) => { 3 * target_info.ptr_size() } @@ -395,7 +400,7 @@ impl RocType { | RocType::RocDict(_, _) | RocType::RocSet(_) | RocType::RocBox(_) => target_info.ptr_alignment_bytes(), - RocType::RocDec => align_of::(), + RocType::Num(num) => num.alignment(target_info), RocType::Bool => align_of::(), RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => { // The smallest alignment this could possibly have is based on the number of tags - e.g. @@ -446,19 +451,6 @@ impl RocType { RocType::Struct { fields, .. } => fields.iter().fold(0, |align, (_, field_id)| { align.max(types.get(*field_id).alignment(types, target_info)) }), - RocType::I8 => IntWidth::I8.alignment_bytes(target_info) as usize, - RocType::U8 => IntWidth::U8.alignment_bytes(target_info) as usize, - RocType::I16 => IntWidth::I16.alignment_bytes(target_info) as usize, - RocType::U16 => IntWidth::U16.alignment_bytes(target_info) as usize, - RocType::I32 => IntWidth::I32.alignment_bytes(target_info) as usize, - RocType::U32 => IntWidth::U32.alignment_bytes(target_info) as usize, - RocType::I64 => IntWidth::I64.alignment_bytes(target_info) as usize, - RocType::U64 => IntWidth::U64.alignment_bytes(target_info) as usize, - RocType::I128 => IntWidth::I128.alignment_bytes(target_info) as usize, - RocType::U128 => IntWidth::U128.alignment_bytes(target_info) as usize, - RocType::F32 => FloatWidth::F32.alignment_bytes(target_info) as usize, - RocType::F64 => FloatWidth::F64.alignment_bytes(target_info) as usize, - RocType::F128 => FloatWidth::F128.alignment_bytes(target_info) as usize, RocType::TransparentWrapper { content, .. } | RocType::TagUnion(RocTagUnion::NullableUnwrapped { non_null_payload: content, From 524ae1e3d8659c7ba7283eb44a3e50bfc3a8a390 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 19:17:10 -0400 Subject: [PATCH 49/65] Use RocType::TagUnionPayload for tag union payloads --- bindgen/src/bindgen.rs | 104 ++++++----------- bindgen/src/bindgen_rs.rs | 219 ++++++++++++++++++++++++------------ bindgen/src/types.rs | 68 ++++++++--- compiler/mono/src/layout.rs | 6 +- 4 files changed, 231 insertions(+), 166 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 9803f74dec..175c55afcb 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -7,7 +7,7 @@ use crate::{ use bumpalo::Bump; use roc_builtins::bitcode::{FloatWidth::*, IntWidth::*}; use roc_collections::VecMap; -use roc_module::ident::{Lowercase, TagName}; +use roc_module::ident::TagName; use roc_module::symbol::{Interns, Symbol}; use roc_mono::layout::{cmp_fields, ext_var_is_empty_tag_union, Builtin, Layout, LayoutCache}; use roc_types::subs::UnionTags; @@ -15,6 +15,7 @@ use roc_types::{ subs::{Content, FlatType, Subs, Variable}, types::RecordField, }; +use std::fmt::Display; pub struct Env<'a> { pub arena: &'a Bump, @@ -98,7 +99,7 @@ fn add_type_help<'a>( .flat_map(|(label, field)| { match field { RecordField::Required(field_var) | RecordField::Demanded(field_var) => { - Some((label.clone(), field_var)) + Some((label.to_string(), field_var)) } RecordField::Optional(_) => { // drop optional fields @@ -112,7 +113,10 @@ fn add_type_help<'a>( None => env.struct_names.get_name(var), }; - add_struct(env, name, it, types) + add_struct(env, name, it, types, |name, fields| RocType::Struct { + name, + fields, + }) } Content::Structure(FlatType::TagUnion(tags, ext_var)) => { debug_assert!(ext_var_is_empty_tag_union(subs, *ext_var)); @@ -240,40 +244,24 @@ fn add_builtin_type<'a>( } } -fn add_struct>( +fn add_struct( env: &mut Env<'_>, name: String, fields: I, types: &mut Types, -) -> TypeId { + to_type: F, +) -> TypeId +where + I: IntoIterator, + L: Display + Ord, + F: FnOnce(String, Vec<(L, TypeId)>) -> RocType, +{ let subs = env.subs; let fields_iter = &mut fields.into_iter(); - let first_field = match fields_iter.next() { - Some(field) => field, - None => { - // This is an empty record; there's no more work to do! - return types.add(RocType::Struct { - name, - fields: Vec::new(), - }); - } - }; - let second_field = match fields_iter.next() { - Some(field) => field, - None => { - // This is a single-field record; put it in a transparent wrapper. - let content = env.add_type(first_field.1, types); - - return types.add(RocType::TransparentWrapper { name, content }); - } - }; let mut sortables = - bumpalo::collections::Vec::with_capacity_in(2 + fields_iter.size_hint().0, env.arena); + bumpalo::collections::Vec::with_capacity_in(fields_iter.size_hint().0, env.arena); - for (label, field_var) in std::iter::once(first_field) - .chain(std::iter::once(second_field)) - .chain(fields_iter) - { + for (label, field_var) in fields_iter { sortables.push(( label, field_var, @@ -298,11 +286,11 @@ fn add_struct>( .map(|(label, field_var, field_layout)| { let type_id = add_type_help(env, field_layout, field_var, None, types); - (label.to_string(), type_id) + (label, type_id) }) - .collect(); + .collect::>(); - types.add(RocType::Struct { name, fields }) + types.add(to_type(name, fields)) } fn add_tag_union( @@ -335,41 +323,14 @@ fn add_tag_union( None => tag_name, }; - match payload_vars.len() { - 0 => { - // This is a single-tag union with no payload, e.g. `[Foo]` - // so just generate an empty record - types.add(RocType::Struct { - name, - fields: Vec::new(), - }) - } - 1 => { - // This is a single-tag union with 1 payload field, e.g.`[Foo Str]`. - // We'll just wrap that. - let var = *payload_vars.get(0).unwrap(); - let content = env.add_type(var, types); + let fields = payload_vars + .iter() + .enumerate() + .map(|(index, payload_var)| (index, *payload_var)); - types.add(RocType::TransparentWrapper { name, content }) - } - _ => { - // This is a single-tag union with multiple payload field, e.g.`[Foo Str U32]`. - // Generate a record. - let fields = payload_vars.iter().enumerate().map(|(index, payload_var)| { - let field_name = format!("f{}", index).into(); - - (field_name, *payload_var) - }); - - // Note that we assume no recursion variable here. If you write something like: - // - // Rec : [Blah Rec] - // - // ...then it's not even theoretically possible to instantiate one, so - // bindgen won't be able to help you do that! - add_struct(env, name, fields, types) - } - } + add_struct(env, name, fields, types, |name, fields| { + RocType::TagUnionPayload { name, fields } + }) } else { // This is a multi-tag union. @@ -412,12 +373,13 @@ fn add_tag_union( (tag_name, Some(payload_id)) } _ => { - // create a struct type for the payload and save it + // create a RocType for the payload and save it let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" - let fields = payload_vars.iter().enumerate().map(|(index, payload_var)| { - (format!("f{}", index).into(), *payload_var) - }); - let struct_id = add_struct(env, struct_name, fields, types); + let fields = payload_vars.iter().copied().enumerate(); + let struct_id = + add_struct(env, struct_name, fields, types, |name, fields| { + RocType::TagUnionPayload { name, fields } + }); (tag_name, Some(struct_id)) } diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 11848bc50a..ef28b9e3a0 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -123,7 +123,10 @@ pub fn emit(types_by_architecture: &[(Architecture, Types)]) -> String { fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut Impls) { match types.get(id) { RocType::Struct { name, fields } => { - add_struct(name, architecture, fields, id, types, impls) + add_struct(name, architecture, fields, id, types, impls, false) + } + RocType::TagUnionPayload { name, fields } => { + add_struct(name, architecture, fields, id, types, impls, true) } RocType::TagUnion(tag_union) => { match tag_union { @@ -570,79 +573,25 @@ pub struct {name} {{ borrowed_ret = format!("&{owned_ret}"); } RocType::Struct { fields, .. } => { - let mut sorted_fields = fields.iter().collect::>(); + let answer = + tag_union_struct_help(fields.iter(), *payload_id, types, false); - sorted_fields.sort_by(|(label1, _), (label2, _)| { - // Convert from e.g. "f12" to 12u64 - // This is necessary because the string "f10" sorts - // to earlier than "f2", whereas the number 10 - // sorts after the number 2. - let num1 = label1[1..].parse::().unwrap(); - let num2 = label2[1..].parse::().unwrap(); + payload_args = answer.payload_args; + args_to_payload = answer.args_to_payload; + owned_ret = answer.owned_ret; + borrowed_ret = answer.borrowed_ret; + owned_ret_type = answer.owned_ret_type; + borrowed_ret_type = answer.borrowed_ret_type; + } + RocType::TagUnionPayload { fields, .. } => { + let answer = tag_union_struct_help(fields.iter(), *payload_id, types, true); - num1.partial_cmp(&num2).unwrap() - }); - - let mut ret_types = Vec::new(); - let mut ret_values = Vec::new(); - - for (label, type_id) in fields { - ret_values.push(format!("payload.{label}")); - ret_types.push(type_name(*type_id, types)); - } - - let payload_type_name = type_name(*payload_id, types); - - payload_args = ret_types - .iter() - .enumerate() - .map(|(index, typ)| format!("arg{index}: {typ}")) - .collect::>() - .join(", "); - args_to_payload = format!( - "core::mem::ManuallyDrop::new({payload_type_name} {{\n{}\n{INDENT}{INDENT}{INDENT}{INDENT}}})", - fields - .iter() - .enumerate() - .map(|(index, (label, _))| { - let mut indents = String::new(); - - for _ in 0..5 { - indents.push_str(INDENT); - } - - format!("{indents}{label}: arg{index},") - }) - .collect::>() - .join("\n") - ); - owned_ret = { - let lines = ret_values - .iter() - .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) - .collect::>() - .join(", "); - - format!("({lines}\n{INDENT}{INDENT})") - }; - borrowed_ret = { - let lines = ret_values - .iter() - .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) - .collect::>() - .join(", "); - - format!("({lines}\n{INDENT}{INDENT})") - }; - owned_ret_type = format!("({})", ret_types.join(", ")); - borrowed_ret_type = format!( - "({})", - ret_types - .iter() - .map(|ret_type| { format!("&{ret_type}") }) - .collect::>() - .join(", ") - ); + payload_args = answer.payload_args; + args_to_payload = answer.args_to_payload; + owned_ret = answer.owned_ret; + borrowed_ret = answer.borrowed_ret; + owned_ret_type = answer.owned_ret_type; + borrowed_ret_type = answer.borrowed_ret_type; } }; @@ -1060,6 +1009,18 @@ pub struct {name} {{ )); } + buf.join("\n") + } + RocType::TagUnionPayload { fields, .. } => { + let mut buf = Vec::new(); + + for (label, _) in fields { + // Needs an "f" prefix + buf.push(format!( + ".field(&({deref_str}{actual_self}.{tag_name}).f{label})" + )); + } + buf.join("\n") } }; @@ -1158,13 +1119,14 @@ fn add_enumeration, S: AsRef + Display>( add_decl(impls, None, architecture, buf); } -fn add_struct( +fn add_struct( name: &str, architecture: Architecture, - fields: &[(String, TypeId)], + fields: &[(S, TypeId)], struct_id: TypeId, types: &Types, impls: &mut Impls, + is_tag_union_payload: bool, ) { match fields.len() { 0 => { @@ -1176,11 +1138,20 @@ fn add_struct( } _ => { let derive = derive_str(types.get(struct_id), types, true); - let mut buf = format!("{derive}\n#[repr(C)]\npub struct {name} {{\n"); + let pub_str = if is_tag_union_payload { "" } else { "pub " }; + let mut buf = format!("{derive}\n#[repr(C)]\n{pub_str}struct {name} {{\n"); for (label, type_id) in fields { let type_str = type_name(*type_id, types); + // Tag union payloads have numbered fields, so we prefix them + // with an "f" because Rust doesn't allow struct fields to be numbers. + let label = if is_tag_union_payload { + format!("f{label}") + } else { + format!("{label}") + }; + buf.push_str(&format!("{INDENT}pub {label}: {type_str},\n",)); } @@ -1218,6 +1189,7 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } + | RocType::TagUnionPayload { name, .. } | RocType::TransparentWrapper { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) @@ -1579,3 +1551,100 @@ fn tagged_pointer_bitmask(architecture: Architecture) -> u8 { Architecture::X86_32 | Architecture::Aarch32 | Architecture::Wasm32 => 0b0000_0011, } } + +struct StructIngredients { + payload_args: String, + args_to_payload: String, + owned_ret: String, + borrowed_ret: String, + owned_ret_type: String, + borrowed_ret_type: String, +} + +fn tag_union_struct_help<'a, I: Iterator, L: Display + PartialOrd + 'a>( + fields: I, + payload_id: TypeId, + types: &Types, + is_tag_union_payload: bool, +) -> StructIngredients { + let mut sorted_fields = fields.collect::>(); + + sorted_fields.sort_by(|(label1, _), (label2, _)| label1.partial_cmp(&label2).unwrap()); + + let mut ret_types = Vec::new(); + let mut ret_values = Vec::new(); + + for (label, type_id) in sorted_fields.iter() { + ret_values.push(format!("payload.{label}")); + ret_types.push(type_name(*type_id, types)); + } + + let payload_type_name = type_name(payload_id, types); + let payload_args = ret_types + .iter() + .enumerate() + .map(|(index, typ)| format!("arg{index}: {typ}")) + .collect::>() + .join(", "); + let args_to_payload = format!( + "core::mem::ManuallyDrop::new({payload_type_name} {{\n{}\n{INDENT}{INDENT}{INDENT}{INDENT}}})", + sorted_fields + .iter() + .enumerate() + .map(|(index, (label, _))| { + let mut indents = String::new(); + + for _ in 0..5 { + indents.push_str(INDENT); + } + + let label = if is_tag_union_payload { + // Tag union payload fields need "f" prefix + // because they're numbers + format!("f{}", label) + } else { + format!("{}", label) + }; + + format!("{indents}{label}: arg{index},") + }) + .collect::>() + .join("\n") + ); + let owned_ret = { + let lines = ret_values + .iter() + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) + .collect::>() + .join(", "); + + format!("({lines}\n{INDENT}{INDENT})") + }; + let borrowed_ret = { + let lines = ret_values + .iter() + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) + .collect::>() + .join(", "); + + format!("({lines}\n{INDENT}{INDENT})") + }; + let owned_ret_type = format!("({})", ret_types.join(", ")); + let borrowed_ret_type = format!( + "({})", + ret_types + .iter() + .map(|ret_type| { format!("&{ret_type}") }) + .collect::>() + .join(", ") + ); + + StructIngredients { + payload_args, + args_to_payload, + owned_ret, + borrowed_ret, + owned_ret_type, + borrowed_ret_type, + } +} diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index 47bf2ae6cd..b491203ab7 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -134,6 +134,10 @@ pub enum RocType { name: String, fields: Vec<(String, TypeId)>, }, + TagUnionPayload { + name: String, + fields: Vec<(usize, TypeId)>, + }, /// Either a single-tag union or a single-field record TransparentWrapper { name: String, @@ -231,6 +235,9 @@ impl RocType { RocType::Struct { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_pointer(types)), + RocType::TagUnionPayload { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_pointer(types)), RocType::TransparentWrapper { content, .. } => types.get(*content).has_pointer(types), } } @@ -263,6 +270,9 @@ impl RocType { RocType::Struct { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_float_help(types, do_not_recurse)), + RocType::TagUnionPayload { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_float_help(types, do_not_recurse)), RocType::TagUnion(RocTagUnion::Recursive { tags, .. }) | RocType::TagUnion(RocTagUnion::NonRecursive { tags, .. }) => { tags.iter().any(|(_, payloads)| { @@ -313,6 +323,9 @@ impl RocType { RocType::Struct { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_enumeration(types)), + RocType::TagUnionPayload { fields, .. } => fields + .iter() + .any(|(_, type_id)| types.get(*type_id).has_enumeration(types)), RocType::TransparentWrapper { content, .. } => { types.get(*content).has_enumeration(types) } @@ -369,23 +382,18 @@ impl RocType { RocTagUnion::NullableWrapped { .. } => todo!(), RocTagUnion::NullableUnwrapped { .. } => todo!(), }, - RocType::Struct { fields, .. } => { - // The "unpadded" size (without taking alignment into account) - // is the sum of all the sizes of the fields. - let size_unpadded = fields.iter().fold(0, |total, (_, field_id)| { - total + types.get(*field_id).size(types, target_info) - }); - - // Round up to the next multiple of alignment, to incorporate - // any necessary alignment padding. - // - // e.g. if we have a record with a Str and a U8, that would be a - // size_unpadded of 25, because Str is three 8-byte pointers and U8 is 1 byte, - // but the 8-byte alignment of the pointers means we'll round 25 up to 32. - let align = self.alignment(types, target_info); - - (size_unpadded / align) * align - } + RocType::Struct { fields, .. } => struct_size( + fields.iter().map(|(_, type_id)| *type_id), + types, + target_info, + self.alignment(types, target_info), + ), + RocType::TagUnionPayload { fields, .. } => struct_size( + fields.iter().map(|(_, type_id)| *type_id), + types, + target_info, + self.alignment(types, target_info), + ), RocType::TransparentWrapper { content, .. } => { types.get(*content).size(types, target_info) } @@ -451,6 +459,11 @@ impl RocType { RocType::Struct { fields, .. } => fields.iter().fold(0, |align, (_, field_id)| { align.max(types.get(*field_id).alignment(types, target_info)) }), + RocType::TagUnionPayload { fields, .. } => { + fields.iter().fold(0, |align, (_, field_id)| { + align.max(types.get(*field_id).alignment(types, target_info)) + }) + } RocType::TransparentWrapper { content, .. } | RocType::TagUnion(RocTagUnion::NullableUnwrapped { non_null_payload: content, @@ -470,6 +483,27 @@ impl RocType { } } +fn struct_size( + fields: impl Iterator, + types: &Types, + target_info: TargetInfo, + align: usize, +) -> usize { + // The "unpadded" size (without taking alignment into account) + // is the sum of all the sizes of the fields. + let size_unpadded = fields.fold(0, |total, field_id| { + total + types.get(field_id).size(types, target_info) + }); + + // Round up to the next multiple of alignment, to incorporate + // any necessary alignment padding. + // + // e.g. if we have a record with a Str and a U8, that would be a + // size_unpadded of 25, because Str is three 8-byte pointers and U8 is 1 byte, + // but the 8-byte alignment of the pointers means we'll round 25 up to 32. + (size_unpadded / align) * align +} + fn size_for_tag_count(num_tags: usize) -> usize { if num_tags == 0 { // empty tag union diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index a3c5c261ba..6a83809d17 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -2971,10 +2971,10 @@ mod test { /// This is called by both code gen and bindgen, so that /// their field orderings agree. #[inline(always)] -pub fn cmp_fields( - label1: &Lowercase, +pub fn cmp_fields( + label1: &L, layout1: &Layout<'_>, - label2: &Lowercase, + label2: &L, layout2: &Layout<'_>, target_info: TargetInfo, ) -> Ordering { From bdf4d9d3cce0f8bca4bd9594ff6112b0230c8e14 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 19:20:08 -0400 Subject: [PATCH 50/65] Remove unnecessary placeholder string --- bindgen/src/bindgen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 175c55afcb..23438779e1 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -337,7 +337,7 @@ fn add_tag_union( // This is a placeholder so that we can get a TypeId for future recursion IDs. // At the end, we will replace this with the real tag union type. let type_id = types.add(RocType::Struct { - name: "[THIS SHOULD BE REMOVED]".to_string(), + name: String::new(), fields: Vec::new(), }); let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); From 7ded31cbc34238e33804a977f0ddcb7c9c922bb7 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 19:20:22 -0400 Subject: [PATCH 51/65] Drop RocTypes::TransparentWrapper --- bindgen/src/bindgen_rs.rs | 75 +++++++++------------------------------ bindgen/src/types.rs | 16 +-------- 2 files changed, 18 insertions(+), 73 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index ef28b9e3a0..6f4931c868 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -213,16 +213,6 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I | RocType::RocSet(_) | RocType::RocList(_) | RocType::RocBox(_) => {} - RocType::TransparentWrapper { name, content } => { - let typ = types.get(id); - let derive = derive_str(typ, types, true); - let body = format!( - "{derive}\n#[repr(transparent)]\npub struct {name}(pub {});", - type_name(*content, types) - ); - - add_decl(impls, None, architecture, body); - } RocType::RecursivePointer { .. } => { // This is recursively pointing to a type that should already have been added, // so no extra work needs to happen. @@ -556,22 +546,6 @@ pub struct {name} {{ owned_ret = "payload".to_string(); borrowed_ret = format!("&{owned_ret}"); } - RocType::TransparentWrapper { content, .. } => { - let payload_type_name = type_name(*payload_id, types); - - // This is a payload with 1 value, so we want to hide the wrapper - // from the public API. - owned_ret_type = type_name(*content, types); - borrowed_ret_type = format!("&{}", owned_ret_type); - payload_args = format!("arg: {owned_ret_type}"); - args_to_payload = if types.get(*content).has_pointer(types) { - format!("core::mem::ManuallyDrop::new({payload_type_name}(arg))") - } else { - format!("{payload_type_name}(arg)") - }; - owned_ret = "payload.0".to_string(); - borrowed_ret = format!("&{owned_ret}"); - } RocType::Struct { fields, .. } => { let answer = tag_union_struct_help(fields.iter(), *payload_id, types, false); @@ -997,9 +971,6 @@ pub struct {name} {{ | RocType::RecursivePointer { .. } => { format!(".field({deref_str}{actual_self}.{tag_name})") } - RocType::TransparentWrapper { .. } => { - format!(".field(&({deref_str}{actual_self}.{tag_name}).0)") - } RocType::Struct { fields, .. } => { let mut buf = Vec::new(); @@ -1128,38 +1099,27 @@ fn add_struct( impls: &mut Impls, is_tag_union_payload: bool, ) { - match fields.len() { - 0 => { - // An empty record is zero-sized and won't end up being passed to/from the host. - } - 1 => { - // Unwrap single-field records - add_type(architecture, fields.first().unwrap().1, types, impls) - } - _ => { - let derive = derive_str(types.get(struct_id), types, true); - let pub_str = if is_tag_union_payload { "" } else { "pub " }; - let mut buf = format!("{derive}\n#[repr(C)]\n{pub_str}struct {name} {{\n"); + let derive = derive_str(types.get(struct_id), types, true); + let pub_str = if is_tag_union_payload { "" } else { "pub " }; + let mut buf = format!("{derive}\n#[repr(C)]\n{pub_str}struct {name} {{\n"); - for (label, type_id) in fields { - let type_str = type_name(*type_id, types); + for (label, type_id) in fields { + let type_str = type_name(*type_id, types); - // Tag union payloads have numbered fields, so we prefix them - // with an "f" because Rust doesn't allow struct fields to be numbers. - let label = if is_tag_union_payload { - format!("f{label}") - } else { - format!("{label}") - }; + // Tag union payloads have numbered fields, so we prefix them + // with an "f" because Rust doesn't allow struct fields to be numbers. + let label = if is_tag_union_payload { + format!("f{label}") + } else { + format!("{label}") + }; - buf.push_str(&format!("{INDENT}pub {label}: {type_str},\n",)); - } - - buf.push('}'); - - add_decl(impls, None, architecture, buf); - } + buf.push_str(&format!("{INDENT}pub {label}: {type_str},\n",)); } + + buf.push('}'); + + add_decl(impls, None, architecture, buf); } fn type_name(id: TypeId, types: &Types) -> String { @@ -1190,7 +1150,6 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), RocType::Struct { name, .. } | RocType::TagUnionPayload { name, .. } - | RocType::TransparentWrapper { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) | RocType::TagUnion(RocTagUnion::Enumeration { name, .. }) diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index b491203ab7..d5b4327d32 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -138,11 +138,6 @@ pub enum RocType { name: String, fields: Vec<(usize, TypeId)>, }, - /// Either a single-tag union or a single-field record - TransparentWrapper { - name: String, - content: TypeId, - }, /// A recursive pointer, e.g. in StrConsList : [Nil, Cons Str StrConsList], /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. @@ -238,7 +233,6 @@ impl RocType { RocType::TagUnionPayload { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_pointer(types)), - RocType::TransparentWrapper { content, .. } => types.get(*content).has_pointer(types), } } @@ -293,7 +287,6 @@ impl RocType { .. }) | RocType::TagUnion(RocTagUnion::NonNullableUnwrapped { content, .. }) - | RocType::TransparentWrapper { content, .. } | RocType::RecursivePointer(content) => { if do_not_recurse.contains(content) { false @@ -326,9 +319,6 @@ impl RocType { RocType::TagUnionPayload { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_enumeration(types)), - RocType::TransparentWrapper { content, .. } => { - types.get(*content).has_enumeration(types) - } } } @@ -394,9 +384,6 @@ impl RocType { target_info, self.alignment(types, target_info), ), - RocType::TransparentWrapper { content, .. } => { - types.get(*content).size(types, target_info) - } RocType::RecursivePointer { .. } => target_info.ptr_size(), } } @@ -464,8 +451,7 @@ impl RocType { align.max(types.get(*field_id).alignment(types, target_info)) }) } - RocType::TransparentWrapper { content, .. } - | RocType::TagUnion(RocTagUnion::NullableUnwrapped { + RocType::TagUnion(RocTagUnion::NullableUnwrapped { non_null_payload: content, .. }) From 8369e376b05af6a2db07032ba579cd292109de83 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 19:23:40 -0400 Subject: [PATCH 52/65] Don't wrap 1-tuples in parens --- bindgen/src/bindgen_rs.rs | 64 ++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 6f4931c868..4322cd5432 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1570,33 +1570,47 @@ fn tag_union_struct_help<'a, I: Iterator, L: Display + P .collect::>() .join("\n") ); - let owned_ret = { - let lines = ret_values - .iter() - .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) - .collect::>() - .join(", "); + let owned_ret; + let borrowed_ret; + let owned_ret_type; + let borrowed_ret_type; - format!("({lines}\n{INDENT}{INDENT})") - }; - let borrowed_ret = { - let lines = ret_values - .iter() - .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) - .collect::>() - .join(", "); + if ret_types.len() == 1 { + owned_ret_type = ret_types.join(""); + borrowed_ret_type = owned_ret_type.clone(); - format!("({lines}\n{INDENT}{INDENT})") - }; - let owned_ret_type = format!("({})", ret_types.join(", ")); - let borrowed_ret_type = format!( - "({})", - ret_types - .iter() - .map(|ret_type| { format!("&{ret_type}") }) - .collect::>() - .join(", ") - ); + let ret_val = ret_values.first().unwrap(); + owned_ret = format!("\n{INDENT}{INDENT}{ret_val}"); + borrowed_ret = format!("\n{INDENT}{INDENT}&{ret_val}"); + } else { + owned_ret_type = format!("({})", ret_types.join(", ")); + borrowed_ret_type = format!( + "({})", + ret_types + .iter() + .map(|ret_type| { format!("&{ret_type}") }) + .collect::>() + .join(", ") + ); + owned_ret = { + let lines = ret_values + .iter() + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}{line}")) + .collect::>() + .join(", "); + + format!("({lines}\n{INDENT}{INDENT})") + }; + borrowed_ret = { + let lines = ret_values + .iter() + .map(|line| format!("\n{INDENT}{INDENT}{INDENT}&{line}")) + .collect::>() + .join(", "); + + format!("({lines}\n{INDENT}{INDENT})") + }; + } StructIngredients { payload_args, From 0fec0e85a64f90e992f970b4cdfce5e50e756e2e Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 19:27:26 -0400 Subject: [PATCH 53/65] Fix missing "f" prefix for tag union payloads --- bindgen/src/bindgen_rs.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 4322cd5432..73ba1abc1c 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1534,6 +1534,14 @@ fn tag_union_struct_help<'a, I: Iterator, L: Display + P let mut ret_values = Vec::new(); for (label, type_id) in sorted_fields.iter() { + let label = if is_tag_union_payload { + // Tag union payload fields need "f" prefix + // because they're numbers + format!("f{}", label) + } else { + format!("{}", label) + }; + ret_values.push(format!("payload.{label}")); ret_types.push(type_name(*type_id, types)); } From f6b1786ab5e72db53a6fbf3145683907759beacb Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 19:28:12 -0400 Subject: [PATCH 54/65] Fix missing dereference --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 73ba1abc1c..fcd31edd7f 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1585,7 +1585,7 @@ fn tag_union_struct_help<'a, I: Iterator, L: Display + P if ret_types.len() == 1 { owned_ret_type = ret_types.join(""); - borrowed_ret_type = owned_ret_type.clone(); + borrowed_ret_type = format!("&{owned_ret_type}"); let ret_val = ret_values.first().unwrap(); owned_ret = format!("\n{INDENT}{INDENT}{ret_val}"); From efbbe12ba12e970908f3f19ff233e87170e628f9 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 20:20:07 -0400 Subject: [PATCH 55/65] Fix constructors, into_, and as_ for nullable-unwrapped --- bindgen/src/bindgen_rs.rs | 62 +++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index fcd31edd7f..f129ba8f86 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1255,6 +1255,52 @@ pub struct {name} {{ ), ); + let owned_ret_type; + let borrowed_ret_type; + let payload_args; + let args_to_payload; + let owned_ret; + let borrowed_ret; + + match payload_type { + RocType::RocStr + | RocType::Bool + | RocType::Num(_) + | RocType::RocList(_) + | RocType::RocDict(_, _) + | RocType::RocSet(_) + | RocType::RocBox(_) + | RocType::TagUnion(_) + | RocType::RecursivePointer { .. } => { + owned_ret_type = type_name(non_null_payload, types); + borrowed_ret_type = format!("&{}", owned_ret_type); + payload_args = format!("arg: {owned_ret_type}"); + args_to_payload = "arg".to_string(); + owned_ret = "payload".to_string(); + borrowed_ret = format!("&{owned_ret}"); + } + RocType::Struct { fields, .. } => { + let answer = tag_union_struct_help(fields.iter(), non_null_payload, types, false); + + payload_args = answer.payload_args; + args_to_payload = answer.args_to_payload; + owned_ret = answer.owned_ret; + borrowed_ret = answer.borrowed_ret; + owned_ret_type = answer.owned_ret_type; + borrowed_ret_type = answer.borrowed_ret_type; + } + RocType::TagUnionPayload { fields, .. } => { + let answer = tag_union_struct_help(fields.iter(), non_null_payload, types, true); + + payload_args = answer.payload_args; + args_to_payload = answer.args_to_payload; + owned_ret = answer.owned_ret; + borrowed_ret = answer.borrowed_ret; + owned_ret_type = answer.owned_ret_type; + borrowed_ret_type = answer.borrowed_ret_type; + } + }; + // Add a convenience constructor function for the tag with the payload, e.g. // // /// Construct a tag named Cons, with the appropriate payload @@ -1277,10 +1323,11 @@ pub struct {name} {{ architecture, format!( r#"/// Construct a tag named {non_null_tag}, with the appropriate payload - pub fn {non_null_tag}(payload: {payload_type_name}) -> Self {{ + pub fn {non_null_tag}({payload_args}) -> Self {{ let payload_align = core::mem::align_of::<{payload_type_name}>(); let self_align = core::mem::align_of::(); let size = self_align + core::mem::size_of::<{payload_type_name}>(); + let payload = {args_to_payload}; unsafe {{ // Store the payload at `self_align` bytes after the allocation, @@ -1288,7 +1335,7 @@ pub struct {name} {{ let alloc_ptr = crate::roc_alloc(size, payload_align as u32); let payload_ptr = alloc_ptr.cast::().add(self_align).cast::>(); - *payload_ptr = core::mem::ManuallyDrop::new(payload); + *payload_ptr = payload; // The reference count is stored immediately before the payload, // which isn't necessarily the same as alloc_ptr - e.g. when alloc_ptr @@ -1317,14 +1364,14 @@ pub struct {name} {{ r#"/// Unsafely assume the given {name} has a .variant() of {non_null_tag} and convert it to {non_null_tag}'s payload. /// (Always examine .variant() first to make sure this is the correct variant!) /// Panics in debug builds if the .variant() doesn't return {non_null_tag}. - pub unsafe fn into_{non_null_tag}(self) -> {payload_type_name} {{ + pub unsafe fn into_{non_null_tag}(self) -> {owned_ret_type} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{non_null_tag}); let payload = {assign_payload}; core::mem::drop::(self); - payload + {owned_ret} }}"#, ), ); @@ -1338,9 +1385,12 @@ pub struct {name} {{ r#"/// Unsafely assume the given {name} has a .variant() of {non_null_tag} and return its payload. /// (Always examine .variant() first to make sure this is the correct variant!) /// Panics in debug builds if the .variant() doesn't return {non_null_tag}. - pub unsafe fn as_{non_null_tag}(&self) -> &{payload_type_name} {{ + pub unsafe fn as_{non_null_tag}(&self) -> {borrowed_ret_type} {{ debug_assert_eq!(self.variant(), {discriminant_name}::{non_null_tag}); - &*self.pointer + + let payload = &*self.pointer; + + {borrowed_ret} }}"#, ), ); From 46a7d341a3143333f17c16753ce05fb49ca2feea Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 20:40:12 -0400 Subject: [PATCH 56/65] Fix nullable-unwrapped Debug instance --- bindgen/src/bindgen_rs.rs | 41 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index f129ba8f86..5093dfd4a8 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1510,6 +1510,41 @@ pub struct {name} {{ let opt_impl = Some(format!("impl core::fmt::Debug for {name}")); let extra_deref = if has_pointer { "*" } else { "" }; + let fields_str = match payload_type { + RocType::RocStr + | RocType::Bool + | RocType::Num(_) + | RocType::RocList(_) + | RocType::RocDict(_, _) + | RocType::RocSet(_) + | RocType::RocBox(_) + | RocType::TagUnion(_) + | RocType::RecursivePointer { .. } => { + format!( + r#"f.debug_tuple("{non_null_tag}").field(&*{extra_deref}self.pointer).finish()"# + ) + } + RocType::Struct { fields, .. } => { + let mut buf = Vec::new(); + + for (label, _) in fields { + buf.push(format!(".field(&(&*{extra_deref}self.pointer).{label})")); + } + + buf.join(&format!("\n{INDENT}{INDENT}{INDENT}{INDENT}{INDENT}")) + } + RocType::TagUnionPayload { fields, .. } => { + let mut buf = Vec::new(); + + for (label, _) in fields { + // Needs an "f" prefix + buf.push(format!(".field(&(&*{extra_deref}self.pointer).f{label})")); + } + + buf.join(&format!("\n{INDENT}{INDENT}{INDENT}{INDENT}{INDENT}")) + } + }; + let body = format!( r#"fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {{ if self.pointer.is_null() {{ @@ -1517,7 +1552,11 @@ pub struct {name} {{ }} else {{ f.write_str("{name}::")?; - unsafe {{ f.debug_tuple("{non_null_tag}").field(&*{extra_deref}self.pointer).finish() }} + unsafe {{ + f.debug_tuple("{non_null_tag}") + {fields_str} + .finish() + }} }} }}"# ); From 368a9d4a4e3043d1ef08824054437b93c97000a4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 20:41:04 -0400 Subject: [PATCH 57/65] Fix nullable-unwrapped bindgen test --- bindgen/tests/fixtures/nullable-unwrapped/src/lib.rs | 7 ++----- bindgen/tests/test_bindgen_cli.rs | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/bindgen/tests/fixtures/nullable-unwrapped/src/lib.rs b/bindgen/tests/fixtures/nullable-unwrapped/src/lib.rs index 872df11366..8b025e07dd 100644 --- a/bindgen/tests/fixtures/nullable-unwrapped/src/lib.rs +++ b/bindgen/tests/fixtures/nullable-unwrapped/src/lib.rs @@ -1,6 +1,6 @@ mod bindings; -use bindings::{StrConsList, StrConsList_Cons}; +use bindings::StrConsList; use indoc::indoc; extern "C" { @@ -38,10 +38,7 @@ pub extern "C" fn rust_main() -> i32 { "# ), tag_union, - StrConsList::Cons(StrConsList_Cons { - f0: "small str".into(), - f1: StrConsList::Nil - }), + StrConsList::Cons("small str".into(), StrConsList::Nil), StrConsList::Nil, ); // Debug diff --git a/bindgen/tests/test_bindgen_cli.rs b/bindgen/tests/test_bindgen_cli.rs index bd075840f4..dad40d231b 100644 --- a/bindgen/tests/test_bindgen_cli.rs +++ b/bindgen/tests/test_bindgen_cli.rs @@ -117,8 +117,8 @@ mod bindgen_cli_run { `Blah 456` is: NonRecursive::Blah(456) "#), nullable_unwrapped:"nullable-unwrapped" => indoc!(r#" - tag_union was: StrConsList::Cons(StrConsList_Cons { f0: "World!", f1: StrConsList::Cons(StrConsList_Cons { f0: "Hello ", f1: StrConsList::Nil }) }) - `Cons "small str" Nil` is: StrConsList::Cons(StrConsList_Cons { f0: "small str", f1: StrConsList::Nil }) + tag_union was: StrConsList::Cons("World!", StrConsList::Cons("Hello ", StrConsList::Nil)) + `Cons "small str" Nil` is: StrConsList::Cons("small str", StrConsList::Nil) `Nil` is: StrConsList::Nil "#), recursive_union:"recursive-union" => indoc!(r#" From 31ef0d117aa001ac185a3ed0d26f624f8fdeaec3 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 22:48:53 -0400 Subject: [PATCH 58/65] Fix into_ and as_ for non-recursive unions --- bindgen/src/bindgen_rs.rs | 60 ++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 5093dfd4a8..2718cb8bb8 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -493,41 +493,63 @@ pub struct {name} {{ let owned_ret; let borrowed_ret; - if payload_type.has_pointer(types) { - owned_get_payload = format!( - r#"unsafe {{ + match recursiveness { + Recursiveness::Recursive => { + if payload_type.has_pointer(types) { + owned_get_payload = format!( + r#"unsafe {{ let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; core::mem::ManuallyDrop::take(&mut (*ptr).{tag_name}) }}"# - ); - borrowed_get_payload = format!( - r#"unsafe {{ + ); + borrowed_get_payload = format!( + r#"unsafe {{ let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; &(*ptr).{tag_name} }}"# - ); - // we need `mut self` for the argument because of ManuallyDrop - self_for_into = "mut self"; - } else { - owned_get_payload = format!( - r#"unsafe {{ + ); + // we need `mut self` for the argument because of ManuallyDrop + self_for_into = "mut self"; + } else { + owned_get_payload = format!( + r#"unsafe {{ let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; core::ptr::read(ptr).{tag_name} }}"# - ); - borrowed_get_payload = format!( - r#"unsafe {{ + ); + borrowed_get_payload = format!( + r#"unsafe {{ let ptr = (self.pointer as usize & !{bitmask}) as *mut {union_name}; (&ptr).{tag_name} }}"# - ); - // we don't need `mut self` unless we need ManuallyDrop - self_for_into = "self"; - }; + ); + // we don't need `mut self` unless we need ManuallyDrop + self_for_into = "self"; + }; + } + Recursiveness::NonRecursive => { + if payload_type.has_pointer(types) { + owned_get_payload = format!( + "unsafe {{ core::mem::ManuallyDrop::take(&mut (*self.pointer).{tag_name}) }}" + ); + borrowed_get_payload = + format!("unsafe {{ &(*self.pointer).{tag_name} }}"); + // we need `mut self` for the argument because of ManuallyDrop + self_for_into = "mut self"; + } else { + owned_get_payload = + format!("unsafe {{ core::ptr::read(self.pointer).{tag_name} }}"); + borrowed_get_payload = + format!("unsafe {{ (&self.pointer).{tag_name} }}"); + // we don't need `mut self` unless we need ManuallyDrop + self_for_into = "self"; + }; + } + } match payload_type { RocType::RocStr From 46ea5a7d3b1f6b5472a50e706b434db393dfde6d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 23:02:53 -0400 Subject: [PATCH 59/65] Fix non-recursive union bindgen --- bindgen/src/bindgen_rs.rs | 119 +++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 48 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 2718cb8bb8..41a7a9980a 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -273,14 +273,14 @@ fn add_tag_union( let target_info = architecture.into(); let discriminant_offset = RocTagUnion::discriminant_offset(tags, types, target_info); let size = typ.size(types, target_info); - let union_name = format!("union_{name}"); - let (actual_self, actual_self_mut, actual_other) = match recursiveness { + let (actual_self, actual_self_mut, actual_other, union_name) = match recursiveness { Recursiveness::Recursive => ( "(&*self.union_pointer())", "(&mut *self.union_pointer())", "(&*other.union_pointer())", + format!("union_{name}"), ), - Recursiveness::NonRecursive => ("self", "self", "other"), + Recursiveness::NonRecursive => ("self", "self", "other", name.to_string()), }; { @@ -534,17 +534,14 @@ pub struct {name} {{ Recursiveness::NonRecursive => { if payload_type.has_pointer(types) { owned_get_payload = format!( - "unsafe {{ core::mem::ManuallyDrop::take(&mut (*self.pointer).{tag_name}) }}" + "unsafe {{ core::mem::ManuallyDrop::take(&mut self.{tag_name}) }}" ); - borrowed_get_payload = - format!("unsafe {{ &(*self.pointer).{tag_name} }}"); + borrowed_get_payload = format!("unsafe {{ &self.{tag_name} }}"); // we need `mut self` for the argument because of ManuallyDrop self_for_into = "mut self"; } else { - owned_get_payload = - format!("unsafe {{ core::ptr::read(self.pointer).{tag_name} }}"); - borrowed_get_payload = - format!("unsafe {{ (&self.pointer).{tag_name} }}"); + owned_get_payload = format!("unsafe {{ self.{tag_name} }}"); + borrowed_get_payload = format!("unsafe {{ &self.{tag_name} }}"); // we don't need `mut self` unless we need ManuallyDrop self_for_into = "self"; }; @@ -563,43 +560,45 @@ pub struct {name} {{ | RocType::RecursivePointer { .. } => { owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); - payload_args = format!("arg: {owned_ret_type}"); - args_to_payload = "arg".to_string(); owned_ret = "payload".to_string(); borrowed_ret = format!("&{owned_ret}"); + payload_args = format!("arg: {owned_ret_type}"); + args_to_payload = if payload_type.has_pointer(types) { + "core::mem::ManuallyDrop::new(arg)".to_string() + } else { + "arg".to_string() + }; } RocType::Struct { fields, .. } => { let answer = tag_union_struct_help(fields.iter(), *payload_id, types, false); - payload_args = answer.payload_args; - args_to_payload = answer.args_to_payload; owned_ret = answer.owned_ret; borrowed_ret = answer.borrowed_ret; owned_ret_type = answer.owned_ret_type; borrowed_ret_type = answer.borrowed_ret_type; + payload_args = answer.payload_args; + args_to_payload = answer.args_to_payload; } RocType::TagUnionPayload { fields, .. } => { let answer = tag_union_struct_help(fields.iter(), *payload_id, types, true); - payload_args = answer.payload_args; - args_to_payload = answer.args_to_payload; owned_ret = answer.owned_ret; borrowed_ret = answer.borrowed_ret; owned_ret_type = answer.owned_ret_type; borrowed_ret_type = answer.borrowed_ret_type; + payload_args = answer.payload_args; + args_to_payload = answer.args_to_payload; } }; - add_decl( - impls, - opt_impl.clone(), - architecture, - format!( - r#"/// Construct a tag named {tag_name}, with the appropriate payload - pub fn {tag_name}({payload_args}) -> Self {{ + { + let body = match recursiveness { + Recursiveness::Recursive => { + format!( + r#" let size = core::mem::size_of::<{union_name}>(); - let align = core::mem::size_of::<{union_name}>() as u32; + let align = core::mem::align_of::<{union_name}>() as u32; unsafe {{ let ptr = crate::roc_alloc(size, align) as *mut {union_name}; @@ -611,10 +610,34 @@ pub struct {name} {{ Self {{ pointer: Self::tag_discriminant(ptr, {discriminant_name}::{tag_name}), }} - }} + }}"# + ) + } + Recursiveness::NonRecursive => { + format!( + r#" + let mut answer = Self {{ + {tag_name}: {args_to_payload} + }}; + + answer.set_discriminant({discriminant_name}::{tag_name}); + + answer"# + ) + } + }; + + add_decl( + impls, + opt_impl.clone(), + architecture, + format!( + r#"/// Construct a tag named {tag_name}, with the appropriate payload + pub fn {tag_name}({payload_args}) -> Self {{{body} }}"# - ), - ); + ), + ); + } add_decl( impls, @@ -1665,30 +1688,30 @@ fn tag_union_struct_help<'a, I: Iterator, L: Display + P .collect::>() .join(", "); let args_to_payload = format!( - "core::mem::ManuallyDrop::new({payload_type_name} {{\n{}\n{INDENT}{INDENT}{INDENT}{INDENT}}})", - sorted_fields - .iter() - .enumerate() - .map(|(index, (label, _))| { - let mut indents = String::new(); + "core::mem::ManuallyDrop::new({payload_type_name} {{\n{}\n{INDENT}{INDENT}{INDENT}{INDENT}}})", + sorted_fields + .iter() + .enumerate() + .map(|(index, (label, _))| { + let mut indents = String::new(); - for _ in 0..5 { - indents.push_str(INDENT); - } + for _ in 0..5 { + indents.push_str(INDENT); + } - let label = if is_tag_union_payload { - // Tag union payload fields need "f" prefix - // because they're numbers - format!("f{}", label) - } else { - format!("{}", label) - }; + let label = if is_tag_union_payload { + // Tag union payload fields need "f" prefix + // because they're numbers + format!("f{}", label) + } else { + format!("{}", label) + }; - format!("{indents}{label}: arg{index},") - }) - .collect::>() - .join("\n") - ); + format!("{indents}{label}: arg{index},") + }) + .collect::>() + .join("\n") + ); let owned_ret; let borrowed_ret; let owned_ret_type; From 0718a15ae098f60d47ebc4695dbefe0c4ec5984b Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 23:12:17 -0400 Subject: [PATCH 60/65] Fix gen_rs tests --- bindgen/tests/gen_rs.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bindgen/tests/gen_rs.rs b/bindgen/tests/gen_rs.rs index 5642520e47..10c510d980 100644 --- a/bindgen/tests/gen_rs.rs +++ b/bindgen/tests/gen_rs.rs @@ -257,7 +257,7 @@ mod test_gen_rs { ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] #[repr(C)] - pub struct UserId { + struct UserId { pub f1: roc_std::RocStr, pub f0: u32, } @@ -269,7 +269,7 @@ mod test_gen_rs { ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] #[repr(C)] - pub struct UserId { + struct UserId { pub f0: u32, pub f1: roc_std::RocStr, } @@ -303,8 +303,10 @@ mod test_gen_rs { target_arch = "wasm32" ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] - #[repr(transparent)] - pub struct UserId(roc_std::RocStr); + #[repr(C)] + struct UserId { + pub f0: roc_std::RocStr, + } "# ) ); From 6948f838a91825fb9c92ab0f1bcbc0765877aef0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 23:17:41 -0400 Subject: [PATCH 61/65] The smallest paperclip --- bindgen/src/bindgen_rs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 41a7a9980a..b0e14738a1 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -1662,7 +1662,7 @@ fn tag_union_struct_help<'a, I: Iterator, L: Display + P ) -> StructIngredients { let mut sorted_fields = fields.collect::>(); - sorted_fields.sort_by(|(label1, _), (label2, _)| label1.partial_cmp(&label2).unwrap()); + sorted_fields.sort_by(|(label1, _), (label2, _)| label1.partial_cmp(label2).unwrap()); let mut ret_types = Vec::new(); let mut ret_values = Vec::new(); From 2c230cc92a09b3cbf9f846819a0a968dfd9cb1f0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 23:21:10 -0400 Subject: [PATCH 62/65] debug_assert that tags is nonempty --- bindgen/src/bindgen_rs.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index b0e14738a1..77805ea051 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -267,6 +267,10 @@ fn add_tag_union( types: &Types, impls: &mut Impls, ) { + // We should never be attempting to bindgen empty tag unions; RocType should not + // have let this happen. + debug_assert_ne!(tags.len(), 0); + let tag_names = tags.iter().map(|(name, _)| name).cloned().collect(); let discriminant_name = add_discriminant(name, architecture, tag_names, types, impls); let typ = types.get(type_id); From 93ec2f84656dab23501ded91d2ef6bbfdd843654 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 23:51:30 -0400 Subject: [PATCH 63/65] bindgen TypeAlias for single-tag unions --- bindgen/src/bindgen.rs | 278 ++++++++++++++++++-------------------- bindgen/src/bindgen_rs.rs | 17 ++- bindgen/src/types.rs | 13 ++ bindgen/tests/gen_rs.rs | 19 ++- 4 files changed, 172 insertions(+), 155 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 23438779e1..be47882f83 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -313,170 +313,154 @@ fn add_tag_union( }) .collect(); - if tags.len() == 1 { - // This is a single-tag union. - let (tag_name, payload_vars) = tags.pop().unwrap(); + let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); + let name = match opt_name { + Some(sym) => sym.as_str(env.interns).to_string(), + None => env.enum_names.get_name(var), + }; - // If there was a type alias name, use that. Otherwise use the tag name. - let name = match opt_name { - Some(sym) => sym.as_str(env.interns).to_string(), - None => tag_name, - }; + // Sort tags alphabetically by tag name + tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - let fields = payload_vars - .iter() - .enumerate() - .map(|(index, payload_var)| (index, *payload_var)); + let is_recursive = is_recursive_tag_union(&layout); - add_struct(env, name, fields, types, |name, fields| { - RocType::TagUnionPayload { name, fields } + let mut tags: Vec<(String, Option)> = tags + .into_iter() + .map(|(tag_name, payload_vars)| { + match struct_fields_needed(env, payload_vars.iter().copied()) { + 0 => { + // no payload + (tag_name, None) + } + 1 if !is_recursive => { + // this isn't recursive and there's 1 payload item, so it doesn't + // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them + // can have payloads of plain old Str, no struct wrapper needed. + let payload_var = payload_vars.get(0).unwrap(); + let layout = env + .layout_cache + .from_var(env.arena, *payload_var, env.subs) + .expect("Something weird ended up in the content"); + let payload_id = add_type_help(env, layout, *payload_var, None, types); + + (tag_name, Some(payload_id)) + } + _ => { + // create a RocType for the payload and save it + let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" + let fields = payload_vars.iter().copied().enumerate(); + let struct_id = add_struct(env, struct_name, fields, types, |name, fields| { + RocType::TagUnionPayload { name, fields } + }); + + (tag_name, Some(struct_id)) + } + } }) - } else { - // This is a multi-tag union. + .collect(); - // This is a placeholder so that we can get a TypeId for future recursion IDs. - // At the end, we will replace this with the real tag union type. - let type_id = types.add(RocType::Struct { - name: String::new(), - fields: Vec::new(), - }); - let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); - let name = match opt_name { - Some(sym) => sym.as_str(env.interns).to_string(), - None => env.enum_names.get_name(var), - }; + let typ = match layout { + Layout::Union(union_layout) => { + use roc_mono::layout::UnionLayout::*; - // Sort tags alphabetically by tag name - tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - - let is_recursive = is_recursive_tag_union(&layout); - - let mut tags: Vec<_> = tags - .into_iter() - .map(|(tag_name, payload_vars)| { - match struct_fields_needed(env, payload_vars.iter().copied()) { - 0 => { - // no payload - (tag_name, None) - } - 1 if !is_recursive => { - // this isn't recursive and there's 1 payload item, so it doesn't - // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them - // can have payloads of plain old Str, no struct wrapper needed. - let payload_var = payload_vars.get(0).unwrap(); - let layout = env - .layout_cache - .from_var(env.arena, *payload_var, env.subs) - .expect("Something weird ended up in the content"); - let payload_id = add_type_help(env, layout, *payload_var, None, types); - - (tag_name, Some(payload_id)) - } - _ => { - // create a RocType for the payload and save it - let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" - let fields = payload_vars.iter().copied().enumerate(); - let struct_id = - add_struct(env, struct_name, fields, types, |name, fields| { - RocType::TagUnionPayload { name, fields } - }); - - (tag_name, Some(struct_id)) - } + match union_layout { + // A non-recursive tag union + // e.g. `Result ok err : [Ok ok, Err err]` + NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), + // A recursive tag union (general case) + // e.g. `Expr : [Sym Str, Add Expr Expr]` + Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), + // A recursive tag union with just one constructor + // Optimization: No need to store a tag ID (the payload is "unwrapped") + // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` + NonNullableUnwrapped(_) => { + todo!() } - }) - .collect(); + // A recursive tag union that has an empty variant + // Optimization: Represent the empty variant as null pointer => no memory usage & fast comparison + // It has more than one other variant, so they need tag IDs (payloads are "wrapped") + // e.g. `FingerTree a : [Empty, Single a, More (Some a) (FingerTree (Tuple a)) (Some a)]` + // see also: https://youtu.be/ip92VMpf_-A?t=164 + NullableWrapped { .. } => { + todo!() + } + // A recursive tag union with only two variants, where one is empty. + // Optimizations: Use null for the empty variant AND don't store a tag ID for the other variant. + // e.g. `ConsList a : [Nil, Cons a (ConsList a)]` + NullableUnwrapped { + nullable_id: null_represents_first_tag, + other_fields: _, // TODO use this! + } => { + // NullableUnwrapped tag unions should always have exactly 2 tags. + debug_assert_eq!(tags.len(), 2); - let typ = match layout { - Layout::Union(union_layout) => { - use roc_mono::layout::UnionLayout::*; + let null_tag; + let non_null; - match union_layout { - // A non-recursive tag union - // e.g. `Result ok err : [Ok ok, Err err]` - NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), - // A recursive tag union (general case) - // e.g. `Expr : [Sym Str, Add Expr Expr]` - Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), - // A recursive tag union with just one constructor - // Optimization: No need to store a tag ID (the payload is "unwrapped") - // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` - NonNullableUnwrapped(_) => { - todo!() + if null_represents_first_tag { + // If nullable_id is true, then the null tag is second, which means + // pop() will return it because it's at the end of the vec. + null_tag = tags.pop().unwrap().0; + non_null = tags.pop().unwrap(); + } else { + // The null tag is first, which means the tag with the payload is second. + non_null = tags.pop().unwrap(); + null_tag = tags.pop().unwrap().0; } - // A recursive tag union that has an empty variant - // Optimization: Represent the empty variant as null pointer => no memory usage & fast comparison - // It has more than one other variant, so they need tag IDs (payloads are "wrapped") - // e.g. `FingerTree a : [Empty, Single a, More (Some a) (FingerTree (Tuple a)) (Some a)]` - // see also: https://youtu.be/ip92VMpf_-A?t=164 - NullableWrapped { .. } => { - todo!() - } - // A recursive tag union with only two variants, where one is empty. - // Optimizations: Use null for the empty variant AND don't store a tag ID for the other variant. - // e.g. `ConsList a : [Nil, Cons a (ConsList a)]` - NullableUnwrapped { - nullable_id: null_represents_first_tag, - other_fields: _, // TODO use this! - } => { - // NullableUnwrapped tag unions should always have exactly 2 tags. - debug_assert_eq!(tags.len(), 2); - let null_tag; - let non_null; + let (non_null_tag, non_null_payload) = non_null; - if null_represents_first_tag { - // If nullable_id is true, then the null tag is second, which means - // pop() will return it because it's at the end of the vec. - null_tag = tags.pop().unwrap().0; - non_null = tags.pop().unwrap(); - } else { - // The null tag is first, which means the tag with the payload is second. - non_null = tags.pop().unwrap(); - null_tag = tags.pop().unwrap().0; - } - - let (non_null_tag, non_null_payload) = non_null; - - RocType::TagUnion(RocTagUnion::NullableUnwrapped { - name, - null_tag, - non_null_tag, - non_null_payload: non_null_payload.unwrap(), - null_represents_first_tag, - }) - } + RocType::TagUnion(RocTagUnion::NullableUnwrapped { + name, + null_tag, + non_null_tag, + non_null_payload: non_null_payload.unwrap(), + null_represents_first_tag, + }) } } - Layout::Builtin(builtin) => match builtin { - Builtin::Int(_) => RocType::TagUnion(RocTagUnion::Enumeration { - name, - tags: tags.into_iter().map(|(tag_name, _)| tag_name).collect(), - }), - Builtin::Bool => RocType::Bool, - Builtin::Float(_) - | Builtin::Decimal - | Builtin::Str - | Builtin::Dict(_, _) - | Builtin::Set(_) - | Builtin::List(_) => unreachable!(), - }, - Layout::Struct { .. } - | Layout::Boxed(_) - | Layout::LambdaSet(_) - | Layout::RecursivePointer => { - unreachable!() - } - }; - - types.replace(type_id, typ); - - if is_recursive { - env.known_recursive_types.insert(var, type_id); } + Layout::Builtin(Builtin::Int(_)) => RocType::TagUnion(RocTagUnion::Enumeration { + name, + tags: tags.into_iter().map(|(tag_name, _)| tag_name).collect(), + }), + Layout::Builtin(_) + | Layout::Struct { .. } + | Layout::Boxed(_) + | Layout::LambdaSet(_) + | Layout::RecursivePointer => { + // Only a single-tag union could have had this layout. + debug_assert_eq!(tags.len(), 1); - type_id + let (_, opt_payload_id) = tags.pop().unwrap(); + + // Return the payload itself. + return match opt_payload_id { + Some(payload_id) => { + // A single-tag union with a payload is a type alias for that payload + types.add(RocType::TypeAlias { + name, + aliasing: payload_id, + }) + } + None => { + // A single-tag union with no payload would be an empty struct. + types.add(RocType::Struct { + name, + fields: Default::default(), + }) + } + }; + } + }; + + let type_id = types.add(typ); + + if is_recursive { + env.known_recursive_types.insert(var, type_id); } + + type_id } fn is_recursive_tag_union(layout: &Layout) -> bool { diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 77805ea051..72c18b0a6a 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -205,6 +205,16 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I } } } + RocType::TypeAlias { name, aliasing } => { + let alias_name = type_name(*aliasing, types); + + add_decl( + impls, + None, + architecture, + format!("pub type {name} = {alias_name};"), + ); + } // These types don't need to be declared in Rust. RocType::Num(_) | RocType::Bool @@ -561,6 +571,7 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) + | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); @@ -1017,6 +1028,7 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) + | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { format!(".field({deref_str}{actual_self}.{tag_name})") } @@ -1197,7 +1209,8 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocSet(elem_id) => format!("roc_std::RocSet<{}>", type_name(*elem_id, types)), RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), - RocType::Struct { name, .. } + RocType::TypeAlias { name, .. } + | RocType::Struct { name, .. } | RocType::TagUnionPayload { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) @@ -1320,6 +1333,7 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) + | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { owned_ret_type = type_name(non_null_payload, types); borrowed_ret_type = format!("&{}", owned_ret_type); @@ -1568,6 +1582,7 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) + | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { format!( r#"f.debug_tuple("{non_null_tag}").field(&*{extra_deref}self.pointer).finish()"# diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index d5b4327d32..bffed9fa4b 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -142,6 +142,10 @@ pub enum RocType { /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. RecursivePointer(TypeId), + TypeAlias { + name: String, + aliasing: TypeId, + }, } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -233,6 +237,7 @@ impl RocType { RocType::TagUnionPayload { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_pointer(types)), + RocType::TypeAlias { aliasing, .. } => types.get(*aliasing).has_pointer(types), } } @@ -298,6 +303,9 @@ impl RocType { types.get(*content).has_float_help(types, &do_not_recurse) } } + RocType::TypeAlias { aliasing, .. } => { + types.get(*aliasing).has_float_help(types, do_not_recurse) + } } } @@ -319,6 +327,7 @@ impl RocType { RocType::TagUnionPayload { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_enumeration(types)), + RocType::TypeAlias { aliasing, .. } => types.get(*aliasing).has_enumeration(types), } } @@ -385,6 +394,7 @@ impl RocType { self.alignment(types, target_info), ), RocType::RecursivePointer { .. } => target_info.ptr_size(), + RocType::TypeAlias { aliasing, .. } => types.get(*aliasing).size(types, target_info), } } @@ -465,6 +475,9 @@ impl RocType { .unwrap() } RocType::RecursivePointer { .. } => target_info.ptr_alignment_bytes(), + RocType::TypeAlias { aliasing, .. } => { + types.get(*aliasing).alignment(types, target_info) + } } } } diff --git a/bindgen/tests/gen_rs.rs b/bindgen/tests/gen_rs.rs index 10c510d980..e670dda9b1 100644 --- a/bindgen/tests/gen_rs.rs +++ b/bindgen/tests/gen_rs.rs @@ -251,13 +251,22 @@ mod test_gen_rs { .unwrap_or_default(), indoc!( r#" + #[cfg(any( + target_arch = "x86_64", + target_arch = "x86", + target_arch = "aarch64", + target_arch = "arm", + target_arch = "wasm32" + ))] + pub type UserId = UserId_Id; + #[cfg(any( target_arch = "x86_64", target_arch = "aarch64" ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] #[repr(C)] - struct UserId { + struct UserId_Id { pub f1: roc_std::RocStr, pub f0: u32, } @@ -269,7 +278,7 @@ mod test_gen_rs { ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] #[repr(C)] - struct UserId { + struct UserId_Id { pub f0: u32, pub f1: roc_std::RocStr, } @@ -302,11 +311,7 @@ mod test_gen_rs { target_arch = "arm", target_arch = "wasm32" ))] - #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] - #[repr(C)] - struct UserId { - pub f0: roc_std::RocStr, - } + pub type UserId = roc_std::RocStr; "# ) ); From 1f9040460a3e763b3421a5fe8471d7b48d579da6 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 28 May 2022 23:56:00 -0400 Subject: [PATCH 64/65] Revert "bindgen TypeAlias for single-tag unions" This reverts commit 93ec2f84656dab23501ded91d2ef6bbfdd843654. --- bindgen/src/bindgen.rs | 280 ++++++++++++++++++++------------------ bindgen/src/bindgen_rs.rs | 17 +-- bindgen/src/types.rs | 13 -- bindgen/tests/gen_rs.rs | 19 +-- 4 files changed, 156 insertions(+), 173 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index be47882f83..23438779e1 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -313,154 +313,170 @@ fn add_tag_union( }) .collect(); - let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); - let name = match opt_name { - Some(sym) => sym.as_str(env.interns).to_string(), - None => env.enum_names.get_name(var), - }; + if tags.len() == 1 { + // This is a single-tag union. + let (tag_name, payload_vars) = tags.pop().unwrap(); - // Sort tags alphabetically by tag name - tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); + // If there was a type alias name, use that. Otherwise use the tag name. + let name = match opt_name { + Some(sym) => sym.as_str(env.interns).to_string(), + None => tag_name, + }; - let is_recursive = is_recursive_tag_union(&layout); + let fields = payload_vars + .iter() + .enumerate() + .map(|(index, payload_var)| (index, *payload_var)); - let mut tags: Vec<(String, Option)> = tags - .into_iter() - .map(|(tag_name, payload_vars)| { - match struct_fields_needed(env, payload_vars.iter().copied()) { - 0 => { - // no payload - (tag_name, None) - } - 1 if !is_recursive => { - // this isn't recursive and there's 1 payload item, so it doesn't - // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them - // can have payloads of plain old Str, no struct wrapper needed. - let payload_var = payload_vars.get(0).unwrap(); - let layout = env - .layout_cache - .from_var(env.arena, *payload_var, env.subs) - .expect("Something weird ended up in the content"); - let payload_id = add_type_help(env, layout, *payload_var, None, types); - - (tag_name, Some(payload_id)) - } - _ => { - // create a RocType for the payload and save it - let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" - let fields = payload_vars.iter().copied().enumerate(); - let struct_id = add_struct(env, struct_name, fields, types, |name, fields| { - RocType::TagUnionPayload { name, fields } - }); - - (tag_name, Some(struct_id)) - } - } + add_struct(env, name, fields, types, |name, fields| { + RocType::TagUnionPayload { name, fields } }) - .collect(); + } else { + // This is a multi-tag union. - let typ = match layout { - Layout::Union(union_layout) => { - use roc_mono::layout::UnionLayout::*; + // This is a placeholder so that we can get a TypeId for future recursion IDs. + // At the end, we will replace this with the real tag union type. + let type_id = types.add(RocType::Struct { + name: String::new(), + fields: Vec::new(), + }); + let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); + let name = match opt_name { + Some(sym) => sym.as_str(env.interns).to_string(), + None => env.enum_names.get_name(var), + }; - match union_layout { - // A non-recursive tag union - // e.g. `Result ok err : [Ok ok, Err err]` - NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), - // A recursive tag union (general case) - // e.g. `Expr : [Sym Str, Add Expr Expr]` - Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), - // A recursive tag union with just one constructor - // Optimization: No need to store a tag ID (the payload is "unwrapped") - // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` - NonNullableUnwrapped(_) => { - todo!() - } - // A recursive tag union that has an empty variant - // Optimization: Represent the empty variant as null pointer => no memory usage & fast comparison - // It has more than one other variant, so they need tag IDs (payloads are "wrapped") - // e.g. `FingerTree a : [Empty, Single a, More (Some a) (FingerTree (Tuple a)) (Some a)]` - // see also: https://youtu.be/ip92VMpf_-A?t=164 - NullableWrapped { .. } => { - todo!() - } - // A recursive tag union with only two variants, where one is empty. - // Optimizations: Use null for the empty variant AND don't store a tag ID for the other variant. - // e.g. `ConsList a : [Nil, Cons a (ConsList a)]` - NullableUnwrapped { - nullable_id: null_represents_first_tag, - other_fields: _, // TODO use this! - } => { - // NullableUnwrapped tag unions should always have exactly 2 tags. - debug_assert_eq!(tags.len(), 2); + // Sort tags alphabetically by tag name + tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - let null_tag; - let non_null; + let is_recursive = is_recursive_tag_union(&layout); - if null_represents_first_tag { - // If nullable_id is true, then the null tag is second, which means - // pop() will return it because it's at the end of the vec. - null_tag = tags.pop().unwrap().0; - non_null = tags.pop().unwrap(); - } else { - // The null tag is first, which means the tag with the payload is second. - non_null = tags.pop().unwrap(); - null_tag = tags.pop().unwrap().0; + let mut tags: Vec<_> = tags + .into_iter() + .map(|(tag_name, payload_vars)| { + match struct_fields_needed(env, payload_vars.iter().copied()) { + 0 => { + // no payload + (tag_name, None) } + 1 if !is_recursive => { + // this isn't recursive and there's 1 payload item, so it doesn't + // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them + // can have payloads of plain old Str, no struct wrapper needed. + let payload_var = payload_vars.get(0).unwrap(); + let layout = env + .layout_cache + .from_var(env.arena, *payload_var, env.subs) + .expect("Something weird ended up in the content"); + let payload_id = add_type_help(env, layout, *payload_var, None, types); - let (non_null_tag, non_null_payload) = non_null; + (tag_name, Some(payload_id)) + } + _ => { + // create a RocType for the payload and save it + let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" + let fields = payload_vars.iter().copied().enumerate(); + let struct_id = + add_struct(env, struct_name, fields, types, |name, fields| { + RocType::TagUnionPayload { name, fields } + }); - RocType::TagUnion(RocTagUnion::NullableUnwrapped { - name, - null_tag, - non_null_tag, - non_null_payload: non_null_payload.unwrap(), - null_represents_first_tag, - }) + (tag_name, Some(struct_id)) + } + } + }) + .collect(); + + let typ = match layout { + Layout::Union(union_layout) => { + use roc_mono::layout::UnionLayout::*; + + match union_layout { + // A non-recursive tag union + // e.g. `Result ok err : [Ok ok, Err err]` + NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), + // A recursive tag union (general case) + // e.g. `Expr : [Sym Str, Add Expr Expr]` + Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), + // A recursive tag union with just one constructor + // Optimization: No need to store a tag ID (the payload is "unwrapped") + // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` + NonNullableUnwrapped(_) => { + todo!() + } + // A recursive tag union that has an empty variant + // Optimization: Represent the empty variant as null pointer => no memory usage & fast comparison + // It has more than one other variant, so they need tag IDs (payloads are "wrapped") + // e.g. `FingerTree a : [Empty, Single a, More (Some a) (FingerTree (Tuple a)) (Some a)]` + // see also: https://youtu.be/ip92VMpf_-A?t=164 + NullableWrapped { .. } => { + todo!() + } + // A recursive tag union with only two variants, where one is empty. + // Optimizations: Use null for the empty variant AND don't store a tag ID for the other variant. + // e.g. `ConsList a : [Nil, Cons a (ConsList a)]` + NullableUnwrapped { + nullable_id: null_represents_first_tag, + other_fields: _, // TODO use this! + } => { + // NullableUnwrapped tag unions should always have exactly 2 tags. + debug_assert_eq!(tags.len(), 2); + + let null_tag; + let non_null; + + if null_represents_first_tag { + // If nullable_id is true, then the null tag is second, which means + // pop() will return it because it's at the end of the vec. + null_tag = tags.pop().unwrap().0; + non_null = tags.pop().unwrap(); + } else { + // The null tag is first, which means the tag with the payload is second. + non_null = tags.pop().unwrap(); + null_tag = tags.pop().unwrap().0; + } + + let (non_null_tag, non_null_payload) = non_null; + + RocType::TagUnion(RocTagUnion::NullableUnwrapped { + name, + null_tag, + non_null_tag, + non_null_payload: non_null_payload.unwrap(), + null_represents_first_tag, + }) + } } } + Layout::Builtin(builtin) => match builtin { + Builtin::Int(_) => RocType::TagUnion(RocTagUnion::Enumeration { + name, + tags: tags.into_iter().map(|(tag_name, _)| tag_name).collect(), + }), + Builtin::Bool => RocType::Bool, + Builtin::Float(_) + | Builtin::Decimal + | Builtin::Str + | Builtin::Dict(_, _) + | Builtin::Set(_) + | Builtin::List(_) => unreachable!(), + }, + Layout::Struct { .. } + | Layout::Boxed(_) + | Layout::LambdaSet(_) + | Layout::RecursivePointer => { + unreachable!() + } + }; + + types.replace(type_id, typ); + + if is_recursive { + env.known_recursive_types.insert(var, type_id); } - Layout::Builtin(Builtin::Int(_)) => RocType::TagUnion(RocTagUnion::Enumeration { - name, - tags: tags.into_iter().map(|(tag_name, _)| tag_name).collect(), - }), - Layout::Builtin(_) - | Layout::Struct { .. } - | Layout::Boxed(_) - | Layout::LambdaSet(_) - | Layout::RecursivePointer => { - // Only a single-tag union could have had this layout. - debug_assert_eq!(tags.len(), 1); - let (_, opt_payload_id) = tags.pop().unwrap(); - - // Return the payload itself. - return match opt_payload_id { - Some(payload_id) => { - // A single-tag union with a payload is a type alias for that payload - types.add(RocType::TypeAlias { - name, - aliasing: payload_id, - }) - } - None => { - // A single-tag union with no payload would be an empty struct. - types.add(RocType::Struct { - name, - fields: Default::default(), - }) - } - }; - } - }; - - let type_id = types.add(typ); - - if is_recursive { - env.known_recursive_types.insert(var, type_id); + type_id } - - type_id } fn is_recursive_tag_union(layout: &Layout) -> bool { diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index 72c18b0a6a..77805ea051 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -205,16 +205,6 @@ fn add_type(architecture: Architecture, id: TypeId, types: &Types, impls: &mut I } } } - RocType::TypeAlias { name, aliasing } => { - let alias_name = type_name(*aliasing, types); - - add_decl( - impls, - None, - architecture, - format!("pub type {name} = {alias_name};"), - ); - } // These types don't need to be declared in Rust. RocType::Num(_) | RocType::Bool @@ -571,7 +561,6 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) - | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { owned_ret_type = type_name(*payload_id, types); borrowed_ret_type = format!("&{}", owned_ret_type); @@ -1028,7 +1017,6 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) - | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { format!(".field({deref_str}{actual_self}.{tag_name})") } @@ -1209,8 +1197,7 @@ fn type_name(id: TypeId, types: &Types) -> String { RocType::RocSet(elem_id) => format!("roc_std::RocSet<{}>", type_name(*elem_id, types)), RocType::RocList(elem_id) => format!("roc_std::RocList<{}>", type_name(*elem_id, types)), RocType::RocBox(elem_id) => format!("roc_std::RocBox<{}>", type_name(*elem_id, types)), - RocType::TypeAlias { name, .. } - | RocType::Struct { name, .. } + RocType::Struct { name, .. } | RocType::TagUnionPayload { name, .. } | RocType::TagUnion(RocTagUnion::NonRecursive { name, .. }) | RocType::TagUnion(RocTagUnion::Recursive { name, .. }) @@ -1333,7 +1320,6 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) - | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { owned_ret_type = type_name(non_null_payload, types); borrowed_ret_type = format!("&{}", owned_ret_type); @@ -1582,7 +1568,6 @@ pub struct {name} {{ | RocType::RocSet(_) | RocType::RocBox(_) | RocType::TagUnion(_) - | RocType::TypeAlias { .. } | RocType::RecursivePointer { .. } => { format!( r#"f.debug_tuple("{non_null_tag}").field(&*{extra_deref}self.pointer).finish()"# diff --git a/bindgen/src/types.rs b/bindgen/src/types.rs index bffed9fa4b..d5b4327d32 100644 --- a/bindgen/src/types.rs +++ b/bindgen/src/types.rs @@ -142,10 +142,6 @@ pub enum RocType { /// this would be the field of Cons containing the (recursive) StrConsList type, /// and the TypeId is the TypeId of StrConsList itself. RecursivePointer(TypeId), - TypeAlias { - name: String, - aliasing: TypeId, - }, } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -237,7 +233,6 @@ impl RocType { RocType::TagUnionPayload { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_pointer(types)), - RocType::TypeAlias { aliasing, .. } => types.get(*aliasing).has_pointer(types), } } @@ -303,9 +298,6 @@ impl RocType { types.get(*content).has_float_help(types, &do_not_recurse) } } - RocType::TypeAlias { aliasing, .. } => { - types.get(*aliasing).has_float_help(types, do_not_recurse) - } } } @@ -327,7 +319,6 @@ impl RocType { RocType::TagUnionPayload { fields, .. } => fields .iter() .any(|(_, type_id)| types.get(*type_id).has_enumeration(types)), - RocType::TypeAlias { aliasing, .. } => types.get(*aliasing).has_enumeration(types), } } @@ -394,7 +385,6 @@ impl RocType { self.alignment(types, target_info), ), RocType::RecursivePointer { .. } => target_info.ptr_size(), - RocType::TypeAlias { aliasing, .. } => types.get(*aliasing).size(types, target_info), } } @@ -475,9 +465,6 @@ impl RocType { .unwrap() } RocType::RecursivePointer { .. } => target_info.ptr_alignment_bytes(), - RocType::TypeAlias { aliasing, .. } => { - types.get(*aliasing).alignment(types, target_info) - } } } } diff --git a/bindgen/tests/gen_rs.rs b/bindgen/tests/gen_rs.rs index e670dda9b1..10c510d980 100644 --- a/bindgen/tests/gen_rs.rs +++ b/bindgen/tests/gen_rs.rs @@ -251,22 +251,13 @@ mod test_gen_rs { .unwrap_or_default(), indoc!( r#" - #[cfg(any( - target_arch = "x86_64", - target_arch = "x86", - target_arch = "aarch64", - target_arch = "arm", - target_arch = "wasm32" - ))] - pub type UserId = UserId_Id; - #[cfg(any( target_arch = "x86_64", target_arch = "aarch64" ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] #[repr(C)] - struct UserId_Id { + struct UserId { pub f1: roc_std::RocStr, pub f0: u32, } @@ -278,7 +269,7 @@ mod test_gen_rs { ))] #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] #[repr(C)] - struct UserId_Id { + struct UserId { pub f0: u32, pub f1: roc_std::RocStr, } @@ -311,7 +302,11 @@ mod test_gen_rs { target_arch = "arm", target_arch = "wasm32" ))] - pub type UserId = roc_std::RocStr; + #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] + #[repr(C)] + struct UserId { + pub f0: roc_std::RocStr, + } "# ) ); From 729c849b51b13f3b725eedaa0783bf0214696656 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 29 May 2022 00:03:30 -0400 Subject: [PATCH 65/65] Don't special-case bindgen for single-tag unions --- bindgen/src/bindgen.rs | 262 ++++++++++++++++++---------------------- bindgen/tests/gen_rs.rs | 78 ------------ 2 files changed, 115 insertions(+), 225 deletions(-) diff --git a/bindgen/src/bindgen.rs b/bindgen/src/bindgen.rs index 23438779e1..52a4273480 100644 --- a/bindgen/src/bindgen.rs +++ b/bindgen/src/bindgen.rs @@ -313,170 +313,138 @@ fn add_tag_union( }) .collect(); - if tags.len() == 1 { - // This is a single-tag union. - let (tag_name, payload_vars) = tags.pop().unwrap(); + let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); + let name = match opt_name { + Some(sym) => sym.as_str(env.interns).to_string(), + None => env.enum_names.get_name(var), + }; - // If there was a type alias name, use that. Otherwise use the tag name. - let name = match opt_name { - Some(sym) => sym.as_str(env.interns).to_string(), - None => tag_name, - }; + // Sort tags alphabetically by tag name + tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - let fields = payload_vars - .iter() - .enumerate() - .map(|(index, payload_var)| (index, *payload_var)); + let is_recursive = is_recursive_tag_union(&layout); - add_struct(env, name, fields, types, |name, fields| { - RocType::TagUnionPayload { name, fields } + let mut tags: Vec<_> = tags + .into_iter() + .map(|(tag_name, payload_vars)| { + match struct_fields_needed(env, payload_vars.iter().copied()) { + 0 => { + // no payload + (tag_name, None) + } + 1 if !is_recursive => { + // this isn't recursive and there's 1 payload item, so it doesn't + // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them + // can have payloads of plain old Str, no struct wrapper needed. + let payload_var = payload_vars.get(0).unwrap(); + let layout = env + .layout_cache + .from_var(env.arena, *payload_var, env.subs) + .expect("Something weird ended up in the content"); + let payload_id = add_type_help(env, layout, *payload_var, None, types); + + (tag_name, Some(payload_id)) + } + _ => { + // create a RocType for the payload and save it + let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" + let fields = payload_vars.iter().copied().enumerate(); + let struct_id = add_struct(env, struct_name, fields, types, |name, fields| { + RocType::TagUnionPayload { name, fields } + }); + + (tag_name, Some(struct_id)) + } + } }) - } else { - // This is a multi-tag union. + .collect(); - // This is a placeholder so that we can get a TypeId for future recursion IDs. - // At the end, we will replace this with the real tag union type. - let type_id = types.add(RocType::Struct { - name: String::new(), - fields: Vec::new(), - }); - let layout = env.layout_cache.from_var(env.arena, var, subs).unwrap(); - let name = match opt_name { - Some(sym) => sym.as_str(env.interns).to_string(), - None => env.enum_names.get_name(var), - }; + let typ = match layout { + Layout::Union(union_layout) => { + use roc_mono::layout::UnionLayout::*; - // Sort tags alphabetically by tag name - tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - - let is_recursive = is_recursive_tag_union(&layout); - - let mut tags: Vec<_> = tags - .into_iter() - .map(|(tag_name, payload_vars)| { - match struct_fields_needed(env, payload_vars.iter().copied()) { - 0 => { - // no payload - (tag_name, None) - } - 1 if !is_recursive => { - // this isn't recursive and there's 1 payload item, so it doesn't - // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them - // can have payloads of plain old Str, no struct wrapper needed. - let payload_var = payload_vars.get(0).unwrap(); - let layout = env - .layout_cache - .from_var(env.arena, *payload_var, env.subs) - .expect("Something weird ended up in the content"); - let payload_id = add_type_help(env, layout, *payload_var, None, types); - - (tag_name, Some(payload_id)) - } - _ => { - // create a RocType for the payload and save it - let struct_name = format!("{}_{}", name, tag_name); // e.g. "MyUnion_MyVariant" - let fields = payload_vars.iter().copied().enumerate(); - let struct_id = - add_struct(env, struct_name, fields, types, |name, fields| { - RocType::TagUnionPayload { name, fields } - }); - - (tag_name, Some(struct_id)) - } + match union_layout { + // A non-recursive tag union + // e.g. `Result ok err : [Ok ok, Err err]` + NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), + // A recursive tag union (general case) + // e.g. `Expr : [Sym Str, Add Expr Expr]` + Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), + // A recursive tag union with just one constructor + // Optimization: No need to store a tag ID (the payload is "unwrapped") + // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` + NonNullableUnwrapped(_) => { + todo!() } - }) - .collect(); + // A recursive tag union that has an empty variant + // Optimization: Represent the empty variant as null pointer => no memory usage & fast comparison + // It has more than one other variant, so they need tag IDs (payloads are "wrapped") + // e.g. `FingerTree a : [Empty, Single a, More (Some a) (FingerTree (Tuple a)) (Some a)]` + // see also: https://youtu.be/ip92VMpf_-A?t=164 + NullableWrapped { .. } => { + todo!() + } + // A recursive tag union with only two variants, where one is empty. + // Optimizations: Use null for the empty variant AND don't store a tag ID for the other variant. + // e.g. `ConsList a : [Nil, Cons a (ConsList a)]` + NullableUnwrapped { + nullable_id: null_represents_first_tag, + other_fields: _, // TODO use this! + } => { + // NullableUnwrapped tag unions should always have exactly 2 tags. + debug_assert_eq!(tags.len(), 2); - let typ = match layout { - Layout::Union(union_layout) => { - use roc_mono::layout::UnionLayout::*; + let null_tag; + let non_null; - match union_layout { - // A non-recursive tag union - // e.g. `Result ok err : [Ok ok, Err err]` - NonRecursive(_) => RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }), - // A recursive tag union (general case) - // e.g. `Expr : [Sym Str, Add Expr Expr]` - Recursive(_) => RocType::TagUnion(RocTagUnion::Recursive { name, tags }), - // A recursive tag union with just one constructor - // Optimization: No need to store a tag ID (the payload is "unwrapped") - // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` - NonNullableUnwrapped(_) => { - todo!() + if null_represents_first_tag { + // If nullable_id is true, then the null tag is second, which means + // pop() will return it because it's at the end of the vec. + null_tag = tags.pop().unwrap().0; + non_null = tags.pop().unwrap(); + } else { + // The null tag is first, which means the tag with the payload is second. + non_null = tags.pop().unwrap(); + null_tag = tags.pop().unwrap().0; } - // A recursive tag union that has an empty variant - // Optimization: Represent the empty variant as null pointer => no memory usage & fast comparison - // It has more than one other variant, so they need tag IDs (payloads are "wrapped") - // e.g. `FingerTree a : [Empty, Single a, More (Some a) (FingerTree (Tuple a)) (Some a)]` - // see also: https://youtu.be/ip92VMpf_-A?t=164 - NullableWrapped { .. } => { - todo!() - } - // A recursive tag union with only two variants, where one is empty. - // Optimizations: Use null for the empty variant AND don't store a tag ID for the other variant. - // e.g. `ConsList a : [Nil, Cons a (ConsList a)]` - NullableUnwrapped { - nullable_id: null_represents_first_tag, - other_fields: _, // TODO use this! - } => { - // NullableUnwrapped tag unions should always have exactly 2 tags. - debug_assert_eq!(tags.len(), 2); - let null_tag; - let non_null; + let (non_null_tag, non_null_payload) = non_null; - if null_represents_first_tag { - // If nullable_id is true, then the null tag is second, which means - // pop() will return it because it's at the end of the vec. - null_tag = tags.pop().unwrap().0; - non_null = tags.pop().unwrap(); - } else { - // The null tag is first, which means the tag with the payload is second. - non_null = tags.pop().unwrap(); - null_tag = tags.pop().unwrap().0; - } - - let (non_null_tag, non_null_payload) = non_null; - - RocType::TagUnion(RocTagUnion::NullableUnwrapped { - name, - null_tag, - non_null_tag, - non_null_payload: non_null_payload.unwrap(), - null_represents_first_tag, - }) - } + RocType::TagUnion(RocTagUnion::NullableUnwrapped { + name, + null_tag, + non_null_tag, + non_null_payload: non_null_payload.unwrap(), + null_represents_first_tag, + }) } } - Layout::Builtin(builtin) => match builtin { - Builtin::Int(_) => RocType::TagUnion(RocTagUnion::Enumeration { - name, - tags: tags.into_iter().map(|(tag_name, _)| tag_name).collect(), - }), - Builtin::Bool => RocType::Bool, - Builtin::Float(_) - | Builtin::Decimal - | Builtin::Str - | Builtin::Dict(_, _) - | Builtin::Set(_) - | Builtin::List(_) => unreachable!(), - }, - Layout::Struct { .. } - | Layout::Boxed(_) - | Layout::LambdaSet(_) - | Layout::RecursivePointer => { - unreachable!() - } - }; - - types.replace(type_id, typ); - - if is_recursive { - env.known_recursive_types.insert(var, type_id); } + Layout::Builtin(Builtin::Int(_)) => RocType::TagUnion(RocTagUnion::Enumeration { + name, + tags: tags.into_iter().map(|(tag_name, _)| tag_name).collect(), + }), + Layout::Builtin(_) + | Layout::Struct { .. } + | Layout::Boxed(_) + | Layout::LambdaSet(_) + | Layout::RecursivePointer => { + // These must be single-tag unions. Bindgen ordinary nonrecursive + // tag unions for them, and let Rust do the unwrapping. + // + // This should be a very rare use case, and it's not worth overcomplicating + // the rest of bindgen to make it do something different. + RocType::TagUnion(RocTagUnion::NonRecursive { name, tags }) + } + }; - type_id + let type_id = types.add(typ); + + if is_recursive { + env.known_recursive_types.insert(var, type_id); } + + type_id } fn is_recursive_tag_union(layout: &Layout) -> bool { diff --git a/bindgen/tests/gen_rs.rs b/bindgen/tests/gen_rs.rs index 10c510d980..d558a8e926 100644 --- a/bindgen/tests/gen_rs.rs +++ b/bindgen/tests/gen_rs.rs @@ -233,82 +233,4 @@ mod test_gen_rs { ) ); } - - #[test] - fn single_tag_union_with_payloads() { - let module = indoc!( - r#" - UserId : [Id U32 Str] - - main : UserId - main = Id 42 "blah" - "# - ); - - assert_eq!( - generate_bindings(module) - .strip_prefix('\n') - .unwrap_or_default(), - indoc!( - r#" - #[cfg(any( - target_arch = "x86_64", - target_arch = "aarch64" - ))] - #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] - #[repr(C)] - struct UserId { - pub f1: roc_std::RocStr, - pub f0: u32, - } - - #[cfg(any( - target_arch = "x86", - target_arch = "arm", - target_arch = "wasm32" - ))] - #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] - #[repr(C)] - struct UserId { - pub f0: u32, - pub f1: roc_std::RocStr, - } - "# - ) - ); - } - - #[test] - fn single_tag_union_with_one_payload_field() { - let module = indoc!( - r#" - UserId : [Id Str] - - main : UserId - main = Id "blah" - "# - ); - - assert_eq!( - generate_bindings(module) - .strip_prefix('\n') - .unwrap_or_default(), - indoc!( - r#" - #[cfg(any( - target_arch = "x86_64", - target_arch = "x86", - target_arch = "aarch64", - target_arch = "arm", - target_arch = "wasm32" - ))] - #[derive(Clone, Debug, Default, Eq, Ord, Hash, PartialEq, PartialOrd)] - #[repr(C)] - struct UserId { - pub f0: roc_std::RocStr, - } - "# - ) - ); - } }