Skip to content

Commit 8d14986

Browse files
committed
mark specialization trait as unsafe and add safety comments
1 parent 68ce47f commit 8d14986

File tree

1 file changed

+28
-5
lines changed

1 file changed

+28
-5
lines changed

library/core/src/iter/adapters/chain.rs

+28-5
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,33 @@ fn and_then_or_clear<T, U>(opt: &mut Option<T>, f: impl FnOnce(&mut T) -> Option
292292
x
293293
}
294294

295+
/// Marks the two generic parameters of Chain as sufficiently equal that their values can be swapped
296+
///
297+
/// # Safety
298+
///
299+
/// This would be trivially safe if both types were identical, including lifetimes.
300+
/// However we can't specify bounds like that and it would be overly restrictive since it's not
301+
/// uncommon for borrowing iterators to have slightly different lifetimes.
302+
///
303+
/// We can relax this by only requiring that the base struct type is the same while ignoring
304+
/// lifetime parameters as long as
305+
/// * the actual runtime lifespan of the values is capped by the shorter of the two lifetimes
306+
/// * all invoked trait methods (and drop code) monomorphize down to the same code
295307
#[rustc_unsafe_specialization_marker]
296-
trait SymmetricalArms {}
297-
298-
impl<A> SymmetricalArms for Chain<A, A> {}
308+
unsafe trait SymmetricalModuloLifetimes {}
309+
310+
/// Safety:
311+
/// * <A, A> ensures that the basic type is the same
312+
/// * actual lifespan of the values is capped by the combined lifetime of Chain's fields as long as
313+
/// there is no way to destructure Chain into. I.e. Chain must not implement `SourceIter`,
314+
/// `into_parts(self)` or similar methods.
315+
/// * we rely on the language currently having no mechanism that would allow lifetime-dependent
316+
/// code paths. Specialization forbids `where T: 'static` and similar bounds (modulo the exposed
317+
/// `#[rustc_unsafe_specialization_marker]` traits).
318+
/// And any trait depending on `Any` would have to be 'static in *both* arms to make a useful Chain.
319+
/// This is only true as long as *all* impls on `Chain` have the same bounds for A and B,
320+
/// which currently is the case.
321+
unsafe impl<A> SymmetricalModuloLifetimes for Chain<A, A> {}
299322

300323
trait SpecChain: Iterator {
301324
fn next(&mut self) -> Option<Self::Item>;
@@ -316,13 +339,13 @@ impl<A, B> SpecChain for Chain<A, B>
316339
where
317340
A: Iterator,
318341
B: Iterator<Item = A::Item>,
319-
Self: SymmetricalArms,
342+
Self: SymmetricalModuloLifetimes,
320343
{
321344
#[inline]
322345
fn next(&mut self) -> Option<A::Item> {
323346
let mut result = and_then_or_clear(&mut self.a, Iterator::next);
324347
if result.is_none() {
325-
// SAFETY: SymmetricalArms guarantees that A and B are the same type.
348+
// SAFETY: SymmetricalModuloLifetimes guarantees that A and B are safe to swap
326349
unsafe { mem::swap(&mut self.a, &mut *(&mut self.b as *mut _ as *mut Option<A>)) };
327350
result = and_then_or_clear(&mut self.a, Iterator::next);
328351
}

0 commit comments

Comments
 (0)