Skip to content

Commit ff3511e

Browse files
author
ffonion
committed
fix(vm): lower string ordered comparisons with lexicographic parity
Compiler-allowed string <, >, <=, >= currently reach the VM as Clt/Cgt opcodes, but both the std interpreter and the no-std runtime route them through numeric comparison and fail with TypeMismatch("number") on string operands, while Ceq already had a string-string fast path. Add a STRING_STRING operand-type-hint arm to the std Clt/Cgt handlers backed by a new string_compare_op that applies Rust str lexicographic ordering, and mirror it in pd-vm-nostd by replacing the numeric-only numeric_compare with a compare helper that dispatches string-string to str ordering and keeps int/float numeric semantics otherwise. Mixed string/number ordering stays a typed TypeMismatch error in both runtimes. Trace JIT and AOT cannot specialize string Clt/Cgt, so they defer to the interpreter (trace abandonment / whole-program AOT refusal); the new JIT tests pin that this produces the exact same result as a pure interpreter run, i.e. no semantic divergence on the JIT path. Tests: std compiler string ordering runtime cases (equality, empty, ASCII prefix, non-ASCII UTF-8, <= >=), mixed-type rejection in both VMs, no-std lexicographic parity via VMBC decode, trace-JIT fallback parity and AOT defer-to-interpreter parity.
1 parent d20d614 commit ff3511e

5 files changed

Lines changed: 329 additions & 11 deletions

File tree

pd-vm-nostd/src/vm.rs

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,16 @@ impl<C> Vm<C> {
475475
let lhs = self.pop()?;
476476
self.stack.push(Value::Bool(lhs == rhs));
477477
}
478-
OpCode::Clt => self.numeric_compare(|lhs, rhs| lhs < rhs, |lhs, rhs| lhs < rhs)?,
479-
OpCode::Cgt => self.numeric_compare(|lhs, rhs| lhs > rhs, |lhs, rhs| lhs > rhs)?,
478+
OpCode::Clt => self.compare(
479+
|lhs, rhs| lhs < rhs,
480+
|lhs, rhs| lhs < rhs,
481+
|lhs, rhs| lhs < rhs,
482+
)?,
483+
OpCode::Cgt => self.compare(
484+
|lhs, rhs| lhs > rhs,
485+
|lhs, rhs| lhs > rhs,
486+
|lhs, rhs| lhs > rhs,
487+
)?,
480488
OpCode::Br => {
481489
let target = self.read_u32()?;
482490
self.jump(target)?;
@@ -968,19 +976,31 @@ impl<C> Vm<C> {
968976
Ok(())
969977
}
970978

971-
fn numeric_compare(
979+
fn compare(
972980
&mut self,
973981
int_op: impl FnOnce(i64, i64) -> bool,
974982
float_op: impl FnOnce(f64, f64) -> bool,
983+
string_op: impl FnOnce(&str, &str) -> bool,
975984
) -> VmResult<()> {
976-
let rhs = self.pop_numeric()?;
977-
let lhs = self.pop_numeric()?;
978-
let result = match (lhs, rhs) {
979-
(NumericValue::Int(lhs), NumericValue::Int(rhs)) => int_op(lhs, rhs),
980-
(lhs, rhs) => float_op(as_float(lhs), as_float(rhs)),
981-
};
982-
self.stack.push(Value::Bool(result));
983-
Ok(())
985+
let rhs = self.pop()?;
986+
let lhs = self.pop()?;
987+
match (lhs, rhs) {
988+
(Value::String(lhs), Value::String(rhs)) => {
989+
self.stack
990+
.push(Value::Bool(string_op(lhs.as_str(), rhs.as_str())));
991+
Ok(())
992+
}
993+
(lhs, rhs) => {
994+
let rhs = numeric_value(rhs)?;
995+
let lhs = numeric_value(lhs)?;
996+
let result = match (lhs, rhs) {
997+
(NumericValue::Int(lhs), NumericValue::Int(rhs)) => int_op(lhs, rhs),
998+
(lhs, rhs) => float_op(as_float(lhs), as_float(rhs)),
999+
};
1000+
self.stack.push(Value::Bool(result));
1001+
Ok(())
1002+
}
1003+
}
9841004
}
9851005

9861006
fn pop(&mut self) -> VmResult<Value> {
@@ -1080,6 +1100,14 @@ impl<C> Vm<C> {
10801100
}
10811101
}
10821102

1103+
fn numeric_value(value: Value) -> VmResult<NumericValue> {
1104+
match value {
1105+
Value::Int(value) => Ok(NumericValue::Int(value)),
1106+
Value::Float(value) => Ok(NumericValue::Float(value)),
1107+
_ => Err(VmError::TypeMismatch("number")),
1108+
}
1109+
}
1110+
10831111
fn as_float(value: NumericValue) -> f64 {
10841112
match value {
10851113
NumericValue::Int(value) => value as f64,

pd-vm-nostd/tests/call_script_tests.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,3 +363,63 @@ fn call_script_binding_outside_frame_fails_typed() {
363363
"expected InvalidFrameState for the out-of-frame binding, got {err:?}"
364364
);
365365
}
366+
367+
#[test]
368+
fn string_ordered_comparison_matches_rust_lexicographic_semantics() {
369+
// Compiler-allowed string `<`/`>`/`<=`/`>=` must lower in the no-std VM
370+
// with the same Rust `str` lexicographic ordering as the std VM, and
371+
// mixed string/number ordering must remain a typed error.
372+
let compiled = compile_source(
373+
r#"
374+
let lt = "abc" < "abd";
375+
let gt = "abd" > "abc";
376+
let eq_le = "abc" <= "abc";
377+
let eq_ge = "abc" >= "abc";
378+
let empty_lt = "" < "a";
379+
let prefix = "ab" < "abc";
380+
let utf8 = ("é" > "e") && ("日本" < "英語");
381+
if lt && gt && eq_le && eq_ge && empty_lt && prefix && utf8 {
382+
1;
383+
} else {
384+
0;
385+
}
386+
"#,
387+
)
388+
.expect("string ordering source should compile");
389+
let bytes = encode_program(&compiled.program.with_local_count(compiled.locals))
390+
.expect("string ordering program should encode as VMBC v14");
391+
let program = decode_program(&bytes).expect("no-std should decode VMBC v14");
392+
393+
let mut vm = EmbeddedVm::new(program);
394+
assert_eq!(
395+
vm.run().expect("string ordering should halt"),
396+
EmbeddedVmStatus::Halted
397+
);
398+
assert_eq!(vm.stack(), &[EmbeddedValue::Int(1)]);
399+
}
400+
401+
#[test]
402+
fn string_numeric_mixed_ordered_comparison_remains_typed_error() {
403+
// `"abc" < 1` must stay a typed error in the no-std VM: the operand type
404+
// hint is (String, Int), which is not the string-string fast path and
405+
// must not be coerced into a numeric or string comparison.
406+
let compiled = compile_source(
407+
r#"
408+
let mixed = "abc" < 1;
409+
mixed;
410+
"#,
411+
)
412+
.expect("mixed ordering source should compile");
413+
let bytes = encode_program(&compiled.program.with_local_count(compiled.locals))
414+
.expect("mixed ordering program should encode");
415+
let program = decode_program(&bytes).expect("no-std should decode mixed ordering program");
416+
417+
let mut vm = EmbeddedVm::new(program);
418+
let err = vm
419+
.run()
420+
.expect_err("mixed string/number ordering must remain a typed error");
421+
assert!(
422+
matches!(err, VmError::TypeMismatch(_)),
423+
"expected TypeMismatch, got {err:?}"
424+
);
425+
}

src/vm/mod.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2780,6 +2780,24 @@ impl Vm {
27802780
Ok(())
27812781
}
27822782

2783+
pub(super) fn string_compare_op(
2784+
&mut self,
2785+
op: impl FnOnce(&str, &str) -> bool,
2786+
) -> VmResult<()> {
2787+
let rhs = match self.pop_value()? {
2788+
Value::String(value) => value,
2789+
_ => return Err(VmError::TypeMismatch("string")),
2790+
};
2791+
let lhs = match self.pop_value()? {
2792+
Value::String(value) => value,
2793+
_ => return Err(VmError::TypeMismatch("string")),
2794+
};
2795+
self.instance
2796+
.stack
2797+
.push(Value::Bool(op(lhs.as_str(), rhs.as_str())));
2798+
Ok(())
2799+
}
2800+
27832801
pub(super) fn null_eq_op(&mut self) -> VmResult<()> {
27842802
let rhs = self.pop_value()?;
27852803
let lhs = self.pop_value()?;
@@ -3669,6 +3687,10 @@ impl Vm {
36693687
self.record_operand_hint_hit();
36703688
self.float_compare_op(|lhs, rhs| lhs < rhs)?
36713689
}
3690+
STRING_STRING_OPERAND_TYPE_HINT => {
3691+
self.record_operand_hint_hit();
3692+
self.string_compare_op(|lhs, rhs| lhs < rhs)?
3693+
}
36723694
_ => {
36733695
self.record_operand_hint_miss();
36743696
self.compare_numeric_op(|lhs, rhs| lhs < rhs, |lhs, rhs| lhs < rhs)?
@@ -3686,6 +3708,10 @@ impl Vm {
36863708
self.record_operand_hint_hit();
36873709
self.float_compare_op(|lhs, rhs| lhs > rhs)?
36883710
}
3711+
STRING_STRING_OPERAND_TYPE_HINT => {
3712+
self.record_operand_hint_hit();
3713+
self.string_compare_op(|lhs, rhs| lhs > rhs)?
3714+
}
36893715
_ => {
36903716
self.record_operand_hint_miss();
36913717
self.compare_numeric_op(|lhs, rhs| lhs > rhs, |lhs, rhs| lhs > rhs)?

tests/compiler/compiler_rustscript_tests.rs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3512,6 +3512,104 @@ fn rustscript_language_runtime_cases_work() {
35123512
run_runtime_cases(&cases);
35133513
}
35143514

3515+
#[test]
3516+
fn rustscript_string_ordered_comparison_runtime_cases_work() {
3517+
let cases = vec![
3518+
RuntimeCase {
3519+
name: "string less-than is lexicographic",
3520+
source: r#"
3521+
("abc" < "abd") && ("abd" > "abc");
3522+
"#,
3523+
flavor: SourceFlavor::RustScript,
3524+
expected_stack: vec![Value::Bool(true)],
3525+
expected_locals: None,
3526+
},
3527+
RuntimeCase {
3528+
name: "equal strings are neither less-than nor greater-than",
3529+
source: r#"
3530+
let equal = ("abc" < "abc") == false && ("abc" > "abc") == false;
3531+
equal;
3532+
"#,
3533+
flavor: SourceFlavor::RustScript,
3534+
expected_stack: vec![Value::Bool(true)],
3535+
expected_locals: None,
3536+
},
3537+
RuntimeCase {
3538+
name: "less-than-or-equal / greater-than-or-equal include equality",
3539+
source: r#"
3540+
("abc" <= "abc") && ("abc" >= "abc") && ("abc" <= "abd") && ("abd" >= "abc");
3541+
"#,
3542+
flavor: SourceFlavor::RustScript,
3543+
expected_stack: vec![Value::Bool(true)],
3544+
expected_locals: None,
3545+
},
3546+
RuntimeCase {
3547+
name: "empty string compares before and after non-empty strings",
3548+
source: r#"
3549+
("" < "a") && (!("a" < "")) && ("" <= "") && ("" == "");
3550+
"#,
3551+
flavor: SourceFlavor::RustScript,
3552+
expected_stack: vec![Value::Bool(true)],
3553+
expected_locals: None,
3554+
},
3555+
RuntimeCase {
3556+
name: "ascii prefix ordering matches byte lexicographic order",
3557+
source: r#"
3558+
("ab" < "abc") && (!("abc" < "ab")) && ("abc" > "ab");
3559+
"#,
3560+
flavor: SourceFlavor::RustScript,
3561+
expected_stack: vec![Value::Bool(true)],
3562+
expected_locals: None,
3563+
},
3564+
RuntimeCase {
3565+
name: "non-ascii utf-8 compares by code-point lexicographic order",
3566+
source: r#"
3567+
("é" > "e") && ("日本" < "英語") && ("中" < "乙") && ("🌍" > "A");
3568+
"#,
3569+
flavor: SourceFlavor::RustScript,
3570+
expected_stack: vec![Value::Bool(true)],
3571+
expected_locals: None,
3572+
},
3573+
];
3574+
run_runtime_cases(&cases);
3575+
}
3576+
3577+
#[test]
3578+
fn rustscript_string_ordered_comparison_rejects_mixed_types() {
3579+
// A compiler-allowed comparison between a string and a number must not
3580+
// silently coerce: both the std VM and the no-std VM keep it a typed
3581+
// runtime error (TypeMismatch), never a lexicographic or numeric answer.
3582+
let source = r#"
3583+
let mixed = "abc" < 1;
3584+
mixed;
3585+
"#;
3586+
let compiled = compile_source_with_flavor(source, SourceFlavor::RustScript)
3587+
.expect("compile should succeed");
3588+
let mut vm = Vm::try_new(compiled.program).expect("test VM construction must not fail");
3589+
let err = vm
3590+
.run()
3591+
.expect_err("mixed string/number ordering must remain a typed error");
3592+
assert!(
3593+
matches!(err, vm::VmError::TypeMismatch(_)),
3594+
"expected TypeMismatch, got {err:?}"
3595+
);
3596+
3597+
let source_gt = r#"
3598+
let mixed = 1 >= "abc";
3599+
mixed;
3600+
"#;
3601+
let compiled_gt = compile_source_with_flavor(source_gt, SourceFlavor::RustScript)
3602+
.expect("compile should succeed");
3603+
let mut vm_gt = Vm::try_new(compiled_gt.program).expect("test VM construction must not fail");
3604+
let err_gt = vm_gt
3605+
.run()
3606+
.expect_err("mixed number/string ordering must remain a typed error");
3607+
assert!(
3608+
matches!(err_gt, vm::VmError::TypeMismatch(_)),
3609+
"expected TypeMismatch, got {err_gt:?}"
3610+
);
3611+
}
3612+
35153613
#[test]
35163614
fn rustscript_language_parse_rejection_cases_work() {
35173615
let cases = vec![

0 commit comments

Comments
 (0)