Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add derive bound option #23

Merged
merged 4 commits into from
Nov 23, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 95 additions & 20 deletions src/gc-arena-derive/src/lib.rs
Original file line number Diff line number Diff line change
@@ -18,44 +18,82 @@ fn collect_derive(mut s: synstructure::Structure) -> TokenStream {
UnsafeDrop,
}

macro_rules! usage_error {
($($t:tt)*) => {{
let msg = format!($($t)*);
panic!("{}. `#[collect(...)]` requires one mode (`require_static`, `no_drop`, or `unsafe_drop`) and optionally `bound = \"...\"`.", msg);
}}
}

let mut mode = None;
let mut override_bound = None;

for attr in &s.ast().attrs {
match attr.parse_meta() {
Ok(syn::Meta::List(syn::MetaList { path, nested, .. })) => {
if path.is_ident("collect") {
if let Some(prev_mode) = mode {
let prev_mode_str = match prev_mode {
Mode::RequireStatic => "require_static",
Mode::NoDrop => "no_drop",
Mode::UnsafeDrop => "unsafe_drop",
};
panic!("`Collect` mode was already specified with `#[collect({})]`, cannot specify twice", prev_mode_str);
if mode.is_some() {
panic!("multiple `#[collect(...)]` attributes found on the same item. consider combining them.");
}

if let Some(syn::NestedMeta::Meta(syn::Meta::Path(path))) = nested.first() {
if path.is_ident("require_static") {
mode = Some(Mode::RequireStatic);
} else if path.is_ident("no_drop") {
mode = Some(Mode::NoDrop);
} else if path.is_ident("unsafe_drop") {
mode = Some(Mode::UnsafeDrop);
} else {
panic!("`#[collect]` requires one of: \"require_static\", \"no_drop\", or \"unsafe_drop\" as an argument");
for nested in nested {
match nested {
syn::NestedMeta::Meta(syn::Meta::Path(path)) => {
if mode.is_some() {
usage_error!("multiple modes specified");
}

if path.is_ident("require_static") {
mode = Some(Mode::RequireStatic);
} else if path.is_ident("no_drop") {
mode = Some(Mode::NoDrop);
} else if path.is_ident("unsafe_drop") {
mode = Some(Mode::UnsafeDrop);
} else {
usage_error!("unknown option")
}
}
syn::NestedMeta::Meta(syn::Meta::NameValue(value)) => {
if override_bound.is_some() {
usage_error!("multiple bounds specified");
}

if value.path.is_ident("bound") {
match value.lit {
syn::Lit::Str(x) => override_bound = Some(x),
_ => usage_error!("bound must be str"),
}
} else {
usage_error!("unknown option");
}
}
_ => usage_error!("unknown option"),
}
}

if mode.is_none() {
usage_error!("missing mode");
}
}
}
_ => {}
}
}

let mode = mode.expect("deriving `Collect` requires a `#[collect(<mode>)]` attribute, where `<mode>` is one of \"require_static\", \"no_drop\", or \"unsafe_drop\"");
let mode = mode.unwrap_or_else(|| {
usage_error!("deriving `Collect` requires a `#[collect(...)]` attribute")
});

let where_clause = if mode == Mode::RequireStatic {
quote!(where Self: 'static)
} else {
quote!()
override_bound
.as_ref()
.map(|x| {
x.parse()
.expect("`#[collect]` failed to parse explicit trait bound expression")
})
.unwrap_or_else(|| quote!())
};

let mut errors = vec![];
@@ -192,7 +230,12 @@ fn collect_derive(mut s: synstructure::Structure) -> TokenStream {
)
});

s.clone().add_bounds(AddBounds::Fields).gen_impl(quote! {
let bounds_type = if override_bound.is_some() {
AddBounds::None
} else {
AddBounds::Generics
};
s.clone().add_bounds(bounds_type).gen_impl(quote! {
gen unsafe impl gc_arena::Collect for @Self #where_clause {
#[inline]
fn needs_trace() -> bool {
@@ -223,4 +266,36 @@ fn collect_derive(mut s: synstructure::Structure) -> TokenStream {
}
}

decl_derive!([Collect, attributes(collect)] => collect_derive);
decl_derive! {
[Collect, attributes(collect)] =>
/// Derives the `Collect` trait needed to trace a gc type.
///
/// To derive `Collect`, an additional attribute is required on the struct/enum called `collect`.
/// This has several optional arguments, but the only required argument is the derive strategy.
/// This can be one of
///
/// - `#[collect(require_static)]` - Adds a `'static` bound, which allows for a no-op trace implementation.
/// This is the ideal choice where possible.
/// - `#[collect(no_drop)]` - The typical safe tracing derive strategy which only has to add a requirement
/// that your struct/enum does not have a custom implementation of `Drop`.
/// - `#[collect(unsafe_drop)]` - The most versatile tracing derive strategy which allows a custom drop implementation.
/// However, this strategy can lead to unsoundness if care is not taken (see the above explanation of `Drop` interactions).
///
/// The `collect` attribute also accepts a number of optional configuration settings:
///
/// - `#[collect(bound = "<code>")]` - Replaces the default generated `where` clause with the given code.
/// This can be an empty string to add no `where` clause, or otherwise must start with `"where"`,
/// e.g., `#[collect(bound = "where T: Collect")]`.
/// Note that this option is ignored for `require_static` mode since the only bound it produces is `Self: 'static`.
/// Also note that providing an explicit bound in this way is safe, and only changes the trait bounds used
/// to enable the implementation of `Collect`.
///
/// Options may be passed to the `collect` attribute together, e.g., `#[collect(no_drop, bound = "")]`.
///
/// The `collect` attribute may also be used on any field of an enum or struct, however the only allowed usage
/// is to specify the strategy as `require_static` (no other strategies are allowed, and no optional settings can be specified).
/// This will add a `'static` bound to the type of the field (regardless of an explicit `bound` setting)
/// in exchange for not having to trace into the given field (the ideal choice where possible).
/// Note that if the entire struct/enum is marked with `require_static` then this is unnecessary.
collect_derive
}
2 changes: 1 addition & 1 deletion src/gc-arena/src/collect.rs
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ use crate::context::CollectionContext;
/// 3. Internal mutability *must* not be used to adopt new `Gc` pointers without calling
/// `Gc::write_barrier` during the same arena mutation.
///
/// It is, however, possible to implement this trait safely by procedurally deriving it, which
/// It is, however, possible to implement this trait safely by procedurally deriving it (see [`gc_arena_derive::Collect`]), which
/// requires that every field in the structure also implement `Collect`, and implements a safe,
/// empty version of `Drop`. Internally mutable types like `Cell` and `RefCell` do not implement
/// `Collect` in such a way that it is possible to store `Gc` pointers inside them, so the write
7 changes: 5 additions & 2 deletions src/gc-sequence/src/and_then.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ use crate::Sequence;

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where S: Collect, I: Collect, F: 'static")]
pub enum AndThen<S, F, I> {
First(S, Option<StaticCollect<F>>),
Second(Option<(I, StaticCollect<F>)>),
@@ -47,7 +47,10 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(
no_drop,
bound = "where S: Collect, C: Collect, I: Collect, F: 'static"
)]
pub enum AndThenWith<S, C, F, I> {
First(S, Option<(C, StaticCollect<F>)>),
Second(Option<(C, I, StaticCollect<F>)>),
4 changes: 2 additions & 2 deletions src/gc-sequence/src/map.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ use crate::Sequence;

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where S: Collect, F: 'static")]
pub struct Map<S, F>(S, Option<StaticCollect<F>>);

impl<S, F> Map<S, F> {
@@ -32,7 +32,7 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where S: Collect, C: Collect, F: 'static")]
pub struct MapWith<S, C, F>(S, Option<(C, StaticCollect<F>)>);

impl<S, C, F> MapWith<S, C, F> {
6 changes: 3 additions & 3 deletions src/gc-sequence/src/map_result.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ use crate::Sequence;

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where S: Collect, F: 'static")]
pub struct MapOk<S, F>(S, Option<StaticCollect<F>>);

impl<S, F> MapOk<S, F> {
@@ -35,7 +35,7 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where S: Collect, C: Collect, F: 'static")]
pub struct MapOkWith<S, C, F>(S, Option<(C, StaticCollect<F>)>);

impl<S, C, F> MapOkWith<S, C, F> {
@@ -66,7 +66,7 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where S: Collect, F: 'static")]
pub struct MapError<S, F>(S, Option<StaticCollect<F>>);

impl<S, F> MapError<S, F> {
4 changes: 2 additions & 2 deletions src/gc-sequence/src/sequence_fn.rs
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where F: 'static")]
pub struct SequenceFn<F>(Option<StaticCollect<F>>);

impl<F> SequenceFn<F> {
@@ -43,7 +43,7 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(no_drop, bound = "where C: Collect, F: 'static")]
pub struct SequenceFnWith<C, F>(Option<(C, StaticCollect<F>)>);

impl<C, F> SequenceFnWith<C, F> {
10 changes: 8 additions & 2 deletions src/gc-sequence/src/then.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,10 @@ use crate::Sequence;

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(
no_drop,
bound = "where <S as Sequence<'gc>>::Output: Collect, F: 'static"
)]
pub enum Then<'gc, S, F>
where
S: Sequence<'gc>,
@@ -49,7 +52,10 @@ where

#[must_use = "sequences do nothing unless stepped"]
#[derive(Debug, Collect)]
#[collect(no_drop)]
#[collect(
no_drop,
bound = "where C: Collect, <S as Sequence<'gc>>::Output: Collect, F: 'static"
)]
pub enum ThenWith<'gc, S, C, F>
where
S: Sequence<'gc>,