Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to call SectionSetter::set again due to too-restrictive lifetimes on method signature #134

Closed
rikyborg opened this issue Aug 1, 2024 · 2 comments · Fixed by #135

Comments

@rikyborg
Copy link
Contributor

rikyborg commented Aug 1, 2024

Once a SectionSetter is obtained by e.g. calling Ini::with_section(), it's possible to set multiple key-value pairs by chaining calls to SectionSetter::set(). However, it is not possible to call set() multiple times on the same variable of type SectionSetter. The cause seems to be too restrictive lifetimes in the signature of SectionSetter::set().

Example

The following code:

use ini::Ini;

fn main() {
    let mut ini = Ini::new();
    let mut section_setter = ini.with_section(Some("section"));

    // chaining set() calls work
    section_setter.set("a", "b").set("c", "d");
    // but calling set() separately doesn't work
    section_setter.set("e", "f"); // <--- error[E0499]
}

fails to compile with the output

error[E0499]: cannot borrow `section_setter` as mutable more than once at a time
  --> src/main.rs:10:5
   |
8  |     section_setter.set("a", "b").set("c", "d");
   |     -------------- first mutable borrow occurs here
9  |     // but calling set() separately doesn't work
10 |     section_setter.set("e", "f");
   |     ^^^^^^^^^^^^^^
   |     |
   |     second mutable borrow occurs here
   |     first borrow later used here

For more information about this error, try `rustc --explain E0499`.
error: could not compile `inisec` (bin "inisec") due to 1 previous error

using rust-ini v0.21.0 and rustc v1.80.0.

Probable cause

The signature of SectionSetter::set() is

    pub fn set<K, V>(&'a mut self, key: K, value: V) -> &'a mut SectionSetter<'a>
    where
        K: Into<String>,
        V: Into<String>,
    {...}

where 'a is the lifetime of the Ini instance borrowed by SectionSetter<'a>.

This signature requires set() to borrow self: SectionSetter for 'a, which is the whole duration of the original borrow of Ini. So once set() is called on the variable section_setter, that variable stays borrowed for its whole lifetime and it's impossible to call set() again on it.

Possible solution

Changing the signature to:

    pub fn set<'b, K, V>(&'b mut self, key: K, value: V) -> &'b mut SectionSetter<'a>
    where
        K: Into<String>,
        V: Into<String>,
        'a: 'b,
    {...}

makes the example code above compile (and all rust-ini tests still pass).

What we do here is introduce a new lifetime 'b, and make the new borrow of SecionSetter<'a> only last for 'b. We also require that 'a is larger than 'b with the bound 'a: 'b.

This way, once the chain of set() calls on the variable section_setter ends, 'b expires and the original variable section_setter is not borrowed anymore and we can explicitly call set() again.

Other methods

I don't have a good-enough grasp of the whole API surface of rust-ini to judge whether this change should be applied to other methods as well. Which is why I'm filing this as an issue rather than a PR.

Most probably this applies to add(), delete() and get() as well.

@zonyitoo
Copy link
Owner

zonyitoo commented Aug 1, 2024

Looks reasonable, could you make a PR for that?

@rikyborg
Copy link
Contributor Author

rikyborg commented Aug 2, 2024

Waiting for PR #135 , a workaround is to define a variable containing &mut SectionSetter and to keep reassigning to it:

use ini::{Ini, SectionSetter};

fn main() {
    let mut ini = Ini::new();
    let mut section_setter: SectionSetter = ini.with_section(Some("section"));
    let mut setter: &mut SectionSetter = &mut section_setter;

    // chained calls
    setter = setter.set("a", "1").set("b", "2");
    // separate calls
    setter = setter.set("c", "3");
    setter = setter.set("d", "4");
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants