Skip to content

Commit 53a5258

Browse files
committed
force &'static lifetime on vortex_expect message
to prevent dynamic format strings Signed-off-by: Andrew Duffy <[email protected]>
1 parent 10c6fc2 commit 53a5258

File tree

5 files changed

+48
-38
lines changed

5 files changed

+48
-38
lines changed

vortex-array/src/arrays/list/array.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::sync::Arc;
55

66
use num_traits::AsPrimitive;
77
use vortex_dtype::{DType, NativePType, match_each_integer_ptype, match_each_native_ptype};
8-
use vortex_error::{VortexExpect, VortexResult, vortex_bail, vortex_ensure, vortex_err};
8+
use vortex_error::{VortexExpect, VortexResult, vortex_bail, vortex_ensure, vortex_panic};
99

1010
use crate::arrays::{ListVTable, PrimitiveVTable};
1111
use crate::compute::{min_max, sub_scalar};
@@ -191,13 +191,13 @@ impl ListArray {
191191

192192
vortex_ensure!(
193193
max_offset
194-
<= P::try_from(elements.len())
195-
.map_err(|e| vortex_err!(
196-
"Offsets type {} must be able to fit elements length {}",
197-
<P as NativePType>::PTYPE,
198-
elements.len()
199-
))
200-
.vortex_expect("Offsets type must fit elements length")
194+
<= P::try_from(elements.len()).unwrap_or_else(|_| vortex_panic!(
195+
"Offsets type {} must be able to fit elements length {}",
196+
<P as NativePType>::PTYPE,
197+
elements.len()
198+
)),
199+
"Max offset {max_offset} is beyond the length of the elements array {}",
200+
elements.len()
201201
);
202202
})
203203
} else {

vortex-array/src/compute/conformance/cast.rs

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
use vortex_dtype::{DType, Nullability, PType};
5-
use vortex_error::{VortexExpect as _, VortexUnwrap};
5+
use vortex_error::{VortexExpect as _, VortexUnwrap, vortex_panic};
66
use vortex_scalar::Scalar;
77

88
use crate::Array;
@@ -120,10 +120,12 @@ fn test_cast_to_non_nullable(array: &dyn Array) {
120120
}
121121
cast(array, &array.dtype().as_nonnullable())
122122
.err()
123-
.vortex_expect(&format!(
124-
"arrays with nulls should error when casting to non-nullable {}",
125-
array,
126-
));
123+
.unwrap_or_else(|| {
124+
vortex_panic!(
125+
"arrays with nulls should error when casting to non-nullable {}",
126+
array,
127+
)
128+
});
127129
}
128130
}
129131

@@ -190,14 +192,16 @@ fn test_cast_to_primitive(array: &dyn Array, target_ptype: PType, test_round_tri
190192
&DType::Primitive(target_ptype, array.dtype().nullability()),
191193
)
192194
.err()
193-
.vortex_expect(&format!(
194-
"Cast must fail because some values are out of bounds. {} {:?} {:?} {} {}",
195-
target_ptype,
196-
min,
197-
max,
198-
array,
199-
array.display_values(),
200-
));
195+
.unwrap_or_else(|| {
196+
vortex_panic!(
197+
"Cast must fail because some values are out of bounds. {} {:?} {:?} {} {}",
198+
target_ptype,
199+
min,
200+
max,
201+
array,
202+
array.display_values(),
203+
)
204+
});
201205
return;
202206
}
203207

@@ -206,11 +210,13 @@ fn test_cast_to_primitive(array: &dyn Array, target_ptype: PType, test_round_tri
206210
array,
207211
&DType::Primitive(target_ptype, array.dtype().nullability()),
208212
)
209-
.vortex_expect(&format!(
210-
"Cast must succeed because all values are within bounds. {} {}",
211-
target_ptype,
212-
array.display_values(),
213-
));
213+
.unwrap_or_else(|e| {
214+
vortex_panic!(
215+
"Cast must succeed because all values are within bounds. {} {}: {e}",
216+
target_ptype,
217+
array.display_values(),
218+
)
219+
});
214220
assert_eq!(array.validity_mask(), casted.validity_mask());
215221
for i in 0..array.len().min(10) {
216222
let original = array.scalar_at(i);

vortex-duckdb/src/duckdb/table_filter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::ptr;
88

99
use cpp::duckdb_vx_table_filter;
1010
use num_traits::AsPrimitive;
11-
use vortex::error::VortexExpect;
11+
use vortex::error::vortex_panic;
1212

1313
use crate::cpp::idx_t;
1414
use crate::duckdb::{Expression, Value, ValueRef};
@@ -47,7 +47,7 @@ impl<'a> IntoIterator for &'a TableFilterSet {
4747
fn into_iter(self) -> Self::IntoIter {
4848
Box::new((0..self.len()).map(move |i| {
4949
self.get(i)
50-
.vortex_expect(format!("inside filter set bounds {i}").as_str())
50+
.unwrap_or_else(|| vortex_panic!("inside filter set bounds {i}"))
5151
}))
5252
}
5353
}

vortex-error/src/lib.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,13 @@ pub trait VortexExpect {
325325

326326
/// Returns the value of the result if it is Ok, otherwise panics with the error.
327327
/// Should be called only in contexts where the error condition represents a bug (programmer error).
328-
fn vortex_expect(self, msg: &str) -> Self::Output;
328+
///
329+
/// # `&'static` message lifetime
330+
///
331+
/// The panic string argument should be a string literal, hence the `&'static` lifetime. If
332+
/// you'd like to panic with a dynamic format string, consider using `unwrap_or_else` combined
333+
/// with the `vortex_panic!` macro instead.
334+
fn vortex_expect(self, msg: &'static str) -> Self::Output;
329335
}
330336

331337
impl<T, E> VortexExpect for Result<T, E>
@@ -335,7 +341,7 @@ where
335341
type Output = T;
336342

337343
#[inline(always)]
338-
fn vortex_expect(self, msg: &str) -> Self::Output {
344+
fn vortex_expect(self, msg: &'static str) -> Self::Output {
339345
self.map_err(|err| err.into())
340346
.unwrap_or_else(|e| vortex_panic!(e.with_context(msg.to_string())))
341347
}
@@ -345,7 +351,7 @@ impl<T> VortexExpect for Option<T> {
345351
type Output = T;
346352

347353
#[inline(always)]
348-
fn vortex_expect(self, msg: &str) -> Self::Output {
354+
fn vortex_expect(self, msg: &'static str) -> Self::Output {
349355
self.unwrap_or_else(|| {
350356
let err = VortexError::AssertionFailed(
351357
msg.to_string().into(),

vortex-session/src/lib.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::ops::{Deref, DerefMut};
1010
use std::sync::Arc;
1111

1212
use dashmap::{DashMap, Entry};
13-
use vortex_error::{VortexExpect, vortex_err, vortex_panic};
13+
use vortex_error::{VortexExpect, vortex_panic};
1414

1515
/// A Vortex session encapsulates the set of extensible arrays, layouts, compute functions, dtypes,
1616
/// etc. that are available for use in a given context.
@@ -77,10 +77,9 @@ impl SessionExt for VortexSession {
7777
Ref(self
7878
.0
7979
.get(&TypeId::of::<V>())
80-
.ok_or_else(|| {
81-
vortex_err!("Session has not been initialized with {}", type_name::<V>())
80+
.unwrap_or_else(|| {
81+
vortex_panic!("Session has not been initialized with {}", type_name::<V>())
8282
})
83-
.vortex_expect("Unitialized session variable")?
8483
.map(|v| {
8584
(**v)
8685
.as_any()
@@ -96,10 +95,9 @@ impl SessionExt for VortexSession {
9695
RefMut(
9796
self.0
9897
.get_mut(&TypeId::of::<V>())
99-
.ok_or_else(|| {
100-
vortex_err!("Session has not been initialized with {}", type_name::<V>())
98+
.unwrap_or_else(|| {
99+
vortex_panic!("Session has not been initialized with {}", type_name::<V>())
101100
})
102-
.vortex_expect("Unitialized session variable")?
103101
.map(|v| {
104102
(**v)
105103
.as_any_mut()

0 commit comments

Comments
 (0)