Skip to content
Draft
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,7 @@ non_zero_suggestions = "warn"
nonstandard_macro_braces = "warn"
option_as_ref_cloned = "warn"
option_option = "warn"
panic = "deny"
path_buf_push_overwrite = "warn"
pathbuf_init_then_push = "warn"
precedence_bits = "warn"
Expand Down
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

msrv = "1.88"

allow-panic-in-tests = true
allow-unwrap-in-tests = true

# https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#avoid-breaking-exported-api
Expand Down
5 changes: 4 additions & 1 deletion crates/build/re_build_tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,10 @@ pub fn export_build_info_vars_for_crate(crate_name: &str) {
// work just fine otherwise.
Err(_err) if environment == Environment::UsedAsDependency => "<error>".to_owned(),

Err(err) => panic!("{err}"),
Err(err) => {
println!("cargo::error={err}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I didn't know about this!

return;
}
};

set_env("RE_BUILD_FEATURES", &features);
Expand Down
12 changes: 8 additions & 4 deletions crates/build/re_build_tools/src/rebuild_detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ pub fn is_tracked_env_var_set(env_var_name: &str) -> bool {
"1" | "yes" | "true" => true,
"0" | "no" | "false" => false,
_ => {
panic!("Failed to understand boolean env-var {env_var_name}={value}");
println!(
"cargo::error=Failed to understand boolean env-var {env_var_name}={value}"
);
false
}
},
}
Expand Down Expand Up @@ -172,10 +175,11 @@ impl<'a> Packages<'a> {
/// dependencies are properly tracked whether they are remote, in-workspace,
/// or locally patched.
pub fn track_implicit_dep(&self, pkg_name: &str, files_to_watch: &mut HashSet<PathBuf>) {
let pkg = self.pkgs.get(pkg_name).unwrap_or_else(|| {
let Some(pkg) = self.pkgs.get(pkg_name) else {
let found_names: Vec<&str> = self.pkgs.values().map(|pkg| pkg.name.as_str()).collect();
panic!("Failed to find package {pkg_name:?} among {found_names:?}")
});
println!("cargo::error=Failed to find package {pkg_name:?} among {found_names:?}");
return;
};

// Track the root package itself
track_crate_files(&pkg.manifest_path, files_to_watch);
Expand Down
4 changes: 4 additions & 0 deletions crates/build/re_dev_tools/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
//! Crate that combines several development utilities.
//!
//! To get an overview over all tools run `pixi run dev-tools --help`.

// Exception for the build crates as it's not production code.
#![allow(clippy::panic)]

use argh::FromArgs;

mod build_examples;
Expand Down
2 changes: 1 addition & 1 deletion crates/build/re_protos_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! definitions in the same file.
//!

#![allow(clippy::unwrap_used, clippy::exit)]
#![allow(clippy::unwrap_used, clippy::panic, clippy::exit)]

use std::path::Path;

Expand Down
2 changes: 2 additions & 0 deletions crates/build/re_types_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@

// TODO(#6330): remove unwrap()
#![allow(clippy::unwrap_used)]
// Exception for the build crates as it's not production code.
#![allow(clippy::panic)]

// NOTE: Official generated code from flatbuffers; ignore _everything_.

Expand Down
72 changes: 44 additions & 28 deletions crates/store/re_chunk/src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl Chunk {
let Some(struct_array) = list_array.values().downcast_array_ref::<ArrowStructArray>()
else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand All @@ -287,7 +287,7 @@ impl Chunk {
.find_map(|(i, field)| (field.name() == field_name).then_some(i))
else {
if cfg!(debug_assertions) {
panic!("field {field_name} not found for {component_descriptor}, data discarded");
panic_field_not_found(component_descriptor, field_name);
} else {
re_log::error_once!(
"field {field_name} not found for {component_descriptor}, data discarded"
Expand All @@ -298,7 +298,7 @@ impl Chunk {

if field_idx >= struct_array.num_columns() {
if cfg!(debug_assertions) {
panic!("field {field_name} not found for {component_descriptor}, data discarded");
panic_field_not_found(component_descriptor, field_name);
} else {
re_log::error_once!(
"field {field_name} not found for {component_descriptor}, data discarded"
Expand Down Expand Up @@ -335,7 +335,6 @@ pub trait ChunkComponentSlicer {
}

/// The actual implementation of `impl_native_type!`, so that we don't have to work in a macro.
#[expect(clippy::needless_pass_by_value)] // The simplest way to avoid lifetime issues.
fn slice_as_native<'a, P, T>(
component_descriptor: ComponentDescriptor,
array: &'a dyn ArrowArray,
Expand All @@ -347,7 +346,7 @@ where
{
let Some(values) = array.downcast_array_ref::<ArrowPrimitiveArray<P>>() else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand Down Expand Up @@ -395,7 +394,6 @@ impl_native_type!(arrow::array::types::Float32Type, f32);
impl_native_type!(arrow::array::types::Float64Type, f64);

/// The actual implementation of `impl_array_native_type!`, so that we don't have to work in a macro.
#[expect(clippy::needless_pass_by_value)] // The simplest way to avoid lifetime issues.
fn slice_as_array_native<'a, const N: usize, P, T>(
component_descriptor: ComponentDescriptor,
array: &'a dyn ArrowArray,
Expand All @@ -408,7 +406,7 @@ where
{
let Some(fixed_size_list_array) = array.downcast_array_ref::<ArrowFixedSizeListArray>() else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand All @@ -420,7 +418,7 @@ where
.downcast_array_ref::<ArrowPrimitiveArray<P>>()
else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand Down Expand Up @@ -475,7 +473,6 @@ impl_array_native_type!(arrow::array::types::Float32Type, f32);
impl_array_native_type!(arrow::array::types::Float64Type, f64);

/// The actual implementation of `impl_buffer_native_type!`, so that we don't have to work in a macro.
#[expect(clippy::needless_pass_by_value)] // The simplest way to avoid lifetime issues.
fn slice_as_buffer_native<'a, P, T>(
component_descriptor: ComponentDescriptor,
array: &'a dyn ArrowArray,
Expand All @@ -487,10 +484,7 @@ where
{
let Some(inner_list_array) = array.downcast_array_ref::<ArrowListArray>() else {
if cfg!(debug_assertions) {
panic!(
"DEBUG BUILD: {component_descriptor} had unexpected datatype: {:?}",
array.data_type()
);
panic_unexpected_data_type(component_descriptor, array);
} else {
re_log::error_once!(
"{component_descriptor} had unexpected datatype: {:?}. Data discarded",
Expand All @@ -505,10 +499,7 @@ where
.downcast_array_ref::<ArrowPrimitiveArray<P>>()
else {
if cfg!(debug_assertions) {
panic!(
"DEBUG BUILD: {component_descriptor} had unexpected datatype: {:?}",
array.data_type()
);
panic_unexpected_data_type(component_descriptor, array);
} else {
re_log::error_once!(
"{component_descriptor} had unexpected datatype: {:?}. Data discarded",
Expand Down Expand Up @@ -618,7 +609,6 @@ impl_buffer_native_type!(arrow::array::types::Float32Type, f32);
impl_buffer_native_type!(arrow::array::types::Float64Type, f64);

/// The actual implementation of `impl_array_list_native_type!`, so that we don't have to work in a macro.
#[expect(clippy::needless_pass_by_value)] // The simplest way to avoid lifetime issues.
fn slice_as_array_list_native<'a, const N: usize, P, T>(
component_descriptor: ComponentDescriptor,
array: &'a dyn ArrowArray,
Expand All @@ -631,7 +621,7 @@ where
{
let Some(inner_list_array) = array.downcast_array_ref::<ArrowListArray>() else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand All @@ -646,7 +636,7 @@ where
.downcast_array_ref::<ArrowFixedSizeListArray>()
else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand All @@ -658,7 +648,7 @@ where
.downcast_array_ref::<ArrowPrimitiveArray<P>>()
else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand Down Expand Up @@ -729,7 +719,7 @@ impl ChunkComponentSlicer for String {
) -> impl Iterator<Item = Vec<ArrowString>> + 'a {
let Some(utf8_array) = array.downcast_array_ref::<ArrowStringArray>() else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand Down Expand Up @@ -761,7 +751,7 @@ impl ChunkComponentSlicer for bool {
) -> impl Iterator<Item = Self::Item<'a>> + 'a {
let Some(values) = array.downcast_array_ref::<ArrowBooleanArray>() else {
if cfg!(debug_assertions) {
panic!("downcast failed for {component_descriptor}, data discarded");
panic_downcast_failed(component_descriptor);
} else {
re_log::error_once!("downcast failed for {component_descriptor}, data discarded");
}
Expand Down Expand Up @@ -950,11 +940,7 @@ impl Chunk {
Ok(values) => values,
Err(err) => {
if cfg!(debug_assertions) {
panic!(
"[DEBUG-ONLY] deserialization failed for {}, data discarded: {}",
C::name(),
re_error::format_ref(&err),
);
panic_deserialization_failed::<C, _>(err);
} else {
re_log::error_once!(
"deserialization failed for {}, data discarded: {}",
Expand All @@ -977,6 +963,36 @@ impl Chunk {
}
}

#[expect(clippy::panic, clippy::needless_pass_by_value)]
fn panic_downcast_failed(component_descriptor: ComponentDescriptor) -> ! {
panic!("downcast failed for {component_descriptor}, data discarded")
}

#[expect(clippy::panic, clippy::needless_pass_by_value)]
fn panic_field_not_found(component_descriptor: ComponentDescriptor, field_name: &str) -> ! {
panic!("field {field_name} not found for {component_descriptor}, data discarded")
}

#[expect(clippy::panic, clippy::needless_pass_by_value)]
fn panic_unexpected_data_type(
component_descriptor: ComponentDescriptor,
array: &dyn ArrowArray,
) -> ! {
panic!(
"DEBUG BUILD: {component_descriptor} had unexpected datatype: {:?}",
array.data_type()
)
}

#[expect(clippy::panic)]
fn panic_deserialization_failed<C: Component, E: std::error::Error>(err: E) -> ! {
panic!(
"[DEBUG-ONLY] deserialization failed for {}, data discarded: {}",
C::name(),
re_error::format_ref(&err),
)
}

// ---

#[cfg(test)]
Expand Down
1 change: 1 addition & 0 deletions crates/store/re_types/src/datatypes/mat3x3_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ impl Mat3x3 {
///
/// Panics if `index` is greater than 2.
#[inline]
#[expect(clippy::panic)]
pub fn col(&self, index: usize) -> Vec3D {
match index {
0 => [self.0[0], self.0[1], self.0[2]].into(),
Expand Down
1 change: 1 addition & 0 deletions crates/store/re_types/src/datatypes/mat4x4_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl Mat4x4 {
///
/// Panics if `index` is greater than 3.
#[inline]
#[expect(clippy::panic)]
pub fn col(&self, index: usize) -> Vec4D {
match index {
0 => [self.0[0], self.0[1], self.0[2], self.0[3]].into(),
Expand Down
1 change: 1 addition & 0 deletions crates/store/re_types/src/view_coordinates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ impl Axis3 {
/// * 1 => [`Self::Y`]
/// * 2 => [`Self::Z`]
#[inline]
#[expect(clippy::panic)]
pub fn from_dim(dim: usize) -> Self {
match dim {
0 => Self::X,
Expand Down
24 changes: 0 additions & 24 deletions crates/store/re_types_core/src/arrow_zip_validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,27 +186,3 @@ where
V: ExactSizeIterator<Item = bool>,
{
}

impl<T, I, V> ZipValidity<T, I, V>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch of dead code!

where
I: Iterator<Item = T>,
V: Iterator<Item = bool>,
{
/// Unwrap into an iterator that has no null values.
pub fn unwrap_required(self) -> I {
if let Self::Required(i) = self {
i
} else {
panic!("Could not 'unwrap_required'. 'ZipValidity' iterator has nulls.");
}
}

/// Unwrap into an iterator that has null values.
pub fn unwrap_optional(self) -> ZipValidityIter<T, I, V> {
if let Self::Optional(i) = self {
i
} else {
panic!("Could not 'unwrap_optional'. 'ZipValidity' iterator has no nulls.");
}
}
}
1 change: 1 addition & 0 deletions crates/store/re_types_core/src/component_batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub trait ComponentBatch {
Ok(array) => Some(array),

#[cfg(debug_assertions)]
#[expect(clippy::panic)]
Err(err) => {
panic!(
"failed to serialize data for {}: {}",
Expand Down
1 change: 1 addition & 0 deletions crates/store/re_types_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ pub fn try_serialize_field<L: Loggable>(
Ok(array) => Some(SerializedComponentBatch::new(array, descriptor)),

#[cfg(debug_assertions)]
#[expect(clippy::panic)]
Err(err) => {
panic!(
"failed to serialize data for {descriptor}: {}",
Expand Down
1 change: 1 addition & 0 deletions crates/utils/re_log/src/result_extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ where
#[track_caller]
fn unwrap_debug_or_log_error(self) -> Option<T> {
if cfg!(debug_assertions) {
#[expect(clippy::panic)]
match self {
Ok(value) => Some(value),
Err(err) => {
Expand Down
2 changes: 2 additions & 0 deletions crates/utils/re_log/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ impl log::Log for PanicOnWarn {
}
}

// Panic is expected only in this method as the logger requires it while the rest of the code must be panic-free.
#[expect(clippy::panic)]
fn log(&self, record: &log::Record<'_>) {
// `enabled` isn't called automatically by the `log!` macros, so we have to call it here.
// (it is only used by `log_enabled!`)
Expand Down
1 change: 1 addition & 0 deletions crates/utils/re_video/src/demux/mp4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ impl VideoDataDescription {
mp4_tracks,
};

#[expect(clippy::panic)]
if cfg!(debug_assertions)
&& let Err(err) = video_data_description.sanity_check()
{
Expand Down
2 changes: 2 additions & 0 deletions crates/viewer/re_renderer/src/file_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ pub trait FileSystem {

fn exists(&self, path: impl AsRef<Path>) -> bool;

#[expect(clippy::panic)]
fn create_dir_all(&self, _path: impl AsRef<Path>) -> anyhow::Result<()> {
panic!("create_dir_all() is not supported on this backend")
}

#[expect(clippy::panic)]
fn create_file(
&self,
_path: impl AsRef<Path>,
Expand Down
Loading