Hacker News new | ask | show | jobs
by lasagnaphil 2622 days ago
I've tried to fix the Rust code, but I'm getting some lifetime errors. Can anyone spot the problem? (Note: I've used C++ for most of the time and am still uncomfortable with Rust)

  use std::collections::HashSet;

  fn main () {
    let strings = vec![
      "apple", "cAt", "cat", "Dog", "apple", "dog", "Cat",
      "Apple", "dOg", "banana", "cat", "dog", "apple",
    ];

    let mut set : HashSet<&'static str> = HashSet::new();
    
    let strings : Vec<&str> = strings
        .into_iter()
        .map(|string| (string, string.to_lowercase()))
        .filter(|(_, lower)| set.insert(lower))
        .map(|(string, _)| string)
        .collect();
    
    println!("{:?}", strings);
  }
1 comments

The lower value is of type &String because the argument to filter is of type &(String, String) and the pattern match needs to propagate the &. The reference is only valid for the body of the filter closure (that is, it is &'short String, for some anonymous/unnamed lifetime 'short), but it is being put into a set that expects &'static str (the explicit type on the let line). The &String to &str type coercion is fine, but it's not correct to treat the &'short as a &'static (the short one is potentially invalid after filter's closure returns).

Changing the type annotations won't make this compile, because it is actually catching a real bug. The iterator is lazy and the full pipeline is executed for each element at once: lowercasing, inserting into the set, inserting into the Vec that's the result of the collect, and deallocating the lower string. This last step is the key/danger: if the HashSet held references/slices to the lower strings (instead of owning them), those references would become dangling immediately and future look-ups into the set won't work right/will trigger undefined behaviour.

The problem is a little clearer (and mostly fixed) if you simplify the code slightly by removing the two map calls, and instead call to_lowercase in the filter directly:

        .into_iter()
        .filter(|string| set.insert(string.to_lowercase()))
        .collect();
This form is a type error, that can be corrected by changing the type annotation to be HashSet<String>, or even removing it entirely and letting type inference handle it. The HashSet owning the strings is the key, so they only disappear after the entire iteration is complete, not after each element.
Even with the hash set solution, it would be nice to not copy the string contents, which unfortunately is needed if the hash set contains lowercased versions of the strings. On the other hand, converting all strings to lowercase then comparing them isn't a great way to case-insensitively compare strings anyway, as Unicode doesn't guarantee that will work.

The UniCase crate defines a wrapper around strings with a case-insensitive Eq implementation, so this works:

    use std::collections::HashSet;
    use unicase::UniCase;

    fn main () {
        let strings = vec![
            "apple", "cAt", "cat", "Dog", "apple", "dog", "Cat",
            "Apple", "dOg", "banana", "cat", "dog", "apple",
        ];
        
        let mut set = HashSet::new();
        
        let strings: Vec<_> = strings
            .iter()
            .filter(|&string| set.insert(UniCase::new(string)))
            .collect();
        
        println!("{:?}", strings);
    }
https://play.rust-lang.org/?version=stable&mode=debug&editio...
This is pretty cool. People might think it's cheating to pull in a crate. But IMO the fact that its super easy to pull in a crate in Rust is one of the best things about the language.
You don't even need the filter, all you need is:

    let dedup = strings.into_iter().map(|s| s.to_lowercase()).collect::<HashSet<_>>().into_iter().collect::<Vec<_>>();
Unfortunately that doesn't preserve case nor order of the input strings.