Skip to content

Commit b807b68

Browse files
stepanchegfacebook-github-bot
stepancheg
authored and
facebook-github-bot
committed
UnpackValue::Error
Summary: `UnpackValue::Error` can be `Infallible` when unpacking is known to not produce an error. `UnpackValue::unpack_value_opt` new function returns `Option<Self>` (without `Error`) when `UnpackValue` is known to be infallible. Reviewed By: JakobDegen Differential Revision: D59437417 fbshipit-source-id: ba95b451b024657cec3739393054f75ffcee12ba
1 parent a46ef36 commit b807b68

35 files changed

+325
-83
lines changed

‎starlark/src/macros.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ macro_rules! starlark_complex_value {
5858
}
5959

6060
impl<'v> $crate::values::UnpackValue<'v> for &'v $x<'v> {
61+
type Error = std::convert::Infallible;
62+
6163
#[inline]
62-
fn unpack_value(x: $crate::values::Value<'v>) -> $crate::Result<Option<&'v $x<'v>>> {
64+
fn unpack_value_impl(x: $crate::values::Value<'v>) -> Result<Option<&'v $x<'v>>, Self::Error> {
6365
Ok($x::from_value(x))
6466
}
6567
}
@@ -207,9 +209,11 @@ macro_rules! starlark_simple_value {
207209
}
208210

209211
impl<'v> $crate::values::UnpackValue<'v> for &'v $x {
212+
type Error = std::convert::Infallible;
213+
210214
#[inline]
211-
fn unpack_value(x: $crate::values::Value<'v>) -> $crate::Result<Option<&'v $x>> {
212-
Ok($x::from_value(x))
215+
fn unpack_value_impl(x: $crate::values::Value<'v>) -> std::result::Result<Option<&'v $x>, Self::Error> {
216+
std::result::Result::Ok($x::from_value(x))
213217
}
214218
}
215219
}

‎starlark/src/stdlib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ impl LibraryExtension {
133133

134134
#[cfg(test)]
135135
mod tests {
136+
use std::convert::Infallible;
137+
136138
use allocative::Allocative;
137139
use derive_more::Display;
138140
use dupe::Dupe;
@@ -199,7 +201,9 @@ mod tests {
199201
}
200202

201203
impl<'v> UnpackValue<'v> for Bool2 {
202-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
204+
type Error = Infallible;
205+
206+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
203207
Ok(Some(*value.downcast_ref::<Bool2>().unwrap()))
204208
}
205209
}

‎starlark/src/tests/uncategorized.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,9 @@ fn test_radd() {
291291
starlark_simple_value!(Select);
292292

293293
impl<'v> UnpackValue<'v> for Select {
294-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
294+
type Error = crate::Error;
295+
296+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<Self>> {
295297
match Select::from_value(value) {
296298
Some(x) => Ok(Some(x.clone())),
297299
None => {

‎starlark/src/values.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ pub use crate::values::types::string;
9494
pub use crate::values::types::structs;
9595
pub use crate::values::types::tuple;
9696
pub use crate::values::unpack::UnpackValue;
97+
pub use crate::values::unpack::UnpackValueError;
98+
pub use crate::values::unpack::UnpackValueErrorInfallible;
9799
pub use crate::values::value_of::ValueOf;
98100
pub use crate::values::value_of_unchecked::FrozenValueOfUnchecked;
99101
pub use crate::values::value_of_unchecked::ValueOfUnchecked;

‎starlark/src/values/layout/complex.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18+
use std::convert::Infallible;
1819
use std::fmt;
1920
use std::marker::PhantomData;
2021

@@ -115,7 +116,9 @@ where
115116
T: ComplexValue<'v>,
116117
T::Frozen: StarlarkValue<'static>,
117118
{
118-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
119+
type Error = Infallible;
120+
121+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
119122
Ok(Self::new(value))
120123
}
121124
}

‎starlark/src/values/layout/typed.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
pub(crate) mod string;
1919

20+
use std::convert::Infallible;
2021
use std::fmt;
2122
use std::fmt::Debug;
2223
use std::fmt::Display;
@@ -366,7 +367,9 @@ impl<'v, T: StarlarkValue<'v>> StarlarkTypeRepr for ValueTyped<'v, T> {
366367
}
367368

368369
impl<'v, T: StarlarkValue<'v>> UnpackValue<'v> for ValueTyped<'v, T> {
369-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
370+
type Error = Infallible;
371+
372+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
370373
Ok(ValueTyped::new(value))
371374
}
372375
}
@@ -392,7 +395,9 @@ impl<'v, T: StarlarkValue<'v>> StarlarkTypeRepr for FrozenValueTyped<'v, T> {
392395
}
393396

394397
impl<'v, T: StarlarkValue<'v>> UnpackValue<'v> for FrozenValueTyped<'v, T> {
395-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
398+
type Error = crate::Error;
399+
400+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<Self>> {
396401
if let Some(value) = value.unpack_frozen() {
397402
if let Some(value) = FrozenValueTyped::new(value) {
398403
return Ok(Some(value));

‎starlark/src/values/layout/value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ impl<'v> Value<'v> {
353353
I: TryFrom<i32>,
354354
I: TryFrom<&'v BigInt>,
355355
{
356-
let Some(num) = StarlarkIntRef::unpack_value(self)? else {
356+
let Some(num) = StarlarkIntRef::unpack_value_opt(self) else {
357357
return Ok(None);
358358
};
359359
let option = match num {

‎starlark/src/values/num/value.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ enum NumError {
4444
/// numerical types and helps in implementation of arithmetical operations
4545
/// between them.
4646
#[derive(Clone, Debug, Dupe, Copy, StarlarkTypeRepr, UnpackValue)]
47-
pub(crate) enum NumRef<'v> {
47+
#[doc(hidden)]
48+
pub enum NumRef<'v> {
4849
Int(StarlarkIntRef<'v>),
4950
// `StarlarkFloat` not `f64` here because `f64` unpacks from `int` too.
5051
Float(StarlarkFloat),

‎starlark/src/values/types/bigint.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,23 +223,23 @@ impl<'v> StarlarkValue<'v> for StarlarkBigInt {
223223
}
224224

225225
fn bit_and(&self, other: Value<'v>, heap: &'v Heap) -> crate::Result<Value<'v>> {
226-
let rhs = match StarlarkIntRef::unpack_value(other)? {
226+
let rhs = match StarlarkIntRef::unpack_value_opt(other) {
227227
Some(rhs) => rhs,
228228
None => return ValueError::unsupported_with(self, "&", other),
229229
};
230230
Ok(heap.alloc(StarlarkIntRef::Big(self) & rhs))
231231
}
232232

233233
fn bit_xor(&self, other: Value<'v>, heap: &'v Heap) -> crate::Result<Value<'v>> {
234-
let rhs = match StarlarkIntRef::unpack_value(other)? {
234+
let rhs = match StarlarkIntRef::unpack_value_opt(other) {
235235
Some(rhs) => rhs,
236236
None => return ValueError::unsupported_with(self, "^", other),
237237
};
238238
Ok(heap.alloc(StarlarkIntRef::Big(self) ^ rhs))
239239
}
240240

241241
fn bit_or(&self, other: Value<'v>, heap: &'v Heap) -> crate::Result<Value<'v>> {
242-
let rhs = match StarlarkIntRef::unpack_value(other)? {
242+
let rhs = match StarlarkIntRef::unpack_value_opt(other) {
243243
Some(rhs) => rhs,
244244
None => return ValueError::unsupported_with(self, "|", other),
245245
};
@@ -251,14 +251,14 @@ impl<'v> StarlarkValue<'v> for StarlarkBigInt {
251251
}
252252

253253
fn left_shift(&self, other: Value<'v>, heap: &'v Heap) -> crate::Result<Value<'v>> {
254-
match StarlarkIntRef::unpack_value(other)? {
254+
match StarlarkIntRef::unpack_value_opt(other) {
255255
None => ValueError::unsupported_with(self, "<<", other),
256256
Some(other) => Ok(heap.alloc(StarlarkIntRef::Big(self).left_shift(other)?)),
257257
}
258258
}
259259

260260
fn right_shift(&self, other: Value<'v>, heap: &'v Heap) -> crate::Result<Value<'v>> {
261-
match StarlarkIntRef::unpack_value(other)? {
261+
match StarlarkIntRef::unpack_value_opt(other) {
262262
None => ValueError::unsupported_with(self, ">>", other),
263263
Some(other) => Ok(heap.alloc(StarlarkIntRef::Big(self).right_shift(other)?)),
264264
}

‎starlark/src/values/types/bigint/convert.rs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,38 +161,50 @@ impl AllocFrozenValue for BigInt {
161161
}
162162

163163
impl<'v> UnpackValue<'v> for u32 {
164-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<u32>> {
164+
type Error = crate::Error;
165+
166+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<u32>> {
165167
value.unpack_integer()
166168
}
167169
}
168170

169171
impl<'v> UnpackValue<'v> for u64 {
170-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<u64>> {
172+
type Error = crate::Error;
173+
174+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<u64>> {
171175
value.unpack_integer()
172176
}
173177
}
174178

175179
impl<'v> UnpackValue<'v> for i64 {
176-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<i64>> {
180+
type Error = crate::Error;
181+
182+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<i64>> {
177183
value.unpack_integer()
178184
}
179185
}
180186

181187
impl<'v> UnpackValue<'v> for usize {
182-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<usize>> {
188+
type Error = crate::Error;
189+
190+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<usize>> {
183191
value.unpack_integer()
184192
}
185193
}
186194

187195
impl<'v> UnpackValue<'v> for isize {
188-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<isize>> {
196+
type Error = crate::Error;
197+
198+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<isize>> {
189199
value.unpack_integer()
190200
}
191201
}
192202

193203
impl<'v> UnpackValue<'v> for BigInt {
194-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<BigInt>> {
195-
let Some(int) = StarlarkIntRef::unpack_value(value)? else {
204+
type Error = crate::Error;
205+
206+
fn unpack_value_impl(value: Value<'v>) -> crate::Result<Option<BigInt>> {
207+
let Some(int) = StarlarkIntRef::unpack_value_opt(value) else {
196208
return Ok(None);
197209
};
198210
Ok(match int {

‎starlark/src/values/types/bool.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
//! Unlike most Starlark values, these aren't actually represented on the [`Heap`], but as special values.
2222
2323
use std::cmp::Ordering;
24+
use std::convert::Infallible;
2425
use std::fmt;
2526
use std::fmt::Display;
2627
use std::hash::Hasher;
@@ -96,7 +97,9 @@ impl StarlarkTypeRepr for bool {
9697
}
9798

9899
impl UnpackValue<'_> for bool {
99-
fn unpack_value(value: Value) -> crate::Result<Option<Self>> {
100+
type Error = Infallible;
101+
102+
fn unpack_value_impl(value: Value) -> Result<Option<Self>, Self::Error> {
100103
Ok(value.unpack_bool())
101104
}
102105
}

‎starlark/src/values/types/dict/refs.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use std::cell::Ref;
1919
use std::cell::RefCell;
2020
use std::cell::RefMut;
21+
use std::convert::Infallible;
2122
use std::ops::Deref;
2223
use std::ops::DerefMut;
2324

@@ -162,7 +163,9 @@ impl<'v> StarlarkTypeRepr for DictRef<'v> {
162163
}
163164

164165
impl<'v> UnpackValue<'v> for DictRef<'v> {
165-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<DictRef<'v>>> {
166+
type Error = Infallible;
167+
168+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<DictRef<'v>>, Infallible> {
166169
Ok(DictRef::from_value(value))
167170
}
168171
}

‎starlark/src/values/types/dict/traits.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use std::collections::BTreeMap;
1919
use std::hash::Hash;
2020

21+
use either::Either;
22+
2123
use crate::collections::SmallMap;
2224
use crate::typing::Ty;
2325
use crate::values::dict::AllocDict;
@@ -84,17 +86,19 @@ impl<K: StarlarkTypeRepr, V: StarlarkTypeRepr> StarlarkTypeRepr for SmallMap<K,
8486
}
8587

8688
impl<'v, K: UnpackValue<'v> + Hash + Eq, V: UnpackValue<'v>> UnpackValue<'v> for SmallMap<K, V> {
87-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
89+
type Error = Either<K::Error, V::Error>;
90+
91+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
8892
let Some(dict) = DictRef::from_value(value) else {
8993
return Ok(None);
9094
};
9195
let it = dict.iter();
9296
let mut r = SmallMap::with_capacity(it.len());
9397
for (k, v) in it {
94-
let Some(k) = K::unpack_value(k)? else {
98+
let Some(k) = K::unpack_value_impl(k).map_err(Either::Left)? else {
9599
return Ok(None);
96100
};
97-
let Some(v) = V::unpack_value(v)? else {
101+
let Some(v) = V::unpack_value_impl(v).map_err(Either::Right)? else {
98102
return Ok(None);
99103
};
100104
// TODO(nga): return error if keys are not unique.
@@ -156,16 +160,18 @@ impl<K: StarlarkTypeRepr, V: StarlarkTypeRepr> StarlarkTypeRepr for BTreeMap<K,
156160
}
157161

158162
impl<'v, K: UnpackValue<'v> + Ord, V: UnpackValue<'v>> UnpackValue<'v> for BTreeMap<K, V> {
159-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
163+
type Error = Either<K::Error, V::Error>;
164+
165+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
160166
let Some(dict) = DictRef::from_value(value) else {
161167
return Ok(None);
162168
};
163169
let mut r = BTreeMap::new();
164170
for (k, v) in dict.iter() {
165-
let Some(k) = K::unpack_value(k)? else {
171+
let Some(k) = K::unpack_value_impl(k).map_err(Either::Left)? else {
166172
return Ok(None);
167173
};
168-
let Some(v) = V::unpack_value(v)? else {
174+
let Some(v) = V::unpack_value_impl(v).map_err(Either::Right)? else {
169175
return Ok(None);
170176
};
171177
// TODO(nga): return error if keys are not unique.

‎starlark/src/values/types/dict/unpack.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
* limitations under the License.
1616
*/
1717

18+
use either::Either;
19+
1820
use crate::values::dict::DictRef;
1921
use crate::values::type_repr::DictType;
2022
use crate::values::type_repr::StarlarkTypeRepr;
@@ -39,16 +41,18 @@ impl<K: StarlarkTypeRepr, V: StarlarkTypeRepr> StarlarkTypeRepr for UnpackDictEn
3941
}
4042

4143
impl<'v, K: UnpackValue<'v>, V: UnpackValue<'v>> UnpackValue<'v> for UnpackDictEntries<K, V> {
42-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
43-
let Some(dict) = DictRef::unpack_value(value)? else {
44+
type Error = Either<K::Error, V::Error>;
45+
46+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
47+
let Some(dict) = DictRef::unpack_value_opt(value) else {
4448
return Ok(None);
4549
};
4650
let mut entries = Vec::with_capacity(dict.len());
4751
for (k, v) in dict.iter() {
48-
let Some(k) = K::unpack_value(k)? else {
52+
let Some(k) = K::unpack_value_impl(k).map_err(Either::Left)? else {
4953
return Ok(None);
5054
};
51-
let Some(v) = V::unpack_value(v)? else {
55+
let Some(v) = V::unpack_value_impl(v).map_err(Either::Right)? else {
5256
return Ok(None);
5357
};
5458
entries.push((k, v));

‎starlark/src/values/types/float/float.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
use std::cmp::Ordering;
19+
use std::convert::Infallible;
1920
use std::fmt;
2021
use std::fmt::Display;
2122
use std::fmt::Write;
@@ -244,7 +245,9 @@ impl AllocFrozenValue for f64 {
244245

245246
/// Allows only a float - an int will not be accepted.
246247
impl<'v> UnpackValue<'v> for StarlarkFloat {
247-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
248+
type Error = Infallible;
249+
250+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
248251
let Some(value) = value.downcast_ref::<StarlarkFloat>() else {
249252
return Ok(None);
250253
};

‎starlark/src/values/types/float/unpack.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ impl StarlarkTypeRepr for UnpackFloat {
3434
}
3535

3636
impl<'v> UnpackValue<'v> for UnpackFloat {
37-
fn unpack_value(value: Value<'v>) -> crate::Result<Option<Self>> {
38-
let Some(num) = NumRef::unpack_value(value)? else {
37+
type Error = <NumRef<'v> as UnpackValue<'v>>::Error;
38+
39+
fn unpack_value_impl(value: Value<'v>) -> Result<Option<Self>, Self::Error> {
40+
let Some(num) = NumRef::unpack_value_impl(value)? else {
3941
return Ok(None);
4042
};
4143
Ok(Some(UnpackFloat(num.as_float())))

0 commit comments

Comments
 (0)