Skip to content

Commit 370d5e3

Browse files
stepanchegfacebook-github-bot
authored andcommitted
Pass bool to eval_equals
Summary: Instead of fn. This is used in the following diffs (we can match on `bool` but cannot match on `fn`). Note I tried to replace `fn(bool) -> bool` and call like `cmp(a == b)` with: * `(a == b) == positive` * creating two different closures for `==` and `!=` but for reasons I don't understand, it leads to perf regression. Reviewed By: ndmitchell Differential Revision: D30888379 fbshipit-source-id: fe936adc019cb69e45f2a51741982b7e36c54617
1 parent 8b1c923 commit 370d5e3

File tree

1 file changed

+24
-6
lines changed

1 file changed

+24
-6
lines changed

starlark/src/eval/fragment/expr.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,22 @@ use gazebo::{coerce::coerce_ref, prelude::*};
4343
use std::cmp::Ordering;
4444
use thiserror::Error;
4545

46+
/// `bool` operation.
47+
#[derive(Copy, Clone, Dupe)]
48+
enum MaybeNot {
49+
Id,
50+
Not,
51+
}
52+
53+
impl MaybeNot {
54+
fn as_fn(self) -> fn(bool) -> bool {
55+
match self {
56+
MaybeNot::Id => |x| x,
57+
MaybeNot::Not => |x| !x,
58+
}
59+
}
60+
}
61+
4662
pub(crate) type ExprCompiled = Box<
4763
dyn for<'v> Fn(&mut Evaluator<'v, '_>) -> Result<Value<'v>, ExprEvalException> + Send + Sync,
4864
>;
@@ -102,12 +118,13 @@ fn eval_compare(
102118
fn try_eval_type_is(
103119
l: ExprCompiledValue,
104120
r: ExprCompiledValue,
105-
cmp: fn(bool) -> bool,
121+
maybe_not: MaybeNot,
106122
) -> Result<ExprCompiledValue, (ExprCompiledValue, ExprCompiledValue)> {
107123
match (l, r) {
108124
(ExprCompiledValue::Type(l), ExprCompiledValue::Value(r)) => {
109125
if let Some(r) = r.downcast_frozen_ref::<StarlarkStr>() {
110126
let t = r.map(|r| r.unpack());
127+
let cmp = maybe_not.as_fn();
111128
Ok(expr!("type_is", l, |_eval| {
112129
Value::new_bool(cmp(l.get_type() == t.as_ref()))
113130
}))
@@ -123,21 +140,22 @@ fn eval_equals(
123140
span: Span,
124141
l: ExprCompiledValue,
125142
r: ExprCompiledValue,
126-
cmp: fn(bool) -> bool,
143+
maybe_not: MaybeNot,
127144
) -> ExprCompiledValue {
145+
let cmp = maybe_not.as_fn();
128146
if let (Some(l), Some(r)) = (l.as_value(), r.as_value()) {
129147
// If comparison fails, let it fail in runtime.
130148
if let Ok(r) = l.equals(r.to_value()) {
131149
return value!(FrozenValue::new_bool(cmp(r)));
132150
}
133151
}
134152

135-
let (l, r) = match try_eval_type_is(l, r, cmp) {
153+
let (l, r) = match try_eval_type_is(l, r, maybe_not) {
136154
Ok(e) => return e,
137155
Err((l, r)) => (l, r),
138156
};
139157

140-
let (r, l) = match try_eval_type_is(r, l, cmp) {
158+
let (r, l) = match try_eval_type_is(r, l, maybe_not) {
141159
Ok(e) => return e,
142160
Err((r, l)) => (r, l),
143161
};
@@ -607,8 +625,8 @@ impl Compiler<'_> {
607625
})
608626
}
609627
}
610-
BinOp::Equal => eval_equals(span, l, r, |x| x),
611-
BinOp::NotEqual => eval_equals(span, l, r, |x| !x),
628+
BinOp::Equal => eval_equals(span, l, r, MaybeNot::Id),
629+
BinOp::NotEqual => eval_equals(span, l, r, MaybeNot::Not),
612630
BinOp::Less => eval_compare(span, l, r, |x| x == Ordering::Less),
613631
BinOp::Greater => eval_compare(span, l, r, |x| x == Ordering::Greater),
614632
BinOp::LessOrEqual => eval_compare(span, l, r, |x| x != Ordering::Greater),

0 commit comments

Comments
 (0)