Understanding when need clone into IntoIterator

help
1

Hello, friends. I hope you've had a great start to the new year. Apologies for asking a basic question.

I have two pieces of code that are quite similar, with only slight differences. However, in the second code snippet, I had to use Clone to fix an error and make it work. While the code runs fine now, I don’t quite understand why this happens and when exactly I need to use Clone. Additionally, I’m curious why Clone wasn’t necessary in the first code snippet.

Code 1:

pub fn extend_hook_object_to_ast<'a>(
    file_content: &str,
    names: impl IntoIterator<Item = &'a str>,
    allocator: &Allocator,
) -> Result<String, String> {
    let mut parsed = source_to_ast(file_content, allocator)?;

    if find_live_socket_node_from_ast(&parsed).is_err() {
        return Err("liveSocket not found.".to_string());
    }

    let maybe_properties = get_properties(&mut parsed.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_init_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 {
            for name in names {
                if !obj_expr.properties.iter().any(|x| match x {
                    ObjectPropertyKind::SpreadProperty(spread) => spread
                        .argument
                        .get_identifier_reference()
                        .map(|ref_id| ref_id.name == name)
                        .unwrap_or(false),
                    ObjectPropertyKind::ObjectProperty(normal) => normal
                        .value
                        .get_identifier_reference()
                        .map(|ref_id| ref_id.name == name)
                        .unwrap_or(false),
                }) {
                    let new_property = create_and_import_object_into_hook(name, allocator);
                    obj_expr.properties.push(new_property);
                }
            }
        }
    } else {
        return Err("properties not found in the AST.".to_string());
    }

    Ok(codegen(&parsed, false))
}

Code 2:

pub fn extend_var_object_property_by_names_to_ast<'a>(
    file_content: &str,
    var_name: &str,
    object_names: impl IntoIterator<Item = &'a str> + Clone,
    allocator: &Allocator,
) -> Result<String, String> {
    let mut parsed = source_to_ast(file_content, allocator)?;

    let result = parsed.program.body.iter_mut().find_map(|node| match node {
        Statement::VariableDeclaration(var_decl) => var_decl
            .declarations
            .iter_mut()
            .map(|decl| {
                if decl.id.kind.get_binding_identifier().unwrap().name == var_name {
                    let get_init = &mut decl.init;
                    if let Some(Expression::ObjectExpression(obj_expr)) = get_init {
                        for name in object_names.clone() {
                            if !obj_expr.properties.iter().any(|x| match x {
                                ObjectPropertyKind::SpreadProperty(spread) => spread
                                    .argument
                                    .get_identifier_reference()
                                    .map(|ref_id| ref_id.name == name)
                                    .unwrap_or(false),
                                ObjectPropertyKind::ObjectProperty(normal) => normal
                                    .value
                                    .get_identifier_reference()
                                    .map(|ref_id| ref_id.name == name)
                                    .unwrap_or(false),
                            }) {
                                let new_property =
                                    create_and_import_object_into_hook(name, allocator);
                                obj_expr.properties.push(new_property);
                            }
                        }
                    }
                    Ok(())
                } else {
                    Err("Variable not found in javascript body".to_string())
                }
            })
            .next(),
        _ => None,
    });

    match result {
        Some(Ok(_)) => Ok(codegen(&parsed, false)),
        Some(Err(e)) => Err(e),
        _ => Err("Variable not found in javascript body or javascript file is invalid".to_string()),
    }
}

As you can see in the first code snippet, I used the input names: impl IntoIterator<Item = &'a str> and called it in a for loop without needing to use Clone at all.

However, in the second code snippet, for the input object_names: impl IntoIterator<Item = &'a str> + Clone, I had to explicitly use clone in order to resolve the issue and make the code work.

Why it has this behaviour and how can use it inside code 2 without clone!
My next question is: Does using Clone here lead to higher resource consumption?

Thank you in advance :pray:t2::heart:

2

Rust has unique ownership. IntoIterator takes ownership for itself and doesn't give it back. When you call into_iter() on a value, that value is gone and can't be used any more.

In the second case you have a closure that may be called multiple times. Without the clone() it would have nothing left to iterate, since it would use up the only copy of the iterator on the first call.

1 Like
3

Aha, the second case has closure that moves the ownership again? base on my basic knowledge the only difference I can find in both code, the closure!! this is correct?

4

The ownership is moved every time you call into_iter(). Not only closures move, but also variable assignments and method calls can move too. So yes, there are multiple moves involved.

1 Like
5

Thank you. my question is I think the Clone is necessary here, but is there another way to handle it? without clone just for learning
Thank you

6

You could require the into iter type to be Copy, which would clone it automatically when needed. References in Rust are Copy, so types like &[X] will work.

1 Like
7

I think - if I understand your code correctly, that you only use the iterator once, right? It looks like whenever the name in object_names happens, you're on a definite path to returning Some from the find_map (by how it only happens inside of the map of an iterator whose next call then propagates to return for the find_map).

In this case, you can avoid the clone if you want, and do the alternative approach of a dynamic “only use once” check in the form of using Option. Something like

pub fn extend_var_object_property_by_names_to_ast<'a>(
    file_content: &str,
    var_name: &str,
-   object_names: impl IntoIterator<Item = &'a str> + Clone,
+   object_names: impl IntoIterator<Item = &'a str>,
    allocator: &Allocator,
) -> Result<String, String> {
    let mut parsed = source_to_ast(file_content, allocator)?;

+   let mut object_names = Some(object_names);

    let result = parsed.program.body.iter_mut().find_map(|node| match node {
        Statement::VariableDeclaration(var_decl) => var_decl
            .declarations
            .iter_mut()
            .map(|decl| {
                if decl.id.kind.get_binding_identifier().unwrap().name == var_name {
                    let get_init = &mut decl.init;
                    if let Some(Expression::ObjectExpression(obj_expr)) = get_init {
-                       for name in object_names.clone() {
+                       for name in object_names.take().unwrap() {
                            if !obj_expr.properties.iter().any(|x| match x {
                                ObjectPropertyKind::SpreadProperty(spread) => spread
                                    .argument
                                    .get_identifier_reference()
                                    .map(|ref_id| ref_id.name == name)
                                    .unwrap_or(false),
                                ObjectPropertyKind::ObjectProperty(normal) => normal
                                    .value
                                    .get_identifier_reference()
                                    .map(|ref_id| ref_id.name == name)
                                    .unwrap_or(false),
                            }) {
                                let new_property =
                                    create_and_import_object_into_hook(name, allocator);
                                obj_expr.properties.push(new_property);
                            }
                        }
                    }
                    Ok(())
                } else {
                    Err("Variable not found in javascript body".to_string())
                }
            })
            .next(),
        _ => None,
    });

    match result {
        Some(Ok(_)) => Ok(codegen(&parsed, false)),
        Some(Err(e)) => Err(e),
        _ => Err("Variable not found in javascript body or javascript file is invalid".to_string()),
    }
}

(untested, because I'm missing all the type definitions to make this compile)

You can probably also refactor this, by moving the map out one level… something like:

    let result = parsed
        .program
        .body
        .iter_mut()
        .find_map(|node| match node {
            Statement::VariableDeclaration(var_decl) => {
                var_decl.declarations.iter_mut().next()
            }
            _ => None,
        })
        .map(|decl| {
            if decl.id.kind.get_binding_identifier().unwrap().name == var_name {
                let get_init = &mut decl.init;
                if let Some(Expression::ObjectExpression(obj_expr)) = get_init {
                    for name in object_names {
                        if !obj_expr.properties.iter().any(|x| match x {
                            ObjectPropertyKind::SpreadProperty(spread) => spread
                                .argument
                                .get_identifier_reference()
                                .map(|ref_id| ref_id.name == name)
                                .unwrap_or(false),
                            ObjectPropertyKind::ObjectProperty(normal) => normal
                                .value
                                .get_identifier_reference()
                                .map(|ref_id| ref_id.name == name)
                                .unwrap_or(false),
                        }) {
                            let new_property = create_and_import_object_into_hook(name, allocator);
                            obj_expr.properties.push(new_property);
                        }
                    }
                }
                Ok(())
            } else {
                Err("Variable not found in javascript body".to_string())
            }
        });

(I'm not sure if this might run into borrow-checking issues with the iter_mut)

which would work because the map now is on Option, and Option::map only needs FnOnce not FnMut.

I'm not sure I understand what the code is doing actually, it seems potentially suspicious that from each var_decl.declarations.iter_mut() only the first item is ever looked at.

1 Like
8

Thank you for help and gave me some Ideas