Skip to content

Commit

Permalink
Merge pull request #207 from Anders429/sync
Browse files Browse the repository at this point in the history
Require components viewed in `Schedule` to implement `Sync`.
  • Loading branch information
Anders429 authored Apr 14, 2023
2 parents 65e1d95 + ddcaebc commit cb3a526
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
- `Entry::query()` no longer requires `V` and `F` to implement `Filter`.
- `system::schedule::Schedule` now only requires a single parameter for indices.
- `World::run_system()` now only requires a single parameter for schedule indices.
### Fixed
- `Schedule`s can now no longer access non-`Sync` components.

## 0.8.2 - 2023-04-02
### Fixed
Expand Down
36 changes: 36 additions & 0 deletions src/system/schedule/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,4 +1149,40 @@ mod tests {
)>()
);
}

/// This test verifies that the schedule compiles just fine even when the registry contains a
/// non-`Sync` component, so long as the schedule itself doesn't borrow said component.
#[test]
fn registry_contains_non_sync_component() {
struct Foo;

impl System for Foo {
type Views<'a> = Views!();
type Filter = filter::None;
type ResourceViews<'a> = Views!();
type EntryViews<'a> = Views!();

fn run<'a, R, S, I, E>(
&mut self,
_query_results: Result<R, S, I, Self::ResourceViews<'a>, Self::EntryViews<'a>, E>,
) where
R: registry::Registry,
I: Iterator<Item = Self::Views<'a>>,
{
unimplemented!()
}
}

assert_eq!(
TypeId::of::<
<(task::System<Foo>, task::Null) as Schedule<
'_,
(alloc::rc::Rc<A>, Registry),
Resources!(),
_,
>>::Stages,
>(),
TypeId::of::<((&mut task::System<Foo>, stage::Null), stages::Null)>()
);
}
}
4 changes: 3 additions & 1 deletion src/system/schedule/task/sealed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ where
R: Registry,
{
/// The components viewed by this task.
type Views: Views<'a>;
type Views: Views<'a> + Send;
/// A filter applied to the components viewed by this task.
type Filter;

Expand All @@ -44,6 +44,7 @@ where
+ registry::ContainsViews<'a, S::EntryViews<'a>, EntryIndices>,
Resources: 'a,
Resources: ContainsViews<'a, S::ResourceViews<'a>, ResourceViewsIndices>,
S::Views<'a>: Send,
S::EntryViews<'a>: view::Disjoint<S::Views<'a>, R, DisjointIndices> + Views<'a>,
{
type Views = S::Views<'a>;
Expand All @@ -68,6 +69,7 @@ where
+ registry::ContainsViews<'a, P::EntryViews<'a>, EntryIndices>,
Resources: 'a,
Resources: ContainsViews<'a, P::ResourceViews<'a>, ResourceViewsIndices>,
P::Views<'a>: Send,
P::EntryViews<'a>: view::Disjoint<P::Views<'a>, R, DisjointIndices> + Views<'a>,
{
type Views = P::Views<'a>;
Expand Down
36 changes: 36 additions & 0 deletions tests/trybuild/schedule/non_send_views.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use brood::{query::{Views, filter, Result}, registry, World, Registry, system::{System, schedule}};
// This import is technically unused, since the macro fails to compile before it would be consumed.
// I'm leaving it here, though, for completeness; user code would use this module, and these tests
// should do their best to simulate user code.
#[allow(unused_imports)]
use brood::system::schedule::task;
use std::rc::Rc;

struct A;

struct Foo;

impl System for Foo {
type Views<'a> = Views!(&'a Rc<A>);
type Filter = filter::None;
type ResourceViews<'a> = Views!();
type EntryViews<'a> = Views!();

fn run<'a, R, S, I, E>(
&mut self,
_query_results: Result<R, S, I, Self::ResourceViews<'a>, Self::EntryViews<'a>, E>,
) where
R: registry::Registry,
I: Iterator<Item = Self::Views<'a>>,
{
unimplemented!()
}
}

fn main() {
let mut world = World::<Registry!(Rc<A>)>::new();

let schedule = schedule!(task::System(Foo));

world.run_schedule(&mut schedule);
}
21 changes: 21 additions & 0 deletions tests/trybuild/schedule/non_send_views.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0277]: `Rc<A>` cannot be shared between threads safely
--> tests/trybuild/schedule/non_send_views.rs:35:24
|
35 | world.run_schedule(&mut schedule);
| ------------ ^^^^^^^^^^^^^ `Rc<A>` cannot be shared between threads safely
| |
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `Rc<A>`
= note: required for `&Rc<A>` to implement `Send`
= note: required because it appears within the type `(&Rc<A>, Null)`
= note: required for `brood::system::schedule::task::System<Foo>` to implement `brood::system::schedule::task::sealed::Task<'_, (Rc<A>, brood::registry::Null), brood::resource::Null, (registry::contains::Null, (registry::contains::Contained, registry::contains::Null), (registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), (resource::contains::Null, resource::contains::Null, resource::contains::Null, resource::contains::Null), (((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), view::disjoint::Null, ((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), view::disjoint::Null), ((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null)>`
= note: required for `(brood::system::schedule::task::System<Foo>, brood::system::schedule::task::Null)` to implement `schedule::stager::Cutoff<'_, (Rc<A>, brood::registry::Null), brood::resource::Null, schedule::claim::decision::Append, ((&Rc<A>, brood::query::view::Null), schedule::claim::Null), schedule::stager::Null, schedule::stager::Null, schedule::stager::Null, schedule::stager::Null, (brood::query::view::Null, schedule::claim::Null), schedule::stager::Null, schedule::stager::Null, schedule::stager::Null, ((registry::contains::Null, (registry::contains::Contained, registry::contains::Null), (registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), schedule::stage::Null), ((resource::contains::Null, resource::contains::Null, resource::contains::Null, resource::contains::Null), schedule::stage::Null), ((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), view::disjoint::Null, ((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), view::disjoint::Null), schedule::stage::Null), (((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), schedule::stage::Null)>`
= note: required for `(brood::system::schedule::task::System<Foo>, brood::system::schedule::task::Null)` to implement `schedule::scheduler::Scheduler<'_, (Rc<A>, brood::registry::Null), brood::resource::Null, ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), (((((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), ((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), (view::merge::Neither, (view::merge::Left, view::merge::Null))), schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), (((registry::contains::Null, (registry::contains::Contained, registry::contains::Null), (registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), schedule::stage::Null), schedule::stages::Null), (((resource::contains::Null, resource::contains::Null, resource::contains::Null, resource::contains::Null), schedule::stage::Null), schedule::stages::Null), (((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), view::disjoint::Null, ((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), view::disjoint::Null), schedule::stage::Null), schedule::stages::Null), ((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), schedule::stage::Null), schedule::stages::Null)>`
= note: required for `(brood::system::schedule::task::System<Foo>, brood::system::schedule::task::Null)` to implement `schedule::sealed::Sealed<'_, (Rc<A>, brood::registry::Null), brood::resource::Null, (((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), (((((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), ((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), (view::merge::Neither, (view::merge::Left, view::merge::Null))), schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), (((registry::contains::Null, (registry::contains::Contained, registry::contains::Null), (registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), schedule::stage::Null), schedule::stages::Null), (((resource::contains::Null, resource::contains::Null, resource::contains::Null, resource::contains::Null), schedule::stage::Null), schedule::stages::Null), (((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), view::disjoint::Null, ((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), view::disjoint::Null), schedule::stage::Null), schedule::stages::Null), ((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), schedule::stage::Null), schedule::stages::Null))>`
= note: required for `(brood::system::schedule::task::System<Foo>, brood::system::schedule::task::Null)` to implement `Schedule<'_, (Rc<A>, brood::registry::Null), brood::resource::Null, (((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::scheduler::Null), (((((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), ((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), (view::merge::Neither, (view::merge::Left, view::merge::Null))), schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), ((schedule::claim::Null, schedule::stager::Null), schedule::stages::Null), (((registry::contains::Null, (registry::contains::Contained, registry::contains::Null), (registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), schedule::stage::Null), schedule::stages::Null), (((resource::contains::Null, resource::contains::Null, resource::contains::Null, resource::contains::Null), schedule::stage::Null), schedule::stages::Null), (((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), view::disjoint::Null, ((registry::contains::NotContained, (&registry::contains::Contained, registry::contains::Null)), (brood::query::result::get::Index, registry::contains::Null), (brood::query::result::get::Index, brood::query::result::reshape::Null)), view::disjoint::Null), schedule::stage::Null), schedule::stages::Null), ((((registry::contains::NotContained, (registry::contains::NotContained, registry::contains::Null)), registry::contains::Null, brood::query::result::reshape::Null), schedule::stage::Null), schedule::stages::Null))>`
note: required by a bound in `World::<R, Resources>::run_schedule`
--> src/world/mod.rs
|
| S: Schedule<'a, R, Resources, Indices>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `World::<R, Resources>::run_schedule`

0 comments on commit cb3a526

Please sign in to comment.