How to use borrowed mut after mach again?

help
1

Hello friends, I have a function that handle some conditions on a mut entry. as you see the commented part, if I uncomment it shows some errors

fn get_property_by_key<'short, 'long>(
    property: &'short mut ObjectPropertyKind<'long>,
    key_name: &str,
    _allocator: &'short Allocator,
) -> Option<&'short mut Expression<'long>> {
    match property {
        ObjectPropertyKind::ObjectProperty(prop) => match &prop.key {
            PropertyKey::StaticIdentifier(key) if key.as_ref().name == key_name => {
                Some(&mut prop.value)
            }
            _ => None,
        },
        _ => None,
    }
    .or_else(|| {
        // if let ObjectPropertyKind::ObjectProperty(prop) = property {
        // println!("{:#?}", property);
        // }
        None
    })
}

For example:

rustc: second borrow occurs due to use of `property` in closure

or

error[E0500]: closure requires unique access to `property` but it is already borrowed
   --> src/parser/ast.rs:283:14
    |
269 |   fn get_property_by_key<'short, 'long>(
    |                          ------ lifetime `'short` defined here
...
274 | /     match property {
275 | |         ObjectPropertyKind::ObjectProperty(prop) => match &prop.key {
    | |                                            ---- borrow occurs here
276 | |             PropertyKey::StaticIdentifier(key) if key.as_ref().name == key_name => {
277 | |                 Some(&mut prop.value)
...   |
283 | |     .or_else(|| {
    | |              ^^ closure construction occurs here
284 | |         if let ObjectPropertyKind::ObjectProperty(prop) = property {
    | |                                                           -------- second borrow occurs due to use of `property` in closure
...   |
287 | |         None
288 | |     })
    | |______- returning this value requires that `property.0` is borrowed for `'short`

I even changed it to this

fn get_property_by_key<'short: 'long, 'long>

But does not work.

I know I've already performed a borrow operation once, but I need to borrow it again.
In this small piece of code, if none of the above conditions are met, I need to make a modification to property as an example.

Thank you in advance

2

Why are you using .or_else to match on property again, instead of putting the logic from the .or_else call inside of the first match? If there is a reason for this that got lost while you tried to construct a more minimal example, would you mind explaining why using .or_else is necessary in your real code?

1 Like
3

Just for learning, how can I reuse a borrowed value after using it for the first time? I got this code from ChatGPT, tried to fix it, but I always encounter errors. So, my question is: how can I reuse it again?

4

Try this, please:

fn get_property_by_key<'short, 'long>(
    property: &'short mut ObjectPropertyKind<'long>,
    key_name: &str,
    _allocator: &'short Allocator,
) -> Option<&'short mut Expression<'long>> {
    match property {
        ObjectPropertyKind::ObjectProperty(prop) => {
            println!("{:#?}", prop);
            match &prop.key {
                PropertyKey::StaticIdentifier(key) if key.as_ref().name == key_name => {
                    Some(&mut prop.value)
                }
                _ => None,
            }
        },
        _ => None,
    }
}

You can't if your code invalidates the exclusivity of a mutable reference. Granted, the callback you pass to or_else is only called when your first match expression returns None and we don't already mutably borrow from property when we construct the Some(&mut prop.value), but seemingly this is not something the borrow checker can reason about.

1 Like
5

The or_else method receives both the Option with the borrowed lifetime, and the closure with the capture. In the Some case, that would be instant UB due to &mut aliasing, for example. There is no sound way for the borrow checker to accept this, with how function boundaries are defined.

Without a different type of function API, no amount of enhanced reasoning can accept this.

3 Likes
6

or_else(closure) immediately borrows everything it will need to use in any case, and the borrowing is checked at the type level.

None still has the same lifetime requirements as Some, because they are the same Option type.

When a lifetime is used with &mut, it's an exclusive loan. This means that when a closure that wants to use it, the closure has to be the only place where this loan is used.

But when you have Option<'exclusive _>, the Option wants to have this lifetime exclusively, and calling a method on it with a closure that also wants to use the 'exclusive lifetime creates a conflict. It's a conflict even if there's nothing in the Option to really conflict with. It's a conflict even when the closure isn't called, because the borrow checker doesn't "see" the logic happening in or_else.

Borrow checking is much smarter when it doesn't go through closures. Inside functions, without closures involved, the borrow checker can track actual usage of borrowed objects and see that if/else or different match arms don't conflict. But closures are opaque to borrow checking and get only coarse grained check for any use of anything inside.

2 Likes
7

Hi again, thank you for all your helps, I am new to rust and learning.

I changed some of code, and I think I did not understand something about the condition with mut stuff

Please see the bellow code.

fn find_hooks_property<'short, 'long>(
    program: &'short mut Program<'long>,
    allocator: &'short Allocator,
) -> Result<(Option<&'short mut Expression<'long>>, bool), String> {
    let live_socket_found = program.body.iter_mut().any(|node| {
        if let Statement::VariableDeclaration(var_decl) = node {
            var_decl
                .declarations
                .iter()
                .any(|decl| is_target_variable(decl, "liveSocket"))
        } else {
            false
        }
    });

    if !live_socket_found {
        return Err("liveSocket not found.".to_string());
    }

    let result = program.body.iter_mut().find_map(|node| {
        let var_decl = match node {
            Statement::VariableDeclaration(var_decl) => var_decl,
            _ => return None,
        };

        var_decl.declarations.iter_mut().find_map(|decl| {
            let obj_expr = get_new_expression(decl)?;

            obj_expr.arguments.iter_mut().find_map(|arg| {
                let obj_expr_inner = get_object_expression(arg)?;

                if let Some(expr) = obj_expr_inner
                    .properties
                    .iter_mut()
                    .find_map(|prop| get_property_by_key(prop, "hooks"))
                {
                    Some(expr)
                } else {
                    // .... some code here
                   // I need to use obj_expr_inner here
                    obj_expr_inner
                        .properties
                        .iter_mut()
                        .find_map(|prop| get_property_by_key(prop, "hooks"))
                }
            })
        })
    });

    let hooks_exist = result.is_some();
    Ok((result, hooks_exist))
}

the else part are using obj_expr_inner var which is used as mut inside if and if the condition is not I want to use it inside else, but I have error

} else {
                    // .... some code here
                   // I need to use obj_expr_inner here
                    obj_expr_inner
                        .properties
                        .iter_mut()
                        .find_map(|prop| get_property_by_key(prop, "hooks"))
                }

And Error

rustc: cannot borrow `obj_expr_inner.properties` as mutable more than once at a time
second mutable borrow occurs here

Thank you in advance

8

Wow, I do not know why Rust so much hard to be understand especially for me :))

fn find_hooks_property<'short, 'long>(
    program: &'short mut Program<'long>,
    allocator: &'short Allocator,
) -> Result<(Option<&'short mut Expression<'long>>, bool), String> {
    let live_socket_found = program.body.iter_mut().any(|node| {
        if let Statement::VariableDeclaration(var_decl) = node {
            var_decl
                .declarations
                .iter()
                .any(|decl| is_target_variable(decl, "liveSocket"))
        } else {
            false
        }
    });

    if !live_socket_found {
        return Err("liveSocket not found.".to_string());
    }

    let result = program.body.iter_mut().find_map(|node| {
        let var_decl = match node {
            Statement::VariableDeclaration(var_decl) => var_decl,
            _ => return None,
        };

        var_decl.declarations.iter_mut().find_map(|decl| {
            let obj_expr = get_new_expression(decl)?;

            obj_expr.arguments.iter_mut().find_map(|arg| {
                let obj_expr_inner = get_object_expression(arg)?;

                let mut iter = obj_expr_inner.properties.iter_mut();

                if let Some(expr) = iter.find_map(|prop| get_property_by_key(prop, "hooks")) {
                    return Some(expr);
                }

                let new_hooks_property = ObjectPropertyKind::ObjectProperty(OXCBox::new_in(
                    ObjectProperty {
                        span: Span::default(),
                        kind: PropertyKind::Init,
                        key: PropertyKey::StaticIdentifier(OXCBox::new_in(
                            IdentifierName {
                                span: Span::default(),
                                name: Atom::from("hooks"),
                            },
                            allocator,
                        )),
                        value: Expression::ObjectExpression(OXCBox::new_in(
                            ObjectExpression {
                                span: Span::default(),
                                properties: OXCVec::new_in(allocator),
                                trailing_comma: None,
                            },
                            allocator,
                        )),
                        method: false,
                        shorthand: false,
                        computed: false,
                    },
                    allocator,
                ));

                iter.push(new_hooks_property).find_map(|prop| get_property_by_key(prop, "hooks"))
            })
        })
    });

    let hooks_exist = result.is_some();
    Ok((result, hooks_exist))
}

For example I change my way not to use 2 times, so

let mut iter = obj_expr_inner.properties.iter_mut();

I created this var, but now I can not push to this iter var, now it does not support the push :smile:

rustc: no method named `push` found for struct `std::slice::IterMut` in the current scope
method not found in `IterMut<'_, ObjectPropertyKind<'_>>`

This way to create let mut iter = obj_expr_inner.properties.iter_mut(); is right way? or rust structure? or I am doing it in a bad way?

9

This is not an answer but could you please always post the full error from running cargo in the terminal, if your IDE does not have a way to get the full error? The full error has much more information, such as notes, hints, etc, and also shows where all the relevant borrows are. Thanks.

4 Likes
10

Where are the types coming from (Program<'_>, Expression<'_>) -- your own library, or a dependency?

If they're your own, consider backing up a ways and using data structures that own or shared-own[1] their data, so that they aren't parameterized by lifetimes. Making your own lifetime parameterized types, especially for non-short-term use,[2] is Rust hard mode. It's going to be a rough time if you attempt something like a zero-copy parser before you have a feel for lifetimes.

                if let Some(expr) = obj_expr_inner
                    .properties
                    .iter_mut()
                    .find_map(|prop| get_property_by_key(prop, "hooks"))
                {
                    Some(expr)
                } else {
                    // .... some code here
                   // I need to use obj_expr_inner here
                    obj_expr_inner
                        .properties
                        .iter_mut()
                        .find_map(|prop| get_property_by_key(prop, "hooks"))
                }

Well, the if let and else branches do the same thing, so that's just...

                obj_expr_inner
                    .properties
                    .iter_mut()
                    .find_map(|prop| get_property_by_key(prop, "hooks"))

...but perhaps you need to do something different. Try ending the if let:

                if let Some(expr) = obj_expr_inner
                    .properties
                    .iter_mut()
                    .find_map(|prop| get_property_by_key(prop, "hooks"))
                {
                    return Some(expr);
                }

                // what use to be the `else` block goes here

(I'm not 100% sure if this will help as there isn't enough context to easily mock this up.)

Iterators are distinct from the collection being iterated, so you can't push into an iterator like it was a Vec. There's not even necessarily a Vec to push into: the data being iterated over might be in array or something else.[3]

  1. e.g. Arc<_> or Rc<_> ↩︎

  2. like longer than create-use-discard ↩︎

  3. Even if you could know there was a Vec underneath from a type perspective, modifying it would be a form of iterator invalidation. ↩︎

3 Likes
11

Were you confused about the type of the variable? If you don't know the type, your IDE may show the type when you hover on it, if it's using rust analyzer. If you do know the type, be sure to check the doc for what methods are supported.

1 Like
12

Sorry about it
the whole code here

13

I sincerely apologize to my friends who are trying to help. There are a lot of topics involved here, and unfortunately, I don't fully understand them.

For example, I can see the types in my editor, and I even dive into the type definitions and read part of the code. However, in the end, I still encounter various lifetime errors or borrowing issues.

The functions linked here are part of my code.
I'm using the OXC library to manipulate JavaScript:

I apologize again. I somewhat understand the explanations provided, but unfortunately, I don't know how to apply them in my code.

15

It's because you are doing things in the language that you have admitted are outside of your comfort zone. That's one way to learn...

Correct, because iterators do not have a push method. Perhaps you want to chain a second iterator onto the first?

Looking over the actual code, I think you just want to push to the Vec that you got the iterator from. You may not need the iterator at all. But what you are asking the compiler to do is something that it has trouble with: analysis of lifetimes with conditional control flow across functions.

The best solution I've found so far (not sure if it works, but it does type-check) is extracting the "get a mutable reference to the vector of properties" into its own function so that the reference can be reused in two places, and also move the "push a new property" to the caller:

diff --git a/native/mishka_parser_nif/src/parser/ast.rs b/native/mishka_parser_nif/src/parser/ast.rs
index 5446968..a097068 100644
--- a/native/mishka_parser_nif/src/parser/ast.rs
+++ b/native/mishka_parser_nif/src/parser/ast.rs
@@ -34,10 +34,10 @@ pub fn source_to_ast<'a>(file_path: &str, allocator: &'a Allocator) -> Result<Pr
     Ok(parse_result.program)
 }
 
-pub fn is_module_imported_from_ast<'a>(
+pub fn is_module_imported_from_ast(
     file_path: &str,
     module_name: &str,
-    allocator: &'a Allocator,
+    allocator: &Allocator,
 ) -> Result<bool, String> {
     let program = source_to_ast(file_path, allocator)?;
 
@@ -52,10 +52,10 @@ pub fn is_module_imported_from_ast<'a>(
     Ok(false)
 }
 
-pub fn insert_import_to_ast<'a>(
+pub fn insert_import_to_ast(
     file_path: &str,
     import_lines: &str,
-    allocator: &'a Allocator,
+    allocator: &Allocator,
 ) -> Result<String, String> {
     let mut program = source_to_ast(file_path, allocator)?;
 
@@ -167,45 +167,33 @@ pub fn find_live_socket_node_from_ast<'a>(program: &'a Program<'a>) -> Result<bo
 }
 
 #[allow(dead_code)]
-pub fn extend_hook_object_to_ast<'a>(
-    file_path: &str,
-    allocator: &'a Allocator,
-) -> Result<String, String> {
+pub fn extend_hook_object_to_ast(file_path: &str, allocator: &Allocator) -> Result<String, String> {
     let mut program = source_to_ast(file_path, allocator)?;
 
-    let hooks_result = find_hooks_property(&mut program, allocator)?;
-    if let Some(hooks_property) = hooks_result {
+    if !has_live_socket(&program) {
+        return Err("liveSocket not found.".to_string());
+    }
+
+    let maybe_properties = get_properties(&mut program.body);
+    if let Some(properties) = maybe_properties {
+        let hooks_property = match find_hooks_property(properties) {
+            Some(prop) => prop,
+            None => {
+                let new_hooks_property = create_hooks(allocator);
+
+                properties.push(new_hooks_property);
+
+                get_property_by_key(properties.last_mut().unwrap(), "hooks").unwrap()
+            }
+        };
+
         if let Expression::ObjectExpression(obj_expr) = hooks_property {
-            let new_property = ObjectPropertyKind::ObjectProperty(OXCBox::new_in(
-                ObjectProperty {
-                    span: Span::default(),
-                    kind: PropertyKind::Init,
-                    key: PropertyKey::StaticIdentifier(OXCBox::new_in(
-                        IdentifierName {
-                            span: Span::default(),
-                            name: Atom::from("OXCTestHook"),
-                        },
-                        allocator,
-                    )),
-                    value: Expression::Identifier(OXCBox::new_in(
-                        IdentifierReference {
-                            span: Span::default(),
-                            name: Atom::from("OXCTestHook"),
-                            reference_id: Cell::new(None),
-                        },
-                        allocator,
-                    )),
-                    method: false,
-                    shorthand: true,
-                    computed: false,
-                },
-                allocator,
-            ));
+            let new_property = create_test_hook(allocator);
 
             obj_expr.properties.push(new_property);
         }
     } else {
-        return Err("liveSocket not found in the AST".to_string());
+        return Err("properties not found in the AST".to_string());
     }
 
     let codegen = Codegen::new();
@@ -213,74 +201,39 @@ pub fn extend_hook_object_to_ast<'a>(
     Ok(generated_code)
 }
 
-fn find_hooks_property<'short, 'long>(
-    program: &'short mut Program<'long>,
-    allocator: &'short Allocator,
-) -> Result<Option<&'short mut Expression<'long>>, String> {
-    let live_socket_found = program.body.iter_mut().any(|node| {
-        if let Statement::VariableDeclaration(var_decl) = node {
-            var_decl
-                .declarations
-                .iter()
-                .any(|decl| is_target_variable(decl, "liveSocket"))
-        } else {
-            false
-        }
-    });
-
-    if !live_socket_found {
-        return Err("liveSocket not found.".to_string());
-    }
-
-    let result = program.body.iter_mut().find_map(|node| {
-        let var_decl = match node {
-            Statement::VariableDeclaration(var_decl) => var_decl,
-            _ => return None,
-        };
-
-        var_decl.declarations.iter_mut().find_map(|decl| {
-            let obj_expr = get_new_expression(decl)?;
-
-            obj_expr.arguments.iter_mut().find_map(|arg| {
-                let obj_expr_inner = get_object_expression(arg)?;
-                let mut iter = obj_expr_inner.properties.iter_mut();
-                if let Some(expr) = iter.find_map(|prop| get_property_by_key(prop, "hooks")) {
-                    return Some(expr);
-                }
+fn has_live_socket(program: &Program<'_>) -> bool {
+    program.body.iter().any(|node| match node {
+        Statement::VariableDeclaration(var_decl) => var_decl
+            .declarations
+            .iter()
+            .any(|decl| is_target_variable(decl, "liveSocket")),
+        _ => false,
+    })
+}
 
-                let new_hooks_property = ObjectPropertyKind::ObjectProperty(OXCBox::new_in(
-                    ObjectProperty {
-                        span: Span::default(),
-                        kind: PropertyKind::Init,
-                        key: PropertyKey::StaticIdentifier(OXCBox::new_in(
-                            IdentifierName {
-                                span: Span::default(),
-                                name: Atom::from("hooks"),
-                            },
-                            allocator,
-                        )),
-                        value: Expression::ObjectExpression(OXCBox::new_in(
-                            ObjectExpression {
-                                span: Span::default(),
-                                properties: OXCVec::new_in(allocator),
-                                trailing_comma: None,
-                            },
-                            allocator,
-                        )),
-                        method: false,
-                        shorthand: false,
-                        computed: false,
-                    },
-                    allocator,
-                ));
-
-                iter.push(new_hooks_property)
-                // .find_map(|prop| get_property_by_key(prop, "hooks"))
+fn get_properties<'short, 'long>(
+    body: &'short mut OXCVec<'long, Statement<'long>>,
+) -> Option<&'short mut OXCVec<'long, ObjectPropertyKind<'long>>> {
+    body.iter_mut().find_map(|node| match node {
+        Statement::VariableDeclaration(var_decl) => {
+            var_decl.declarations.iter_mut().find_map(|decl| {
+                let obj_expr = get_new_expression(decl)?;
+                obj_expr.arguments.iter_mut().find_map(|arg| {
+                    let obj_expr_inner = get_object_expression(arg)?;
+                    Some(&mut obj_expr_inner.properties)
+                })
             })
-        })
-    });
+        }
+        _ => None,
+    })
+}
 
-    Ok(result)
+fn find_hooks_property<'short, 'long>(
+    properties: &'short mut OXCVec<'long, ObjectPropertyKind<'long>>,
+) -> Option<&'short mut Expression<'long>> {
+    properties
+        .iter_mut()
+        .find_map(|prop| get_property_by_key(prop, "hooks"))
 }
 
 fn is_target_variable(decl: &VariableDeclarator, name: &str) -> bool {
@@ -323,6 +276,62 @@ fn get_property_by_key<'short, 'long>(
     }
 }
 
+fn create_test_hook(allocator: &Allocator) -> ObjectPropertyKind {
+    ObjectPropertyKind::ObjectProperty(OXCBox::new_in(
+        ObjectProperty {
+            span: Span::default(),
+            kind: PropertyKind::Init,
+            key: PropertyKey::StaticIdentifier(OXCBox::new_in(
+                IdentifierName {
+                    span: Span::default(),
+                    name: Atom::from("OXCTestHook"),
+                },
+                allocator,
+            )),
+            value: Expression::Identifier(OXCBox::new_in(
+                IdentifierReference {
+                    span: Span::default(),
+                    name: Atom::from("OXCTestHook"),
+                    reference_id: Cell::new(None),
+                },
+                allocator,
+            )),
+            method: false,
+            shorthand: true,
+            computed: false,
+        },
+        allocator,
+    ))
+}
+
+fn create_hooks(allocator: &Allocator) -> ObjectPropertyKind {
+    ObjectPropertyKind::ObjectProperty(OXCBox::new_in(
+        ObjectProperty {
+            span: Span::default(),
+            kind: PropertyKind::Init,
+            key: PropertyKey::StaticIdentifier(OXCBox::new_in(
+                IdentifierName {
+                    span: Span::default(),
+                    name: Atom::from("hooks"),
+                },
+                allocator,
+            )),
+            value: Expression::ObjectExpression(OXCBox::new_in(
+                ObjectExpression {
+                    span: Span::default(),
+                    properties: OXCVec::new_in(allocator),
+                    trailing_comma: None,
+                },
+                allocator,
+            )),
+            method: false,
+            shorthand: false,
+            computed: false,
+        },
+        allocator,
+    ))
+}
+
 // TODO: delete an object from hook
 #[cfg(test)]
 mod tests {

Bare with me, this is a long diff. (Sorry, I kind of rewrote half of it, But there are good reasons.)

First, I took the liberty to remove the unnecessary parts (eliding lifetimes, removing the allocator reference now that find_hooks_property() no longer allocates, etc). But I did not fix all of the lints. Run cargo clippy to find those.

Secondly, I inlined the problematic control flow part into the caller. Because extend_hook_object_to_ast() doesn't need to return a reference at all, there are no borrow errors of the "Problem case #3" flavor.

By inlining the push lower in the stack, we can get away with dropping the borrow returned by find_hooks_property() and create a new one after the push has completed. NLL is able to see all of the borrows since we're no longer doing conditional control flow across a function call.

It uses a few unwraps, but this is fine because these are guaranteed to be populated because of the push().

To bring it all together, I split your find_hooks_property() into three distinct functions. I also moved the creation of new objects into their own functions to clean up the call sites. Now extend_hook_object_to_ast() is super simple:

  1. Calls has_live_socket() to return an error if needed.
  2. Calls get_properties() to walk the AST.
  3. Calls find_hooks_property() to get a reference if one exists, or else a new "hooks" property is created and pushed to the properties reference received on step 2.
  4. Finally, create the "test hook" if the returned expression is an ObjectExpression.

This is my best interpretation of what was originally wanted. It's crucial that the find-or-push operation is done in multiple stages so that the compiler can reason about the references.

BTW, you might be trying to do things a bit too cleverly. Pushing to a vector while holding a reference to it will never work. The allocation may need to be resized, which could in turn move everything to a new location in memory. The borrowing errors prevent that safety hazard.

5 Likes
16

Yes, I think this learning process will take quite a long time for me. After years of working with a language like Elixir, I’m now experiencing something completely different.

I don’t even know how to express my gratitude to you. In both of my posts, you completely wrote the project for me—I didn’t actually do anything at all :joy:.

I’ve tried several times to read through your comment as well as review the parts of the code you’ve modified, step by step. Honestly, I don’t think I could have resolved this issue without your help. There are many foundational areas where, unfortunately, I still struggle.

In any case, I’ve broadly understood that a significant part of the changes revolves around the output type of functions being adjusted to a Vec tailored for the OXC project.

I’ve also somewhat grasped your approach to using if statements and got a partial understanding of how transitions between functions are structured. It’s fascinating to me that in Rust, you always have to strive to write smarter code—it really feels like playing a game.

I think, with the sheer amount of information I’ve absorbed here, I’ll need a few days to mentally adapt, especially to let go of comparisons with Elixir :smiling_face_with_tear:.

I’m not sure if my approach to learning in this way is correct or not!!?
Unfortunately, I don’t think I’ve reached the level yet where I can fully understand how to approach code editing in this way. I won’t stop trying, but if you have any suggestions, I’d greatly appreciate it if you could share them with me.

Once again, thank you so much! I’ve learned more from you in this short time than I would have in hours of independent effort while posting on this forum.

2 Likes
17

Be patient, it will all come together with time. :smiley: Everyone is different, and everyone learns in their own way. There is no "correct and incorrect" here. Just keep trying until you find what works best for you!

If it's any help, I can say that I've been studying and using Rust daily for around 9 years. I've hit these exact issues several times and I've picked up some "good habits" that automatically avoid them. I'm sure you will, too. But thanks to this experience, I am sometimes able to help others with their questions.

There's one thing I need to admit, though. It's a bit of an impostor syndrome thing: The post above kind of made this whole thing look a lot easier than it really is. I did have to readjust a few times and try more than one approach before I landed on that implementation as an answer. It took some real time and effort to do.

For instance, my first approach was to just kind of brute force your original design to compile. I got as far as making find_hooks_property() do the push before returning a reference in a very similar way: Use a non-borrowing check on the short-circuit return path, avoiding "Problem case #3". This kind of worked, except the push() call extended the 'short lifetime to 'long, which was awesome to see! It failed to compile and reminded me that the push will possibly invalidate references to the vector. So, I abandoned the original design as impossible. [1]

What I mean is, I kind of hid all of the hard stuff that I went through. Not on purpose, but because it wasn't relevant to the question. Just know that even if it looks like I magically had the answer or the right design in mind from the start, I did not. I had to work for it, even after all of these years of practice.

Which brings me back to those "good habits"! These days when I am coding, I think broadly in terms of "big picture" or "architecture". I see a whole Cathedral, not individual stones and mortar. Borrowing and lifetimes are a part of the architecture, in my mind. If I cannot explain the purpose of my borrows, they don't belong there. Knowing that oxc uses an arena (bumpalo) is also helpful: 'long always refers to the lifetime of the arena; Allocator. [2]

I elide lifetimes when I can, or else I reason about them: "I know I can take a temporary borrow to any deep part of this complex structure that is disconnected from the arena that it is borrowing." And I can directly take advantage of that knowledge: "I'll break this out into its own function so that I can reuse the temporary borrow."

Mix in a few workarounds for compiler limitations that I know of, and it's good enough to cover the majority of cases.

I hope this is helpful. I know it isn't anything groundbreaking, but perhaps some insight into my thought process on some of these challenges will be of use.

  1. It may be possible to do in safe Rust with the new borrow checker, I didn't actually try it. I think there's technically no reason that the push() needs to extend 'short, but it could be a fundamental problem with the invariance of &mut T. I didn't really dive into that theoretical possibility before I abandoned the approach. ↩︎

  2. Coincidentally, your tests leak an Allocator to create &'static Allocator, but you've named its lifetime 'a in the constructor mishka_chelekom/native/mishka_parser_nif/src/parser/ast.rs at rustler-nifs · shahryarjb/mishka_chelekom (and named it 'long in other functions).
    The leaking can be fixed by replacing let allocator = create_allocator(); with let allocator = Allocator::default(); and then changing the usage sites in the tests to &allocator. ↩︎

6 Likes
18

I have nothing to add to the solution @parasyte kindly provided. The below are more along the lines of follow-ups and thread meta-comments.

I think there may have been a lifetime mismatch here. This is the original signature:

fn find_hooks_property<'short, 'long>(
    program: &'short mut Program<'long>,
    allocator: &'short Allocator,
) -> Result<Option<&'short mut Expression<'long>>, String> {

But if you're going to create a new ObjectProperty<'a> to push into the guts of the Program<'long>, you're going to need 'a == 'long. But you only have a &'short Allocator.[1]

I'd probably try to pick some more descriptive lifetime name than 'long for everything that's tied together across the entire[2] program, 'oxc or 'bump or 'alloc perhaps.

fn find_hooks_property<'short, 'oxc>(
    program: &'short mut Program<'oxc>,
    allocator: &'oxc Allocator,
) -> Result<Option<&'short mut Expression<'oxc>>, String> {
Parenthetical aside

There's no suggestion for the code base here, just daydreaming.

In some hypothetical world where we had generic modules:

mod post_parse<'oxc> {
    type AllocRef = &'oxc Allocator;
    type Program = oxc::SomePath::Program<'oxc>;
    type Expression = oxc::SomePath::Expression<'oxc>;

    fn find_hooks_property(
        program: &mut Program,
        allocator: AllocRef,
    ) -> Result<Option<&mut Expression>, String> {

That really does clean up nicely! I guess you could simulate this somewhat with a trait, but it wouldn't be as ergonomic.[3]

FYI that basically means you don't really have a choice when it comes to having lifetimes everywhere (without backing up so far you pick an entirely new framework to rely on (or leaking memory to make everything 'static)). I.e. my comments about making your own lifetime-parameterized types don't apply (they're not your own, they're somebody else's).

There was nothing to apologize for as far as I'm concerned. Hopefully I didn't come across as berating! I was just trying to gather more information, and giving some general advice and specific feedback.

  1. I didn't test this out, but I'm pretty sure that's what's going on. ↩︎

  2. more or less ↩︎

  3. Self::Alias everywhere ↩︎

3 Likes
19

Ha! Your analysis is spot on. I didn't even recognize &'short Allocator, which is for sure what was causing the problem. Fixing the lifetime name does in fact make it work. (Sometimes I should just pay closer attention.)

And I agree with the suggestion to change the lifetime conventions! 'alloc is what is used in oxc::Vec, but oxc::Program unhelpfully calls it 'a.

2 Likes