From 2574990a3f147de597380ccb135ab12bd18f9b3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rn=20Lode?= Date: Sun, 6 Sep 2015 03:20:53 +0200 Subject: [PATCH] Rustonomicon: Fix bug in implementation of Vec::drain() In the last code snippet on the following page there is a bug in the implementation of Vec::drain(). https://doc.rust-lang.org/nightly/nomicon/vec-drain.html ```rust pub fn drain(&mut self) -> Drain { // Oops, setting it to 0 while we still need the old value! self.len = 0; unsafe { Drain { // len is used to create a &[T] from &self here, // so we end up always creating an empty slice. iter: RawValIter::new(&self), vec: PhantomData, } } } ``` A simple test to verify that Drain is broken can be found here: https://play.rust-lang.org/?gist=30f579565e4bbf4836ce&version=nightly And here's one with a fixed implementation: https://play.rust-lang.org/?gist=2ec0c1a6dcf5defd7a53&version=nightly --- vec-drain.md | 14 ++++++++------ vec-final.md | 13 ++++++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/vec-drain.md b/vec-drain.md index 4521bbd..edad06c 100644 --- a/vec-drain.md +++ b/vec-drain.md @@ -129,14 +129,16 @@ impl<'a, T> Drop for Drain<'a, T> { impl Vec { pub fn drain(&mut self) -> Drain { - // this is a mem::forget safety thing. If Drain is forgotten, we just - // leak the whole Vec's contents. Also we need to do this eventually - // anyway, so why not do it now? - self.len = 0; - unsafe { + let iter = RawValIter::new(&self); + + // this is a mem::forget safety thing. If this is forgotten, we just + // leak the whole Vec's contents. Also we need to do this *eventually* + // anyway, so why not do it now? + self.len = 0; + Drain { - iter: RawValIter::new(&self), + iter: iter, vec: PhantomData, } } diff --git a/vec-final.md b/vec-final.md index 847957e..ace8f20 100644 --- a/vec-final.md +++ b/vec-final.md @@ -155,13 +155,16 @@ impl Vec { } pub fn drain(&mut self) -> Drain { - // this is a mem::forget safety thing. If this is forgotten, we just - // leak the whole Vec's contents. Also we need to do this *eventually* - // anyway, so why not do it now? - self.len = 0; unsafe { + let iter = RawValIter::new(&self); + + // this is a mem::forget safety thing. If this is forgotten, we just + // leak the whole Vec's contents. Also we need to do this *eventually* + // anyway, so why not do it now? + self.len = 0; + Drain { - iter: RawValIter::new(&self), + iter: iter, vec: PhantomData, } }