Auto merge of #33 - servo:…, r=nox

Port NonZero optimization to stable Rust

… and various drive-by fixes.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/tendril/33)
<!-- Reviewable:end -->

.travis.yml

@@ -7,7 +7,7 @@ script:
   - cargo build
   - cargo doc
   - cargo test
-  - "if [ $TRAVIS_RUST_VERSION = nightly ]; then cargo test --features unstable; fi"
+  - "if [ $TRAVIS_RUST_VERSION = nightly ]; then cargo test --features bench; fi"
   - "if [ $TRAVIS_RUST_VERSION = nightly ]; then (cd capi/ctest; ./build-and-test.sh); fi"
 notifications:
   webhooks: http://build.servo.org:54856/travis

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tendril"
-version = "0.3.0"
+version = "0.4.0"
 authors = ["Keegan McAllister <[email protected]>",
            "Simon Sapin <[email protected]>",
            "Chris Morgan <[email protected]>"]
@@ -19,4 +19,4 @@ utf-8 = "0.7"
 rand = "0"
 
 [features]
-unstable = []
+bench = []

capi/Cargo.toml

@@ -16,7 +16,6 @@ libc = "0.1"
 
 [dependencies.tendril]
 path = "../"
-features = ["unstable"]  # Drop flags and C API don’t play friends
 
 [build-dependencies]
 gcc = "0"

capi/include/tendril.h

@@ -102,7 +102,7 @@ void tendril_debug_dump(const tendril *t, FILE *stream);
 //// implementation details follow
 ////
 
-__attribute__((packed)) struct tendril_impl {
+struct tendril_impl {
     uintptr_t __ptr;
     uint32_t __a;
     uint32_t __b;

examples/fuzz.rs

@@ -105,7 +105,7 @@ fn fuzz() {
 fn random_boundary<R: Rng>(rng: &mut R, text: &str) -> usize {
     loop {
         let i = Range::new(0, text.len()+1).ind_sample(rng);
-        if is_char_boundary(text, i) {
+        if text.is_char_boundary(i) {
             return i;
         }
     }
@@ -115,25 +115,16 @@ fn random_slice<R: Rng>(rng: &mut R, text: &str) -> (usize, usize) {
     loop {
         let start = Range::new(0, text.len()+1).ind_sample(rng);
         let end = Range::new(start, text.len()+1).ind_sample(rng);
-        if !is_char_boundary(text, start) {
+        if !text.is_char_boundary(start) {
             continue;
         }
-        if end < text.len() && !is_char_boundary(text, end) {
+        if end < text.len() && !text.is_char_boundary(end) {
             continue;
         }
         return (start, end);
     }
 }
 
-// Copy of the str::is_char_boundary method, which is unstable.
-fn is_char_boundary(s: &str, index: usize) -> bool {
-    if index == s.len() { return true; }
-    match s.as_bytes().get(index) {
-        None => false,
-        Some(&b) => b < 128 || b >= 192,
-    }
-}
-
 static TEXT: &'static str =
     "It was from the artists and poets that the pertinent answers came, and I \
      know that panic would have broken loose had they been able to compare notes. \

src/buf32.rs

@@ -16,7 +16,6 @@ pub const MAX_LEN: usize = u32::MAX as usize;
 
 /// A buffer points to a header of type `H`, which is followed by `MIN_CAP` or more
 /// bytes of storage.
-#[repr(packed)]
 pub struct Buf32<H> {
     pub ptr: *mut H,
     pub len: u32,

src/lib.rs

@@ -4,19 +4,15 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![cfg_attr(feature = "unstable", feature(nonzero))]
-#![cfg_attr(all(test, feature = "unstable"), feature(test))]
+#![cfg_attr(all(test, feature = "bench"), feature(test))]
 #![cfg_attr(test, deny(warnings))]
 
-#[cfg(feature = "unstable")] extern crate core;
+#[cfg(all(test, feature = "bench"))] extern crate test;
 #[cfg(feature = "encoding")] pub extern crate encoding;
 #[macro_use] extern crate mac;
 extern crate futf;
 extern crate utf8;
 
-#[cfg(all(test, feature = "unstable"))]
-extern crate test;
-
 pub use tendril::{Tendril, ByteTendril, StrTendril, SliceExt, ReadExt, SubtendrilError};
 pub use tendril::{SendTendril, Atomicity, Atomic, NonAtomic};
 pub use fmt::Format;

src/tendril.rs

@@ -24,18 +24,18 @@ use buf32::{self, Buf32};
 use fmt::{self, Slice};
 use fmt::imp::Fixup;
 use util::{unsafe_slice, unsafe_slice_mut, copy_and_advance, copy_lifetime_mut, copy_lifetime,
-           NonZero};
+           NonZeroUsize};
 use OFLOW;
 
 const MAX_INLINE_LEN: usize = 8;
 const MAX_INLINE_TAG: usize = 0xF;
 const EMPTY_TAG: usize = 0xF;
 
 #[inline(always)]
-fn inline_tag(len: u32) -> NonZero<usize> {
+fn inline_tag(len: u32) -> NonZeroUsize {
     debug_assert!(len <= MAX_INLINE_LEN as u32);
     unsafe {
-        NonZero::new(if len == 0 {
+        NonZeroUsize::new(if len == 0 {
             EMPTY_TAG
         } else {
             len as usize
@@ -47,7 +47,7 @@ fn inline_tag(len: u32) -> NonZero<usize> {
 ///
 /// Exactly two types implement this trait:
 ///
-/// - `Atomic`: use this in your tendril and you will have a `Send + Sync` tendril which works
+/// - `Atomic`: use this in your tendril and you will have a `Send` tendril which works
 ///   across threads; this is akin to `Arc`.
 ///
 /// - `NonAtomic`: use this in your tendril and you will have a tendril which is neither
@@ -104,8 +104,8 @@ unsafe impl Atomicity for NonAtomic {
 
 /// A marker of an atomic (and hence concurrent) tendril.
 ///
-/// This is used as the second, optional type parameter of a `Tendril`; `Tendril<F, Atomic>` thus
-/// implements both `Send` and `Sync`.
+/// This is used as the second, optional type parameter of a `Tendril`;
+/// `Tendril<F, Atomic>` thus implements`Send`.
 ///
 /// This is akin to using `Arc` for reference counting.
 pub struct Atomic(AtomicUsize);
@@ -182,24 +182,23 @@ pub enum SubtendrilError {
 ///
 /// The type parameter `A` indicates the atomicity of the tendril; it is by
 /// default `NonAtomic`, but can be specified as `Atomic` to get a tendril
-/// which implements `Send` and `Sync` (viz. a thread-safe tendril).
+/// which implements `Send` (viz. a thread-safe tendril).
 ///
 /// The maximum length of a `Tendril` is 4 GB. The library will panic if
 /// you attempt to go over the limit.
-#[repr(packed)]
+#[repr(C)]
 pub struct Tendril<F, A = NonAtomic>
     where F: fmt::Format,
           A: Atomicity,
 {
-    ptr: Cell<NonZero<usize>>,
+    ptr: Cell<NonZeroUsize>,
     len: u32,
     aux: Cell<u32>,
     marker: PhantomData<*mut F>,
     refcount_marker: PhantomData<A>,
 }
 
 unsafe impl<F, A> Send for Tendril<F, A> where F: fmt::Format, A: Atomicity + Sync { }
-unsafe impl<F, A> Sync for Tendril<F, A> where F: fmt::Format, A: Atomicity + Sync { }
 
 /// `Tendril` for storing native Rust strings.
 pub type StrTendril = Tendril<fmt::UTF8>;
@@ -586,7 +585,7 @@ impl<F, A> Tendril<F, A>
     #[inline]
     pub fn clear(&mut self) {
         if self.ptr.get().get() <= MAX_INLINE_TAG {
-            self.ptr.set(unsafe { NonZero::new(EMPTY_TAG) });
+            self.ptr.set(unsafe { NonZeroUsize::new(EMPTY_TAG) });
         } else {
             let (_, shared, _) = unsafe { self.assume_buf() };
             if shared {
@@ -966,7 +965,7 @@ impl<F, A> Tendril<F, A>
             let header = p as *mut Header<A>;
             (*header).cap = self.aux.get();
 
-            self.ptr.set(NonZero::new(p | 1));
+            self.ptr.set(NonZeroUsize::new(p | 1));
             self.aux.set(0);
         }
     }
@@ -989,7 +988,7 @@ impl<F, A> Tendril<F, A>
         self.make_owned();
         let mut buf = self.assume_buf().0;
         buf.grow(cap);
-        self.ptr.set(NonZero::new(buf.ptr as usize));
+        self.ptr.set(NonZeroUsize::new(buf.ptr as usize));
         self.aux.set(buf.cap);
     }
 
@@ -1032,7 +1031,7 @@ impl<F, A> Tendril<F, A>
     #[inline]
     unsafe fn owned(x: Buf32<Header<A>>) -> Tendril<F, A> {
         Tendril {
-            ptr: Cell::new(NonZero::new(x.ptr as usize)),
+            ptr: Cell::new(NonZeroUsize::new(x.ptr as usize)),
             len: x.len,
             aux: Cell::new(x.cap),
             marker: PhantomData,
@@ -1052,7 +1051,7 @@ impl<F, A> Tendril<F, A>
     #[inline]
     unsafe fn shared(buf: Buf32<Header<A>>, off: u32, len: u32) -> Tendril<F, A> {
         Tendril {
-            ptr: Cell::new(NonZero::new((buf.ptr as usize) | 1)),
+            ptr: Cell::new(NonZeroUsize::new((buf.ptr as usize) | 1)),
             len: len,
             aux: Cell::new(off),
             marker: PhantomData,
@@ -1548,7 +1547,7 @@ impl<'a, A> From<&'a Tendril<fmt::UTF8, A>> for String
 }
 
 
-#[cfg(all(test, feature = "unstable"))]
+#[cfg(all(test, feature = "bench"))]
 #[path="bench.rs"]
 mod bench;
 
@@ -1561,7 +1560,6 @@ mod test {
     use std::thread;
 
     fn assert_send<T: Send>() { }
-    fn assert_sync<T: Sync>() { }
 
     #[test]
     fn smoke_test() {
@@ -1589,9 +1587,6 @@ mod test {
         assert_eq!(correct, mem::size_of::<ByteTendril>());
         assert_eq!(correct, mem::size_of::<StrTendril>());
 
-        // Check that the NonZero<T> optimization is working, if on unstable Rust.
-        let option_tag = if cfg!(feature = "unstable") { 0 } else { 1 };
-        let correct = correct + option_tag;
         assert_eq!(correct, mem::size_of::<Option<ByteTendril>>());
         assert_eq!(correct, mem::size_of::<Option<StrTendril>>());
 
@@ -2211,7 +2206,6 @@ mod test {
     #[test]
     fn atomic() {
         assert_send::<Tendril<fmt::UTF8, Atomic>>();
-        assert_sync::<Tendril<fmt::UTF8, Atomic>>();
         let s: Tendril<fmt::UTF8, Atomic> = Tendril::from_slice("this is a string");
         assert!(!s.is_shared());
         let mut t = s.clone();

src/util.rs

@@ -40,19 +40,18 @@ pub unsafe fn copy_lifetime<'a, S: ?Sized, T: ?Sized + 'a>
     mem::transmute(ptr)
 }
 
-#[cfg(feature = "unstable")] pub use core::nonzero::NonZero;
-
-#[cfg(not(feature = "unstable"))]
 #[derive(Copy, Clone)]
-pub struct NonZero<T>(T);
+pub struct NonZeroUsize(&'static u8);
 
-#[cfg(not(feature = "unstable"))]
-impl<T> NonZero<T> {
-    pub unsafe fn new(x: T) -> NonZero<T> { NonZero(x) }
-}
+impl NonZeroUsize {
+    #[inline]
+    pub unsafe fn new(value: usize) -> Self {
+        debug_assert!(value != 0);
+        NonZeroUsize(&*(value as *const u8))
+    }
 
-#[cfg(not(feature = "unstable"))]
-impl<T> NonZero<T> {
     #[inline]
-    pub fn get(self) -> T { self.0 }
+    pub fn get(self) -> usize {
+        self.0 as *const u8 as usize
+    }
 }