Skip to content

Commit

Permalink
Cleanup usage of unwrap() in re_data_store (rerun-io#6334)
Browse files Browse the repository at this point in the history
<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

### What

Part of rerun-io#6330
Removes or explicitly allows unwrap() where it makes sense.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6334?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6334?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [ ] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6334)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
Artxiom authored May 16, 2024
1 parent 48ff64e commit 692b6b6
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 12 deletions.
7 changes: 1 addition & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ unnested_or_patterns = "warn"
unused_peekable = "warn"
unused_rounding = "warn"
unused_self = "warn"
unwrap_used = "warn"
useless_transmute = "warn"
verbose_file_reads = "warn"
wildcard_dependencies = "warn"
Expand All @@ -440,12 +441,6 @@ let_underscore_untyped = "allow"
missing_assert_message = "allow"
missing_errors_doc = "allow"

# TODO(#3408): Forbid `.unwrap()`
# This would be nice to enable, but we have way too many unwraps right now 😭
# Enabling this lint in 2023-04-27 yielded 556 hits.
# Enabling this lint in 2023-09-23 yielded 352 hits
unwrap_used = "warn"

[patch.crates-io]
# Try to avoid patching crates! It prevents us from publishing the crates on crates.io.
# If you do patch always prefer to patch to a commit on the trunk of the upstream repo.
Expand Down
4 changes: 2 additions & 2 deletions crates/re_data_store/src/arrow_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ impl ArrayExt for dyn Array {
fn get_child_length(&self, child_nr: usize) -> usize {
self.as_any()
.downcast_ref::<ListArray<i32>>()
.unwrap()
.expect("not a ListArray<i32>")
.offsets()
.lengths()
.nth(child_nr)
.unwrap()
.unwrap_or_else(|| panic!("no child at index {child_nr}"))
}
}
4 changes: 0 additions & 4 deletions crates/re_data_store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@
#![doc = document_features::document_features!()]
//!
// TODO(#3408): remove unwrap()
#![allow(clippy::unwrap_used)]

mod arrow_util;
mod store;
mod store_arrow;
Expand All @@ -34,7 +31,6 @@ mod store_write;
#[doc(hidden)]
pub mod test_util;

pub use self::arrow_util::ArrayExt;
pub use self::store::{DataStore, DataStoreConfig, StoreGeneration};
pub use self::store_event::{StoreDiff, StoreDiffKind, StoreEvent};
pub use self::store_gc::{GarbageCollectionOptions, GarbageCollectionTarget};
Expand Down
1 change: 1 addition & 0 deletions crates/re_data_store/src/store_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ impl DataStore {
}

#[cfg(debug_assertions)]
#[allow(clippy::unwrap_used)]
self.sanity_check().unwrap();

// NOTE: only temporal data and row metadata get purged!
Expand Down
2 changes: 2 additions & 0 deletions crates/re_data_store/src/store_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ impl IndexedTable {
pub fn find_bucket(&self, time: TimeInt) -> (TimeInt, &IndexedBucket) {
// This cannot fail, `iter_bucket` is guaranteed to always yield at least one bucket,
// since indexed tables always spawn with a default bucket that covers [-∞;+∞].
#[allow(clippy::unwrap_used)]
self.range_buckets_rev(..=time).next().unwrap()
}

Expand All @@ -466,6 +467,7 @@ impl IndexedTable {
pub fn find_bucket_mut(&mut self, time: TimeInt) -> (TimeInt, &mut IndexedBucket) {
// This cannot fail, `iter_bucket_mut` is guaranteed to always yield at least one bucket,
// since indexed tables always spawn with a default bucket that covers [-∞;+∞].
#[allow(clippy::unwrap_used)]
self.range_bucket_rev_mut(..=time).next().unwrap()
}

Expand Down
2 changes: 2 additions & 0 deletions crates/re_data_store/src/store_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ impl IndexedBucket {
*size_bytes += size_bytes_added;

#[cfg(debug_assertions)]
#[allow(clippy::unwrap_used)]
{
drop(inner);
self.sanity_check().unwrap();
Expand Down Expand Up @@ -534,6 +535,7 @@ impl IndexedBucket {

// sanity checks
#[cfg(debug_assertions)]
#[allow(clippy::unwrap_used)]
{
drop(inner1); // sanity checking will grab the lock!
self.sanity_check().unwrap();
Expand Down
2 changes: 2 additions & 0 deletions crates/re_data_store/src/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,14 @@ pub fn sanity_unwrap(store: &DataStore) {
if let err @ Err(_) = store.sanity_check() {
store.sort_indices_if_needed();
eprintln!("{store}");
#[allow(clippy::unwrap_used)] // we want to panic here
err.unwrap();
}
}

// We very often re-use RowIds when generating test data.
pub fn insert_table_with_retries(store: &mut DataStore, table: &DataTable) {
#[allow(clippy::unwrap_used)] // ok for tests
for row in table.to_rows() {
let mut row = row.unwrap();
loop {
Expand Down

0 comments on commit 692b6b6

Please sign in to comment.