From dcb22efd27ba660e2e2b31380fdbc7bfe599ad7e Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Mon, 24 Aug 2020 16:23:38 -0400 Subject: [PATCH 1/5] fix potential issue with pointer provenance --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index e8560de..1f5046e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -261,7 +261,7 @@ impl<'a, T> RequestBuf<'a, T> { /// Get the untyped `Request` reference for this `RequestBuf`. fn request(self: Pin<&mut Self>) -> Pin<&mut Request<'a>> { // safety: projecting Pin onto our `request` field. - unsafe { self.map_unchecked_mut(|this| &mut this.request) } + unsafe { self.map_unchecked_mut(|this| &mut *(this as *mut Self as *mut Request<'a>)) } } /// Take a value previously provided to this `RequestBuf`. From 4ad0a589d2cf5c94256f2d133318f897c545a5eb Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Mon, 24 Aug 2020 19:31:04 -0400 Subject: [PATCH 2/5] First pass at a dyn Trait based solution --- src/lib.rs | 284 ++++++++++++++++++++++++++--------------------------- 1 file changed, 140 insertions(+), 144 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1f5046e..aa715ee 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,7 +14,7 @@ //! # path: PathBuf, //! # } //! # impl ObjectProvider for MyProvider { -//! # fn provide<'a>(&'a self, request: Pin<&mut Request<'a>>) { +//! # fn provide<'a>(&'a self, request: &mut dyn Request<'a>) { //! # request //! # .provide_ref::(&self.path) //! # .provide_ref::(&self.path) @@ -57,7 +57,7 @@ //! } //! //! impl ObjectProvider for MyProvider { -//! fn provide<'a>(&'a self, request: Pin<&mut Request<'a>>) { +//! fn provide<'a>(&'a self, request: &mut dyn Request<'a>) { //! request //! .provide_ref::(&self.path) //! .provide_ref::(&self.path) @@ -67,22 +67,71 @@ //! ``` use core::any::TypeId; -use core::fmt; -use core::marker::{PhantomData, PhantomPinned}; -use core::pin::Pin; +use core::ptr; -struct ReqRef(&'static T); -struct ReqVal(T); +mod private { + pub struct Private; + pub trait Sealed {} +} /// A dynamic request for an object based on its type. -#[repr(C)] -pub struct Request<'a> { - type_id: TypeId, - _pinned: PhantomPinned, - _marker: PhantomData<&'a ()>, +pub unsafe trait Request<'a>: private::Sealed { + /// /!\ THIS IS NOT PUBLIC API /!\ + /// + /// Provide a reference with the given type to the request. + /// + /// The returned pointer, if non-null, can be cast to point to an `Option<&'a T>`, + /// where `T` is the type `TypeId` was derived from. + /// + /// The lifetime of the returned pointer is the lifetime of `self`. + #[doc(hidden)] + fn provide_ref_internal(&mut self, _: private::Private, _type_id: TypeId) -> *mut () { + ptr::null_mut() + } + + /// /!\ THIS IS NOT PUBLIC API /!\ + /// + /// Provide a value with the given type to the request. + /// + /// The returned pointer, if non-null, will point to an `Option`, where + /// `T` is the type `TypeId` was derived from. + /// + /// The lifetime of the returned pointer is the lifetime of `self`. + #[doc(hidden)] + fn provide_value_internal(&mut self, _: private::Private, _type_id: TypeId) -> *mut () { + ptr::null_mut() + } +} + +impl<'a> dyn Request<'a> + '_ { + /// Type-safe wrapper for calling `provide_ref_internal`. + /// + /// See `Request::provide_ref_internal`'s documentation for the invariants + /// being held by this method. + fn provide_ref_place<'b, T: ?Sized + 'static>(&'b mut self) -> Option<&'b mut Option<&'a T>> { + let ptr = self.provide_ref_internal(private::Private, TypeId::of::()); + if ptr.is_null() { + None + } else { + Some(unsafe { &mut *(ptr as *mut Option<&'a T>) }) + } + } + + /// Type-safe wrapper for calling `provide_value_internal`. + /// + /// See `Request::provide_value_internal`'s documentation for the invariants + /// being held by this method. + fn provide_value_place<'b, T: 'static>(&'b mut self) -> Option<&'b mut Option> { + let ptr = self.provide_value_internal(private::Private, TypeId::of::()); + if ptr.is_null() { + None + } else { + Some(unsafe { &mut *(ptr as *mut Option) }) + } + } } -impl<'a> Request<'a> { +impl<'a> dyn Request<'a> + '_ { /// Provides a reference of type `&'a T` in response to this request. /// /// If a reference of type `&'a T` has already been provided for this @@ -91,7 +140,7 @@ impl<'a> Request<'a> { /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. - pub fn provide_ref(self: Pin<&mut Self>, value: &'a T) -> Pin<&mut Self> { + pub fn provide_ref(&mut self, value: &'a T) -> &mut Self { self.provide_ref_with(|| value) } @@ -106,18 +155,13 @@ impl<'a> Request<'a> { /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. - pub fn provide_ref_with( - mut self: Pin<&mut Self>, - cb: F, - ) -> Pin<&mut Self> + pub fn provide_ref_with(&mut self, cb: F) -> &mut Self where + T: ?Sized + 'static, F: FnOnce() -> &'a T, { - if self.is_ref::() { - // safety: `self.is_ref::()` ensured the data field is `&'a T`. - unsafe { - *self.as_mut().downcast_unchecked::<&'a T>() = Some(cb()); - } + if let Some(place) = self.provide_ref_place::() { + *place = Some(cb()); } self } @@ -129,7 +173,7 @@ impl<'a> Request<'a> { /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. - pub fn provide_value(self: Pin<&mut Self>, value: T) -> Pin<&mut Self> { + pub fn provide_value(&mut self, value: T) -> &mut Self { self.provide_value_with(|| value) } @@ -143,164 +187,116 @@ impl<'a> Request<'a> { /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. - pub fn provide_value_with(mut self: Pin<&mut Self>, cb: F) -> Pin<&mut Self> + pub fn provide_value_with(&mut self, cb: F) -> &mut Self where + T: 'static, F: FnOnce() -> T, { - if self.is_value::() { - // safety: `self.is_value::()` ensured the data field is `T`. - unsafe { - *self.as_mut().downcast_unchecked::() = Some(cb()); - } + if let Some(place) = self.provide_value_place::() { + *place = Some(cb()); } self } +} - /// Returns `true` if the requested type is `&'a T` - pub fn is_ref(&self) -> bool { - self.type_id == TypeId::of::>() - } - - /// Returns `true` if the requested type is `T` - pub fn is_value(&self) -> bool { - self.type_id == TypeId::of::>() - } +/// Trait to provide other objects based on a requested type at runtime. +/// +/// See also the [`ObjectProviderExt`] trait which provides the `request_ref` and +/// `request_value` methods. +pub trait ObjectProvider { + /// Provide an object in response to `request`. + fn provide<'a>(&'a self, request: &mut dyn Request<'a>); +} - // internal implementation detail - performs an unchecked downcast. - unsafe fn downcast_unchecked(self: Pin<&mut Self>) -> &mut Option { - let ptr = self.get_unchecked_mut() as *mut Self as *mut RequestBuf<'a, T>; - &mut (*ptr).value - } +/// Methods supported by all [`ObjectProvider`] implementors. +pub trait ObjectProviderExt { + /// Request a reference of type `&T` from an object provider. + fn request_ref(&self) -> Option<&T>; - /// Calls the provided closure with a request for the the type `&'a T`, - /// returning `Some(&T)` if the request was fulfilled, and `None` otherwise. - /// - /// The `ObjectProviderExt` trait provides helper methods specifically for - /// types implementing `ObjectProvider`. - pub fn request_ref(f: F) -> Option<&'a T> - where - F: FnOnce(Pin<&mut Request<'a>>), - { - let mut buf = RequestBuf::for_ref(); - // safety: We never move `buf` after creating `pinned`. - let mut pinned = unsafe { Pin::new_unchecked(&mut buf) }; - f(pinned.as_mut().request()); - pinned.take() - } + /// Request an owned value of type `T` from an object provider. + fn request_value(&self) -> Option; +} - /// Calls the provided closure with a request for the the type `T`, - /// returning `Some(T)` if the request was fulfilled, and `None` otherwise. - /// - /// The `ObjectProviderExt` trait provides helper methods specifically for - /// types implementing `ObjectProvider`. - pub fn request_value(f: F) -> Option - where - F: FnOnce(Pin<&mut Request<'a>>), - { - let mut buf = RequestBuf::for_value(); - // safety: We never move `buf` after creating `pinned`. - let mut pinned = unsafe { Pin::new_unchecked(&mut buf) }; - f(pinned.as_mut().request()); - pinned.take() +impl ObjectProviderExt for O { + fn request_ref(&self) -> Option<&T> { + let mut request = RequestRef::default(); + self.provide(&mut request); + request.value } -} -impl<'a> fmt::Debug for Request<'a> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Request") - .field("type_id", &self.type_id) - .finish() + fn request_value(&self) -> Option { + let mut request = RequestValue::default(); + self.provide(&mut request); + request.value } } -/// Low level buffer API used to create typed object requests. +/// A request for a reference of type `&'a T`. /// -/// Due to a heavy dependency on [`Pin`], this type is inconvenient to use -/// directly. Prefer using the [`ObjectProviderExt`] trait and [`Request::with`] -/// APIs when possible. -// Needs to have a known layout so we can do unsafe pointer shenanigans. -#[repr(C)] +/// This type implements `Request<'a>`, meaning it can be passed to types +/// expecting `&mut dyn Request<'a>`. #[derive(Debug)] -struct RequestBuf<'a, T> { - request: Request<'a>, - value: Option, +pub struct RequestRef<'a, T: ?Sized> { + /// The value provided to this request, or `None`. + pub value: Option<&'a T>, } -impl<'a, T: ?Sized + 'static> RequestBuf<'a, &'a T> { - /// Create a new `RequestBuf` object. - /// - /// This type must be pinned before it can be used. - fn for_ref() -> Self { - // safety: ReqRef is a marker type for `&'a T` - unsafe { Self::new_internal(TypeId::of::>()) } +impl<'a, T: ?Sized> Default for RequestRef<'a, T> { + fn default() -> Self { + RequestRef { value: None } } } -impl<'a, T: 'static> RequestBuf<'a, T> { - /// Create a new `RequestBuf` object. - /// - /// This type must be pinned before it can be used. - fn for_value() -> Self { - // safety: ReqVal is a marker type for `T` - unsafe { Self::new_internal(TypeId::of::>()) } - } -} +impl<'a, T: ?Sized + 'static> private::Sealed for RequestRef<'a, T> {} -impl<'a, T> RequestBuf<'a, T> { - unsafe fn new_internal(type_id: TypeId) -> Self { - RequestBuf { - request: Request { - type_id, - _pinned: PhantomPinned, - _marker: PhantomData, - }, - value: None, +unsafe impl<'a, T: ?Sized + 'static> Request<'a> for RequestRef<'a, T> { + /// See `Request::provide_ref_internal`'s documentation for the invariants + /// being upheld by this method. + #[doc(hidden)] + fn provide_ref_internal(&mut self, _: private::Private, type_id: TypeId) -> *mut () { + if type_id == TypeId::of::() { + &mut self.value as *mut Option<&'a T> as *mut () + } else { + ptr::null_mut() } } - - /// Get the untyped `Request` reference for this `RequestBuf`. - fn request(self: Pin<&mut Self>) -> Pin<&mut Request<'a>> { - // safety: projecting Pin onto our `request` field. - unsafe { self.map_unchecked_mut(|this| &mut *(this as *mut Self as *mut Request<'a>)) } - } - - /// Take a value previously provided to this `RequestBuf`. - fn take(self: Pin<&mut Self>) -> Option { - // safety: we don't project Pin onto our `value` field. - unsafe { self.get_unchecked_mut().value.take() } - } } -/// Trait to provide other objects based on a requested type at runtime. +/// A request for a value of type `T`. /// -/// See also the [`ObjectProviderExt`] trait which provides the `request` method. -pub trait ObjectProvider { - /// Provide an object of a given type in response to an untyped request. - fn provide<'a>(&'a self, request: Pin<&mut Request<'a>>); +/// This type implements [`Request`], meaning it can be passed to functions +/// expecting a `&mut dyn Request<'a>` trait object. +#[derive(Debug)] +pub struct RequestValue { + /// The value provided to this request, or `None`. + pub value: Option, } -/// Methods supported by all [`ObjectProvider`] implementors. -pub trait ObjectProviderExt { - /// Request a reference of type `&T` from an object provider. - fn request_ref(&self) -> Option<&T>; - - /// Request an owned value of type `T` from an object provider. - fn request_value(&self) -> Option; +impl Default for RequestValue { + fn default() -> Self { + RequestValue { value: None } + } } -impl ObjectProviderExt for O { - fn request_ref(&self) -> Option<&T> { - Request::request_ref::(|req| self.provide(req)) - } +impl private::Sealed for RequestValue {} - fn request_value(&self) -> Option { - Request::request_value::(|req| self.provide(req)) +unsafe impl<'a, T: 'static> Request<'a> for RequestValue { + /// See `Request::provide_value_internal`'s documentation for the invariants + /// being upheld by this method. + #[doc(hidden)] + fn provide_value_internal(&mut self, _: private::Private, type_id: TypeId) -> *mut () { + if type_id == TypeId::of::() { + &mut self.value as *mut Option as *mut () + } else { + ptr::null_mut() + } } } #[cfg(test)] mod test { use super::*; + use std::fmt; use std::path::{Path, PathBuf}; #[test] @@ -310,7 +306,7 @@ mod test { path: PathBuf, } impl ObjectProvider for HasContext { - fn provide<'a>(&'a self, request: Pin<&mut Request<'a>>) { + fn provide<'a>(&'a self, request: &mut dyn Request<'a>) { request .provide_ref::(&self.int) .provide_ref::(&self.path) From 5380303c37e1a2e718760189ca986ebe6b9fa892 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Mon, 24 Aug 2020 22:53:20 -0400 Subject: [PATCH 3/5] use private dst suffix for request response --- src/lib.rs | 234 +++++++++++++++++++++++++---------------------------- 1 file changed, 108 insertions(+), 126 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index aa715ee..bdf7710 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,7 +14,7 @@ //! # path: PathBuf, //! # } //! # impl ObjectProvider for MyProvider { -//! # fn provide<'a>(&'a self, request: &mut dyn Request<'a>) { +//! # fn provide<'a>(&'a self, request: &mut Request<'a>) { //! # request //! # .provide_ref::(&self.path) //! # .provide_ref::(&self.path) @@ -57,7 +57,7 @@ //! } //! //! impl ObjectProvider for MyProvider { -//! fn provide<'a>(&'a self, request: &mut dyn Request<'a>) { +//! fn provide<'a>(&'a self, request: &mut Request<'a>) { //! request //! .provide_ref::(&self.path) //! .provide_ref::(&self.path) @@ -67,71 +67,67 @@ //! ``` use core::any::TypeId; -use core::ptr; +use core::marker::PhantomData; mod private { - pub struct Private; - pub trait Sealed {} + pub trait Response {} + impl Response for Option {} } +/// When used by `Request`, `TypeId::of::>()` indicates that +/// `response` is of type `Option<&'a T>`. +struct RefMarker(T); + +/// When used by `Request`, `TypeId::of::>()` indicates that +/// `response` is of type `Option`. +struct ValueMarker(T); + /// A dynamic request for an object based on its type. -pub unsafe trait Request<'a>: private::Sealed { - /// /!\ THIS IS NOT PUBLIC API /!\ - /// - /// Provide a reference with the given type to the request. - /// - /// The returned pointer, if non-null, can be cast to point to an `Option<&'a T>`, - /// where `T` is the type `TypeId` was derived from. - /// - /// The lifetime of the returned pointer is the lifetime of `self`. - #[doc(hidden)] - fn provide_ref_internal(&mut self, _: private::Private, _type_id: TypeId) -> *mut () { - ptr::null_mut() - } +pub struct Request<'a, R = dyn private::Response + 'a> +where + R: ?Sized + private::Response, +{ + marker: PhantomData<&'a ()>, - /// /!\ THIS IS NOT PUBLIC API /!\ + /// A `TypeId` marker for the type stored in `R`. /// - /// Provide a value with the given type to the request. - /// - /// The returned pointer, if non-null, will point to an `Option`, where - /// `T` is the type `TypeId` was derived from. + /// Will be the TypeId of either `RefMarker` or `ValueMarker`. + type_id: TypeId, + + /// A (potentially type-erased) `Option` containing the response value. /// - /// The lifetime of the returned pointer is the lifetime of `self`. - #[doc(hidden)] - fn provide_value_internal(&mut self, _: private::Private, _type_id: TypeId) -> *mut () { - ptr::null_mut() - } + /// Will be either `Option<&'a T>` if `type_id` is `RefMarker`, or + /// `Option` if `type_id` is `ValueMarker`. + response: R, } -impl<'a> dyn Request<'a> + '_ { - /// Type-safe wrapper for calling `provide_ref_internal`. - /// - /// See `Request::provide_ref_internal`'s documentation for the invariants - /// being held by this method. - fn provide_ref_place<'b, T: ?Sized + 'static>(&'b mut self) -> Option<&'b mut Option<&'a T>> { - let ptr = self.provide_ref_internal(private::Private, TypeId::of::()); - if ptr.is_null() { - None +impl<'a> Request<'a> { + /// Perform a checked downcast of `response` to `Option<&'a T>` + fn downcast_ref_response<'b, T: ?Sized + 'static>( + &'b mut self, + ) -> Option<&'b mut Option<&'a T>> { + if self.is_ref::() { + // safety: If `self.is_ref::()` returns true, `response` must be + // of the correct type. This is enforced by the private `type_id` + // field. + Some(unsafe { &mut *(&mut self.response as *mut _ as *mut Option<&'a T>) }) } else { - Some(unsafe { &mut *(ptr as *mut Option<&'a T>) }) + None } } - /// Type-safe wrapper for calling `provide_value_internal`. - /// - /// See `Request::provide_value_internal`'s documentation for the invariants - /// being held by this method. - fn provide_value_place<'b, T: 'static>(&'b mut self) -> Option<&'b mut Option> { - let ptr = self.provide_value_internal(private::Private, TypeId::of::()); - if ptr.is_null() { - None + /// Perform a checked downcast of `response` to `Option` + fn downcast_value_response<'b, T: 'static>(&'b mut self) -> Option<&'b mut Option> { + if self.is_value::() { + // safety: If `self.is_value::()` returns true, `response` must + // be of the correct type. This is enforced by the private `type_id` + // field. + Some(unsafe { &mut *(&mut self.response as *mut _ as *mut Option) }) } else { - Some(unsafe { &mut *(ptr as *mut Option) }) + None } } -} -impl<'a> dyn Request<'a> + '_ { /// Provides a reference of type `&'a T` in response to this request. /// /// If a reference of type `&'a T` has already been provided for this @@ -155,13 +151,12 @@ impl<'a> dyn Request<'a> + '_ { /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. - pub fn provide_ref_with(&mut self, cb: F) -> &mut Self + pub fn provide_ref_with(&mut self, cb: F) -> &mut Self where - T: ?Sized + 'static, F: FnOnce() -> &'a T, { - if let Some(place) = self.provide_ref_place::() { - *place = Some(cb()); + if let Some(response) = self.downcast_ref_response::() { + *response = Some(cb()); } self } @@ -187,16 +182,64 @@ impl<'a> dyn Request<'a> + '_ { /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. - pub fn provide_value_with(&mut self, cb: F) -> &mut Self + pub fn provide_value_with(&mut self, cb: F) -> &mut Self where - T: 'static, F: FnOnce() -> T, { - if let Some(place) = self.provide_value_place::() { - *place = Some(cb()); + if let Some(response) = self.downcast_value_response::() { + *response = Some(cb()); } self } + + /// Returns `true` if the requested type is `&'a T` + pub fn is_ref(&self) -> bool { + self.type_id == TypeId::of::>() + } + + /// Returns `true` if the requested type is `T` + pub fn is_value(&self) -> bool { + self.type_id == TypeId::of::>() + } +} + +impl<'a, T: ?Sized + 'static> Request<'a, Option<&'a T>> { + /// Create a new reference request object. + /// + /// The returned value will unsize to `Request<'a>`, and can be passed to + /// functions accepting it as an argument to request `&'a T` references. + pub fn new_ref() -> Self { + // safety: Initializes `type_id` to `RefMarker`, which corresponds to + // the response type `Option<&'a T>`. + Request { + marker: PhantomData, + type_id: TypeId::of::>(), + response: None, + } + } +} + +impl Request<'_, Option> { + /// Create a new value request object. + /// + /// The returned value will unsize to `Request<'a>`, and can be passed to + /// functions accepting it as an argument to request `T` values. + pub fn new_value() -> Self { + // safety: Initializes `type_id` to `ValueMarker`, which corresponds + // to the response type `Option`. + Request { + marker: PhantomData, + type_id: TypeId::of::>(), + response: None, + } + } +} + +impl Request<'_, Option> { + /// Extract the response from this request object, consuming it. + pub fn into_response(self) -> Option { + self.response + } } /// Trait to provide other objects based on a requested type at runtime. @@ -205,7 +248,7 @@ impl<'a> dyn Request<'a> + '_ { /// `request_value` methods. pub trait ObjectProvider { /// Provide an object in response to `request`. - fn provide<'a>(&'a self, request: &mut dyn Request<'a>); + fn provide<'a>(&'a self, request: &mut Request<'a>); } /// Methods supported by all [`ObjectProvider`] implementors. @@ -219,77 +262,15 @@ pub trait ObjectProviderExt { impl ObjectProviderExt for O { fn request_ref(&self) -> Option<&T> { - let mut request = RequestRef::default(); + let mut request = Request::new_ref(); self.provide(&mut request); - request.value + request.into_response() } fn request_value(&self) -> Option { - let mut request = RequestValue::default(); + let mut request = Request::new_value(); self.provide(&mut request); - request.value - } -} - -/// A request for a reference of type `&'a T`. -/// -/// This type implements `Request<'a>`, meaning it can be passed to types -/// expecting `&mut dyn Request<'a>`. -#[derive(Debug)] -pub struct RequestRef<'a, T: ?Sized> { - /// The value provided to this request, or `None`. - pub value: Option<&'a T>, -} - -impl<'a, T: ?Sized> Default for RequestRef<'a, T> { - fn default() -> Self { - RequestRef { value: None } - } -} - -impl<'a, T: ?Sized + 'static> private::Sealed for RequestRef<'a, T> {} - -unsafe impl<'a, T: ?Sized + 'static> Request<'a> for RequestRef<'a, T> { - /// See `Request::provide_ref_internal`'s documentation for the invariants - /// being upheld by this method. - #[doc(hidden)] - fn provide_ref_internal(&mut self, _: private::Private, type_id: TypeId) -> *mut () { - if type_id == TypeId::of::() { - &mut self.value as *mut Option<&'a T> as *mut () - } else { - ptr::null_mut() - } - } -} - -/// A request for a value of type `T`. -/// -/// This type implements [`Request`], meaning it can be passed to functions -/// expecting a `&mut dyn Request<'a>` trait object. -#[derive(Debug)] -pub struct RequestValue { - /// The value provided to this request, or `None`. - pub value: Option, -} - -impl Default for RequestValue { - fn default() -> Self { - RequestValue { value: None } - } -} - -impl private::Sealed for RequestValue {} - -unsafe impl<'a, T: 'static> Request<'a> for RequestValue { - /// See `Request::provide_value_internal`'s documentation for the invariants - /// being upheld by this method. - #[doc(hidden)] - fn provide_value_internal(&mut self, _: private::Private, type_id: TypeId) -> *mut () { - if type_id == TypeId::of::() { - &mut self.value as *mut Option as *mut () - } else { - ptr::null_mut() - } + request.into_response() } } @@ -306,12 +287,13 @@ mod test { path: PathBuf, } impl ObjectProvider for HasContext { - fn provide<'a>(&'a self, request: &mut dyn Request<'a>) { + fn provide<'a>(&'a self, request: &mut Request<'a>) { request .provide_ref::(&self.int) .provide_ref::(&self.path) .provide_ref::(&self.int) - .provide_value::(self.int); + .provide_value::(self.int) + .provide_value_with::(|| self.int as i64); } } From 8fbd57479abb247e9c7bfc3a19c7b3a1cd47b248 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Tue, 25 Aug 2020 00:43:07 -0400 Subject: [PATCH 4/5] add doc examples --- src/lib.rs | 130 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 119 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bdf7710..1d278ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -61,7 +61,10 @@ //! request //! .provide_ref::(&self.path) //! .provide_ref::(&self.path) -//! .provide_ref::(&self.path); +//! .provide_ref::(&self.path) +//! .provide_value_with::(|| { +//! self.path.to_string_lossy().into_owned() +//! }); //! } //! } //! ``` @@ -131,11 +134,31 @@ impl<'a> Request<'a> { /// Provides a reference of type `&'a T` in response to this request. /// /// If a reference of type `&'a T` has already been provided for this - /// request, the existing value will be replaced by the newly provided - /// value. + /// request, or if the request is for a different type, this call will be + /// ignored. /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. + /// + /// # Example + /// + /// ``` + /// # use object_provider::{ObjectProvider, Request}; + /// # use std::fmt; + /// struct MyProvider { + /// name: String, + /// } + /// + /// impl ObjectProvider for MyProvider { + /// fn provide<'a>(&'a self, request: &mut Request<'a>) { + /// request + /// .provide_ref::(&self) + /// .provide_ref::(&self.name) + /// .provide_ref::(&self.name) + /// .provide_ref::(&self.name); + /// } + /// } + /// ``` pub fn provide_ref(&mut self, value: &'a T) -> &mut Self { self.provide_ref_with(|| value) } @@ -143,19 +166,42 @@ impl<'a> Request<'a> { /// Lazily provides a reference of type `&'a T` in response to this request. /// /// If a reference of type `&'a T` has already been provided for this - /// request, the existing value will be replaced by the newly provided - /// value. + /// request, or if the request is for a different type, this call will be + /// ignored. /// /// The passed closure is only called if the value will be successfully /// provided. /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. + /// + /// # Example + /// + /// ``` + /// # use object_provider::{ObjectProvider, Request}; + /// # fn expensive_condition() -> bool { true } + /// struct MyProvider { + /// a: String, + /// b: String, + /// } + /// + /// impl ObjectProvider for MyProvider { + /// fn provide<'a>(&'a self, request: &mut Request<'a>) { + /// request.provide_ref_with::(|| { + /// if expensive_condition() { + /// &self.a + /// } else { + /// &self.b + /// } + /// }); + /// } + /// } + /// ``` pub fn provide_ref_with(&mut self, cb: F) -> &mut Self where F: FnOnce() -> &'a T, { - if let Some(response) = self.downcast_ref_response::() { + if let Some(response @ None) = self.downcast_ref_response::() { *response = Some(cb()); } self @@ -163,30 +209,64 @@ impl<'a> Request<'a> { /// Provides an value of type `T` in response to this request. /// - /// If a value of type `T` has already been provided for this request, the - /// existing value will be replaced by the newly provided value. + /// If a value of type `T` has already been provided for this request, or if + /// the request is for a different type, this call will be ignored. /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. + /// + /// # Example + /// + /// ``` + /// # use object_provider::{ObjectProvider, Request}; + /// struct MyProvider { + /// count: u32, + /// } + /// + /// impl ObjectProvider for MyProvider { + /// fn provide<'a>(&'a self, request: &mut Request<'a>) { + /// request + /// .provide_value::(self.count) + /// .provide_value::<&'static str>("hello, world!"); + /// } + /// } + /// ``` pub fn provide_value(&mut self, value: T) -> &mut Self { self.provide_value_with(|| value) } /// Lazily provides a value of type `T` in response to this request. /// - /// If a value of type `T` has already been provided for this request, the - /// existing value will be replaced by the newly provided value. + /// If a value of type `T` has already been provided for this request, or if + /// the request is for a different type, this call will be ignored. /// /// The passed closure is only called if the value will be successfully /// provided. /// /// This method can be chained within `provide` implementations to concisely /// provide multiple objects. + /// + /// # Example + /// + /// ``` + /// # use object_provider::{ObjectProvider, Request}; + /// struct MyProvider { + /// count: u32, + /// } + /// + /// impl ObjectProvider for MyProvider { + /// fn provide<'a>(&'a self, request: &mut Request<'a>) { + /// request + /// .provide_value_with::(|| self.count / 10) + /// .provide_value_with::(|| format!("{}", self.count)); + /// } + /// } + /// ``` pub fn provide_value_with(&mut self, cb: F) -> &mut Self where F: FnOnce() -> T, { - if let Some(response) = self.downcast_value_response::() { + if let Some(response @ None) = self.downcast_value_response::() { *response = Some(cb()); } self @@ -208,6 +288,20 @@ impl<'a, T: ?Sized + 'static> Request<'a, Option<&'a T>> { /// /// The returned value will unsize to `Request<'a>`, and can be passed to /// functions accepting it as an argument to request `&'a T` references. + /// + /// See also the [`ObjectProviderExt`] trait, which provides the + /// `request_ref` helper method. + /// + /// # Example + /// + /// ``` + /// # use object_provider::Request; + /// fn provider(request: &mut Request<'_>) { /* ... */ } + /// + /// let mut request = Request::<'_, Option<&str>>::new_ref(); + /// provider(&mut request); + /// let response = request.into_response(); + /// ``` pub fn new_ref() -> Self { // safety: Initializes `type_id` to `RefMarker`, which corresponds to // the response type `Option<&'a T>`. @@ -224,6 +318,20 @@ impl Request<'_, Option> { /// /// The returned value will unsize to `Request<'a>`, and can be passed to /// functions accepting it as an argument to request `T` values. + /// + /// See also the [`ObjectProviderExt`] trait, which provides the + /// `request_value` helper method. + /// + /// # Example + /// + /// ``` + /// # use object_provider::Request; + /// fn provider(request: &mut Request<'_>) { /* ... */ } + /// + /// let mut request = Request::<'_, Option>::new_value(); + /// provider(&mut request); + /// let response = request.into_response(); + /// ``` pub fn new_value() -> Self { // safety: Initializes `type_id` to `ValueMarker`, which corresponds // to the response type `Option`. From 9f77eb6a6bb521cf143d9e7777a9d7a775d58b29 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Tue, 25 Aug 2020 11:19:41 -0400 Subject: [PATCH 5/5] Fix potential variance soundness hole --- src/lib.rs | 153 ++++++++++++++++++++++++++++------------------------- 1 file changed, 81 insertions(+), 72 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1d278ab..ce4cbf2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -73,34 +73,32 @@ use core::any::TypeId; use core::marker::PhantomData; mod private { - pub trait Response {} - impl Response for Option {} + pub trait Response<'a>: 'a {} } -/// When used by `Request`, `TypeId::of::>()` indicates that -/// `response` is of type `Option<&'a T>`. -struct RefMarker(T); +/// A response to a ref request. +struct RefResponse<'a, T: ?Sized + 'static>(Option<&'a T>); +impl<'a, T: ?Sized + 'static> private::Response<'a> for RefResponse<'a, T> {} -/// When used by `Request`, `TypeId::of::>()` indicates that -/// `response` is of type `Option`. -struct ValueMarker(T); +/// A response to a value request. +struct ValueResponse(Option); +impl<'a, T: 'static> private::Response<'a> for ValueResponse {} /// A dynamic request for an object based on its type. -pub struct Request<'a, R = dyn private::Response + 'a> +pub struct Request<'a, R = dyn private::Response<'a>> where - R: ?Sized + private::Response, + R: ?Sized + private::Response<'a>, { marker: PhantomData<&'a ()>, /// A `TypeId` marker for the type stored in `R`. /// - /// Will be the TypeId of either `RefMarker` or `ValueMarker`. + /// Will be the TypeId of either `RefResponse<'static, T>` or + /// `ValueResponse`. type_id: TypeId, - /// A (potentially type-erased) `Option` containing the response value. - /// - /// Will be either `Option<&'a T>` if `type_id` is `RefMarker`, or - /// `Option` if `type_id` is `ValueMarker`. + /// A type erased `RefResponse` or `ValueResponse` containing the response + /// value. response: R, } @@ -108,24 +106,24 @@ impl<'a> Request<'a> { /// Perform a checked downcast of `response` to `Option<&'a T>` fn downcast_ref_response<'b, T: ?Sized + 'static>( &'b mut self, - ) -> Option<&'b mut Option<&'a T>> { + ) -> Option<&'b mut RefResponse<'a, T>> { if self.is_ref::() { // safety: If `self.is_ref::()` returns true, `response` must be // of the correct type. This is enforced by the private `type_id` // field. - Some(unsafe { &mut *(&mut self.response as *mut _ as *mut Option<&'a T>) }) + Some(unsafe { &mut *(&mut self.response as *mut _ as *mut RefResponse<'a, T>) }) } else { None } } /// Perform a checked downcast of `response` to `Option` - fn downcast_value_response<'b, T: 'static>(&'b mut self) -> Option<&'b mut Option> { + fn downcast_value_response<'b, T: 'static>(&'b mut self) -> Option<&'b mut ValueResponse> { if self.is_value::() { // safety: If `self.is_value::()` returns true, `response` must // be of the correct type. This is enforced by the private `type_id` // field. - Some(unsafe { &mut *(&mut self.response as *mut _ as *mut Option) }) + Some(unsafe { &mut *(&mut self.response as *mut _ as *mut ValueResponse) }) } else { None } @@ -201,7 +199,7 @@ impl<'a> Request<'a> { where F: FnOnce() -> &'a T, { - if let Some(response @ None) = self.downcast_ref_response::() { + if let Some(RefResponse(response @ None)) = self.downcast_ref_response::() { *response = Some(cb()); } self @@ -266,7 +264,7 @@ impl<'a> Request<'a> { where F: FnOnce() -> T, { - if let Some(response @ None) = self.downcast_value_response::() { + if let Some(ValueResponse(response @ None)) = self.downcast_value_response::() { *response = Some(cb()); } self @@ -274,79 +272,94 @@ impl<'a> Request<'a> { /// Returns `true` if the requested type is `&'a T` pub fn is_ref(&self) -> bool { - self.type_id == TypeId::of::>() + self.type_id == TypeId::of::>() } /// Returns `true` if the requested type is `T` pub fn is_value(&self) -> bool { - self.type_id == TypeId::of::>() + self.type_id == TypeId::of::>() } -} -impl<'a, T: ?Sized + 'static> Request<'a, Option<&'a T>> { - /// Create a new reference request object. - /// - /// The returned value will unsize to `Request<'a>`, and can be passed to - /// functions accepting it as an argument to request `&'a T` references. + /// Calls the provided closure with a request for the the type `&'a T`, + /// returning `Some(&T)` if the request was fulfilled, and `None` otherwise. /// - /// See also the [`ObjectProviderExt`] trait, which provides the - /// `request_ref` helper method. + /// The `ObjectProviderExt` trait provides helper methods specifically for + /// types implementing `ObjectProvider`. /// /// # Example /// /// ``` /// # use object_provider::Request; - /// fn provider(request: &mut Request<'_>) { /* ... */ } - /// - /// let mut request = Request::<'_, Option<&str>>::new_ref(); - /// provider(&mut request); - /// let response = request.into_response(); + /// let response: Option<&str> = Request::request_ref(|request| { + /// // ... + /// request.provide_ref::("hello, world"); + /// }); + /// assert_eq!(response, Some("hello, world")); /// ``` - pub fn new_ref() -> Self { - // safety: Initializes `type_id` to `RefMarker`, which corresponds to - // the response type `Option<&'a T>`. - Request { - marker: PhantomData, - type_id: TypeId::of::>(), - response: None, - } + pub fn request_ref(cb: F) -> Option<&'a T> + where + F: FnOnce(&mut Request<'a>), + { + let mut request = Request::new_ref(); + cb(&mut request); + request.response.0 } -} -impl Request<'_, Option> { - /// Create a new value request object. + /// Calls the provided closure with a request for the the type `T`, + /// returning `Some(T)` if the request was fulfilled, and `None` otherwise. /// - /// The returned value will unsize to `Request<'a>`, and can be passed to - /// functions accepting it as an argument to request `T` values. - /// - /// See also the [`ObjectProviderExt`] trait, which provides the - /// `request_value` helper method. + /// The `ObjectProviderExt` trait provides helper methods specifically for + /// types implementing `ObjectProvider`. /// /// # Example /// /// ``` /// # use object_provider::Request; - /// fn provider(request: &mut Request<'_>) { /* ... */ } - /// - /// let mut request = Request::<'_, Option>::new_value(); - /// provider(&mut request); - /// let response = request.into_response(); + /// let response: Option = Request::request_value(|request| { + /// // ... + /// request.provide_value::(5); + /// }); + /// assert_eq!(response, Some(5)); /// ``` - pub fn new_value() -> Self { - // safety: Initializes `type_id` to `ValueMarker`, which corresponds - // to the response type `Option`. + pub fn request_value(cb: F) -> Option + where + F: FnOnce(&mut Request<'a>), + { + let mut request = Request::new_value(); + cb(&mut request); + request.response.0 + } +} + +impl<'a, T: ?Sized + 'static> Request<'a, RefResponse<'a, T>> { + /// Create a new reference request object. + /// + /// The returned value will unsize to `Request<'a>`, and can be passed to + /// functions accepting it as an argument to request `&'a T` references. + fn new_ref() -> Self { + // safety: Initializes `type_id` to `RefResponse<'static, T>`, which + // corresponds to the response type `RefResponse<'a, T>`. Request { marker: PhantomData, - type_id: TypeId::of::>(), - response: None, + type_id: TypeId::of::>(), + response: RefResponse(None), } } } -impl Request<'_, Option> { - /// Extract the response from this request object, consuming it. - pub fn into_response(self) -> Option { - self.response +impl Request<'_, ValueResponse> { + /// Create a new value request object. + /// + /// The returned value will unsize to `Request<'a>`, and can be passed to + /// functions accepting it as an argument to request `T` values. + fn new_value() -> Self { + // safety: Initializes `type_id` to `ValueResponse`, which + // corresponds to the response type `ValueResponse`. + Request { + marker: PhantomData, + type_id: TypeId::of::>(), + response: ValueResponse(None), + } } } @@ -370,15 +383,11 @@ pub trait ObjectProviderExt { impl ObjectProviderExt for O { fn request_ref(&self) -> Option<&T> { - let mut request = Request::new_ref(); - self.provide(&mut request); - request.into_response() + Request::request_ref(|request| self.provide(request)) } fn request_value(&self) -> Option { - let mut request = Request::new_value(); - self.provide(&mut request); - request.into_response() + Request::request_value(|request| self.provide(request)) } }