🐛 ✨ transmit_all, fix UB if *Context::position was modified

stackline/src/context.rs

@@ -16,7 +16,7 @@ use super::*;
 // SAFETY: `pane[position].cell` is borrow mutably, while a pointer to the original Pane is kept;
 // thus, no other reference to `pane[position].cell` may be done
 pub struct UpdateContext<'a> {
-    pub position: (usize, usize),
+    position: (usize, usize),
     pane: *const Pane,
 
     phantom: PhantomData<&'a Pane>,
@@ -38,6 +38,11 @@ impl<'a> UpdateContext<'a> {
         Some((res, tile))
     }
 
+    #[inline]
+    pub fn position(&self) -> (usize, usize) {
+        self.position
+    }
+
     #[inline]
     pub fn signal<'b>(&'b self) -> Option<&'b Rc<Signal>> where 'a: 'b {
         let pane = unsafe { self.pane() };
@@ -87,7 +92,7 @@ impl<'a> UpdateContext<'a> {
 // SAFETY: this structures ensures that it has exlusive, mutable access to `∀x, pane[x].signal` and `pane.signals`.
 // Other parts of `pane` may be accessed and returned immutably.
 pub struct TransmitContext<'a> {
-    pub position: (usize, usize),
+    position: (usize, usize),
     pane: *mut Pane,
 
     phantom: PhantomData<&'a mut Pane>,
@@ -112,6 +117,11 @@ impl<'a> TransmitContext<'a> {
         Some((res, tile, signal))
     }
 
+    #[inline]
+    pub fn position(&self) -> (usize, usize) {
+        self.position
+    }
+
     /// Returns an immutable reference to the [tile](AnyTile) at `pos` in the current [Pane].
     /// Returns `None` if that tile does not exist.
     #[inline]
@@ -124,22 +134,12 @@ impl<'a> TransmitContext<'a> {
 
     /// Sends a signal to be stored in a cell (may be the current one), the signal overrides that of the other cell
     /// Returns true if the signal was stored in a cell, false otherwise
-    pub fn send<'b>(&'b self, pos: (usize, usize), signal: Signal) -> bool where 'a: 'b {
+    pub fn send<'b>(&'b self, pos: (usize, usize), signal: Signal) -> Option<Weak<Signal>> where 'a: 'b {
         // SAFETY: we do not return any reference to any data borrowed in this function
         // SAFETY: we only access `pane[pos].signal` and `pane.signals`
         let pane = unsafe { self.pane_mut() };
 
-        match pane.get_mut(pos) {
-            Some(ref mut tile) => {
-                if let Some(weak) = tile.set_signal(signal) {
-                    pane.signals.push(weak);
-                    true
-                } else {
-                    false
-                }
-            }
-            _ => false
-        }
+        pane.set_signal(pos, signal)
     }
 
     /// Returns `Some((position.x + Δx, position.y + Δy))` iff `(x + Δx, y + Δy)` is inside the pane
@@ -165,3 +165,45 @@ impl<'a> TransmitContext<'a> {
         &mut *self.pane
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_update_exclusivity() {
+        // Check that UpdateContext does not allow any other reference to `tiles[position].cell` to be made
+        let mut pane = Pane::empty(4, 4).unwrap();
+        let mut tile = FullTile::from(Wire::new(Orientation::Any));
+        tile.set_signal(Signal::empty((1, 2), Direction::Up)).unwrap();
+        *pane.get_mut((1, 2)).unwrap() = tile;
+
+        let (ctx, _tile) = UpdateContext::new(&mut pane, (1, 2)).unwrap();
+
+        assert_eq!(ctx.position(), (1, 2));
+
+        let tile_self: Option<&FullTile> = ctx.get((1, 2)); // The FullTile may not be read
+        assert!(tile_self.is_none());
+
+        assert!(ctx.signal().is_some()); // Our Signal may be read, though
+    }
+
+    #[test]
+    fn test_transmit() {
+        let mut pane = Pane::empty(4, 4).unwrap();
+        let mut tile = FullTile::from(Wire::new(Orientation::Any));
+        tile.set_signal(Signal::empty((1, 2), Direction::Up)).unwrap();
+        *pane.get_mut((1, 2)).unwrap() = tile;
+
+        let (ctx, _tile, _signal) = TransmitContext::new(&mut pane, (1, 2)).unwrap();
+
+        assert_eq!(ctx.position(), (1, 2));
+
+        let tile_self: Option<&AnyTile> = ctx.get((1, 2));
+        assert!(tile_self.is_some()); // We may read our AnyTile, as they are under an immutable borrow
+
+        // Check that the signal was dropped
+        std::mem::drop(ctx);
+        assert!(pane.get((1, 2)).unwrap().signal().is_none());
+    }
+}

stackline/src/lib.rs

@@ -1,86 +1,21 @@
 use std::num::NonZeroUsize;
 use std::rc::Weak;
 
+mod signal;
+pub use signal::*;
+
+mod pane;
+pub use pane::*;
+
 pub mod utils;
 use utils::*;
 
 pub mod tile;
 use tile::*;
 
-mod signal;
-pub use signal::*;
-
 pub mod context;
 use context::*;
 
 pub struct World {
     panes: Vec<Pane>,
 }
-
-pub struct Pane {
-    tiles: Vec<FullTile>,
-    width: NonZeroUsize,
-    height: NonZeroUsize,
-
-    signals: Vec<Weak<Signal>>,
-}
-
-impl Pane {
-    pub fn empty(width: usize, height: usize) -> Option<Self> {
-        // TODO: check that width * height is a valid usize
-
-        Some(Self {
-            width: width.try_into().ok()?,
-            height: height.try_into().ok()?,
-            tiles: vec![FullTile::default(); width * height],
-
-            signals: vec![],
-        })
-    }
-
-    /// Returns `Some((x + Δx, y + Δy))` iff `(x + Δx, y + Δy)` is inside the world
-    // SAFETY: this function may *not* access `self.signals`, `∀x, self.tiles[x].cell` or `∀x, self.tiles[x].signal`
-    #[inline]
-    pub fn offset(&self, position: (usize, usize), offset: (i8, i8)) -> Option<(usize, usize)> {
-        if offset.0 < 0 && (-offset.0) as usize > position.0
-            || offset.1 < 0 && (-offset.1) as usize > position.1
-        {
-            return None;
-        }
-
-        // TODO: check that position and position + offset are valid isize values
-        let new_pos = (
-            (position.0 as isize + offset.0 as isize) as usize,
-            (position.1 as isize + offset.1 as isize) as usize,
-        );
-
-        if new_pos.0 < self.width.get() && new_pos.1 < self.height.get() {
-            Some(new_pos)
-        } else {
-            None
-        }
-    }
-
-    #[inline]
-    pub fn get<'b>(&'b self, position: (usize, usize)) -> Option<&'b FullTile> {
-        if !self.in_bounds(position) {
-            return None;
-        }
-
-        self.tiles.get(position.1 * self.width.get() + position.0)
-    }
-
-    #[inline]
-    pub fn get_mut<'b>(&'b mut self, position: (usize, usize)) -> Option<&'b mut FullTile> {
-        if !self.in_bounds(position) {
-            return None;
-        }
-
-        self.tiles.get_mut(position.1 * self.width.get() + position.0)
-    }
-
-    #[inline]
-    pub fn in_bounds(&self, position: (usize, usize)) -> bool {
-        position.0 < self.width.get() && position.1 < self.height.get()
-    }
-}

stackline/src/pane.rs

@@ -0,0 +1,116 @@
+use super::*;
+
+#[derive(Debug)]
+pub struct Pane {
+    tiles: Vec<FullTile>,
+    width: NonZeroUsize,
+    height: NonZeroUsize,
+
+    signals: Vec<Weak<Signal>>,
+}
+
+impl Pane {
+    pub fn empty(width: usize, height: usize) -> Option<Self> {
+        // TODO: check that width * height is a valid usize
+        let length = width.checked_mul(height)?;
+
+        Some(Self {
+            width: width.try_into().ok()?,
+            height: height.try_into().ok()?,
+            tiles: vec![FullTile::default(); length],
+
+            signals: vec![],
+        })
+    }
+
+    /// Returns `Some((x + Δx, y + Δy))` iff `(x + Δx, y + Δy)` is inside the world
+    // SAFETY: this function may *not* access `self.signals`, `∀x, self.tiles[x].cell` or `∀x, self.tiles[x].signal`
+    #[inline]
+    pub fn offset(&self, position: (usize, usize), offset: (i8, i8)) -> Option<(usize, usize)> {
+        if offset.0 < 0 && (-offset.0) as usize > position.0
+            || offset.1 < 0 && (-offset.1) as usize > position.1
+        {
+            return None;
+        }
+
+        // TODO: check that position and position + offset are valid isize values
+        let new_pos = (
+            (position.0 as isize + offset.0 as isize) as usize,
+            (position.1 as isize + offset.1 as isize) as usize,
+        );
+
+        if new_pos.0 < self.width.get() && new_pos.1 < self.height.get() {
+            Some(new_pos)
+        } else {
+            None
+        }
+    }
+
+    // SAFETY: this function may not access `self.signals`, nor may it read the contents of `self.tiles[position]`
+    #[inline]
+    pub fn get<'b>(&'b self, position: (usize, usize)) -> Option<&'b FullTile> {
+        if !self.in_bounds(position) {
+            return None;
+        }
+
+        self.tiles.get(position.1 * self.width.get() + position.0)
+    }
+
+    #[inline]
+    pub fn get_mut<'b>(&'b mut self, position: (usize, usize)) -> Option<&'b mut FullTile> {
+        if !self.in_bounds(position) {
+            return None;
+        }
+
+        self.tiles.get_mut(position.1 * self.width.get() + position.0)
+    }
+
+    // SAFETY: may only access `self[pos].signal` and `self.signals`
+    #[inline]
+    pub fn set_signal(&mut self, position: (usize, usize), signal: Signal) -> Option<Weak<Signal>> {
+        let signal = self.get_mut(position)?.set_signal(signal)?;
+        self.signals.push(signal.clone());
+        Some(signal)
+    }
+
+    #[inline]
+    pub fn in_bounds(&self, position: (usize, usize)) -> bool {
+        position.0 < self.width.get() && position.1 < self.height.get()
+    }
+
+    #[inline]
+    pub fn update(&mut self, position: (usize, usize)) -> Option<()> {
+        let (ctx, tile) = UpdateContext::new(self, position)?;
+
+        tile.update(ctx);
+
+        Some(())
+    }
+
+    // TODO: update_all (requires FullTile::state)
+
+    #[inline]
+    pub fn transmit(&mut self, position: (usize, usize)) -> Option<()> {
+        let (ctx, tile, signal) = TransmitContext::new(self, position)?;
+
+        tile.transmit(signal, ctx);
+
+        Some(())
+    }
+
+    pub fn transmit_all(&mut self) {
+        // TODO: store a second buffer and perform swap reads
+        for signal in std::mem::replace(&mut self.signals, vec![]) {
+            if let Some(upgraded) = signal.upgrade() {
+                let position = upgraded.position();
+                let _ = self.transmit(position); // May return None if the signal was aliased
+            }
+        }
+    }
+
+    /// Returns an iterator over the tiles and their coordinates
+    #[inline]
+    pub fn tiles<'b>(&'b self) -> impl Iterator<Item=(usize, usize, &'b FullTile)> + 'b {
+        self.tiles.iter().enumerate().map(move |(i, v)| (i % self.width, i / self.width, v))
+    }
+}

stackline/src/signal.rs

@@ -2,13 +2,35 @@ use super::*;
 
 #[derive(Clone, Debug)]
 pub struct Signal {
-    pub direction: Direction,
+    direction: Direction,
+    position: (usize, usize),
 }
 
 impl Signal {
-    pub fn clone_with_dir(&self, direction: Direction) -> Self {
+    pub fn empty(position: (usize, usize), direction: Direction) -> Self {
+        Self {
+            direction,
+            position
+        }
+    }
+
+    pub fn clone_move(&self, direction: Direction) -> Option<Self> {
         let mut res = self.clone();
         res.direction = direction;
-        res
+
+        let (dx, dy) = direction.into_offset();
+
+        res.position.0 = (res.position.0 as isize + dx as isize).try_into().ok()?;
+        res.position.1 = (res.position.1 as isize + dy as isize).try_into().ok()?;
+
+        Some(res)
+    }
+
+    pub fn direction(&self) -> Direction {
+        self.direction
+    }
+
+    pub fn position(&self) -> (usize, usize) {
+        self.position
     }
 }

stackline/src/tile/mod.rs

@@ -47,39 +47,60 @@ impl FullTile {
     }
 
     /// Returns the internal state of this full tile
+    #[inline]
     pub fn get<'b>(&'b self) -> Option<&'b AnyTile> {
         self.cell.as_ref()
     }
 
     /// Returns the signal of this tile
+    #[inline]
     pub fn signal<'b>(&'b self) -> Option<&'b Rc<Signal>> {
         self.signal.as_ref()
     }
 
+    #[inline]
     pub(crate) fn take_signal(&mut self) -> Option<Rc<Signal>> {
         std::mem::take(&mut self.signal)
     }
 
+    #[inline]
     pub(crate) fn get_mut<'b>(&'b mut self) -> Option<&'b mut AnyTile> {
         self.cell.as_mut()
     }
 }
 
 impl Default for FullTile {
+    #[inline]
     fn default() -> Self {
         Self::new(None)
     }
 }
 
+impl<T: Tile + 'static> From<T> for FullTile {
+    #[inline]
+    fn from(tile: T) -> Self {
+        Self::new(Some(AnyTile::new(tile)))
+    }
+}
+
+impl From<()> for FullTile {
+    #[inline]
+    fn from(_empty: ()) -> Self {
+        Self::new(None)
+    }
+}
+
 pub trait Tile: DynClone + std::fmt::Debug {
     /// Function to be called when the tile needs to update its internal state.
     /// During the "update" phase, the tile may access its signal and the other tiles immutably.
+    #[inline]
     fn update<'b>(&'b mut self, _context: UpdateContext<'b>) {}
 
     /// Function that will be called if the tile has a signal.
     fn transmit<'b>(&'b self, signal: Rc<Signal>, context: TransmitContext<'b>);
 
     /// Should return true iff the tile accepts a signal travelling in `Direction`
+    #[inline]
     fn accepts_signal(&self, _direction: Direction) -> bool {
         true
     }
@@ -89,17 +110,85 @@ pub trait Tile: DynClone + std::fmt::Debug {
 pub struct AnyTile(Box<dyn Tile>);
 
 impl AnyTile {
-    fn update<'b>(&'b mut self, ctx: UpdateContext<'b>) {
+    #[inline]
+    pub fn new<T: Tile + 'static>(tile: T) -> Self {
+        Self(Box::new(tile))
+    }
+
+    #[inline]
+    pub fn update<'b>(&'b mut self, ctx: UpdateContext<'b>) {
         self.0.update(ctx)
     }
 
-    fn accepts_signal(&self, direction: Direction) -> bool {
+    #[inline]
+    pub fn transmit<'b>(&'b self, signal: Rc<Signal>, context: TransmitContext<'b>) {
+        self.0.transmit(signal, context)
+    }
+
+    #[inline]
+    pub fn accepts_signal(&self, direction: Direction) -> bool {
         self.0.accepts_signal(direction)
     }
 }
 
 impl Clone for AnyTile {
+    #[inline]
     fn clone(&self) -> Self {
         Self(clone_box(self.0.as_ref()))
     }
 }
+
+#[cfg(test)]
+mod crate_macros {
+    #[macro_export]
+    macro_rules! test_tile_setup {
+        ( $width:expr, $height:expr, [ $( $x:expr ),* ] ) => {{
+            assert!($width > 0);
+            assert!($height > 0);
+            let mut pane = Pane::empty($width, $height).unwrap();
+            let mut index = 0;
+
+            $(
+                {
+                    let x = index % $width;
+                    let y = index / $width;
+                    *pane.get_mut((x, y)).unwrap() = FullTile::from($x);
+                    index += 1;
+                }
+            )*
+
+            assert!(index == $width * $height);
+
+            pane
+        }}
+    }
+
+    #[macro_export]
+    macro_rules! test_set_signal {
+        ( $pane:expr, $pos:expr, $dir:expr ) => {
+            $pane.set_signal($pos, Signal::empty($pos, $dir)).unwrap();
+        }
+    }
+
+    #[macro_export]
+    macro_rules! assert_signal {
+        ( $pane:expr, $pos:expr ) => {{
+            let signal = $pane.get($pos).expect(&format!("Couldn't get tile at {:?}", $pos)).signal();
+            assert!(signal.is_some());
+            signal
+        }};
+
+        ( $pane:expr, $pos:expr, [ $( $data:expr ),* ] ) => {{
+            let signal = assert_signal!($pane, $pos);
+            // TODO: check that signal.data == data
+        }}
+    }
+
+    #[macro_export]
+    macro_rules! assert_no_signal {
+        ( $pane:expr, $pos:expr) => {{
+            let signal = $pane.get($pos).expect(&format!("Couldn't get tile at {:?}", $pos)).signal();
+            assert!(signal.is_none());
+        }}
+    }
+}

stackline/src/tile/wire.rs

@@ -5,17 +5,23 @@ use super::*;
 #[derive(Clone, Debug)]
 pub struct Wire(Orientation);
 
+impl Wire {
+    pub fn new(orientation: Orientation) -> Self {
+        Self(orientation)
+    }
+}
+
 impl Tile for Wire {
     fn transmit<'b>(&'b self, signal: Rc<Signal>, context: TransmitContext<'b>) {
         for &direction in self.0.into_directions() {
-            if direction == signal.direction.opposite() {
+            if direction == signal.direction().opposite() {
                 continue;
             }
 
             if let Some(new_pos) = context.offset(direction.into_offset()) {
                 let tile = context.get(new_pos);
                 if tile.map(|t| t.accepts_signal(direction)).unwrap_or(false) {
-                    context.send(new_pos, (*signal).clone_with_dir(direction));
+                    context.send(new_pos, signal.clone_move(direction).unwrap());
                 }
             }
         }
@@ -29,6 +35,12 @@ impl Tile for Wire {
 #[derive(Clone, Debug)]
 pub struct Diode(Direction);
 
+impl Diode {
+    pub fn new(direction: Direction) -> Self {
+        Self(direction)
+    }
+}
+
 // impl Tile for Diode {
 //     fn update(&mut self, world: &mut World, signal: &mut Option<Rc<Signal>>, pos: (usize, usize)) {
 //         if let Some(signal) = std::mem::take(signal) {
@@ -41,3 +53,57 @@ pub struct Diode(Direction);
 //         }
 //     }
 // }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_wire_transmit() {
+        use crate::Orientation::*;
+
+        let mut pane = test_tile_setup!(3, 2, [
+            Wire::new(Horizontal), Wire::new(Any), Wire::new(Horizontal),
+            (), Wire::new(Vertical), ()
+        ]);
+
+        // Test the signal going from left to right
+        test_set_signal!(pane, (0, 0), Direction::Right);
+
+        pane.transmit_all();
+
+        assert_signal!(pane, (1, 0));
+        assert_no_signal!(pane, (0, 0));
+        assert_no_signal!(pane, (2, 0));
+        assert_no_signal!(pane, (1, 1));
+
+        pane.transmit_all();
+
+        assert_signal!(pane, (2, 0));
+        assert_signal!(pane, (1, 1));
+        assert_no_signal!(pane, (0, 0));
+        assert_no_signal!(pane, (1, 0));
+
+        pane.transmit_all();
+        for (_, _, tile) in pane.tiles() {
+            assert!(tile.signal().is_none());
+        }
+
+        // Test the signal going from right to left
+        test_set_signal!(pane, (2, 0), Direction::Left);
+
+        pane.transmit_all();
+
+        assert_signal!(pane, (1, 0));
+        assert_no_signal!(pane, (0, 0));
+        assert_no_signal!(pane, (2, 0));
+        assert_no_signal!(pane, (1, 1));
+
+        pane.transmit_all();
+
+        assert_signal!(pane, (0, 0));
+        assert_signal!(pane, (1, 1));
+        assert_no_signal!(pane, (2, 0));
+        assert_no_signal!(pane, (1, 0));
+    }
+}