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]

4 files changed, 70 insertions, 15 deletions

diff --git a/src/libcollections/dlist.rs b/src/libcollections/dlist.rs
index 643da970364..0ed946aa947 100644
--- a/src/libcollections/dlist.rs
+++ b/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()
         }
     }
 
diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs
index eb9bede85d8..d1bea25dded 100644
--- a/src/libcore/ptr.rs
+++ b/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]
diff --git a/src/libcoretest/ptr.rs b/src/libcoretest/ptr.rs
index 9058ae56c45..754391a284d 100644
--- a/src/libcoretest/ptr.rs
+++ b/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);
+        }
     }
 }
 
diff --git a/src/liblog/lib.rs b/src/liblog/lib.rs
index 32222fb16ce..6461a6cf402 100644
--- a/src/liblog/lib.rs
+++ b/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,
         _ => {}
     }