auto merge of #16634 : apoelstra/rust/to-option-fix, r=aturon

As outlined in

  https://aturon.github.io/style/naming/conversions.html

`to_` functions names should only be used for expensive operations.
Thus `to_option` is better named `as_option`. Also, putting type
names into method names is considered bad style; what the user is
really trying to get is a reference. This `as_ref` is even better.

Also, we are missing a mutable version of this method.

Finally, there is a bug in the signature of `to_option` which has
been around since lifetime elision: originally the returned reference
had 'static lifetime, but since the elision changes this become
the lifetime of the raw pointer (which does not make sense, since
the pointer lifetime and referent lifetime are unrelated). We fix
the bug to return a reference with a fresh lifetime which will be
inferred from the calling context.

[breaking-change]

src/libcollections/dlist.rs

@@ -90,7 +90,7 @@ impl<T> Rawlink<T> {
     /// Convert the `Rawlink` into an Option value
     fn resolve_immut<'a>(&self) -> Option<&'a T> {
         unsafe {
-            mem::transmute(self.p.to_option())
+            self.p.as_ref()
         }
     }
 

src/libcore/ptr.rs

@@ -256,27 +256,46 @@ pub unsafe fn position<T>(buf: *const T, f: |&T| -> bool) -> uint {
 pub trait RawPtr<T> {
     /// Returns the null pointer.
     fn null() -> Self;
+
     /// Returns true if the pointer is equal to the null pointer.
     fn is_null(&self) -> bool;
+
     /// Returns true if the pointer is not equal to the null pointer.
     fn is_not_null(&self) -> bool { !self.is_null() }
+
     /// Returns the value of this pointer (ie, the address it points to)
     fn to_uint(&self) -> uint;
-    /// Returns `None` if the pointer is null, or else returns the value wrapped
-    /// in `Some`.
+
+    /// Returns `None` if the pointer is null, or else returns a reference to the
+    /// value wrapped in `Some`.
     ///
     /// # Safety Notes
     ///
-    /// While this method is useful for null-safety, it is important to note
-    /// that this is still an unsafe operation because the returned value could
-    /// be pointing to invalid memory.
-    unsafe fn to_option(&self) -> Option<&T>;
+    /// While this method and its mutable counterpart are useful for null-safety,
+    /// it is important to note that this is still an unsafe operation because
+    /// the returned value could be pointing to invalid memory.
+    unsafe fn as_ref<'a>(&self) -> Option<&'a T>;
+
+    /// A synonym for `as_ref`, except with incorrect lifetime semantics
+    #[deprecated="Use `as_ref` instead"]
+    unsafe fn to_option<'a>(&'a self) -> Option<&'a T> {
+        mem::transmute(self.as_ref())
+    }
+
     /// Calculates the offset from a pointer. The offset *must* be in-bounds of
     /// the object, or one-byte-past-the-end.  `count` is in units of T; e.g. a
     /// `count` of 3 represents a pointer offset of `3 * sizeof::<T>()` bytes.
     unsafe fn offset(self, count: int) -> Self;
 }
 
+/// Methods on mutable raw pointers
+pub trait RawMutPtr<T>{
+    /// Returns `None` if the pointer is null, or else returns a mutable reference
+    /// to the value wrapped in `Some`. As with `as_ref`, this is unsafe because
+    /// it cannot verify the validity of the returned pointer.
+    unsafe fn as_mut<'a>(&self) -> Option<&'a mut T>;
+}
+
 impl<T> RawPtr<T> for *const T {
     #[inline]
     fn null() -> *const T { null() }
@@ -293,7 +312,7 @@ impl<T> RawPtr<T> for *const T {
     }
 
     #[inline]
-    unsafe fn to_option(&self) -> Option<&T> {
+    unsafe fn as_ref<'a>(&self) -> Option<&'a T> {
         if self.is_null() {
             None
         } else {
@@ -318,7 +337,7 @@ impl<T> RawPtr<T> for *mut T {
     }
 
     #[inline]
-    unsafe fn to_option(&self) -> Option<&T> {
+    unsafe fn as_ref<'a>(&self) -> Option<&'a T> {
         if self.is_null() {
             None
         } else {
@@ -327,6 +346,17 @@ impl<T> RawPtr<T> for *mut T {
     }
 }
 
+impl<T> RawMutPtr<T> for *mut T {
+    #[inline]
+    unsafe fn as_mut<'a>(&self) -> Option<&'a mut T> {
+        if self.is_null() {
+            None
+        } else {
+            Some(&mut **self)
+        }
+    }
+}
+
 // Equality for pointers
 impl<T> PartialEq for *const T {
     #[inline]

src/libcoretest/ptr.rs

@@ -102,19 +102,44 @@ fn test_is_null() {
 }
 
 #[test]
-fn test_to_option() {
+fn test_as_ref() {
     unsafe {
         let p: *const int = null();
-        assert_eq!(p.to_option(), None);
+        assert_eq!(p.as_ref(), None);
 
         let q: *const int = &2;
-        assert_eq!(q.to_option().unwrap(), &2);
+        assert_eq!(q.as_ref().unwrap(), &2);
 
         let p: *mut int = mut_null();
-        assert_eq!(p.to_option(), None);
+        assert_eq!(p.as_ref(), None);
 
         let q: *mut int = &mut 2;
-        assert_eq!(q.to_option().unwrap(), &2);
+        assert_eq!(q.as_ref().unwrap(), &2);
+
+        // Lifetime inference
+        let u = 2i;
+        {
+            let p: *const int = &u as *const _;
+            assert_eq!(p.as_ref().unwrap(), &2);
+        }
+    }
+}
+
+#[test]
+fn test_as_mut() {
+    unsafe {
+        let p: *mut int = mut_null();
+        assert!(p.as_mut() == None);
+
+        let q: *mut int = &mut 2;
+        assert!(q.as_mut().unwrap() == &mut 2);
+
+        // Lifetime inference
+        let mut u = 2i;
+        {
+            let p: *mut int = &mut u as *mut _;
+            assert!(p.as_mut().unwrap() == &mut 2);
+        }
     }
 }
 

src/liblog/lib.rs

@@ -282,7 +282,7 @@ impl Drop for DefaultLogger {
 pub fn log(level: u32, loc: &'static LogLocation, args: &fmt::Arguments) {
     // Test the literal string from args against the current filter, if there
     // is one.
-    match unsafe { FILTER.to_option() } {
+    match unsafe { FILTER.as_ref() } {
         Some(filter) if filter.is_match(args.to_string().as_slice()) => return,
         _ => {}
     }