Protect guard needs to be bound with let

[t-kalinowski]

Hi, very nice crate + package + book!

I'm reading through the code and came across this:

savvy/src/eval.rs

Line 52 in 2e98d9c

local_protect(charsxp);

I'm pretty sure this should be let _guard = local_protect(charsxp);

Otherwise, the LocalProtection object is immediately dropped.

Here is a minimal example demonstrating:

struct MyStruct {
    name: String,
}

impl Drop for MyStruct {
    fn drop(&mut self) {
        println!("Dropping MyStruct: {}", self.name);
    }
}

fn foo(instance_name: &str) -> MyStruct {
    println!("Creating MyStruct: {}", instance_name);
    MyStruct { name: instance_name.to_string() }
}

fn main() {
    println!("Test 1: (no let)");
    {
        println!("Start Test 1 block");
        foo("Immediate Instance"); // Instance is dropped immediately after this line.
        println!("End Test 1 block");
    }

    println!("Test 2: let _ =");
    {
        println!("Start Test 2 block");
        let _ = foo("Temp Binding Instance"); // Instance is dropped immediately after this line.
        println!("End Test 2 block");
    }

    println!("Test 3: let _x =");
    {
        println!("Start Test 3 block");
        let _x = foo("Normal Binding Instance"); // `_x` holds the instance until the end of this block.
        println!("End Test 3 block");
    }
}

outputs:

Test 1: (no let)
Start Test 1 block
Creating MyStruct: Immediate Instance
Dropping MyStruct: Immediate Instance
End Test 1 block
Test 2: let _ =
Start Test 2 block
Creating MyStruct: Temp Binding Instance
Dropping MyStruct: Temp Binding Instance
End Test 2 block
Test 3: let _x =
Start Test 3 block
Creating MyStruct: Normal Binding Instance
End Test 3 block
Dropping MyStruct: Normal Binding Instance

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=88fb5bb69f6a0666f51148be56d7ead0

[t-kalinowski]

Also, in that block, it might be simpler to just use R_ParseEvalString instead of parsing and evaling separately. It would cut down on the amount of code needed.

[yutannihilation]

it might be simpler to just use R_ParseEvalString instead of parsing and evaling separately. It would cut down on the amount of code needed.

Thanks, I didn't use it because

1. R_ParseEvalString was not explained in WRE at that time, which means it's not an "API"
2. R_ParseEvalString is not very widely used among CRAN packages: https://github.com/search?q=org%3Acran+R_ParseEvalString&type=code

But, now that reason 1 is no longer the case, it sounds good to me.