Merge pull request #765 from Imberflur/safety

Safety

.config/nextest.toml

@@ -0,0 +1,3 @@
+[profile.default-miri]
+slow-timeout = { period = "30s", terminate-after = 1 }
+fail-fast = false

.github/workflows/ci.yml

@@ -25,9 +25,9 @@ jobs:
       fail-fast: true
       matrix:
         os: [macos-latest, windows-latest, ubuntu-latest]
-        toolchain: [stable, beta, nightly, 1.65.0]
+        toolchain: [stable, beta, nightly, 1.70.0]
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       # install the toolchain we are going to compile and test with
       - name: install ${{ matrix.toolchain }} toolchain
@@ -77,3 +77,18 @@ jobs:
 
       # - run: mdbook test -L ./target/debug/deps docs/book
       #   if: matrix.toolchain == 'stable' && matrix.os == 'ubuntu-latest'
+
+  miri:
+    name: "Miri"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install Miri
+        run: |
+          rustup toolchain install nightly --component miri
+          rustup override set nightly
+          cargo miri setup
+      - name: Install latest nextest release
+        uses: taiki-e/install-action@nextest
+      - name: Test with Miri
+        run: ./miri.sh

.github/workflows/deny.yml

@@ -17,8 +17,9 @@ jobs:
     # Prevent sudden announcement of a new advisory from failing ci:
     continue-on-error: ${{ matrix.checks == 'advisories' }}
 
+
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - uses: EmbarkStudios/cargo-deny-action@v1
         with:
           command: check ${{ matrix.checks }}

.rustfmt.toml

@@ -1,9 +1,8 @@
 hard_tabs = false
-imports_granularity = "Crate"
 reorder_impl_items = true
 use_field_init_shorthand = true
 use_try_shorthand = true
 format_code_in_doc_comments = true
 wrap_comments = true
-edition = "2018"
+edition = "2021"
 version = "Two"

CHANGELOG.md

@@ -1,13 +1,23 @@
+# Unreleased
+
+* MSRV to 1.70.0 ([#765])
+* Significant refactor of `Join` and related traits to alleviate soundness
+  issues. Includes introduction of a lending/streaming join via the `LendJoin`
+  trait which is the new common denominator implemented by joinable types.
+  ([#765])
+
+[#765]: https://github.com/amethyst/specs/pull/765
+
 # 0.19.0 (2023-06-10)
 
 * Bump MSRV to 1.65.0 ([#766])
 * Added index where entity deletion stopped to the error returned from `WorldExt::delete_entities` ([#766])
 * Fix bug where deleting an entity with the wrong generation could clear the components of an existing entity. ([#766])
-* Bump shred to version `0.14.1`, MSRV to 1.60.0 ([shred changelog][shred-changelog], [#756])
+* Bump shred to version `0.14.1`, MSRV to 1.60.0 ([shred changelog][shred_changelog], [#756])
 
+[shred_changelog]: https://github.com/amethyst/shred/blob/master/CHANGELOG.md#0141-2022-07-14
 [#756]: https://github.com/amethyst/specs/pull/756
 [#766]: https://github.com/amethyst/specs/pull/766
-[shred-changelog]: https://github.com/amethyst/shred/blob/6b754812e304cf6c63ba0364a82a7e0e5025aaa4/CHANGELOG.md#0140-2022-07-12
 
 # 0.18.0 (2022-07-02)
 

Cargo.toml

@@ -14,19 +14,20 @@ license = "MIT OR Apache-2.0"
 authors = ["slide-rs hackers"]
 include = ["/src", "/examples", "/benches", "/README.md", "/LICENSE-MIT", "/LICENSE-APACHE"]
 edition = "2021"
-rust-version = "1.65.0"
+rust-version = "1.70.0"
 
 # the `storage_cmp` and `storage_sparse` benches are called from `benches_main`
 autobenches = false
 
 [dependencies]
 ahash = "0.7.6"
 crossbeam-queue = "0.3"
-hibitset = { version = "0.6.3", default-features = false }
+hibitset = { version = "0.6.4", default-features = false }
 log = "0.4.8"
-shred = { version = "0.14.1", default-features = false }
+shred = { version = "0.15.0", default-features = false }
 shrev = "1.1.1"
 tuple_utils = "0.4.0"
+nougat = "0.2.3"
 
 rayon = { version = "1.5.1", optional = true }
 serde = { version = "1.0.104", optional = true, features = ["serde_derive"] }
@@ -40,7 +41,7 @@ uuid_entity = ["uuid", "serde"]
 stdweb = ["uuid/js"]
 storage-event-control = []
 derive = ["shred-derive", "specs-derive"]
-nightly = []
+nightly = ["shred/nightly"]
 
 shred-derive = ["shred/shred-derive"]
 
@@ -53,30 +54,31 @@ criterion = "0.3.1"
 ron = "0.7.1"
 rand = "0.8"
 serde_json = "1.0.48"
-shred = { version = "0.14.1", default-features = false, features = ["shred-derive"] }
+shred = { version = "0.15.0", default-features = false, features = ["shred-derive"] }
 specs-derive = { path = "specs-derive", version = "0.4.1" }
 
+[[example]]
+name = "async"
 [[example]]
 name = "basic"
-
 [[example]]
-name = "full"
-
+name = "bitset"
 [[example]]
 name = "cluster_bomb"
-
 [[example]]
-name = "bitset"
-
+name = "full"
 [[example]]
-name = "track"
-
+name = "lend_join"
+test = true
 [[example]]
 name = "ordered_track"
-
 [[example]]
 name = "saveload"
 required-features = ["serde"]
+[[example]]
+name = "slices"
+[[example]]
+name = "track"
 
 [[bench]]
 name = "benches_main"

README.md

@@ -35,7 +35,7 @@ Unlike most other ECS libraries out there, it provides
       other and you can use barriers to force several stages in system execution
 * high performance for real-world applications
 
-Minimum Rust version: 1.65
+Minimum Rust version: 1.70
 
 ## [Link to the book][book]
 

benches/parallel.rs

@@ -55,6 +55,7 @@ impl Component for Lifetime {
 }
 
 #[derive(Clone, Copy, Debug)]
+#[allow(dead_code)]
 struct Ball {
     radius: f32,
 }
@@ -64,6 +65,7 @@ impl Component for Ball {
 }
 
 #[derive(Clone, Copy, Debug)]
+#[allow(dead_code)]
 struct Rect {
     a: f32,
     b: f32,
@@ -91,6 +93,7 @@ impl Component for SpawnRequests {
 }
 
 #[derive(Clone, Copy, Debug)]
+#[allow(dead_code)]
 struct Collision {
     a: Entity,
     b: Entity,
@@ -102,6 +105,7 @@ impl Component for Collision {
 }
 
 #[derive(Clone, Copy, Debug)]
+#[allow(dead_code)]
 struct Room {
     inner_width: f32,
     inner_height: f32,

docs/tutorials/src/12_tracked.md

@@ -117,8 +117,8 @@ fetch the component as mutable if/when needed.
 for (entity, mut comp) in (&entities, &mut comps.restrict_mut()).join() {
     // Check whether this component should be modified, without fetching it as
     // mutable.
-    if comp.get_unchecked().condition < 5 {
-         let mut comp = comp.get_mut_unchecked();
+    if comp.get().condition < 5 {
+         let mut comp = comp.get_mut();
          // ...
     }
 }
@@ -137,4 +137,4 @@ simply call `storage.set_event_emission(true)`.
 
 _See
 [FlaggedStorage Doc](https://docs.rs/specs/latest/specs/struct.FlaggedStorage.html)
-for more into._
+for more into._

examples/lend_join.rs

@@ -0,0 +1,52 @@
+use specs::prelude::*;
+struct Pos(f32);
+
+impl Component for Pos {
+    type Storage = VecStorage<Self>;
+}
+
+fn main() {
+    let mut world = World::new();
+
+    world.register::<Pos>();
+
+    let entity0 = world.create_entity().with(Pos(0.0)).build();
+    world.create_entity().with(Pos(1.6)).build();
+    world.create_entity().with(Pos(5.4)).build();
+
+    let mut pos = world.write_storage::<Pos>();
+    let entities = world.entities();
+
+    // Unlike `join` the type return from `lend_join` does not implement
+    // `Iterator`. Instead, a `next` method is provided that only allows one
+    // element to be accessed at once.
+    let mut lending = (&mut pos).lend_join();
+
+    // We copy the value out here so the borrow of `lending` is released.
+    let a = lending.next().unwrap().0;
+    // Here we keep the reference from `lending.next()` alive, so `lending`
+    // remains exclusively borrowed for the lifetime of `b`.
+    let b = lending.next().unwrap();
+    // This right fails to compile since `b` is used below:
+    // let d = lending.next().unwrap();
+    b.0 = a;
+
+    // Items can be iterated with `while let` loop:
+    let mut lending = (&mut pos).lend_join();
+    while let Some(pos) = lending.next() {
+        pos.0 *= 1.5;
+    }
+
+    // A `for_each` method is also available:
+    (&mut pos).lend_join().for_each(|pos| {
+        pos.0 += 1.0;
+    });
+
+    // Finally, there is one bonus feature which `.join()` can't soundly provide.
+    let mut lending = (&mut pos).lend_join();
+    // That is, there is a method to get the joined result for a particular
+    // entity:
+    if let Some(pos) = lending.get(entity0, &entities) {
+        pos.0 += 5.0;
+    }
+}

examples/slices.rs

@@ -42,7 +42,7 @@ impl<'a> System<'a> for SysA {
         // Both the `Pos` and `Vel` components use `DefaultVecStorage`, which supports
         // `as_slice()` and `as_mut_slice()`. This lets us access components without
         // indirection.
-        let mut pos_slice = pos.as_mut_slice();
+        let pos_slice = pos.as_mut_slice();
         let vel_slice = vel.as_slice();
 
         // Note that an entity which has position but not velocity will still have

examples/track.rs

@@ -70,7 +70,7 @@ impl<'a> System<'a> for SysB {
     fn run(&mut self, (entities, mut tracked): Self::SystemData) {
         for (entity, mut restricted) in (&entities, &mut tracked.restrict_mut()).join() {
             if entity.id() % 2 == 0 {
-                let mut comp = restricted.get_mut_unchecked();
+                let comp = restricted.get_mut();
                 comp.0 += 1;
             }
         }

miri.sh

@@ -0,0 +1,31 @@
+#!/bin/bash
+#
+# Convenience script for running Miri, also the same one that the CI runs!
+
+set -e
+
+# use half the available threads since miri can be a bit memory hungry
+test_threads=$((($(nproc) - 1) / 2 + 1))
+echo using $test_threads threads
+
+# filters out long running tests
+filter='not (test(100k) | test(map_test::wrap) | test(map_test::insert_same_key) | test(=mixed_create_merge)| test(=par_join_many_entities_and_systems) | test(=stillborn_entities))'
+echo "using filter: \"$filter\""
+
+# Miri currently reports leaks in some tests so we disable that check
+# here (might be due to ptr-int-ptr in crossbeam-epoch so might be
+# resolved in future versions of that crate).
+MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-ignore-leaks" \
+    cargo +nightly miri nextest run \
+    -E "$filter" \
+    --test-threads="$test_threads" \
+    # use nocapture or run miri directly to see warnings from miri
+    #--nocapture
+
+# Run tests only available when parallel feature is disabled.
+MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-ignore-leaks" \
+    cargo +nightly miri nextest run \
+    --no-default-features \
+    -E "binary(no_parallel)" \
+    --test-threads="$test_threads"
+