significant code quality improvement

Cargo.lock

@@ -4,4 +4,4 @@ version = 4
 
 [[package]]
 name = "microlock"
-version = "0.2.2"
+version = "0.3.0"

Cargo.toml

@@ -3,7 +3,7 @@ name = "microlock"
 description = "A crate for waiting: Small locks and other timing things!"
 license = "MIT"
 repository = "https://git.tudbut.de/tudbut/microlock"
-version = "0.2.2"
+version = "0.3.0"
 edition = "2021"
 
 [dependencies]

shell.nix

@@ -5,5 +5,6 @@ pkgs.mkShell {
     cargo
     rustc
     cargo-watch
+    clippy
   ];
 }

src/lib.rs

@@ -1,3 +1,4 @@
+#[deny(clippy::missing_const_for_fn)]
 mod timed;
 pub mod timer;
 mod untimed;

src/timed.rs

@@ -5,16 +5,6 @@ use crate::{
     Lock, UntimedLock,
 };
 
-struct TimedLockData {
-    /// The timer governing when the lock should unlock. This is None if it is
-    /// in untimed mode or if it is not locked.
-    time: Option<Timer>,
-    /// When calling lock(t) on an already locked lock, it will have to unlock
-    /// and relock the inner UntimedLock. This flag prevents exiting in such a
-    /// situation, so wait_here* is never exited prematurely.
-    remain_locked: bool,
-}
-
 /// A timed lock. This can be locked and unlocked, and it will unlock on
 /// its own after a timeout, if specified (see [`UntimedLock`] for a
 /// non-expiring lock). If a thread calls wait_here on a locked lock, it will
@@ -25,18 +15,28 @@ pub struct TimedLock {
     inner: UntimedLock,
     /// The data specifying how the lock is currently meant to act. (Most
     /// importantly, it contains the Timer)
-    data: Mutex<TimedLockData>,
+    timer: Mutex<Timer>,
 }
 
 impl TimedLock {
     /// Creates a new timed lock that is either unlocked or locked.
     pub const fn new(locked: bool) -> Self {
+        let duration = if locked {
+            TimerDuration::Infinite
+        } else {
+            TimerDuration::Elapsed
+        };
         Self {
             inner: UntimedLock::new(locked),
-            data: Mutex::new(TimedLockData {
-                time: None,
-                remain_locked: false,
-            }),
+            timer: Mutex::new(Timer::new_const(duration)),
+        }
+    }
+
+    /// Creates a new timed lock that is either unlocked or locked.
+    pub fn new_for(duration: TimerDuration) -> Self {
+        Self {
+            inner: UntimedLock::new(!duration.is_elapsed()),
+            timer: Mutex::new(Timer::new(duration)),
         }
     }
 
@@ -54,8 +54,8 @@ impl TimedLock {
     /// or if it is currently unlocked. The timer may already have elapsed
     /// if the lock wasn't unlocked explicitly and nobody was waiting on it
     /// to trigger an update.
-    pub fn get_timer(&self) -> Option<Timer> {
-        self.data.lock().unwrap().time
+    pub fn get_timer(&self) -> Timer {
+        *self.timer.lock().unwrap()
     }
 }
 
@@ -70,11 +70,7 @@ impl Timed for TimedLock {
         if !self.is_locked() {
             return TimerDuration::Elapsed;
         }
-        self.data
-            .lock()
-            .unwrap()
-            .time
-            .map_or(TimerDuration::Infinite, |x| x.time_left())
+        self.timer.lock().unwrap().time_left()
     }
 }
 
@@ -84,19 +80,19 @@ impl TimedLock {
     /// any waiting threads, even if relocking is necessary.
     pub fn lock_for(&self, duration: TimerDuration) {
         if duration == TimerDuration::Elapsed {
+            self.unlock();
             return;
         }
 
-        let mut data = self.data.lock().unwrap();
-        data.time = Some(Timer::new(duration));
-        data.remain_locked = true;
-        drop(data);
-        if self.is_locked() {
-            self.inner.unlock();
-        }
-        self.inner.lock()
+        *self.timer.lock().unwrap() = Timer::new(duration);
+
+        self.relock();
     }
 
+    /// Relocks (without waking up waiting threads) the lock with a different duration,
+    /// taken from the function given the current time left as the input.
+    ///
+    /// If the lock is unlocked, the function will instead be given `if_unlocked`.
     pub fn change_lock_time(
         &self,
         f: impl FnOnce(TimerDuration) -> TimerDuration,
@@ -107,20 +103,22 @@ impl TimedLock {
             return;
         }
 
-        let data = self.data.lock().unwrap();
+        let timer = self.timer.lock().unwrap();
 
-        let new_time = if let Some(old_time) = data.time {
-            let old_time = old_time.time_left();
-            f(old_time)
-        } else {
-            f(TimerDuration::Infinite)
-        };
+        let old_time = timer.time_left();
+        let new_time = f(old_time);
 
-        drop(data);
+        drop(timer);
 
         self.lock_for(new_time);
     }
 
+    /// Relocks (without waking up waiting threads) the lock with a duration that is
+    /// either the current lock duration (=> no change), or the given duration,
+    /// whichever is shorter.
+    ///
+    /// If the lock is unlocked, `lock_if_unlocked` decides whether to leave it
+    /// unlocked or lock it with the given duration.
     pub fn reduce_lock_time(&self, duration: TimerDuration, lock_if_unlocked: bool) {
         self.change_lock_time(
             |old_time| old_time.min(duration),
@@ -132,12 +130,26 @@ impl TimedLock {
         );
     }
 
+    /// Relocks (without waking up waiting threads) the lock with a duration that is
+    /// either the current lock duration (=> no change), or the given duration,
+    /// whichever is longer.
     pub fn increase_lock_time(&self, duration: TimerDuration) {
         self.change_lock_time(|old_time| old_time.max(duration), TimerDuration::Elapsed);
     }
 
     pub fn lock_for_ms(&self, duration_ms: u64) {
-        self.lock_for(TimerDuration::Real(Duration::from_millis(duration_ms)))
+        self.lock_for(TimerDuration::from(duration_ms))
+    }
+
+    /// Unlocks the inner untimed lock and then locks it again. This does *not* wake
+    /// up the threads waiting on this lock.
+    ///
+    /// If the lock is not currently locked, it will be.
+    fn relock(&self) {
+        if self.inner.is_locked() {
+            self.inner.unlock();
+        }
+        self.inner.lock();
     }
 }
 
@@ -146,31 +158,22 @@ impl Lock for TimedLock {
     /// returns that. This updating is NOT necessary to release waiting
     /// threads.
     fn is_locked(&self) -> bool {
-        let data = self.data.lock().unwrap();
-        if !self.inner.is_locked() {
+        let timer = self.timer.lock().unwrap();
+        if timer.has_elapsed() {
+            drop(timer);
+            self.unlock();
             return false;
         }
-        if let Some(time) = data.time {
-            if time.has_elapsed() {
-                drop(data);
-                self.unlock();
-                return false;
-            }
-        }
         true
     }
 
     fn lock(&self) {
-        let mut data = self.data.lock().unwrap();
-        data.time = None;
-        data.remain_locked = true;
-        self.inner.lock()
+        self.lock_for(TimerDuration::Infinite);
     }
 
     fn try_lock(&self) -> bool {
-        let mut data = self.data.lock().unwrap();
-        data.time = None;
-        data.remain_locked = true;
+        *self.timer.lock().unwrap() = Timer::new(TimerDuration::Infinite);
+
         if self.inner.is_locked() {
             self.inner.unlock();
         }
@@ -178,37 +181,22 @@ impl Lock for TimedLock {
     }
 
     fn unlock(&self) {
-        let mut data = self.data.lock().unwrap();
-        data.time = None;
-        data.remain_locked = false;
+        *self.timer.lock().unwrap() = Timer::new(TimerDuration::Elapsed);
         self.inner.unlock()
     }
 
     fn wait_here(&self) {
-        loop {
-            if !self.is_locked() || !self.data.lock().unwrap().remain_locked {
-                break;
-            }
-            if let Some(time) = self.get_timer() {
-                self.inner.wait_here_for(time.time_left());
-            } else {
-                self.inner.wait_here();
-            }
+        while self.is_locked() {
+            let time = self.timer.lock().unwrap().time_left();
+            self.inner.wait_here_for(time);
         }
     }
 
     fn wait_here_for(&self, timeout: TimerDuration) {
         let timer = Timer::new(timeout);
-        loop {
-            if let Some(time) = self.get_timer() {
-                self.inner
-                    .wait_here_for(time.time_left().min(timer.time_left()));
-            } else {
-                self.inner.wait_here_for(timer.time_left());
-            }
-            if !self.is_locked() || !self.data.lock().unwrap().remain_locked {
-                break;
-            }
+        while !timer.has_elapsed() && self.is_locked() {
+            let time = self.timer.lock().unwrap().time_left();
+            self.inner.wait_here_for(time.min(timer.time_left()));
         }
     }
 

src/timer.rs

@@ -45,6 +45,18 @@ impl TimerDuration {
             _ => Duration::new(0, 0),
         }
     }
+
+    pub const fn is_elapsed(&self) -> bool {
+        matches!(self, TimerDuration::Elapsed)
+    }
+
+    pub const fn is_real(&self) -> bool {
+        matches!(self, TimerDuration::Real(_))
+    }
+
+    pub const fn is_infinite(&self) -> bool {
+        matches!(self, TimerDuration::Infinite)
+    }
 }
 impl From<Duration> for TimerDuration {
     fn from(value: Duration) -> Self {
@@ -57,6 +69,17 @@ impl From<Duration> for TimerDuration {
         Self::Real(value)
     }
 }
+impl From<u64> for TimerDuration {
+    fn from(value: u64) -> Self {
+        if value == 0 {
+            return Self::Elapsed;
+        }
+        if value == u64::MAX {
+            return Self::Infinite;
+        }
+        Self::Real(Duration::from_millis(value))
+    }
+}
 impl PartialOrd for TimerDuration {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.cmp(other))
@@ -79,14 +102,21 @@ impl Ord for TimerDuration {
 #[derive(Clone, Copy)]
 /// Holds a start time and a TimerDuration and allows for the time elapsed
 /// and the time left to be queried. Timers are immutable.
-pub struct Timer(Instant, TimerDuration);
+pub struct Timer(Option<Instant>, TimerDuration);
 impl Timer {
     pub fn new(d: TimerDuration) -> Self {
-        Self(Instant::now(), d)
+        Self(Some(Instant::now()), d)
+    }
+
+    pub const fn new_const(d: TimerDuration) -> Self {
+        if d.is_real() {
+            panic!("TimerDuration is only const-compatible when Elapsed or Infinite.");
+        }
+        Self(None, d)
     }
 
     pub fn time_elapsed(&self) -> Duration {
-        self.0.elapsed()
+        self.0.unwrap_or_else(Instant::now).elapsed()
     }
 }
 
@@ -100,9 +130,18 @@ pub trait Timed {
 }
 impl Timed for Timer {
     fn has_elapsed(&self) -> bool {
-        self.0.elapsed() >= self.1.to_real()
+        if self.1.is_elapsed() {
+            return true;
+        }
+        if self.1.is_infinite() {
+            return false;
+        }
+        self.0.unwrap().elapsed() >= self.1.to_real()
     }
     fn time_left(&self) -> TimerDuration {
-        TimerDuration::from_difference(self.1, self.0.elapsed())
+        if !self.1.is_real() {
+            return self.1;
+        }
+        TimerDuration::from_difference(self.1, self.0.unwrap().elapsed())
     }
 }