From b853350a3800e38d2cb9950355b80bc8b8d3959c Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 9 Nov 2021 17:31:24 -0800 Subject: [PATCH] Require inventory element type to be const constructed --- examples/flags.rs | 2 +- src/lib.rs | 108 ++++++++++++++-------------- tests/ui/collect-unsized.stderr | 19 ----- tests/ui/submit-unrecognized.stderr | 19 +++-- 4 files changed, 64 insertions(+), 84 deletions(-) diff --git a/examples/flags.rs b/examples/flags.rs index 308c2d4..12f2f4b 100644 --- a/examples/flags.rs +++ b/examples/flags.rs @@ -4,7 +4,7 @@ struct Flag { } impl Flag { - fn new(short: char, name: &'static str) -> Self { + const fn new(short: char, name: &'static str) -> Self { Flag { short, name } } } diff --git a/src/lib.rs b/src/lib.rs index bfaed7f..f465f96 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -61,7 +61,7 @@ //! # struct Flag; //! # //! # impl Flag { -//! # fn new(short: char, name: &'static str) -> Self { +//! # const fn new(short: char, name: &'static str) -> Self { //! # Flag //! # } //! # } @@ -113,26 +113,48 @@ clippy::semicolon_if_nothing_returned, // https://github.com/rust-lang/rust-clippy/issues/7324 )] -extern crate alloc; +// Not public API. +#[doc(hidden)] +pub extern crate core; // Not public API. #[doc(hidden)] pub use ctor::ctor; -use alloc::boxed::Box; +use core::cell::UnsafeCell; +use core::marker::PhantomData; use core::ops::Deref; use core::ptr; use core::sync::atomic::{AtomicPtr, Ordering}; // Not public API. Used by generated code. #[doc(hidden)] -pub struct Registry { - head: AtomicPtr>, +pub struct Registry { + head: AtomicPtr, +} + +// Not public API. Used by generated code. +#[doc(hidden)] +pub struct Node { + pub value: &'static dyn ErasedNode, + pub next: UnsafeCell>, +} + +// The `value` is Sync, and `next` is only mutated during submit, which is prior +// to any reads. +unsafe impl Sync for Node {} + +// Not public API. Used by generated code. +#[doc(hidden)] +pub trait ErasedNode: Sync { + // SAFETY: requires *node.value is of type Self. + unsafe fn submit(&self, node: &'static Node); } -struct Node { - value: T, - next: Option<&'static Node>, +impl ErasedNode for T { + unsafe fn submit(&self, node: &'static Node) { + T::registry().submit(node); + } } /// Trait bound corresponding to types that can be iterated by inventory::iter. @@ -151,18 +173,10 @@ struct Node { /// ``` pub trait Collect: Sync + Sized + 'static { #[doc(hidden)] - fn registry() -> &'static Registry; + fn registry() -> &'static Registry; } -// Not public API. Used by generated code. -#[doc(hidden)] -pub fn submit(value: T) { - // TODO: Avoid allocation by storing node in a static mut Option> - // after existential type is stable. See comment in inventory-impl. - T::registry().submit(Box::new(Node { value, next: None })); -} - -impl Registry { +impl Registry { // Not public API. Used by generated code. pub const fn new() -> Self { Registry { @@ -170,16 +184,16 @@ impl Registry { } } - fn submit(&'static self, new: Box>) { - let mut new = ptr::NonNull::from(Box::leak(new)); + // SAFETY: requires type of *new.value matches the $ty surrounding the + // declaration of this registry in inventory::collect macro. + unsafe fn submit(&'static self, new: &'static Node) { let mut head = self.head.load(Ordering::SeqCst); loop { - // `new` is always a valid Node, and is not yet visible through the registry. - // `head` is always null or valid &'static Node. - unsafe { new.as_mut().next = head.as_ref() }; + *new.next.get() = head.as_ref(); + let new_ptr = new as *const Node as *mut Node; match self .head - .compare_exchange(head, new.as_ptr(), Ordering::SeqCst, Ordering::SeqCst) + .compare_exchange(head, new_ptr, Ordering::SeqCst, Ordering::SeqCst) { Ok(_) => return, Err(prev) => head = prev, @@ -237,8 +251,9 @@ const ITER: () = { fn into_iter() -> Iter { let head = T::registry().head.load(Ordering::SeqCst); Iter { - // Head pointer is always null or valid &'static Node. + // Head pointer is always null or valid &'static Node. node: unsafe { head.as_ref() }, + marker: PhantomData, } } @@ -261,7 +276,8 @@ const ITER: () = { #[derive(Clone)] pub struct Iter { - node: Option<&'static Node>, + node: Option<&'static Node>, + marker: PhantomData, } impl Iterator for Iter { @@ -269,9 +285,11 @@ const ITER: () = { fn next(&mut self) -> Option { let node = self.node?; - let value = &node.value; - self.node = node.next; - Some(value) + unsafe { + let value_ptr = node.value as *const dyn ErasedNode as *const T; + self.node = *node.next.get(); + Some(&*value_ptr) + } } } }; @@ -310,8 +328,8 @@ macro_rules! collect { ($ty:ty) => { impl $crate::Collect for $ty { #[inline] - fn registry() -> &'static $crate::Registry { - static REGISTRY: $crate::Registry<$ty> = $crate::Registry::new(); + fn registry() -> &'static $crate::Registry { + static REGISTRY: $crate::Registry = $crate::Registry::new(); ®ISTRY } } @@ -337,7 +355,7 @@ macro_rules! collect { /// # struct Flag; /// # /// # impl Flag { -/// # fn new(short: char, name: &'static str) -> Self { +/// # const fn new(short: char, name: &'static str) -> Self { /// # Flag /// # } /// # } @@ -374,27 +392,11 @@ macro_rules! submit { #[allow(non_upper_case_globals)] #[$crate::ctor] fn __init() { - // TODO: once existential type is stable, store the caller's - // expression into a static and string those statics together - // into an intrusive linked list without needing allocation. - // - // existential type This; - // - // static mut VALUE: Option> = None; - // - // fn value() -> This { - // $($value)* - // } - // - // unsafe { - // VALUE = Some(inventory::Node { - // value: value(), - // next: None, - // }); - // inventory::submit(VALUE.as_mut().unwrap()); - // } - - $crate::submit({ $($value)* }); + static __INVENTORY: $crate::Node = $crate::Node { + value: &{ $($value)* }, + next: $crate::core::cell::UnsafeCell::new($crate::core::option::Option::None), + }; + unsafe { $crate::ErasedNode::submit(__INVENTORY.value, &__INVENTORY) } } }; } diff --git a/tests/ui/collect-unsized.stderr b/tests/ui/collect-unsized.stderr index c6f5841..6675f42 100644 --- a/tests/ui/collect-unsized.stderr +++ b/tests/ui/collect-unsized.stderr @@ -16,22 +16,3 @@ note: required by a bound in `Collect` | pub trait Collect: Sync + Sized + 'static { | ^^^^^ required by this bound in `Collect` = note: this error originates in the macro `inventory::collect` (in Nightly builds, run with -Z macro-backtrace for more info) - -error[E0277]: the size for values of type `str` cannot be known at compilation time - --> tests/ui/collect-unsized.rs:3:1 - | -3 | inventory::collect!(Unsized); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time - | - = help: within `Unsized`, the trait `Sized` is not implemented for `str` -note: required because it appears within the type `Unsized` - --> tests/ui/collect-unsized.rs:1:12 - | -1 | pub struct Unsized(str); - | ^^^^^^^ -note: required by a bound in `Registry` - --> src/lib.rs - | - | pub struct Registry { - | ^ required by this bound in `Registry` - = note: this error originates in the macro `inventory::collect` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/submit-unrecognized.stderr b/tests/ui/submit-unrecognized.stderr index e20afbc..b69931e 100644 --- a/tests/ui/submit-unrecognized.stderr +++ b/tests/ui/submit-unrecognized.stderr @@ -1,12 +1,9 @@ error[E0277]: the trait bound `Thing: Collect` is not satisfied - --> tests/ui/submit-unrecognized.rs:3:1 - | -3 | inventory::submit!(Thing); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Collect` is not implemented for `Thing` - | -note: required by a bound in `submit` - --> src/lib.rs - | - | pub fn submit(value: T) { - | ^^^^^^^ required by this bound in `submit` - = note: this error originates in the macro `inventory::submit` (in Nightly builds, run with -Z macro-backtrace for more info) + --> tests/ui/submit-unrecognized.rs:3:1 + | +3 | inventory::submit!(Thing); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Collect` is not implemented for `Thing` + | + = note: required because of the requirements on the impl of `ErasedNode` for `Thing` + = note: required for the cast to the object type `dyn ErasedNode` + = note: this error originates in the macro `inventory::submit` (in Nightly builds, run with -Z macro-backtrace for more info)