diff --git a/examples/src/fail_tests/move_ref_not_borrowed.rs b/examples/src/fail_tests/move_ref_not_borrowed.rs new file mode 100644 index 0000000..829e3b2 --- /dev/null +++ b/examples/src/fail_tests/move_ref_not_borrowed.rs @@ -0,0 +1,29 @@ +use ouroboros::self_referencing; + +#[self_referencing] +struct CopyRef { + data: String, + #[borrows(data)] + #[covariant] + ref1: Option<&'this str>, + other: String, + #[borrows(other)] + #[covariant] + ref2: Option<&'this str>, +} + +fn main() { + let mut s = CopyRefBuilder { + data: "test".to_string(), + ref1_builder: |_| None, + other: "other".to_string(), + ref2_builder: |_| None, + } + .build(); + + s.with_mut(|f| { + *f.ref1 = Some(f.other); + }); + + drop(s); +} diff --git a/examples/src/fail_tests/move_ref_not_borrowed.stderr b/examples/src/fail_tests/move_ref_not_borrowed.stderr new file mode 100644 index 0000000..08e0852 --- /dev/null +++ b/examples/src/fail_tests/move_ref_not_borrowed.stderr @@ -0,0 +1,33 @@ +error[E0597]: `s` does not live long enough + --> src/fail_tests/move_ref_not_borrowed.rs:24:5 + | +16 | let mut s = CopyRefBuilder { + | ----- binding `s` declared here +... +24 | s.with_mut(|f| { + | ^ borrowed value does not live long enough + | _____| + | | +25 | | *f.ref1 = Some(f.other); +26 | | }); + | |______- argument requires that `s` is borrowed for `'static` +... +29 | } + | - `s` dropped here while still borrowed + +error[E0505]: cannot move out of `s` because it is borrowed + --> src/fail_tests/move_ref_not_borrowed.rs:28:10 + | +16 | let mut s = CopyRefBuilder { + | ----- binding `s` declared here +... +24 | s.with_mut(|f| { + | - borrow of `s` occurs here + | _____| + | | +25 | | *f.ref1 = Some(f.other); +26 | | }); + | |______- argument requires that `s` is borrowed for `'static` +27 | +28 | drop(s); + | ^ move out of `s` occurs here diff --git a/examples/src/fail_tests/move_ref_out_of_bound.rs b/examples/src/fail_tests/move_ref_out_of_bound.rs new file mode 100644 index 0000000..b22ce9a --- /dev/null +++ b/examples/src/fail_tests/move_ref_out_of_bound.rs @@ -0,0 +1,30 @@ +use ouroboros::self_referencing; + +#[self_referencing] +struct CopyRef { + data: String, + #[borrows(data)] + #[covariant] + ref1: Option<&'this str>, + other: String, + #[borrows(data, other)] + #[covariant] + ref2: Option<&'this str>, +} + +fn main() { + let mut s = CopyRefBuilder { + data: "test".to_string(), + ref1_builder: |_| None, + other: "other".to_string(), + ref2_builder: |x, _| Some(x), + } + .build(); + + s.with_mut(|f| { + *f.ref2 = Some(f.other); + *f.ref1 = *f.ref2; + }); + + drop(s); +} diff --git a/examples/src/fail_tests/move_ref_out_of_bound.stderr b/examples/src/fail_tests/move_ref_out_of_bound.stderr new file mode 100644 index 0000000..435f975 --- /dev/null +++ b/examples/src/fail_tests/move_ref_out_of_bound.stderr @@ -0,0 +1,35 @@ +error[E0597]: `s` does not live long enough + --> src/fail_tests/move_ref_out_of_bound.rs:24:5 + | +16 | let mut s = CopyRefBuilder { + | ----- binding `s` declared here +... +24 | s.with_mut(|f| { + | ^ borrowed value does not live long enough + | _____| + | | +25 | | *f.ref2 = Some(f.other); +26 | | *f.ref1 = *f.ref2; +27 | | }); + | |______- argument requires that `s` is borrowed for `'static` +... +30 | } + | - `s` dropped here while still borrowed + +error[E0505]: cannot move out of `s` because it is borrowed + --> src/fail_tests/move_ref_out_of_bound.rs:29:10 + | +16 | let mut s = CopyRefBuilder { + | ----- binding `s` declared here +... +24 | s.with_mut(|f| { + | - borrow of `s` occurs here + | _____| + | | +25 | | *f.ref2 = Some(f.other); +26 | | *f.ref1 = *f.ref2; +27 | | }); + | |______- argument requires that `s` is borrowed for `'static` +28 | +29 | drop(s); + | ^ move out of `s` occurs here diff --git a/examples/src/ok_tests.rs b/examples/src/ok_tests.rs index 87674e1..2f9558b 100644 --- a/examples/src/ok_tests.rs +++ b/examples/src/ok_tests.rs @@ -35,6 +35,28 @@ struct ChainedAndUndocumented { ref2: &'this &'this i32, } +#[self_referencing] +struct CopyRef1 { + data1: i32, + data2: i32, + #[borrows(data1, data2)] + ref1: &'this i32, + #[borrows(data1, data2)] + ref2: &'this i32, +} + +#[self_referencing] +struct CopyRef2 { + data: String, + #[borrows(data)] + #[covariant] + ref1: Option<&'this str>, + other: String, + #[borrows(data, other)] + #[covariant] + ref2: Option<&'this str>, +} + #[self_referencing] struct BoxCheckWithLifetimeParameter<'t> { external_data: &'t (), @@ -208,6 +230,48 @@ fn box_and_mut_ref() { assert!(bar.with_dref(|dref| **dref) == 34); } +#[test] +fn copy_ref1() { + let mut s = CopyRef1Builder { + data1: 1, + data2: 2, + ref1_builder: |x, _| x, + ref2_builder: |_, x| x, + } + .build(); + assert!(s.with_ref2(|d| **d) == 2); + s.with_mut(|fields| { + *fields.ref2 = *fields.ref1; + }); + assert!(s.with_ref2(|d| **d) == 1); +} + +#[test] +fn copy_ref2() { + let mut s = CopyRef2Builder { + data: "test".to_string(), + ref1_builder: |_| None, + other: "other".to_string(), + ref2_builder: |x, _| Some(x), + } + .build(); + + assert!(s.with_ref2(|d| d.unwrap()) == "test"); + s.with_mut(|f| { + *f.ref2 = *f.ref1; + }); + assert!(s.with_ref2(|d| d.is_none())); + s.with_mut(|f| { + *f.ref2 = Some(f.other); + }); + assert!(s.with_ref2(|d| d.unwrap()) == "other"); + + // compile error + // s.with_mut(|f| { + // *f.ref1 = *f.ref2; + // }); +} + // #[test] // fn template_mess() { // let ext_str = "Hello World!".to_owned(); diff --git a/ouroboros_macro/src/generate/with_mut.rs b/ouroboros_macro/src/generate/with_mut.rs index 221367d..d701c54 100644 --- a/ouroboros_macro/src/generate/with_mut.rs +++ b/ouroboros_macro/src/generate/with_mut.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use crate::{ info_structures::{FieldType, Options, StructInfo}, utils::{replace_this_with_lifetime, uses_this_lifetime}, @@ -18,13 +20,20 @@ pub fn make_with_all_mut_function( let mut mut_fields = Vec::new(); let mut mut_field_assignments = Vec::new(); let mut lifetime_idents = Vec::new(); + let mut borrowed_vars: Vec> = Vec::new(); + // I don't think the reverse is necessary but it does make the expanded code more uniform. - for field in info.fields.iter().rev() { + for (i, field) in info.fields.iter().enumerate().rev() { let field_name = &field.name; let field_type = &field.typ; let lifetime = format_ident!("this{}", lifetime_idents.len()); if uses_this_lifetime(quote! { #field_type }) || field.field_type == FieldType::Borrowed { lifetime_idents.push(lifetime.clone()); + let mut borrowed: HashSet<_> = field.borrows.iter().map(|b| b.index).collect(); + if borrowed.is_empty() { + borrowed.insert(i); + } + borrowed_vars.push(borrowed); } let field_type = replace_this_with_lifetime(quote! { #field_type }, lifetime.clone()); if field.field_type == FieldType::Tail { @@ -86,13 +95,20 @@ pub fn make_with_all_mut_function( .predicates .extend(extra.predicates.into_iter()); } - for idents in lifetime_idents.windows(2) { - let lt = Lifetime::new(&format!("'{}", idents[1]), Span::call_site()); - let outlives = Lifetime::new(&format!("'{}", idents[0]), Span::call_site()); - let extra: WhereClause = syn::parse_quote! { where #lt: #outlives }; - generic_where - .predicates - .extend(extra.predicates.into_iter()); + for (i, (s1, ident1)) in borrowed_vars.iter().zip(&lifetime_idents).enumerate() { + let lt = Lifetime::new(&format!("'{ident1}"), Span::call_site()); + for (j, (s2, ident2)) in borrowed_vars.iter().zip(&lifetime_idents).enumerate() { + if i == j { + continue; + } + if s1.is_subset(s2) { + let outlives = Lifetime::new(&format!("'{ident2}"), Span::call_site()); + let extra: WhereClause = syn::parse_quote! { where #lt: #outlives }; + generic_where + .predicates + .extend(extra.predicates.into_iter()); + } + } } let struct_defs = quote! { #[doc=#mut_struct_documentation]