Skip to content

Commit 719ed47

Browse files
timfennisclaude
andcommitted
Migrate cmp/math/regex stdlib to VM-native types; fix cross-type numeric equality
- regex.rs: replace interpreter Value with ndc_vm::value::Value in nums, unsigned_nums, captures, capture_once — no bridge on the hot path - cmp.rs: assert_eq/assert_ne take ndc_vm::value::Value directly - math.rs: float() and int() match on VM Value patterns (Bool, Object::String, to_f64/to_number); drop unused interpreter Sequence import - value.rs: extend Value and Object PartialEq to handle cross-type numeric comparisons via ndc_core::Number (e.g. Rational(5/1) == Int(5)), consistent with the existing PartialOrd behaviour; latent bug exposed by assert_eq migration Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 17dcb46 commit 719ed47

4 files changed

Lines changed: 77 additions & 66 deletions

File tree

ndc_stdlib/src/cmp.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
mod inner {
33
use anyhow::anyhow;
44

5-
use ndc_interpreter::value::Value;
65
use std::cmp::Ordering;
76

87
/// Produces an error if the argument is not true.
@@ -15,7 +14,10 @@ mod inner {
1514
}
1615

1716
/// Produces an error if the arguments aren't equal to each other.
18-
pub fn assert_eq(left: &Value, right: &Value) -> anyhow::Result<()> {
17+
pub fn assert_eq(
18+
left: ndc_vm::value::Value,
19+
right: ndc_vm::value::Value,
20+
) -> anyhow::Result<()> {
1921
if left == right {
2022
Ok(())
2123
} else {
@@ -26,7 +28,10 @@ mod inner {
2628
}
2729

2830
/// Produces an error if the arguments are equal to each other.
29-
pub fn assert_ne(left: &Value, right: &Value) -> anyhow::Result<()> {
31+
pub fn assert_ne(
32+
left: ndc_vm::value::Value,
33+
right: ndc_vm::value::Value,
34+
) -> anyhow::Result<()> {
3035
if left == right {
3136
Err(anyhow!(format!(
3237
"failed asserting that {left} does not equal {right}"

ndc_stdlib/src/math.rs

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use factorial::Factorial;
22
use ndc_interpreter::environment::Environment;
33
use ndc_interpreter::num::{BinaryOperatorError, Number};
4-
use ndc_interpreter::sequence::Sequence;
54
use ndc_macros::export_module;
65
use num::ToPrimitive;
76
use std::ops::{Add, Mul};
@@ -13,8 +12,6 @@ mod inner {
1312
use anyhow::Context;
1413
use ndc_interpreter::int::Int;
1514
use ndc_interpreter::num::Number;
16-
use ndc_interpreter::sequence::Sequence;
17-
use ndc_interpreter::value::Value;
1815
use num::{BigInt, BigRational, BigUint, Integer, complex::Complex64};
1916

2017
/// Returns the sign of a number.
@@ -110,28 +107,18 @@ mod inner {
110107
Ok(left.sub(right)?.abs())
111108
}
112109

113-
pub fn float(value: &Value) -> anyhow::Result<f64> {
114-
match value {
115-
Value::Number(Number::Int(Int::BigInt(i))) => i
116-
.to_f64()
117-
.ok_or_else(|| anyhow::anyhow!("failed to convert int to float (overflow?)")),
118-
Value::Number(Number::Int(Int::Int64(i))) => i
110+
pub fn float(value: ndc_vm::value::Value) -> anyhow::Result<f64> {
111+
match &value {
112+
ndc_vm::value::Value::Bool(b) => Ok(if *b { 1.0 } else { 0.0 }),
113+
ndc_vm::value::Value::Object(obj) => match obj.as_ref() {
114+
ndc_vm::value::Object::String(s) => Ok(s.borrow().parse::<f64>()?),
115+
_ => value
116+
.to_f64()
117+
.ok_or_else(|| anyhow::anyhow!("cannot convert {} to float", value.static_type())),
118+
},
119+
_ => value
119120
.to_f64()
120-
.ok_or_else(|| anyhow::anyhow!("failed to convert int to float (overflow?)")),
121-
Value::Number(Number::Rational(r)) => r
122-
.to_f64()
123-
.ok_or_else(|| anyhow::anyhow!("failed to convert rational to float (overflow?)")),
124-
Value::Number(Number::Float(f)) => Ok(*f),
125-
Value::Bool(true) => Ok(1.0),
126-
Value::Bool(false) => Ok(0.0),
127-
Value::Sequence(Sequence::String(string)) => {
128-
let string = string.borrow();
129-
Ok(string.parse::<f64>()?)
130-
}
131-
value => Err(anyhow::anyhow!(
132-
"cannot convert {} to float",
133-
value.static_type()
134-
)),
121+
.ok_or_else(|| anyhow::anyhow!("cannot convert {} to float", value.static_type())),
135122
}
136123
}
137124

@@ -147,20 +134,25 @@ mod inner {
147134
/// - Rational numbers are rounded down
148135
/// - `true` is converted to `1`, and `false` to `0`
149136
/// - Strings are parsed as decimal integers; other representations result in an error
150-
pub fn int(value: &Value) -> anyhow::Result<Number> {
151-
match value {
152-
Value::Number(number) => Ok(number.to_int_lossy()?),
153-
Value::Bool(true) => Ok(Number::from(1)),
154-
Value::Bool(false) => Ok(Number::from(0)),
155-
Value::Sequence(Sequence::String(string)) => {
156-
let string = string.borrow();
157-
let bi = string.parse::<BigInt>()?;
158-
Ok(Number::Int(Int::BigInt(bi).simplified()))
159-
}
160-
value => Err(anyhow::anyhow!(
161-
"cannot convert {} to int",
162-
value.static_type()
163-
)),
137+
pub fn int(value: ndc_vm::value::Value) -> anyhow::Result<Number> {
138+
match &value {
139+
ndc_vm::value::Value::Bool(b) => Ok(Number::from(if *b { 1i32 } else { 0i32 })),
140+
ndc_vm::value::Value::Object(obj) => match obj.as_ref() {
141+
ndc_vm::value::Object::String(s) => {
142+
let bi = s.borrow().parse::<BigInt>()?;
143+
Ok(Number::Int(Int::BigInt(bi).simplified()))
144+
}
145+
_ => value
146+
.to_number()
147+
.ok_or_else(|| anyhow::anyhow!("cannot convert {} to int", value.static_type()))?
148+
.to_int_lossy()
149+
.map_err(|e| anyhow::anyhow!("{e}")),
150+
},
151+
_ => value
152+
.to_number()
153+
.ok_or_else(|| anyhow::anyhow!("cannot convert {} to int", value.static_type()))?
154+
.to_int_lossy()
155+
.map_err(|e| anyhow::anyhow!("{e}")),
164156
}
165157
}
166158
}

ndc_stdlib/src/regex.rs

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,38 @@
1-
use ndc_interpreter::value::Value;
21
use once_cell::sync::Lazy;
32
use regex::Regex;
43

54
#[ndc_macros::export_module]
65
mod inner {
6+
use ndc_vm::value::Value;
77

88
/// Extracts all signed integers from the given string.
99
#[function(return_type = Vec<i64>)]
10-
pub fn nums(haystack: &str) -> Value {
10+
pub fn nums(haystack: &str) -> ndc_vm::value::Value {
1111
static RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"-?\d+").unwrap());
1212

13-
Value::collect_list(RE.captures_iter(haystack).filter_map(|cap| {
14-
let (full, []) = cap.extract();
15-
full.parse::<i64>().ok()
16-
}))
13+
Value::list(
14+
RE.captures_iter(haystack)
15+
.filter_map(|cap| {
16+
let (full, []) = cap.extract();
17+
full.parse::<i64>().ok().map(Value::Int)
18+
})
19+
.collect(),
20+
)
1721
}
1822

1923
/// Extracts all unsigned integers from the given string.
2024
#[function(return_type = Vec<i64>)]
21-
pub fn unsigned_nums(haystack: &str) -> Value {
25+
pub fn unsigned_nums(haystack: &str) -> ndc_vm::value::Value {
2226
static RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"\d+").unwrap());
2327

24-
Value::collect_list(RE.captures_iter(haystack).filter_map(|cap| {
25-
let (full, []) = cap.extract();
26-
full.parse::<i64>().ok()
27-
}))
28+
Value::list(
29+
RE.captures_iter(haystack)
30+
.filter_map(|cap| {
31+
let (full, []) = cap.extract();
32+
full.parse::<i64>().ok().map(Value::Int)
33+
})
34+
.collect(),
35+
)
2836
}
2937

3038
/// Returns `true` if the string matches the given regular expression.
@@ -35,7 +43,7 @@ mod inner {
3543

3644
/// Returns all capture groups from the first match of the regular expression.
3745
#[function(return_type = Vec<String>)]
38-
pub fn captures(haystack: &str, regex: &str) -> Result<Value, regex::Error> {
46+
pub fn captures(haystack: &str, regex: &str) -> Result<ndc_vm::value::Value, regex::Error> {
3947
let r = Regex::new(regex)?;
4048

4149
let list = r
@@ -44,7 +52,7 @@ mod inner {
4452
Value::list(
4553
captures
4654
.iter()
47-
.filter_map(|x| x.map(|x| Value::from(x.as_str())))
55+
.filter_map(|x| x.map(|x| Value::string(x.as_str())))
4856
.collect::<Vec<_>>(),
4957
)
5058
})
@@ -55,16 +63,19 @@ mod inner {
5563

5664
/// Returns the first capture group from the first match of the regular expression.
5765
#[function(return_type = Vec<String>)]
58-
pub fn capture_once(haystack: &str, regex: &str) -> Result<Value, regex::Error> {
66+
pub fn capture_once(
67+
haystack: &str,
68+
regex: &str,
69+
) -> Result<ndc_vm::value::Value, regex::Error> {
5970
let r = Regex::new(regex)?;
6071

6172
let Some(captures) = r.captures(haystack) else {
62-
return Ok(Value::empty_list());
73+
return Ok(Value::list(vec![]));
6374
};
6475

6576
let list = captures
6677
.iter()
67-
.filter_map(|x| x.map(|x| Value::from(x.as_str())))
78+
.filter_map(|x| x.map(|x| Value::string(x.as_str())))
6879
.collect::<Vec<_>>();
6980

7081
Ok(Value::list(list))

ndc_vm/src/value.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -627,14 +627,15 @@ impl PartialEq for Value {
627627
match (self, other) {
628628
(Self::Int(a), Self::Int(b)) => a == b,
629629
(Self::Float(a), Self::Float(b)) => OrderedFloat(*a) == OrderedFloat(*b),
630-
// Cross-type numeric equality: consistent with PartialOrd's cross-numeric path.
631-
(Self::Int(_), Self::Float(_)) | (Self::Float(_), Self::Int(_)) => {
632-
vm_value_to_number(self) == vm_value_to_number(other)
633-
}
634630
(Self::Bool(a), Self::Bool(b)) => a == b,
635631
(Self::None, Self::None) => true,
636632
(Self::Object(a), Self::Object(b)) => a == b,
637-
_ => false,
633+
// Cross-type numeric equality: delegate to Number, consistent with PartialOrd.
634+
// Covers Int vs Float, Int vs Rational, Int vs BigInt, Float vs Rational, etc.
635+
(a, b) => match (vm_value_to_number(a), vm_value_to_number(b)) {
636+
(Some(a), Some(b)) => a == b,
637+
_ => false,
638+
},
638639
}
639640
}
640641
}
@@ -677,9 +678,6 @@ impl PartialEq for Object {
677678
fn eq(&self, other: &Self) -> bool {
678679
match (self, other) {
679680
(Self::Some(a), Self::Some(b)) => a == b,
680-
(Self::BigInt(a), Self::BigInt(b)) => a == b,
681-
(Self::Complex(a), Self::Complex(b)) => a == b,
682-
(Self::Rational(a), Self::Rational(b)) => a == b,
683681
(Self::String(a), Self::String(b)) => a.borrow().eq(&*b.borrow()),
684682
(Self::List(a), Self::List(b)) => a.borrow().eq(&*b.borrow()),
685683
(Self::Tuple(a), Self::Tuple(b)) => a == b,
@@ -720,7 +718,12 @@ impl PartialEq for Object {
720718
// address is equivalent to comparing the outer Rc pointers.
721719
(Self::MinHeap(a), Self::MinHeap(b)) => std::ptr::eq(a, b),
722720
(Self::MaxHeap(a), Self::MaxHeap(b)) => std::ptr::eq(a, b),
723-
_ => false,
721+
// Numeric types: delegate to Number for cross-type equality
722+
// (e.g. BigInt(5) == Rational(5/1), Rational(5/1) == Complex(5+0i)).
723+
(a, b) => match (obj_to_number(a), obj_to_number(b)) {
724+
(Some(a), Some(b)) => a == b,
725+
_ => false,
726+
},
724727
}
725728
}
726729
}

0 commit comments

Comments
 (0)