#Convert OsString to &str inside a closure ?

15 messages · Page 1 of 1 (latest)

crystal kraken
#

Hello, I'm currently doing a personal project inspired by Kondo, to clean projects dependencies & artifacts.
The goal is to improve my knowledge of the Rust language.
I was wondering about a portion of the code in Kondo's codebase : https://github.com/tbillington/kondo/blob/master/kondo-lib/src/lib.rs#L326

After building an OsString iterator, they try to turn it into a Option<&str>, and if it gives None they just skip this iteration. So I was wondering, could it be possible to turn the iteration variable directly to a &str, directly into the functions chain used in the for loop ? That way we wouldn't have to create file_name from the OsString.

My first attempt to do that failed though, as the compiler won't allow me to use filter_map and to_str() directly in a closure. The message, more precisely, is cannot return reference to temporary value returns a reference to data owned by the current function.

Attempt :

for file_name in rd
    .filter_map(|rd| rd.ok())
    .filter(|de| de.file_type().map(|ft| ft.is_file()).unwrap_or(false))
    .filter_map(|de| de.file_name().to_str())
{
  //...
}

Then, I started to try weird things with to_owned or String::from, but didn't succeed. I think I misunderstand something about the Rust compiler.
I hope this is clear, and can give more info if needed. Thank you !

vital dagger
#

to_owned implies an allocation, which is just unescessary, especially in a loop

#

and with the borrow mecanics, given the OsString is generated by the iterator, you cannot simply map it to something that borrows it, as you'd get rid of the original by not keeping it around

#

the only way would be to try to convert it into an owned String instead (which should not do a rellocation)

#

using OsString::into_string would be more approriate in this case

#

filter_map(|os| os.into_string().ok()) simply (shame that we dont have function currying)

#

this would yield an owned string you could just use normally, and it'd be properly dropped when needed

crystal kraken
#

Thank you for your answer, I think I understand here it makes more sense to turn it into an owned String, so to use into_string(). By the way, why/how should it not do a reallocation ?

#

Also, I'm wondering if there is a reason why they wrote it differently in Kondo's source code ? Is it more idiomatic or is it simply a different style, with no particular performance/optimization concerns ?

#

In the end, I decided to refactor it. I also don't know if it fits with how usual Rust code looks...

fn get_filenames(dir: &DirEntry) -> Option<impl Iterator<Item = String>> {
    let rd = match dir.path().read_dir() {
        Err(_e) => return None,
        Ok(rd) => rd,
    };

    Some(
        rd.filter_map(|rd| rd.ok())
            .filter(|de| de.file_type().map(|ft| ft.is_file()).unwrap_or(false))
            .filter_map(|de| de.file_name().into_string().ok()),
    )
}

But I kind of prefer being able to write that :

for file_name in get_filenames(&dir)? {
    //...
}
vital dagger
#

That does look better

vital dagger
crystal kraken
#

I'll pay attention to that when using loops then. So if I understand correctly, the into_string() I'm using on the OsString type just converts the OsString without reallocating, right ? So it's better than doing something like String::from or to_owned

vital dagger
#

String::from is not applicable here, so is to_owned not even on String

#

its if you go through a string slice instead, that you might reallocate as in this cass you needed an owned value in the iterator