New borrow checker warning for regex.replace_all captures

help
1 
    let r = Regex::new("aaa")?;
    r.replace_all("aaa", |c: &Captures<'_> | {
        c.get(0).unwrap().as_str()
    });

playground

Warns about UB:

3|     r.replace_all("aaa", |c: &regex::Captures<'_> | {
 |                                               --  - return type of closure is &'2 str
 |                                               |
 |                                               let's call this `'1`
4|         c.get(0).unwrap().as_str()
 |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
 |

= warning: this error has been downgraded to a warning for backwards compatibility with previous releases
= warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future

However, I think it should work:

  • replace_all takes &'t str
  • get() returns Match<'t>
  • as_str() returns &'t str

Is it a problem with my code? Is it a problem with the regex crate? Is it a problem with NLL?

1 Like
2

I have had similar issues, when I changed the closure to a function these messages disappeared. I think that this has to do with closures not inferring the most general lifetimes for its arguments

3

That code looks correct to me. I'd also guess that code like that isn't altogether uncommon. I might file a bug and see if someone could do a crater run to see if those warnings are appearing in other folks' code.

4

If I understand correctly, replace_all expects this signature:

fn<'a, 'b, 'c>(_: &'a regex::Captures<'b>) -> &'c str

i.e. it expects the returned string to be unrelated to the argument. A potential PR would have to relax the bounds on this blanket impl.

5

Yeah, it looks like it. I'm unable to figure out the right way to make a blanket impl with the explicit lifetime. It seems that replace_append's arg has an implicit lifetime that can't be named for the closure.

This isn't compatible with the trait:

impl<'t, F> Replacer for F where F: FnMut(&Captures<'t>) -> &'t str {
    fn replace_append(&mut self, caps: &Captures<'t>, dst: &mut String) {
        dst.push_str((*self)(caps).as_ref());
    }
}

 = note: expected type `fn(&mut F, &re_unicode::Captures<'_>, &mut std::string::String)`
            found type `fn(&mut F, &re_unicode::Captures<'t>, &mut std::string::String)`

So it seems this is what actually happens:


impl<'a, F> Replacer for F where F: for<'t> FnMut(&Captures<'t>) -> &'a str {
    fn replace_append<'t>(&mut self, caps: &Captures<'t>, dst: &mut String) {
        dst.push_str((*self)(caps).as_ref());
    }
}

(I'm using &str instead of T: AsRef<str>, as the extra layer of indirection made things even harder).

I think for<'t: 'a> would be needed to express the relationship, but:

lifetime bounds cannot be used in this context

impl<'a, F> Replacer for F where F: for<'t: 'a> FnMut(&Captures<'t>) -> &'a str {
6

I suspect it's not a borrow checker bug, but regex might need to change the code, so I've filed a bug about this:

https://github.com/rust-lang/regex/issues/601

1 Like
7

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.