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 #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.