From 6c8b5497f63c25c3bec8107f734813ba26d2f237 Mon Sep 17 00:00:00 2001 From: Jan Behrens Date: Fri, 21 Jul 2023 13:58:51 +0200 Subject: [PATCH] More conservative impl of Replacer for closures 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`). --- src/regex/string.rs | 14 ++++++++++---- tests/misc.rs | 23 +++++++---------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/regex/string.rs b/src/regex/string.rs index d94f06c54..879e2a136 100644 --- a/src/regex/string.rs +++ b/src/regex/string.rs @@ -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>) -> >::Output, + Self: FnMut(&'a Captures<'_>) -> >::Output, { /// Return type of the closure (may depend on lifetime `'a`). type Output: AsRef; } impl<'a, F: ?Sized, O> ReplacerClosure<'a> for F where - F: FnMut(&'a Captures<'a>) -> O, + F: FnMut(&'a Captures<'_>) -> O, O: AsRef, { type Output = O; @@ -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` (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 /// @@ -2580,7 +2586,7 @@ impl<'a> Replacer for &'a Cow<'a, str> { /// ```ignore /// impl Replacer for F /// where -/// F: for<'a> FnMut(&'a Captures<'a>) -> T, +/// F: for<'a, 'b> FnMut(&'a Captures<'b>) -> T, /// T: AsRef, // `T` may also depend on `'a`, which cannot be expressed easily /// { /// /* … */ diff --git a/tests/misc.rs b/tests/misc.rs index d4ae31feb..c6900863f 100644 --- a/tests/misc.rs +++ b/tests/misc.rs @@ -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 FnMut(&Captures<'b>) -> Cow<'b, str>>(f: F) -> F { @@ -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 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"); - } + */ }