Skip to content

Commit e1961a9

Browse files
stepanchegfacebook-github-bot
authored andcommitted
When evaluating type(x) == "y" do identity comparison before string content comparison
Reviewed By: ndmitchell Differential Revision: D30919274 fbshipit-source-id: 73c3148a3476bc1857e68bac103643e9b7177c33
1 parent 64025cb commit e1961a9

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

starlark/src/eval/fragment/expr.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ use crate::{
3434
dict::Dict,
3535
function::{BoundMethod, NativeAttribute},
3636
list::List,
37-
string::StarlarkStr,
3837
tuple::{FrozenTuple, Tuple},
39-
AttrType, FrozenHeap, FrozenValue, Heap, Value, ValueError, ValueLike,
38+
AttrType, FrozenHeap, FrozenStringValue, FrozenValue, Heap, Value, ValueError, ValueLike,
4039
},
4140
};
4241
use gazebo::{coerce::coerce_ref, prelude::*};
@@ -122,11 +121,10 @@ fn try_eval_type_is(
122121
) -> Result<ExprCompiledValue, (ExprCompiledValue, ExprCompiledValue)> {
123122
match (l, r) {
124123
(ExprCompiledValue::Type(l), ExprCompiledValue::Value(r)) => {
125-
if let Some(r) = r.downcast_frozen_ref::<StarlarkStr>() {
126-
let t = r.map(|r| r.unpack());
124+
if let Some(t) = FrozenStringValue::new(r) {
127125
let cmp = maybe_not.as_fn();
128126
Ok(expr!("type_is", l, |_eval| {
129-
Value::new_bool(cmp(l.get_type() == t.as_ref()))
127+
Value::new_bool(cmp(l.get_type_value() == t))
130128
}))
131129
} else {
132130
Err((ExprCompiledValue::Type(l), ExprCompiledValue::Value(r)))

starlark/src/values/layout/constant.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use std::{
2828
fmt,
2929
fmt::{Debug, Formatter},
3030
intrinsics::copy_nonoverlapping,
31+
ptr,
3132
};
3233

3334
/// A constant string that can be converted to a [`FrozenValue`].
@@ -100,6 +101,14 @@ unsafe impl<'v> CoerceKey<StringValue<'v>> for FrozenStringValue {}
100101
unsafe impl<'v> Coerce<StringValue<'v>> for StringValue<'v> {}
101102
unsafe impl<'v> CoerceKey<StringValue<'v>> for StringValue<'v> {}
102103

104+
impl PartialEq for FrozenStringValue {
105+
fn eq(&self, other: &Self) -> bool {
106+
ptr::eq(self, other) || self.as_str() == other.as_str()
107+
}
108+
}
109+
110+
impl Eq for FrozenStringValue {}
111+
103112
impl FrozenStringValue {
104113
/// Obtain the [`FrozenValue`] for a [`FrozenStringValue`].
105114
pub fn unpack(self) -> FrozenValue {
@@ -113,6 +122,20 @@ impl FrozenStringValue {
113122
debug_assert!(value.unpack_str().is_some());
114123
FrozenStringValue(&*(value.0.ptr_value() as *const AValueRepr<StarlarkStr>))
115124
}
125+
126+
/// Construct from a value. Returns [`None`] if a value does not contain a string.
127+
pub(crate) fn new(value: FrozenValue) -> Option<FrozenStringValue> {
128+
if value.unpack_str().is_some() {
129+
Some(unsafe { Self::new_unchecked(value) })
130+
} else {
131+
None
132+
}
133+
}
134+
135+
/// Get a string.
136+
pub(crate) fn as_str(self) -> &'static str {
137+
self.0.payload.unpack()
138+
}
116139
}
117140

118141
impl<'v> StringValue<'v> {

0 commit comments

Comments
 (0)