Skip to content

Commit

Permalink
Fix rustdoc, strict lints, and macro cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
Lee-Janggun committed Jun 26, 2023
1 parent 9b861e6 commit ab6779f
Show file tree
Hide file tree
Showing 29 changed files with 120 additions and 166 deletions.
9 changes: 4 additions & 5 deletions homework/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ check-loom = ["loom"]
arr_macro = "0.2.1"
cfg-if = "1.0.0"
crossbeam-channel = "0.5.8"
crossbeam-epoch = "0.9.14"
crossbeam-utils = "0.8.15"
ctrlc = "3.2.5"
crossbeam-epoch = "0.9.15"
crossbeam-utils = "0.8.16"
ctrlc = "3.4.0"
either = "1.8.1"
itertools = "0.10.5"
once_cell = "1.17.1"
cs431 = { git = "https://github.com/kaist-cp/cs431" }
# cs431 = { path = "../cs431" }
loom = { version = "0.5.6", optional = true }
rand = "0.8.5"
regex = "1.8.1"
regex = "1.8.4"
8 changes: 4 additions & 4 deletions homework/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Tips

- Read the paper and the skeleton code carefully. I'll ask questions on those in the exams.
- Read the paper and the skeleton code carefully. I'll ask questions about those in the exams.

- Read [the Rust book](https://doc.rust-lang.org/book/), especially the ["getting started"
section](https://doc.rust-lang.org/book/ch01-00-getting-started.html) for learning how to build
Expand Down Expand Up @@ -39,11 +39,11 @@
## Using LLVM Sanitizers

We are going to use the LLVM sanitizers for grading.
Sanitizers are dynamic analysis tools that detects buggy behaviors during runtime. For example,
Sanitizers are dynamic analysis tools that detect buggy behaviors during runtime. For example,
[AddressSanitizer](https://clang.llvm.org/docs/AddressSanitizer.html) detects memory bugs like use-after-free and
[ThreadSanitizer](https://clang.llvm.org/docs/ThreadSanitizer.html) detects data races.

You can run the tests with sanitizers using following commands:
You can run the tests with sanitizers using the following commands:
```
source scripts/grade-utils.sh
# This may take some time because of `rustup toolchain update stable nightly` in the script.
Expand All @@ -65,6 +65,6 @@ cargo_tsan SUBCOMMAND
cargo_tsan test --test growable_array
```

While (safe) Rust's type system guarantees memory safety and absence of data race,
While (safe) Rust's type system guarantees memory safety and the absence of data race,
this guarantee relies on the correctness of the libraries implemented with unsafe features.
Therefore tools like sanitizers are still essential when we use unsafe Rust.
2 changes: 1 addition & 1 deletion homework/rust-toolchain
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.69.0
1.70.0
2 changes: 1 addition & 1 deletion homework/src/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,6 @@ impl<T: fmt::Debug> fmt::Debug for Arc<T> {

impl<T> fmt::Pointer for Arc<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Pointer::fmt(&(&**self as *const T), f)
fmt::Pointer::fmt(&(&**self), f)
}
}
20 changes: 12 additions & 8 deletions homework/src/art/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use arr_macro::arr;
use itertools::izip;

/// The sentinel value for index.
pub const KEY_ENDMARK: u8 = 0xffu8;
pub const KEY_INVALID: u8 = 0xfeu8;
pub(crate) const KEY_ENDMARK: u8 = 0xffu8;
pub(crate) const KEY_INVALID: u8 = 0xfeu8;

/// The header of a node.
#[derive(Default, Debug, Clone)]
pub struct NodeHeader {
pub(crate) struct NodeHeader {
/// The length of the key fragment.
length: u8,
/// The key fragment of the node used for path compression optimization.
Expand Down Expand Up @@ -62,7 +62,7 @@ struct NodeBodyV<V> {
}

/// The trait for the body of an internal node.
pub trait NodeBodyI<V> {
pub(crate) trait NodeBodyI<V> {
/// Lookups the child of `key`.
///
/// Returns `Some((i, n))` if `n` is the child of `key` at the internal index `i`.
Expand Down Expand Up @@ -415,7 +415,11 @@ impl<V> NodeBox<V> {
/// # Panics
///
/// Panics if the number of `children` or `min_size` exceeds 256.
pub fn newi(header: NodeHeader, children: Vec<(u8, NodeBox<V>)>, min_size: usize) -> Self {
pub(crate) fn newi(
header: NodeHeader,
children: Vec<(u8, NodeBox<V>)>,
min_size: usize,
) -> Self {
let size = cmp::max(children.len(), min_size);
let mut node = if (0..=4).contains(&size) {
Self::new_inner_default::<NodeBody4<V>>(header, 0)
Expand All @@ -431,7 +435,7 @@ impl<V> NodeBox<V> {

let base = node.deref_mut().unwrap().1.left().unwrap();
for (i, c) in children.into_iter() {
base.update(i, c).map_err(|_| ()).unwrap();
let _unused = base.update(i, c).map_err(|_| ()).unwrap();
}

node
Expand Down Expand Up @@ -542,7 +546,7 @@ impl<V> NodeBox<V> {
/// an internal node and `body` is its body, or `Right(value)`, if it's a leaf node and `value`
/// is a reference to the leaf node's value.
#[allow(clippy::type_complexity)]
pub fn deref(&self) -> Option<(&NodeHeader, Either<&dyn NodeBodyI<V>, &V>)> {
pub(crate) fn deref(&self) -> Option<(&NodeHeader, Either<&dyn NodeBodyI<V>, &V>)> {
let ptr = self.inner & !TAG_MASK;
if ptr == 0 {
return None;
Expand Down Expand Up @@ -580,7 +584,7 @@ impl<V> NodeBox<V> {
///
/// See the comments for `Self::deref()`.
#[allow(clippy::type_complexity)]
pub fn deref_mut(
pub(crate) fn deref_mut(
&mut self,
) -> Option<(&mut NodeHeader, Either<&mut dyn NodeBodyI<V>, &mut V>)> {
let ptr = self.inner & !TAG_MASK;
Expand Down
16 changes: 8 additions & 8 deletions homework/src/bst/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ pub trait AtomicRW {
// because it's a generally accepted practice.
impl<T> AtomicRW for T {
unsafe fn atomic_write(&self, value: T) {
let this = self as *const _ as *mut _;
let this = (self as *const T).cast_mut();
ptr::write_volatile(this, value);
}

unsafe fn atomic_swap(&self, value: T) -> Self {
let this = self as *const _ as *mut _;
let this = (self as *const T).cast_mut();
let result = ptr::read_volatile(this);
ptr::write_volatile(this, value);
result
Expand All @@ -31,7 +31,7 @@ impl<T> AtomicRW for T {

/// Node's inner data protected by sequence lock.
#[derive(Debug)]
pub struct NodeInner<K: Ord, V> {
pub(crate) struct NodeInner<K: Ord, V> {
/// Value on the node. `None` means it's vacant.
pub(crate) value: Option<V>,

Expand All @@ -43,7 +43,7 @@ pub struct NodeInner<K: Ord, V> {
}

#[derive(Debug)]
pub struct Node<K: Ord, V> {
pub(crate) struct Node<K: Ord, V> {
/// Key on the node.
pub(crate) key: K,

Expand All @@ -53,7 +53,7 @@ pub struct Node<K: Ord, V> {

/// Direction (left or right).
#[derive(Debug, Clone, Copy)]
pub enum Dir {
pub(crate) enum Dir {
/// Left dirction.
L,

Expand Down Expand Up @@ -88,7 +88,7 @@ pub struct Bst<K: Ord, V> {

impl<K: Ord, V> NodeInner<K, V> {
#[inline]
pub fn child(&self, dir: Dir) -> &Atomic<Node<K, V>> {
pub(crate) fn child(&self, dir: Dir) -> &Atomic<Node<K, V>> {
match dir {
Dir::L => &self.left,
Dir::R => &self.right,
Expand All @@ -99,15 +99,15 @@ impl<K: Ord, V> NodeInner<K, V> {
impl Dir {
/// Returns the opposite direction.
#[inline]
pub fn opposite(self) -> Self {
pub(crate) fn opposite(self) -> Self {
match self {
Self::L => Self::R,
Self::R => Self::L,
}
}
}

impl<'g, K: Ord, V> Cursor<'g, K, V> {
impl<K: Ord, V> Cursor<'_, K, V> {
/// Checks if it is at the root.
pub fn is_root(&self) -> bool {
self.ancestors.is_empty()
Expand Down
6 changes: 3 additions & 3 deletions homework/src/elim_stack/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use crossbeam_epoch::{pin, Atomic, Guard, Owned};
use rand::{thread_rng, Rng};
use std::time;

pub const ELIM_SIZE: usize = 16;
pub const ELIM_DELAY: time::Duration = time::Duration::from_millis(10);
pub(crate) const ELIM_SIZE: usize = 16;
pub(crate) const ELIM_DELAY: time::Duration = time::Duration::from_millis(10);

#[inline]
pub fn get_random_elim_index() -> usize {
pub(crate) fn get_random_elim_index() -> usize {
thread_rng().gen::<usize>() % ELIM_SIZE
}

Expand Down
5 changes: 2 additions & 3 deletions homework/src/elim_stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ mod base;
mod elim;
mod treiber_stack;

pub use base::Stack;

/// Elimination-backoff stack based on Treiber's stack.
pub type ElimStack<T> = base::ElimStack<T, treiber_stack::TreiberStack<T>>;

#[cfg(test)]
mod test {
use super::*;
use base::Stack;
use std::thread::scope;

#[test]
Expand All @@ -20,7 +19,7 @@ mod test {

scope(|scope| {
for _ in 0..10 {
scope.spawn(|| {
let _unused = scope.spawn(|| {
for i in 0..10_000 {
stack.push(i);
assert!(stack.pop().is_some());
Expand Down
9 changes: 6 additions & 3 deletions homework/src/elim_stack/treiber_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ impl<T> Stack<T> for TreiberStack<T> {

fn try_pop(&self, guard: &Guard) -> Result<Option<T>, ()> {
let head = self.head.load(Ordering::Acquire, guard);
let head_ref = some_or!(unsafe { head.as_ref() }, return Ok(None));
let Some(head_ref) = (unsafe { head.as_ref() }) else {
return Ok(None);
};
let next = head_ref.next.load(Ordering::Relaxed, guard);

self.head
let _ = self
.head
.compare_exchange(head, next, Ordering::Relaxed, Ordering::Relaxed, guard)
.map_err(|_| ())?;

Expand Down Expand Up @@ -103,7 +106,7 @@ mod test {

scope(|scope| {
for _ in 0..10 {
scope.spawn(|| {
let _unused = scope.spawn(|| {
for i in 0..10_000 {
stack.push(i);
assert!(stack.pop().is_some());
Expand Down
4 changes: 2 additions & 2 deletions homework/src/hazard_pointer/hazard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ mod tests {
for data in VALUES {
let src = AtomicPtr::new(data as *mut ());
let shield = Shield::new(&hazard_bag);
shield.protect(&src);
let _ = shield.protect(&src);
// leak the shield so that it is not unprotected.
mem::forget(shield);
}
Expand All @@ -209,7 +209,7 @@ mod tests {
for data in VALUES {
let src = AtomicPtr::new(data as *mut ());
let shield = Shield::new(&hazard_bag);
shield.protect(&src);
let _ = shield.protect(&src);
}
})
})
Expand Down
2 changes: 1 addition & 1 deletion homework/src/hazard_pointer/retire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ mod tests {
struct Tester(Rc<RefCell<HashSet<usize>>>, usize);
impl Drop for Tester {
fn drop(&mut self) {
self.0.borrow_mut().insert(self.1);
let _ = self.0.borrow_mut().insert(self.1);
}
}
let hazards = HazardBag::new();
Expand Down
7 changes: 4 additions & 3 deletions homework/src/hello_server/handler.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Request handler with a cache.

use once_cell::sync::Lazy;
use regex::bytes::Regex;
use std::io::prelude::*;
use std::net::TcpStream;
use std::sync::Arc;
use std::sync::OnceLock;
use std::thread;
use std::time::Duration;

Expand Down Expand Up @@ -53,9 +53,10 @@ impl Handler {
let mut buf = [0; 512];
let _ = stream.read(&mut buf).unwrap();

static REQUEST_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"GET /(?P<key>\w+) HTTP/1.1\r\n").unwrap());
static REQUEST_REGEX: OnceLock<Regex> = OnceLock::<Regex>::new();

let key = REQUEST_REGEX
.get_or_init(|| Regex::new(r"GET /(?P<key>\w+) HTTP/1.1\r\n").unwrap())
.captures(&buf)
.and_then(|cap| cap.name("key"))
.map(|key| String::from_utf8_lossy(key.as_bytes()));
Expand Down
6 changes: 3 additions & 3 deletions homework/src/hello_server/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ impl CancellableTcpListener {

/// Returns an iterator over the connections being received on this listener. The returned
/// iterator will return `None` if the listener is `cancel`led.
pub fn incoming(&self) -> Incoming {
pub fn incoming(&self) -> Incoming<'_> {
Incoming { listener: self }
}
}

impl<'a> Iterator for Incoming<'a> {
impl Iterator for Incoming<'_> {
type Item = io::Result<TcpStream>;
/// Returns None if the listener is `cancel()`led.
fn next(&mut self) -> Option<io::Result<TcpStream>> {
let stream: io::Result<TcpStream> = self.listener.inner.accept().map(|p| p.0);
let stream = self.listener.inner.accept().map(|p| p.0);
todo!()
}
}
3 changes: 0 additions & 3 deletions homework/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
#![allow(unused_imports)]
#![allow(unused_mut)]

#[macro_use]
mod utils;

mod arc;
mod art;
mod bst;
Expand Down
4 changes: 2 additions & 2 deletions homework/src/linked_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct Node<T> {
///
/// [`iter`]: struct.LinkedList.html#method.iter
/// [`LinkedList`]: struct.LinkedList.html
pub struct Iter<'a, T: 'a> {
pub struct Iter<'a, T> {
head: *mut Node<T>,
tail: *mut Node<T>,
len: usize,
Expand All @@ -55,7 +55,7 @@ impl<T> Clone for Iter<'_, T> {
///
/// [`iter_mut`]: struct.LinkedList.html#method.iter_mut
/// [`LinkedList`]: struct.LinkedList.html
pub struct IterMut<'a, T: 'a> {
pub struct IterMut<'a, T> {
// We do *not* exclusively own the entire list here, references to node's `element`
// have been handed out by the iterator! So be careful when using this; the methods
// called must be aware that there can be aliasing pointers to `element`.
Expand Down
4 changes: 2 additions & 2 deletions homework/src/list_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<T> OrderedListSet<T> {
}

impl<T: Ord> OrderedListSet<T> {
fn find(&self, key: &T) -> (bool, Cursor<T>) {
fn find(&self, key: &T) -> (bool, Cursor<'_, T>) {
todo!()
}

Expand All @@ -76,7 +76,7 @@ pub struct Iter<'l, T>(Option<MutexGuard<'l, *mut Node<T>>>);

impl<T> OrderedListSet<T> {
/// An iterator visiting all elements.
pub fn iter(&self) -> Iter<T> {
pub fn iter(&self) -> Iter<'_, T> {
Iter(Some(self.head.lock().unwrap()))
}
}
Expand Down
21 changes: 0 additions & 21 deletions homework/src/utils.rs

This file was deleted.

Loading

0 comments on commit ab6779f

Please sign in to comment.