From 00ff5aac4ef48615321610b73f30da825700fb78 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 20 Aug 2014 13:06:34 -0700 Subject: [PATCH] Rename `RawPtr::to_option()` to `RawPtr::as_ref()` 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. So add a new trait `RawMutPtr` with a corresponding `as_mut` methode. 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). 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 | 2 +- src/libcore/ptr.rs | 46 ++++++++++++++++++++++++++++++------- src/libcoretest/ptr.rs | 35 ++++++++++++++++++++++++---- src/liblog/lib.rs | 2 +- 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 Rawlink { /// 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(buf: *const T, f: |&T| -> bool) -> uint { pub trait RawPtr { /// 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::()` bytes. unsafe fn offset(self, count: int) -> Self; } +/// Methods on mutable raw pointers +pub trait RawMutPtr{ + /// 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 RawPtr for *const T { #[inline] fn null() -> *const T { null() } @@ -293,7 +312,7 @@ impl RawPtr 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 RawPtr 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 RawPtr for *mut T { } } +impl RawMutPtr 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 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, _ => {} }