Search code examples
rustborrow-checker

Mutable borrow problem with inserting Vacant entry into HashMap


I'm trying to write a simple parser for some yml config files (with serde) which involves some additional parsing for custom formats inside many params. My additional parsers are based on simple Regex'es, and I created a registry so that they are "compiled" only once. The registry is essentially a HashMap<String, _>. Now of course the method registering a Regex first checks if the same was already inserted, by means of the HashMap::entry() method.

Problem is, if I write something like this:

    fn register_pattern(&mut self, pattern: &str) {
        if let Vacant(entry) = self.regex_registry.entry(pattern.to_string()) {
            let asc = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Ascii);
            let jap = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Japanese);
            let parse_localized_regex = ...expression involving asc, jap...;
        }
    }

...the compiler yells at me like this:

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/parsing/parsers/parser_service.rs:65:23
   |
63 |         if let Vacant(entry) = self.regex_registry.entry(pattern.to_string()) {
   |                                ------------------- first mutable borrow occurs here
64 |             let asc = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Ascii);
65 |             let jap = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Japanese);
   |                       ^^^^ second mutable borrow occurs here
66 |             let parse_localized_regex = ParseLocalized::new(asc, jap);
67 |             entry.insert(parse_localized_regex);
   |             ----- first borrow later used here

The solution I've found works, but seems too complex to me:

    fn register_pattern(&mut self, pattern: &str) {
        if let Vacant(_) = self.regex_registry.entry(pattern.to_string()) {
            let asc = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Ascii);
            let jap = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Japanese);
            if let Vacant(entry) = self.regex_registry.entry(pattern.to_string()) {
                entry.insert(ParseLocalized::new(asc, jap));
            }
        }
    }

Solution

  • I believe in this case you should not use the entry API anyway - even without considering this issue.

    This is because the entry API, in order to be able to insert the key if missing, needs an owned key. For you that means that you cannot lookup with &str, you must call to_string() on it - and that means an allocation, which is a bigger cost than a second lookup, and assuming (as I understand) that most of the times the pattern is already registered, the cost of the allocation will dwarf the cost of the double lookup.

    So the best solution for you is to use a simple structure:

    fn register_pattern(&mut self, pattern: &str) {
        if !self.regex_registry.contains(pattern) {
            let asc = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Ascii);
            let jap = self.parse_locale_pattern(pattern.to_string(), ParseLocale::Japanese);
            let parse_localized_regex = ...expression involving asc, jap...;
    
            self.regex_registry.insert(pattern.to_string(), ParseLocalized::new(asc, jap));
        }
    }