Skip to content

Commit

Permalink
Fix up Clippy issues and do some manual cleanups (#255)
Browse files Browse the repository at this point in the history
* Apply `cargo clippy --fix` and `cargo fmt`

Clippy has to be run twice to reproduce these results (to cover some
spots that get hit by one lint and then another).

* Fix a few more things that `cargo clippy --fix` doesn't cover

`cargo clippy` now reports no warnings on the latest stable Rust
version (1.78.0).

* Use Rust 1.78.0 (current latest stable) in CI

This updates the version for formatting, Clippy, and docs while not
touching the main testing version (to continue checking compiler
compatibility against the older version).

* Bump Cargo.lock after 1.0.1 bump
  • Loading branch information
dzhu committed May 13, 2024
1 parent 977fc4b commit e29d260
Show file tree
Hide file tree
Showing 25 changed files with 68 additions and 87 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/native.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
- name: Install Rust
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: 1.74.0
toolchain: 1.78.0
components: rustfmt
- name: Cargo fmt check
run: cargo fmt --all -- --check
Expand Down Expand Up @@ -152,7 +152,7 @@ jobs:
- name: Install Rust
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: 1.74.0
toolchain: 1.78.0
components: clippy
- name: Cargo Clippy
run: cargo clippy -- -D warnings
Expand All @@ -168,7 +168,7 @@ jobs:
- name: Install Rust
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: 1.74.0
toolchain: 1.78.0
- name: Cargo doc`
run: cargo doc --all --no-deps

Expand Down
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions rustfst-ffi/src/algorithms/compose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub unsafe extern "C" fn fst_matcher_config_destroy(
return Ok(());
}

unsafe { drop(Box::from_raw(ptr)) }
drop(unsafe { Box::from_raw(ptr) });
Ok(())
})
}
Expand All @@ -296,7 +296,7 @@ pub unsafe extern "C" fn fst_compose_config_destroy(
return Ok(());
}

unsafe { drop(Box::from_raw(ptr)) }
drop(unsafe { Box::from_raw(ptr) });
Ok(())
})
}
Expand Down
2 changes: 1 addition & 1 deletion rustfst-ffi/src/fst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ pub unsafe fn fst_destroy(fst_ptr: *mut CFst) -> RUSTFST_FFI_RESULT {
return Ok(());
}

unsafe { drop(Box::from_raw(fst_ptr)) }
drop(unsafe { Box::from_raw(fst_ptr) });
Ok(())
})
}
6 changes: 3 additions & 3 deletions rustfst-ffi/src/iterators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub unsafe extern "C" fn trs_iterator_destroy(iter_ptr: *mut CTrsIterator) -> RU
return Ok(());
}

unsafe { drop(Box::from_raw(iter_ptr)) }
drop(unsafe { Box::from_raw(iter_ptr) });
Ok(())
})
}
Expand Down Expand Up @@ -313,7 +313,7 @@ pub unsafe extern "C" fn mut_trs_iterator_destroy(
return Ok(());
}

unsafe { drop(Box::from_raw(iter_ptr)) }
drop(unsafe { Box::from_raw(iter_ptr) });
Ok(())
})
}
Expand Down Expand Up @@ -384,7 +384,7 @@ pub unsafe extern "C" fn state_iterator_destroy(
return Ok(());
}

unsafe { drop(Box::from_raw(iter_ptr)) }
drop(unsafe { Box::from_raw(iter_ptr) });
Ok(())
})
}
2 changes: 1 addition & 1 deletion rustfst-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub enum RUSTFST_FFI_RESULT {
}

thread_local! {
pub(crate) static LAST_ERROR: RefCell<Option<String>> = RefCell::new(None);
pub(crate) static LAST_ERROR: RefCell<Option<String>> = const { RefCell::new(None) };
}

pub fn wrap<F: FnOnce() -> Result<()>>(func: F) -> RUSTFST_FFI_RESULT {
Expand Down
2 changes: 1 addition & 1 deletion rustfst-ffi/src/string_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub unsafe extern "C" fn string_path_destroy(iter_ptr: *mut CStringPath) -> RUST
return Ok(());
}

unsafe { drop(Box::from_raw(iter_ptr)) }
drop(unsafe { Box::from_raw(iter_ptr) });
Ok(())
})
}
Expand Down
2 changes: 1 addition & 1 deletion rustfst-ffi/src/string_paths_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub unsafe extern "C" fn string_paths_iterator_destroy(
return Ok(());
}

unsafe { drop(Box::from_raw(iter_ptr)) }
drop(unsafe { Box::from_raw(iter_ptr) });
Ok(())
})
}
2 changes: 1 addition & 1 deletion rustfst-ffi/src/symbol_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ pub unsafe extern "C" fn symt_destroy(symt_ptr: *mut CSymbolTable) -> RUSTFST_FF
return Ok(());
}

unsafe { drop(Box::from_raw(symt_ptr)) }
drop(unsafe { Box::from_raw(symt_ptr) });
Ok(())
})
}
2 changes: 1 addition & 1 deletion rustfst-ffi/src/tr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ pub unsafe extern "C" fn tr_delete(tr_ptr: *mut CTr) -> RUSTFST_FFI_RESULT {
return Ok(());
}

unsafe { drop(Box::from_raw(tr_ptr)) }
drop(unsafe { Box::from_raw(tr_ptr) });
Ok(())
})
}
2 changes: 1 addition & 1 deletion rustfst-ffi/src/trs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pub unsafe extern "C" fn trs_vec_delete(trs_ptr: *mut CTrs) -> RUSTFST_FFI_RESUL
return Ok(());
}

unsafe { drop(Box::from_raw(trs_ptr)) }
drop(unsafe { Box::from_raw(trs_ptr) });
Ok(())
})
}
10 changes: 5 additions & 5 deletions rustfst/src/algorithms/connect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,13 @@ impl<'a, W: Semiring, F: 'a + ExpandedFst<W>> Visitor<'a, W, F> for ConnectVisit
}
}
}
if let Some(_p) = parent {
let _p = _p as usize;
if let Some(p) = parent {
let p = p as usize;
if self.coaccess[s] {
self.coaccess[_p] = true;
self.coaccess[p] = true;
}
if self.lowlink[s] < self.lowlink[_p] {
self.lowlink[_p] = self.lowlink[s];
if self.lowlink[s] < self.lowlink[p] {
self.lowlink[p] = self.lowlink[s];
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions rustfst/src/algorithms/minimize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,10 +602,7 @@ impl TrIterCompare {
&self,
x: &TrsIterCollected<W, T>,
y: &TrsIterCollected<W, T>,
) -> bool
where
W: Semiring,
{
) -> bool {
let xarc = x.value().unwrap();
let yarc = y.value().unwrap();
xarc.ilabel > yarc.ilabel
Expand Down
4 changes: 2 additions & 2 deletions rustfst/src/algorithms/queues/fifo_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ mod test {

queue.enqueue(2);
queue.enqueue(3);
assert_eq!(queue.is_empty(), false);
assert!(!queue.is_empty());
assert_eq!(queue.head(), Some(2));
queue.clear();
assert_eq!(queue.head(), None);
assert_eq!(queue.is_empty(), true);
assert!(queue.is_empty());

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions rustfst/src/algorithms/queues/lifo_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ mod test {

queue.enqueue(2);
queue.enqueue(3);
assert_eq!(queue.is_empty(), false);
assert!(!queue.is_empty());
assert_eq!(queue.head(), Some(3));
queue.clear();
assert_eq!(queue.head(), None);
assert_eq!(queue.is_empty(), true);
assert!(queue.is_empty());

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions rustfst/src/algorithms/queues/trivial_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ mod test {

queue.enqueue(2);
queue.enqueue(3);
assert_eq!(queue.is_empty(), false);
assert!(!queue.is_empty());
assert_eq!(queue.head(), Some(3));
queue.clear();
assert_eq!(queue.head(), None);
assert_eq!(queue.is_empty(), true);
assert!(queue.is_empty());
Ok(())
}
}
6 changes: 1 addition & 5 deletions rustfst/src/algorithms/shortest_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,6 @@ where
heap.push(final_state);

let weight_threshold = W::zero();
let state_threshold: Option<StateId> = None;
let limit = distance[istart as usize].times(weight_threshold)?;

let mut r = vec![];
Expand All @@ -464,10 +463,7 @@ where
} else {
W::one()
};
#[allow(clippy::unnecessary_literal_unwrap)]
if natural_less(&limit, &d.times(&p.1)?)?
|| (state_threshold.is_some() && ofst.num_states() >= state_threshold.unwrap() as usize)
{
if natural_less(&limit, &d.times(&p.1)?)? {
continue;
}

Expand Down
1 change: 1 addition & 0 deletions rustfst/src/algorithms/state_sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{StateId, Trs};
/// `order[i]` gives the the state ID after sorting that corresponds
/// to the state ID i before sorting; it must therefore be a
/// permutation of the input FST's states ID sequence.
#[allow(clippy::assigning_clones)]
pub fn state_sort<W, F>(fst: &mut F, order: &[StateId]) -> Result<()>
where
W: Semiring,
Expand Down
8 changes: 0 additions & 8 deletions rustfst/src/fst_impls/vector_fst/serializable_fst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,6 @@ impl<W: SerializableSemiring> SerializableFst<W> for VectorFst<W> {

static VECTOR_MIN_FILE_VERSION: i32 = 2;

#[derive(Debug, PartialEq)]
struct Transition {
ilabel: i32,
olabel: i32,
weight: f32,
nextstate: i32,
}

fn parse_vector_fst_state<W: SerializableSemiring>(
i: &[u8],
) -> IResult<&[u8], VectorFstState<W>, NomCustomError<&[u8]>> {
Expand Down
6 changes: 1 addition & 5 deletions rustfst/src/fst_traits/paths_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,7 @@ mod tests {

let mut paths_ref = Counter::new();
paths_ref.update(vec![FstPath::new(vec![], vec![], IntegerWeight::new(38))]);
paths_ref.update(vec![FstPath::new(
vec![1],
vec![1],
IntegerWeight::new(1 * 41),
)]);
paths_ref.update(vec![FstPath::new(vec![1], vec![1], IntegerWeight::new(41))]);
paths_ref.update(vec![FstPath::new(
vec![2],
vec![2],
Expand Down
16 changes: 8 additions & 8 deletions rustfst/src/semirings/boolean_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,16 @@ mod tests {
let b_false = BooleanWeight::new(false);

// Test plus
assert_eq!(b_true.plus(&b_true)?, b_true);
assert_eq!(b_true.plus(&b_false)?, b_true);
assert_eq!(b_false.plus(&b_true)?, b_true);
assert_eq!(b_false.plus(&b_false)?, b_false);
assert_eq!(b_true.plus(b_true)?, b_true);
assert_eq!(b_true.plus(b_false)?, b_true);
assert_eq!(b_false.plus(b_true)?, b_true);
assert_eq!(b_false.plus(b_false)?, b_false);

// Test times
assert_eq!(b_true.times(&b_true)?, b_true);
assert_eq!(b_true.times(&b_false)?, b_false);
assert_eq!(b_false.times(&b_true)?, b_false);
assert_eq!(b_false.times(&b_false)?, b_false);
assert_eq!(b_true.times(b_true)?, b_true);
assert_eq!(b_true.times(b_false)?, b_false);
assert_eq!(b_false.times(b_true)?, b_false);
assert_eq!(b_false.times(b_false)?, b_false);
Ok(())
}
}
18 changes: 9 additions & 9 deletions rustfst/src/symbol_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,25 +412,25 @@ mod tests {

assert_eq!(symt.len(), 3);

assert_eq!(symt.is_empty(), false);
assert!(!symt.is_empty());

assert_eq!(symt.get_label(EPS_SYMBOL), Some(0));
assert_eq!(symt.get_label("a"), Some(1));
assert_eq!(symt.get_label("b"), Some(2));

assert_eq!(symt.contains_symbol(EPS_SYMBOL), true);
assert_eq!(symt.contains_symbol("a"), true);
assert_eq!(symt.contains_symbol("b"), true);
assert_eq!(symt.contains_symbol("c"), false);
assert!(symt.contains_symbol(EPS_SYMBOL));
assert!(symt.contains_symbol("a"));
assert!(symt.contains_symbol("b"));
assert!(!symt.contains_symbol("c"));

assert_eq!(symt.get_symbol(0), Some(EPS_SYMBOL));
assert_eq!(symt.get_symbol(1), Some("a"));
assert_eq!(symt.get_symbol(2), Some("b"));

assert_eq!(symt.contains_label(0), true);
assert_eq!(symt.contains_label(1), true);
assert_eq!(symt.contains_label(2), true);
assert_eq!(symt.contains_label(3), false);
assert!(symt.contains_label(0));
assert!(symt.contains_label(1));
assert!(symt.contains_label(2));
assert!(!symt.contains_label(3));
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions rustfst/src/tests_openfst/algorithms/compose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ where

type TComposeFilter<S, F1, F2> = TPushLabelsFilter<S, F1, F2>;

let fst1: VectorFst<_> = fst_raw.clone().into();
let fst1: VectorFst<_> = fst_raw.clone();
let mut fst2: VectorFst<_> = compose_test_data.fst_2.clone();

let graph1look = Arc::new(TLaFst::new_with_relabeling(fst1, &mut fst2, true)?);
Expand Down Expand Up @@ -246,7 +246,7 @@ where
test_eq_fst(
&compose_test_data.result,
&static_fst,
format!("Compose failed : filter_name = lookahead"),
"Compose failed : filter_name = lookahead".to_string(),
);

Ok(())
Expand Down
6 changes: 3 additions & 3 deletions rustfst/src/tests_openfst/test_symt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ fn run_test_openfst_symt(test_name: &str) -> Result<()> {

#[test]
fn test_openfst_symt_000() -> Result<()> {
run_test_openfst_symt("symt_000").map_err(|v| v.into())
run_test_openfst_symt("symt_000")
}

#[test]
fn test_openfst_symt_001() -> Result<()> {
run_test_openfst_symt("symt_001").map_err(|v| v.into())
run_test_openfst_symt("symt_001")
}

#[test]
fn test_openfst_symt_002() -> Result<()> {
run_test_openfst_symt("symt_002").map_err(|v| v.into())
run_test_openfst_symt("symt_002")
}
Loading

0 comments on commit e29d260

Please sign in to comment.