Skip to content

Commit

Permalink
More conservative impl of Replacer for closures
Browse files Browse the repository at this point in the history
This is an alternative flavor of PR rust-lang#1048:

Require `Replacer` closures to take a `&'a Captures<'b>` (for all `'a,
'b: 'a`) as argument and have a return type that must not depend on `'b`
(but only on `'a`).
  • Loading branch information
JanBeh committed Jul 21, 2023
1 parent 664a0f2 commit 6c8b549
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 20 deletions.
14 changes: 10 additions & 4 deletions src/regex/string.rs
Expand Up @@ -2376,16 +2376,21 @@ mod replacer_closure {
use super::*;
/// If a closure implements this for all `'a`, then it also implements
/// [`Replacer`].
// TODO: Use two lifetimes `'a` and `'b`: one for the reference and one for
// the lifetime argument `'b` of `Captures<'b>`. This requires a syntax
// like `for<'a, 'b: 'a> ReplacerClosure<'a, 'b> when using this trait,
// which currently doesn't exist.
// See also: https://github.com/rust-lang/rfcs/pull/3261
pub trait ReplacerClosure<'a>
where
Self: FnMut(&'a Captures<'a>) -> <Self as ReplacerClosure<'a>>::Output,
Self: FnMut(&'a Captures<'_>) -> <Self as ReplacerClosure<'a>>::Output,
{
/// Return type of the closure (may depend on lifetime `'a`).
type Output: AsRef<str>;
}
impl<'a, F: ?Sized, O> ReplacerClosure<'a> for F
where
F: FnMut(&'a Captures<'a>) -> O,
F: FnMut(&'a Captures<'_>) -> O,
O: AsRef<str>,
{
type Output = O;
Expand Down Expand Up @@ -2430,7 +2435,8 @@ use replacer_closure::*;
///
/// Closures that take an argument of type `&'a Captures<'b>` for any `'a` and
/// `'b: 'a` and which return a type `T: AsRef<str>` (that may depend on `'a`
/// or `'b`) implement the `Replacer` trait through a [blanket implementation].
/// but not on `'b`) implement the `Replacer` trait through a [blanket
/// implementation].
///
/// [blanket implementation]: Self#impl-Replacer-for-F
///
Expand Down Expand Up @@ -2580,7 +2586,7 @@ impl<'a> Replacer for &'a Cow<'a, str> {
/// ```ignore
/// impl<F, T> Replacer for F
/// where
/// F: for<'a> FnMut(&'a Captures<'a>) -> T,
/// F: for<'a, 'b> FnMut(&'a Captures<'b>) -> T,
/// T: AsRef<str>, // `T` may also depend on `'a`, which cannot be expressed easily
/// {
/// /* … */
Expand Down
23 changes: 7 additions & 16 deletions tests/misc.rs
Expand Up @@ -159,6 +159,12 @@ mod replacer_closure_lifetimes {
.replace_all("x", coerce(|caps| Cow::Borrowed(&caps[0])));
assert_eq!(s, "x");
}
// The following test is commented out yet because currently `Replacer` is
// not implemented for closures whose return type depends on the lifetime
// parameter `'b` of the `Captures<'b>` type.
// TODO: Fix this when/if the hidden `ReplacerClosure` helper trait gets
// two lifetime parameters.
/*
#[test]
fn parameter_lifetime() {
fn coerce<F: for<'b> FnMut(&Captures<'b>) -> Cow<'b, str>>(f: F) -> F {
Expand All @@ -170,20 +176,5 @@ mod replacer_closure_lifetimes {
);
assert_eq!(s, "x");
}
// Additionally demand that its sufficient if the closure accepts a single
// lifetime `'u` which is used both for the reference to and the lifetime
// argument of the `Captures` argument. Note that `Captures<'u>` is
// covariant over `'u`.
#[test]
fn unified_lifetime() {
fn coerce<F: for<'u> FnMut(&'u Captures<'u>) -> Cow<'u, str>>(
f: F,
) -> F {
f
}
let s = Regex::new("x")
.unwrap()
.replace_all("x", coerce(|caps| Cow::Borrowed(&caps[0])));
assert_eq!(s, "x");
}
*/
}

0 comments on commit 6c8b549

Please sign in to comment.