Skip to content

Commit dad62a0

Browse files
olwangclaude
andcommitted
perf(reg-vm): elision Slice 2 — scalar pattern-binds don't taint the scrutinee
Thread the scrutinee's static type into the reg-VM pattern lowerers so a scalar element/field/payload extracted by a pattern bind is marked `note_scalar` and no longer taints its (read-param) source. Covers list-pattern elements, struct-field binds, and variant/Option/Result scalar payload binds. Makes `UnwrapSome` symmetric with `UnwrapVariantValue` in the elision analysis, which unblocks the `Option<Scalar>` unwrap chain. Conservative (unmarked) where the scrutinee type is statically unavailable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7f031c1 commit dad62a0

4 files changed

Lines changed: 190 additions & 42 deletions

File tree

crates/rsscript/src/reg_vm/lower.rs

Lines changed: 107 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,17 @@ fn list_elem_type(collection_ty: &str) -> Option<&str> {
3737
}
3838
}
3939

40+
/// The `index`-th top-level generic argument of a (possibly `None`) type spelling,
41+
/// or `None` when the type is unknown / not generic / has too few arguments.
42+
/// Used to derive a pattern payload type from the scrutinee's static type:
43+
/// arg 0 of `Option<Int>` / `Result<Int, E>` is the `Some`/`Ok` payload, arg 1 of
44+
/// `Result<T, Int>` is the `Err` payload. `None` ⇒ conservative: bind stays tainted.
45+
fn nth_type_arg(type_name: Option<&str>, index: usize) -> Option<&str> {
46+
crate::text_util::type_arg_names(type_name?)?
47+
.get(index)
48+
.copied()
49+
}
50+
4051
impl RegLowerer<'_> {
4152
/// Record that `dst` holds a `Copy` scalar when `elem_ty` is a known scalar
4253
/// type (`Int`/`Bool`/`Float`/`Char`/…). Extracting such a value is a bit-copy
@@ -2018,12 +2029,16 @@ impl RegLowerer<'_> {
20182029
}
20192030

20202031
let src = self.expr(value)?;
2032+
// Scrutinee's static type threads into pattern lowering so a scalar
2033+
// element/field/payload extraction can be marked `note_scalar` and thus
2034+
// not taint the (read-param) scrutinee. `None` ⇒ conservative (unmarked).
2035+
let scrutinee_ty = reg_expr_type_name(value);
20212036
let mut failure_patches = Vec::new();
20222037
let mut end_jumps = Vec::new();
20232038
for arm in arms {
20242039
let arm_ip = self.function.code.len();
20252040
self.patch_match_failures(failure_patches, arm_ip);
2026-
failure_patches = self.lower_match_pattern(&arm.pattern, src)?;
2041+
failure_patches = self.lower_match_pattern(&arm.pattern, src, scrutinee_ty)?;
20272042
if let Some(guard) = &arm.guard {
20282043
let guard_failure = self.condition_jump(guard, false, usize::MAX)?;
20292044
failure_patches.push(MatchFailurePatch::Jump(guard_failure));
@@ -2056,13 +2071,14 @@ impl RegLowerer<'_> {
20562071
}
20572072

20582073
let src = self.expr(value)?;
2074+
let scrutinee_ty = reg_expr_type_name(value);
20592075
let dst = self.temp();
20602076
let mut failure_patches = Vec::new();
20612077
let mut end_jumps = Vec::new();
20622078
for arm in arms {
20632079
let arm_ip = self.function.code.len();
20642080
self.patch_match_failures(failure_patches, arm_ip);
2065-
failure_patches = self.lower_match_pattern(&arm.pattern, src)?;
2081+
failure_patches = self.lower_match_pattern(&arm.pattern, src, scrutinee_ty)?;
20662082
if let Some(guard) = &arm.guard {
20672083
let guard_failure = self.condition_jump(guard, false, usize::MAX)?;
20682084
failure_patches.push(MatchFailurePatch::Jump(guard_failure));
@@ -2088,6 +2104,7 @@ impl RegLowerer<'_> {
20882104
&mut self,
20892105
pattern: &MatchPattern,
20902106
src: Reg,
2107+
scrutinee_ty: Option<&str>,
20912108
) -> Result<Vec<MatchFailurePatch>, EvalError> {
20922109
match pattern {
20932110
MatchPattern::Binding { name, .. } => {
@@ -2097,7 +2114,9 @@ impl RegLowerer<'_> {
20972114
}
20982115
MatchPattern::Wildcard(_) => Ok(Vec::new()),
20992116
MatchPattern::Variant { name, bindings, .. } if name == "Some" => {
2100-
self.lower_option_some_pattern(src, bindings.first())
2117+
// `Some(x)` on `Option<T>` unwraps the payload `T` (arg 0).
2118+
let payload_ty = nth_type_arg(scrutinee_ty, 0);
2119+
self.lower_option_some_pattern(src, bindings.first(), payload_ty)
21012120
}
21022121
MatchPattern::Variant { name, .. } if name == "None" => {
21032122
let match_ip = self.emit(RegInstr::MatchOption {
@@ -2110,7 +2129,9 @@ impl RegLowerer<'_> {
21102129
Ok(vec![MatchFailurePatch::OptionSome(match_ip)])
21112130
}
21122131
MatchPattern::Variant { name, bindings, .. } if name == "Ok" || name == "Err" => {
2113-
self.lower_result_variant_pattern(src, name, bindings.first())
2132+
// `Ok`/`Err` on `Result<T, E>` unwrap arg 0 / arg 1 respectively.
2133+
let payload_ty = nth_type_arg(scrutinee_ty, if name == "Ok" { 0 } else { 1 });
2134+
self.lower_result_variant_pattern(src, name, bindings.first(), payload_ty)
21142135
}
21152136
MatchPattern::Variant { name, bindings, .. }
21162137
if self.hir.sum_type_for_variant(name).is_some() =>
@@ -2122,13 +2143,15 @@ impl RegLowerer<'_> {
21222143
{
21232144
self.lower_user_struct_variant_pattern(src, name, fields)
21242145
}
2125-
MatchPattern::Struct { fields, .. } => self.lower_struct_field_patterns(src, fields),
2146+
MatchPattern::Struct { fields, .. } => {
2147+
self.lower_struct_field_patterns(src, fields, scrutinee_ty)
2148+
}
21262149
MatchPattern::List {
21272150
prefix,
21282151
rest,
21292152
suffix,
21302153
..
2131-
} => self.lower_list_pattern(src, prefix, rest, suffix),
2154+
} => self.lower_list_pattern(src, prefix, rest, suffix, scrutinee_ty),
21322155
MatchPattern::Literal { value, .. } => self.lower_literal_pattern(src, value),
21332156
_ => Err(EvalError::Runtime(
21342157
"reg VM v0 does not support this match pattern.".to_string(),
@@ -2182,20 +2205,33 @@ impl RegLowerer<'_> {
21822205
&mut self,
21832206
src: Reg,
21842207
fields: &[MatchFieldPattern],
2208+
scrutinee_ty: Option<&str>,
21852209
) -> Result<Vec<MatchFailurePatch>, EvalError> {
21862210
let mut failures = Vec::new();
2211+
// Resolve the struct decl once (decoupled from `self` so `note_scalar`'s
2212+
// `&mut self` borrow is free); each field's declared type gates the mark.
2213+
let hir = self.hir;
2214+
let owner = scrutinee_ty
2215+
.map(|ty| crate::text_util::type_root_name(ty))
2216+
.and_then(|root| hir.type_info(root));
21872217
for field in fields {
21882218
if field.ignored {
21892219
continue;
21902220
}
2221+
let field_ty = owner
2222+
.and_then(|info| info.fields.get(&field.name))
2223+
.map(|info| info.type_name.as_str());
21912224
let field_reg = self.temp();
21922225
self.emit(RegInstr::GetField {
21932226
dst: field_reg,
21942227
base: src,
21952228
name: field.name.clone(),
21962229
});
2230+
// A scalar field is a bit-copy: marking `field_reg` stops the `GetField`
2231+
// from tainting the scrutinee (the read param). Non-scalar ⇒ unmarked.
2232+
self.note_scalar(field_reg, field_ty);
21972233
if let Some(pattern) = field.pattern.as_deref() {
2198-
failures.extend(self.lower_match_pattern(pattern, field_reg)?);
2234+
failures.extend(self.lower_match_pattern(pattern, field_reg, field_ty)?);
21992235
} else if let Some(binding) = field.binding.as_ref() {
22002236
let dst = self.local(binding);
22012237
self.emit(RegInstr::Move {
@@ -2221,7 +2257,11 @@ impl RegLowerer<'_> {
22212257
prefix: &[MatchPattern],
22222258
rest: &Option<Option<String>>,
22232259
suffix: &[MatchPattern],
2260+
scrutinee_ty: Option<&str>,
22242261
) -> Result<Vec<MatchFailurePatch>, EvalError> {
2262+
// Element type of the matched `List<T>`/`Deque<T>` (the `ListGet` result);
2263+
// a scalar `T` lets the per-element extraction skip tainting the scrutinee.
2264+
let elem_ty = scrutinee_ty.and_then(list_elem_type);
22252265
let mut failures = Vec::new();
22262266
let required = (prefix.len() + suffix.len()) as i64;
22272267
let len = self.temp();
@@ -2267,7 +2307,8 @@ impl RegLowerer<'_> {
22672307
list: src,
22682308
index: idx,
22692309
});
2270-
failures.extend(self.lower_match_pattern(pattern, elem)?);
2310+
self.note_scalar(elem, elem_ty);
2311+
failures.extend(self.lower_match_pattern(pattern, elem, elem_ty)?);
22712312
}
22722313
if let Some(Some(rest_name)) = rest {
22732314
let start = self.temp();
@@ -2318,7 +2359,8 @@ impl RegLowerer<'_> {
23182359
list: src,
23192360
index: idx,
23202361
});
2321-
failures.extend(self.lower_match_pattern(pattern, elem)?);
2362+
self.note_scalar(elem, elem_ty);
2363+
failures.extend(self.lower_match_pattern(pattern, elem, elem_ty)?);
23222364
}
23232365
}
23242366
Ok(failures)
@@ -2379,6 +2421,7 @@ impl RegLowerer<'_> {
23792421
&mut self,
23802422
src: Reg,
23812423
binding: Option<&MatchPattern>,
2424+
payload_ty: Option<&str>,
23822425
) -> Result<Vec<MatchFailurePatch>, EvalError> {
23832426
let match_ip = self.emit(RegInstr::MatchOption {
23842427
src,
@@ -2393,12 +2436,16 @@ impl RegLowerer<'_> {
23932436
MatchPattern::Binding { name, .. } => {
23942437
let dst = self.local(name);
23952438
self.emit(RegInstr::UnwrapSome { dst, src });
2439+
// A scalar `Some` payload is a bit-copy ⇒ the unwrap must not
2440+
// taint the scrutinee (the read-param `Option<Scalar>`).
2441+
self.note_scalar(dst, payload_ty);
23962442
}
23972443
MatchPattern::Wildcard(_) => {}
23982444
_ => {
23992445
let payload = self.temp();
24002446
self.emit(RegInstr::UnwrapSome { dst: payload, src });
2401-
failures.extend(self.lower_match_pattern(binding, payload)?);
2447+
self.note_scalar(payload, payload_ty);
2448+
failures.extend(self.lower_match_pattern(binding, payload, payload_ty)?);
24022449
}
24032450
}
24042451
}
@@ -2410,6 +2457,7 @@ impl RegLowerer<'_> {
24102457
src: Reg,
24112458
variant: &str,
24122459
binding: Option<&MatchPattern>,
2460+
payload_ty: Option<&str>,
24132461
) -> Result<Vec<MatchFailurePatch>, EvalError> {
24142462
let match_ip = self.emit(RegInstr::MatchResult {
24152463
src,
@@ -2437,6 +2485,8 @@ impl RegLowerer<'_> {
24372485
src,
24382486
expected: variant.to_string(),
24392487
});
2488+
// Scalar `Ok`/`Err` payload ⇒ bit-copy ⇒ don't taint scrutinee.
2489+
self.note_scalar(dst, payload_ty);
24402490
}
24412491
MatchPattern::Wildcard(_) => {}
24422492
_ => {
@@ -2446,7 +2496,8 @@ impl RegLowerer<'_> {
24462496
src,
24472497
expected: variant.to_string(),
24482498
});
2449-
failures.extend(self.lower_match_pattern(binding, payload)?);
2499+
self.note_scalar(payload, payload_ty);
2500+
failures.extend(self.lower_match_pattern(binding, payload, payload_ty)?);
24502501
}
24512502
}
24522503
}
@@ -2459,13 +2510,11 @@ impl RegLowerer<'_> {
24592510
variant: &str,
24602511
bindings: &[MatchPattern],
24612512
) -> Result<Vec<MatchFailurePatch>, EvalError> {
2462-
let field_names: Vec<String> = self
2463-
.hir
2464-
.sum_variant_fields(variant)
2465-
.unwrap_or(&[])
2466-
.iter()
2467-
.map(|field| field.name.clone())
2468-
.collect();
2513+
// Variant field decls (name + declared type). Bound to a `&'a` slice via a
2514+
// local copy of `self.hir` so `note_scalar`'s `&mut self` is free below.
2515+
let hir = self.hir;
2516+
let field_infos = hir.sum_variant_fields(variant).unwrap_or(&[]);
2517+
let field_names: Vec<String> = field_infos.iter().map(|field| field.name.clone()).collect();
24692518
let match_ip = self.emit(RegInstr::MatchVariant {
24702519
src,
24712520
expected: variant.to_string(),
@@ -2487,40 +2536,50 @@ impl RegLowerer<'_> {
24872536
[] => {}
24882537
// Single-payload sugar keeps the `UnwrapVariantValue` projection so the
24892538
// native scalar-replacement pass can still dissolve the variant.
2490-
[binding] => match binding {
2491-
MatchPattern::Binding { name, .. } => {
2492-
let dst = self.local(name);
2493-
self.emit(RegInstr::UnwrapVariantValue {
2494-
dst,
2495-
src,
2496-
expected: variant.to_string(),
2497-
});
2498-
}
2499-
MatchPattern::Wildcard(_) => {}
2500-
_ => {
2501-
let payload = self.temp();
2502-
self.emit(RegInstr::UnwrapVariantValue {
2503-
dst: payload,
2504-
src,
2505-
expected: variant.to_string(),
2506-
});
2507-
failures.extend(self.lower_match_pattern(binding, payload)?);
2539+
[binding] => {
2540+
// Single-field variant: the payload type is the sole field's type.
2541+
let payload_ty = field_infos.first().map(|field| field.type_name.as_str());
2542+
match binding {
2543+
MatchPattern::Binding { name, .. } => {
2544+
let dst = self.local(name);
2545+
self.emit(RegInstr::UnwrapVariantValue {
2546+
dst,
2547+
src,
2548+
expected: variant.to_string(),
2549+
});
2550+
self.note_scalar(dst, payload_ty);
2551+
}
2552+
MatchPattern::Wildcard(_) => {}
2553+
_ => {
2554+
let payload = self.temp();
2555+
self.emit(RegInstr::UnwrapVariantValue {
2556+
dst: payload,
2557+
src,
2558+
expected: variant.to_string(),
2559+
});
2560+
self.note_scalar(payload, payload_ty);
2561+
failures.extend(self.lower_match_pattern(binding, payload, payload_ty)?);
2562+
}
25082563
}
2509-
},
2564+
}
25102565
// Positional multi-field binding routes through the same per-field
25112566
// `GetField` projection as `lower_user_struct_variant_pattern`, so the
25122567
// reg-VM and AOT share the struct-variant field-projection semantics.
25132568
_ => {
2514-
for (binding, field_name) in bindings.iter().zip(field_names.iter()) {
2569+
for (index, (binding, field_name)) in
2570+
bindings.iter().zip(field_names.iter()).enumerate()
2571+
{
25152572
if matches!(binding, MatchPattern::Wildcard(_)) {
25162573
continue;
25172574
}
2575+
let field_ty = field_infos.get(index).map(|field| field.type_name.as_str());
25182576
let field_reg = self.temp();
25192577
self.emit(RegInstr::GetField {
25202578
dst: field_reg,
25212579
base: src,
25222580
name: field_name.clone(),
25232581
});
2582+
self.note_scalar(field_reg, field_ty);
25242583
match binding {
25252584
MatchPattern::Binding { name, .. } => {
25262585
let dst = self.local(name);
@@ -2530,7 +2589,7 @@ impl RegLowerer<'_> {
25302589
});
25312590
}
25322591
_ => {
2533-
failures.extend(self.lower_match_pattern(binding, field_reg)?);
2592+
failures.extend(self.lower_match_pattern(binding, field_reg, field_ty)?);
25342593
}
25352594
}
25362595
}
@@ -2554,18 +2613,26 @@ impl RegLowerer<'_> {
25542613
let pass_ip = self.function.code.len();
25552614
self.patch_jump(match_ip, pass_ip);
25562615
let mut failures = vec![MatchFailurePatch::VariantOther(match_ip)];
2616+
// Struct-variant field decls (name → declared type), decoupled from `self`.
2617+
let hir = self.hir;
2618+
let field_infos = hir.sum_variant_fields(variant).unwrap_or(&[]);
25572619
for field in fields {
25582620
if field.ignored {
25592621
continue;
25602622
}
2623+
let field_ty = field_infos
2624+
.iter()
2625+
.find(|info| info.name == field.name)
2626+
.map(|info| info.type_name.as_str());
25612627
let field_reg = self.temp();
25622628
self.emit(RegInstr::GetField {
25632629
dst: field_reg,
25642630
base: src,
25652631
name: field.name.clone(),
25662632
});
2633+
self.note_scalar(field_reg, field_ty);
25672634
if let Some(pattern) = field.pattern.as_deref() {
2568-
failures.extend(self.lower_match_pattern(pattern, field_reg)?);
2635+
failures.extend(self.lower_match_pattern(pattern, field_reg, field_ty)?);
25692636
} else if let Some(binding) = field.binding.as_ref() {
25702637
let dst = self.local(binding);
25712638
self.emit(RegInstr::Move {

crates/rsscript/src/reg_vm/model.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,7 +2362,7 @@ fn deepcopy_intrinsic_class(intrinsic: RegIntrinsic) -> IntrinsicTaintClass {
23622362
/// Classification:
23632363
/// * In-place mutation of a tainted heap receiver → keep (mirrors native's receiver set;
23642364
/// the broader interpreter-only mutators fall through to the conservative default).
2365-
/// * Alias-PROPAGATION reads (`Move`/`*Get`/`GetField*`/`UnwrapVariantValue`/`DequePop*`) →
2365+
/// * Alias-PROPAGATION reads (`Move`/`*Get`/`GetField*`/`UnwrapSome`/`UnwrapVariantValue`/`DequePop*`) →
23662366
/// safe: `dst` is already tainted by the closure, and the op does not itself mutate/escape.
23672367
/// * Calls (`CallKnown`/`CallDynamic`/`CallNative`/`CallClosure`) → keep ONLY if a tainted
23682368
/// value sits in a `mut_args` position; `read` args (and the `closure` receiver) are safe
@@ -2393,6 +2393,7 @@ fn deepcopy_instr_forces_keep(instr: &RegInstr, tainted: &[bool], n_regs: usize)
23932393
| RegInstr::MapGet { .. }
23942394
| RegInstr::GetField { .. }
23952395
| RegInstr::GetFieldSlot { .. }
2396+
| RegInstr::UnwrapSome { .. }
23962397
| RegInstr::UnwrapVariantValue { .. }
23972398
| RegInstr::DequePopFront { .. }
23982399
| RegInstr::DequePopBack { .. } => false,
@@ -2514,7 +2515,7 @@ fn deepcopy_elidable_param_regs(
25142515
}
25152516
}
25162517
// Extractions (`ListGet`/`MapGet`/`GetField`/`GetFieldSlot`/
2517-
// `UnwrapVariantValue`/`DequePop*`) pull an INTERIOR value out of a
2518+
// `UnwrapSome`/`UnwrapVariantValue`/`DequePop*`) pull an INTERIOR value out of a
25182519
// collection/struct/variant. When the lowerer proved `dst` holds a
25192520
// `Copy` scalar (`Int`/`Bool`/`Float`/`Char`/…), that value is inline
25202521
// with no interior `Rc`: it cannot alias `src` or carry `src`'s `Rc`
@@ -2526,6 +2527,7 @@ fn deepcopy_elidable_param_regs(
25262527
| RegInstr::MapGet { dst, map: src, .. }
25272528
| RegInstr::GetField { dst, base: src, .. }
25282529
| RegInstr::GetFieldSlot { dst, base: src, .. }
2530+
| RegInstr::UnwrapSome { dst, src }
25292531
| RegInstr::UnwrapVariantValue { dst, src, .. }
25302532
| RegInstr::DequePopFront { dst, deque: src }
25312533
| RegInstr::DequePopBack { dst, deque: src } = instr

0 commit comments

Comments
 (0)