From f68a8b951d43992b5791c63a84cd22ef6a19c7c0 Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Tue, 21 Jul 2026 20:49:45 -0600 Subject: [PATCH 1/9] test(compiler): cover if/else-if predicate local escape Locals assigned in if and else-if predicates remain visible after the chain; locks flatten semantics vs nested else-scope strip. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- .../if_statement/else_if_predicate_local_escape.vrl | 10 ++++++++++ .../if_statement/if_predicate_local_escape.vrl | 7 +++++++ 2 files changed, 17 insertions(+) create mode 100644 lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl create mode 100644 lib/tests/tests/expressions/if_statement/if_predicate_local_escape.vrl diff --git a/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl b/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl new file mode 100644 index 0000000000..64acdf4515 --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl @@ -0,0 +1,10 @@ +# result: 1 + +# Locals assigned in an else-if predicate are visible after the chain +# (same as the first if predicate; flatten peels nested else { if }). +if false { + null +} else if (x = 1; true) { + null +} +x diff --git a/lib/tests/tests/expressions/if_statement/if_predicate_local_escape.vrl b/lib/tests/tests/expressions/if_statement/if_predicate_local_escape.vrl new file mode 100644 index 0000000000..4b919bbeed --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/if_predicate_local_escape.vrl @@ -0,0 +1,7 @@ +# result: 1 + +# Locals assigned in an if predicate are visible after the if. +if (x = 1; true) { + null +} +x From 72632ff61c110a1e0a8da0e9e07111823357653d Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Tue, 21 Jul 2026 20:49:45 -0600 Subject: [PATCH 2/9] perf(compiler): flatten else-if chains during compile Peel parser else { if ... } into multi-arm IfStatement so typecheck visits each arm once instead of re-walking nested suffixes. Keeps predicate side-effect order and join semantics. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- src/compiler/compiler.rs | 77 +++++++++++---- src/compiler/expression.rs | 2 +- src/compiler/expression/if_statement.rs | 120 +++++++++++++++--------- src/compiler/expression/op.rs | 26 ++--- 4 files changed, 154 insertions(+), 71 deletions(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 2ef0138822..784ee67c1c 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -4,7 +4,7 @@ use crate::compiler::{ CompileConfig, Function, Program, TypeDef, expression::{ Abort, Array, Assignment, Block, Container, Expr, Expression, FunctionArgument, - FunctionCall, Group, IfStatement, Literal, Noop, Not, Object, Op, Predicate, Query, Return, + FunctionCall, Group, IfArm, IfStatement, Literal, Noop, Not, Object, Op, Predicate, Query, Return, Target, Unary, Variable, assignment, function_call, literal, predicate, query, }, parser::ast::RootExpr, @@ -384,32 +384,37 @@ impl<'a> Compiler<'a> { else_node, } = node.into_inner(); + // Peel parser-emitted `else { if ... }` chains into a flat arm list so + // typecheck visits each arm once (nested form re-walked suffixes). + let (ast_arms, final_else) = flatten_ast_else_if_chain(predicate, if_node, else_node); + let original_state = state.clone(); + let mut pred_state = original_state.clone(); + let mut arms = Vec::with_capacity(ast_arms.len()); - let predicate = self - .compile_predicate(predicate, state)? - .map_err(|err| self.diagnostics.push(Box::new(err))) - .ok()?; + for (predicate, if_node) in ast_arms { + *state = pred_state.clone(); - let after_predicate_state = state.clone(); + let predicate = self + .compile_predicate(predicate, state)? + .map_err(|err| self.diagnostics.push(Box::new(err))) + .ok()?; - let if_block = self.compile_block(if_node, state)?; + pred_state = state.clone(); + let block = self.compile_block(if_node, state)?; + arms.push(IfArm { predicate, block }); + } - let else_block = if let Some(else_node) = else_node { - *state = after_predicate_state; + let else_block = if let Some(else_node) = final_else { + *state = pred_state; Some(self.compile_block(else_node, state)?) } else { None }; - let if_statement = IfStatement { - predicate, - if_block, - else_block, - }; + let if_statement = IfStatement::new(arms, else_block); - // The current state is from one of the branches. Restore it and calculate - // the type state from the full "if statement" expression. + // Restore and calculate type state from the full flat if expression once. *state = original_state; if_statement.apply_type_info(state); Some(if_statement) @@ -946,3 +951,43 @@ impl<'a> Compiler<'a> { self.skip_missing_query_target.push(query); } } + +/// Peel parser `else { if ... }` nesting into a flat list of arms. +/// +/// The VRL parser builds `else if` as `else` wrapping a block that contains a +/// single nested `if`. Walking that nest during compile/typecheck re-visits +/// suffixes and is quadratic in arm count. +fn flatten_ast_else_if_chain( + predicate: Node, + if_node: Node, + mut else_node: Option>, +) -> ( + Vec<(Node, Node)>, + Option>, +) { + let mut arms = vec![(predicate, if_node)]; + + loop { + let Some(else_block) = else_node.take() else { + return (arms, None); + }; + + let is_else_if = matches!( + else_block.inner().0.as_slice(), + [stmt] if matches!(stmt.inner(), ast::Expr::IfStatement(_)) + ); + + if !is_else_if { + return (arms, Some(else_block)); + } + + let mut stmts = else_block.into_inner().into_inner(); + let only = stmts.pop().expect("checked single stmt"); + let ast::Expr::IfStatement(inner) = only.into_inner() else { + unreachable!("checked IfStatement"); + }; + let inner = inner.into_inner(); + arms.push((inner.predicate, inner.if_node)); + else_node = inner.else_node; + } +} diff --git a/src/compiler/expression.rs b/src/compiler/expression.rs index ae2d8c5672..7828a89e9d 100644 --- a/src/compiler/expression.rs +++ b/src/compiler/expression.rs @@ -12,7 +12,7 @@ pub use function::FunctionExpression; pub use function_argument::FunctionArgument; pub use function_call::FunctionCall; pub use group::Group; -pub use if_statement::IfStatement; +pub use if_statement::{IfArm, IfStatement}; pub use literal::Literal; pub use noop::Noop; pub use not::Not; diff --git a/src/compiler/expression/if_statement.rs b/src/compiler/expression/if_statement.rs index 1590f33372..06a71dbe7d 100644 --- a/src/compiler/expression/if_statement.rs +++ b/src/compiler/expression/if_statement.rs @@ -1,6 +1,6 @@ use std::fmt; -use crate::value::Value; +use crate::value::{Kind, Value}; use crate::compiler::state::{TypeInfo, TypeState}; use crate::compiler::{ @@ -10,69 +10,103 @@ use crate::compiler::{ }; #[derive(Debug, Clone, PartialEq)] -pub struct IfStatement { +pub struct IfArm { pub predicate: Predicate, - pub if_block: Block, + pub block: Block, +} + +/// `if` / `else if` / `else` as a flat multi-arm node. +/// +/// The parser still emits nested `else { if ... }` for `else if`, but the compiler +/// peels that chain into [`IfStatement::arms`] so typecheck is O(arms) instead of +/// re-walking nested suffixes (quadratic in arm count). +#[derive(Debug, Clone, PartialEq)] +pub struct IfStatement { + pub arms: Vec, pub else_block: Option, } +impl IfStatement { + #[must_use] + pub fn new(arms: Vec, else_block: Option) -> Self { + assert!(!arms.is_empty(), "if statement requires at least one arm"); + Self { arms, else_block } + } +} + impl Expression for IfStatement { fn resolve(&self, ctx: &mut Context) -> Resolved { - let predicate = self.predicate.resolve(ctx)?.try_boolean()?; - - if predicate { - self.if_block.resolve(ctx) - } else { - self.else_block - .as_ref() - .map_or(Ok(Value::Null), |block| block.resolve(ctx)) + for arm in &self.arms { + let predicate = arm.predicate.resolve(ctx)?.try_boolean()?; + if predicate { + return arm.block.resolve(ctx); + } } + + self.else_block + .as_ref() + .map_or(Ok(Value::Null), |block| block.resolve(ctx)) } fn type_info(&self, state: &TypeState) -> TypeInfo { - let mut state = state.clone(); - let predicate_info = self.predicate.apply_type_info(&mut state); - - let if_info = self.if_block.type_info(&state); - - if let Some(else_block) = &self.else_block { - let else_info = else_block.type_info(&state); - - // final state will be from either the "if" or "else" block, but not the original - let final_state = if_info.state.merge(else_info.state); + // Match nested else-if semantics: arm i is typed after predicates 0..=i + // have been applied (previous predicates ran and were false at runtime, + // but their type-level side effects still apply). + let mut running = state.clone(); + let mut returns = Kind::never(); + let mut arm_states: Vec = Vec::with_capacity(self.arms.len()); + let mut result_def: Option = None; + + for arm in &self.arms { + let predicate_info = arm.predicate.apply_type_info(&mut running); + returns.merge_keep(predicate_info.returns().clone(), false); + + let arm_info = arm.block.type_info(&running); + result_def = Some(match result_def { + None => arm_info.result, + Some(prev) => prev.union(arm_info.result), + }); + arm_states.push(arm_info.state); + } - // result is from either "if" or the "else" block - let mut result = if_info.result.union(else_info.result); + let mut result = result_def.expect("at least one arm"); - // predicate can also return - result - .returns_mut() - .merge_keep(predicate_info.returns().clone(), false); + let final_state = if let Some(else_block) = &self.else_block { + let else_info = else_block.type_info(&running); + result = result.union(else_info.result); - TypeInfo::new(final_state, result) + arm_states + .into_iter() + .fold(else_info.state, |acc, arm_state| acc.merge(arm_state)) } else { - // state changes from the "if block" are optional, so merge it with the original - let final_state = if_info.state.merge(state); - - // if the predicate is false, "null" is returned. - let mut result = if_info.result.or_null(); - - // predicate can also return - result - .returns_mut() - .merge_keep(predicate_info.returns().clone(), false); - - TypeInfo::new(final_state, result) - } + // All predicates false → null, and state is the post-predicate state + // merged with every arm (same as nested `if` without `else`). + result = result.or_null(); + arm_states + .into_iter() + .fold(running, |acc, arm_state| acc.merge(arm_state)) + }; + + result.returns_mut().merge_keep(returns, false); + TypeInfo::new(final_state, result) } } impl fmt::Display for IfStatement { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut arms = self.arms.iter(); + let first = arms.next().expect("at least one arm"); f.write_str("if ")?; - self.predicate.fmt(f)?; + first.predicate.fmt(f)?; f.write_str(" ")?; - self.if_block.fmt(f)?; + first.block.fmt(f)?; + + for arm in arms { + f.write_str(" else if ")?; + arm.predicate.fmt(f)?; + f.write_str(" ")?; + arm.block.fmt(f)?; + } if let Some(alt) = &self.else_block { f.write_str(" else")?; diff --git a/src/compiler/expression/op.rs b/src/compiler/expression/op.rs index 19617c5d0f..65a26a9912 100644 --- a/src/compiler/expression/op.rs +++ b/src/compiler/expression/op.rs @@ -443,7 +443,7 @@ mod tests { Opcode::{Add, And, Div, Eq, Err, Ge, Gt, Le, Lt, Mul, Ne, Or, Sub}, }; - use crate::compiler::expression::{Block, IfStatement, Literal, Predicate, Variable}; + use crate::compiler::expression::{Block, IfArm, IfStatement, Literal, Predicate, Variable}; use crate::test_type_def; use super::*; @@ -843,11 +843,13 @@ mod tests { or_nullable { expr: |_| Op { lhs: Box::new( - IfStatement { - predicate: Predicate::new_unchecked(vec![Literal::from(true).into()]), - if_block: Block::new_scoped(vec![Literal::from("string").into()]), - else_block: None, - }.into()), + IfStatement::new( + vec![IfArm { + predicate: Predicate::new_unchecked(vec![Literal::from(true).into()]), + block: Block::new_scoped(vec![Literal::from("string").into()]), + }], + None, + ).into()), rhs: Box::new(Literal::from("another string").into()), opcode: Or, }, @@ -857,11 +859,13 @@ mod tests { or_not_nullable { expr: |_| Op { lhs: Box::new( - IfStatement { - predicate: Predicate::new_unchecked(vec![Literal::from(true).into()]), - if_block: Block::new_scoped(vec![Literal::from("string").into()]), - else_block: Some(Block::new_scoped(vec![Literal::from(42).into()])) - }.into()), + IfStatement::new( + vec![IfArm { + predicate: Predicate::new_unchecked(vec![Literal::from(true).into()]), + block: Block::new_scoped(vec![Literal::from("string").into()]), + }], + Some(Block::new_scoped(vec![Literal::from(42).into()])), + ).into()), rhs: Box::new(Literal::from("another string").into()), opcode: Or, }, From d4385464d55caf44ee1613a9953d666976fac61d Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Tue, 21 Jul 2026 20:49:45 -0600 Subject: [PATCH 3/9] perf(compiler): mutate Kind inserts and assignment types in place Avoid deep-cloning exact object/array Collections and TypeDefs on every nested path write during progressive typecheck. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- src/compiler/expression/assignment.rs | 78 +++++++++++++++------------ src/compiler/state.rs | 12 +++++ src/compiler/type_def.rs | 18 ++++--- src/value/kind/crud/insert.rs | 19 +++++-- 4 files changed, 79 insertions(+), 48 deletions(-) diff --git a/src/compiler/expression/assignment.rs b/src/compiler/expression/assignment.rs index 33034cafab..fca395c615 100644 --- a/src/compiler/expression/assignment.rs +++ b/src/compiler/expression/assignment.rs @@ -311,7 +311,15 @@ impl Expression for Assignment { } fn type_info(&self, state: &TypeState) -> TypeInfo { - self.variant.type_info(state) + let mut state = state.clone(); + let result = self.apply_type_info(&mut state); + TypeInfo::new(state, result) + } + + /// Mutate `state` in place. The default `apply_type_info` clones via `type_info`; + /// assignments are dense in remap programs so avoid that full `TypeState` clone. + fn apply_type_info(&self, state: &mut TypeState) -> TypeDef { + self.variant.apply_type_info(state) } } @@ -353,33 +361,30 @@ impl Target { match self { Self::Noop => {} Self::Internal(ident, path) => { - let type_def = match state.local.variable(ident) { - None => TypeDef::never().with_type_inserted(path, new_type_def), - Some(Details { type_def, .. }) => { - type_def.clone().with_type_inserted(path, new_type_def) - } - }; - - let details = Details { type_def, value }; - state.local.insert_variable(ident.clone(), details); + if let Some(details) = state.local.variable_mut(ident) { + details.type_def.insert_type(path, new_type_def); + details.value = value; + } else { + let type_def = TypeDef::never().with_type_inserted(path, new_type_def); + state + .local + .insert_variable(ident.clone(), Details { type_def, value }); + } } Self::External(target_path) => match target_path.prefix { PathPrefix::Event => { - state.external.update_target(Details { - type_def: state - .external - .target() - .type_def - .clone() - .with_type_inserted(&target_path.path, new_type_def), - value, - }); + let details = state.external.target_mut(); + details + .type_def + .insert_type(&target_path.path, new_type_def); + details.value = value; } PathPrefix::Metadata => { - let mut kind = state.external.metadata_kind().clone(); - kind.insert(&target_path.path, new_type_def.kind().clone()); - state.external.update_metadata(kind); + state + .external + .metadata_kind_mut() + .insert(&target_path.path, new_type_def.kind().clone()); } }, } @@ -551,15 +556,14 @@ where Ok(value) } - fn type_info(&self, state: &TypeState) -> TypeInfo { - let mut state = state.clone(); + fn apply_type_info(&self, state: &mut TypeState) -> TypeDef { match &self { Variant::Single { target, expr } => { - let expr_result = expr.apply_type_info(&mut state).impure(); + let expr_result = expr.apply_type_info(state).impure(); - let const_value = expr.resolve_constant(&state); - target.insert_type_def(&mut state, expr_result.clone(), const_value); - TypeInfo::new(state, expr_result) + let const_value = expr.resolve_constant(state); + target.insert_type_def(state, expr_result.clone(), const_value); + expr_result } Variant::Infallible { ok, @@ -567,7 +571,7 @@ where expr, default, } => { - let expr_result = expr.apply_type_info(&mut state); + let expr_result = expr.apply_type_info(state); // The "ok" type is either the result of the expression, or a "default" value when the expression fails. let ok_type = expr_result @@ -575,20 +579,24 @@ where .union(TypeDef::from(default.kind())) .infallible(); - let const_value = expr.resolve_constant(&state); - ok.insert_type_def(&mut state, ok_type, const_value); + let const_value = expr.resolve_constant(state); + ok.insert_type_def(state, ok_type, const_value); // The "err" type is either the error message "bytes" or "null" (not undefined). let err_type = TypeDef::from(Kind::bytes().or_null()); - err.insert_type_def(&mut state, err_type, None); + err.insert_type_def(state, err_type, None); // Return type of the assignment expression itself is either the "expr" type or "bytes (the error message). - let assignment_result = expr_result.infallible().impure().or_bytes(); - - TypeInfo::new(state, assignment_result) + expr_result.infallible().impure().or_bytes() } } } + + fn type_info(&self, state: &TypeState) -> TypeInfo { + let mut state = state.clone(); + let result = self.apply_type_info(&mut state); + TypeInfo::new(state, result) + } } impl fmt::Display for Variant diff --git a/src/compiler/state.rs b/src/compiler/state.rs index f497c4d940..c2cb804306 100644 --- a/src/compiler/state.rs +++ b/src/compiler/state.rs @@ -66,6 +66,10 @@ impl LocalEnv { self.bindings.get(ident) } + pub(crate) fn variable_mut(&mut self, ident: &Ident) -> Option<&mut Details> { + self.bindings.get_mut(ident) + } + pub(crate) fn insert_variable(&mut self, ident: Ident, details: Details) { self.bindings.insert(ident, details); } @@ -145,6 +149,10 @@ impl ExternalEnv { &self.target } + pub(crate) fn target_mut(&mut self) -> &mut Details { + &mut self.target + } + pub fn target_kind(&self) -> &Kind { self.target().type_def.kind() } @@ -161,6 +169,10 @@ impl ExternalEnv { &self.metadata } + pub(crate) fn metadata_kind_mut(&mut self) -> &mut Kind { + &mut self.metadata + } + pub(crate) fn update_target(&mut self, details: Details) { self.target = details; } diff --git a/src/compiler/type_def.rs b/src/compiler/type_def.rs index 767a6a5b8d..6512f79b59 100644 --- a/src/compiler/type_def.rs +++ b/src/compiler/type_def.rs @@ -497,14 +497,16 @@ impl TypeDef { #[must_use] pub fn with_type_inserted<'a>(self, path: impl ValuePath<'a>, other: Self) -> Self { - let mut kind = self.kind; - kind.insert(path, other.kind); - Self { - fallibility: Fallibility::merge(&self.fallibility, &other.fallibility), - kind, - purity: Purity::merge(&self.purity, &other.purity), - returns: self.returns.clone(), - } + let mut this = self; + this.insert_type(path, other); + this + } + + /// Insert `other`'s kind at `path` within `self`, mutating in place. + pub fn insert_type<'a>(&mut self, path: impl ValuePath<'a>, other: Self) { + self.fallibility = Fallibility::merge(&self.fallibility, &other.fallibility); + self.kind.insert(path, other.kind); + self.purity = Purity::merge(&self.purity, &other.purity); } #[must_use] diff --git a/src/value/kind/crud/insert.rs b/src/value/kind/crud/insert.rs index a14e384f68..d3b91cf7c4 100644 --- a/src/value/kind/crud/insert.rs +++ b/src/value/kind/crud/insert.rs @@ -42,10 +42,15 @@ impl Kind { match segment { BorrowedSegment::Field(field) => { // Field insertion converts the value to an object, so remove all other types. - *self = - Self::object(self.as_object().cloned().unwrap_or_else(Collection::empty)); + // If already an exact object, mutate in place: cloning the Collection on every + // nested field write makes progressive event typing O(known²). + if !(self.is_object() && self.object.is_some()) { + *self = Self::object( + self.as_object().cloned().unwrap_or_else(Collection::empty), + ); + } - let collection = self.object.as_mut().expect("object was just inserted"); + let collection = self.object.as_mut().expect("object present after coerce"); let unknown_kind = collection.unknown_kind(); collection @@ -56,8 +61,12 @@ impl Kind { } BorrowedSegment::Index(mut index) => { // Array insertion converts the value to an array, so remove all other types. - *self = Self::array(self.as_array().cloned().unwrap_or_else(Collection::empty)); - let collection = self.array.as_mut().expect("array was just inserted"); + // Same in-place fast path as objects when already an exact array. + if !(self.is_array() && self.array.is_some()) { + *self = + Self::array(self.as_array().cloned().unwrap_or_else(Collection::empty)); + } + let collection = self.array.as_mut().expect("array present after coerce"); if index < 0 { let largest_known_index = collection.largest_known_index(); From 5611966d2304f4f153076477e0bd1c353c8f100c Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Tue, 21 Jul 2026 20:49:46 -0600 Subject: [PATCH 4/9] perf(compiler): share Collection known maps via Arc Wrap known fields in SharedMap so TypeState forks clone cheaply and writes copy-on-write through make_mut. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- src/value/kind/collection.rs | 126 +++++++++++++++++++-------- src/value/kind/collection/field.rs | 2 +- src/value/kind/collection/unknown.rs | 3 +- 3 files changed, 94 insertions(+), 37 deletions(-) diff --git a/src/value/kind/collection.rs b/src/value/kind/collection.rs index 69c14fda77..182c0bd93d 100644 --- a/src/value/kind/collection.rs +++ b/src/value/kind/collection.rs @@ -4,7 +4,10 @@ mod field; mod index; mod unknown; +use std::cmp::Ordering; use std::collections::BTreeMap; +use std::ops::Deref; +use std::sync::Arc; use crate::path::OwnedSegment; use crate::path::OwnedValuePath; @@ -18,13 +21,48 @@ pub trait CollectionKey { fn to_segment(&self) -> OwnedSegment; } +/// Shared known-key map for [`Collection`]. +/// +/// `Kind` / `TypeState` clones (e.g. forking at `if` arms) share the map until a write goes through +/// [`Self::make_mut`]. Private so callers cannot mutate the `Arc` without copy-on-write. +#[derive(Debug, Clone, Eq, PartialEq)] +struct SharedMap(Arc>); + +impl SharedMap { + fn new(map: BTreeMap) -> Self { + Self(Arc::new(map)) + } + + fn empty() -> Self { + Self::new(BTreeMap::new()) + } +} + +impl SharedMap { + fn make_mut(&mut self) -> &mut BTreeMap { + Arc::make_mut(&mut self.0) + } + + fn into_map(self) -> BTreeMap { + Arc::try_unwrap(self.0).unwrap_or_else(|arc| (*arc).clone()) + } +} + +impl Deref for SharedMap { + type Target = BTreeMap; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + /// The kinds of a collection (e.g. array or object). /// /// A collection contains one or more kinds for known positions within the collection (e.g. indices /// or fields), and contains a global "unknown" state that applies to all unknown paths. -#[derive(Debug, Clone, Eq, PartialEq, PartialOrd)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct Collection { - known: BTreeMap, + known: SharedMap, /// The kind of other unknown fields. /// @@ -34,12 +72,25 @@ pub struct Collection { unknown: Unknown, } +impl PartialOrd for Collection +where + T: PartialOrd, + BTreeMap: PartialOrd, +{ + fn partial_cmp(&self, other: &Self) -> Option { + match self.known.deref().partial_cmp(other.known.deref()) { + Some(Ordering::Equal) => self.unknown.partial_cmp(&other.unknown), + non_eq => non_eq, + } + } +} + impl Collection { /// Create a new collection from its parts. #[must_use] pub fn from_parts(known: BTreeMap, unknown: impl Into) -> Self { Self { - known, + known: SharedMap::new(known), unknown: unknown.into().into(), } } @@ -60,7 +111,7 @@ impl Collection { #[must_use] pub fn from_unknown(unknown: impl Into) -> Self { Self { - known: BTreeMap::default(), + known: SharedMap::empty(), unknown: unknown.into().into(), } } @@ -69,7 +120,7 @@ impl Collection { #[must_use] pub fn empty() -> Self { Self { - known: BTreeMap::default(), + known: SharedMap::empty(), unknown: Kind::undefined().into(), } } @@ -78,7 +129,7 @@ impl Collection { #[must_use] pub fn any() -> Self { Self { - known: BTreeMap::default(), + known: SharedMap::empty(), unknown: Unknown::any(), } } @@ -87,7 +138,7 @@ impl Collection { #[must_use] pub fn json() -> Self { Self { - known: BTreeMap::default(), + known: SharedMap::empty(), unknown: Unknown::json(), } } @@ -107,9 +158,11 @@ impl Collection { } /// Get a mutable reference to the "known" elements in the collection. + /// + /// Clones the map if other `Kind` values still share it. #[must_use] pub fn known_mut(&mut self) -> &mut BTreeMap { - &mut self.known + self.known.make_mut() } /// Gets the type of "unknown" elements in the collection. @@ -172,17 +225,19 @@ impl Collection { /// has an object with a field "bar" results in a collection of which any field can have an /// object that has a field "bar". pub fn anonymize(&mut self) { - let known_unknown = self - .known - .values_mut() - .reduce(|lhs, rhs| { - lhs.merge_keep(rhs.clone(), false); - lhs - }) - .cloned() - .unwrap_or(Kind::never()); - - self.known.clear(); + let known_unknown = { + let known = self.known_mut(); + let known_unknown = known + .values_mut() + .reduce(|lhs, rhs| { + lhs.merge_keep(rhs.clone(), false); + lhs + }) + .cloned() + .unwrap_or(Kind::never()); + known.clear(); + known_unknown + }; self.unknown = self.unknown.to_kind().union(known_unknown).into(); } @@ -201,23 +256,26 @@ impl Collection { /// For *unknown fields or indices*: /// /// - Both `Unknown`s are merged, similar to merging two `Kind`s. - pub fn merge(&mut self, mut other: Self, overwrite: bool) { - for (key, self_kind) in &mut self.known { - if let Some(other_kind) = other.known.remove(key) { + pub fn merge(&mut self, other: Self, overwrite: bool) { + let mut other_known = other.known.into_map(); + let other_unknown = other.unknown; + + for (key, self_kind) in self.known_mut() { + if let Some(other_kind) = other_known.remove(key) { if overwrite { *self_kind = other_kind; } else { self_kind.merge_keep(other_kind, overwrite); } - } else if other.unknown_kind().contains_any_defined() { + } else if other_unknown.to_kind().contains_any_defined() { if overwrite { // the specific field being merged isn't guaranteed to exist, so merge it with the known type of self - *self_kind = other - .unknown_kind() + *self_kind = other_unknown + .to_kind() .without_undefined() .union(self_kind.clone()); } else { - self_kind.merge_keep(other.unknown_kind(), overwrite); + self_kind.merge_keep(other_unknown.to_kind(), overwrite); } } else if !overwrite { // other is missing this field, which returns null @@ -227,21 +285,21 @@ impl Collection { let self_unknown_kind = self.unknown_kind(); if self_unknown_kind.contains_any_defined() { - for (key, mut other_kind) in other.known { + for (key, mut other_kind) in other_known { if !overwrite { other_kind.merge_keep(self_unknown_kind.clone(), overwrite); } self.known_mut().insert(key, other_kind); } } else if overwrite { - self.known.extend(other.known); + self.known_mut().extend(other_known); } else { - for (key, other_kind) in other.known { + for (key, other_kind) in other_known { // self is missing this field, which returns null - self.known.insert(key, other_kind.or_undefined()); + self.known_mut().insert(key, other_kind.or_undefined()); } } - self.unknown.merge(other.unknown, overwrite); + self.unknown.merge(other_unknown, overwrite); } /// Return the reduced `Kind` of the items within the collection. @@ -281,7 +339,7 @@ impl Collection { // All known fields in `other` need to either be a subset of a matching known field in // `self`, or a subset of self's `unknown` type state. - for (key, other_kind) in &other.known { + for (key, other_kind) in other.known.iter() { match self.known.get(key) { Some(self_kind) => { self_kind @@ -298,7 +356,7 @@ impl Collection { // All known fields in `self` not known in `other` need to be a superset of other's // `unknown` type state. - for (key, self_kind) in &self.known { + for (key, self_kind) in self.known.iter() { if !other.known.contains_key(key) { self_kind .is_superset(&other.unknown_kind()) @@ -332,7 +390,7 @@ pub enum EmptyState { impl From> for Collection { fn from(known: BTreeMap) -> Self { Self { - known, + known: SharedMap::new(known), unknown: Kind::undefined().into(), } } diff --git a/src/value/kind/collection/field.rs b/src/value/kind/collection/field.rs index a28d913878..fa5f610ab6 100644 --- a/src/value/kind/collection/field.rs +++ b/src/value/kind/collection/field.rs @@ -31,7 +31,7 @@ impl CollectionRemove for Collection { type Key = Field; fn remove_known(&mut self, key: &Field) { - self.known.remove(key); + self.known_mut().remove(key); } } diff --git a/src/value/kind/collection/unknown.rs b/src/value/kind/collection/unknown.rs index 85d8c49aad..e919072ed0 100644 --- a/src/value/kind/collection/unknown.rs +++ b/src/value/kind/collection/unknown.rs @@ -1,5 +1,4 @@ use crate::path::OwnedValuePath; -use std::collections::BTreeMap; use super::Collection; use crate::value::Kind; @@ -346,7 +345,7 @@ impl From for Kind { impl From for Collection { fn from(infinite: Infinite) -> Self { Self { - known: BTreeMap::default(), + known: super::SharedMap::empty(), unknown: Unknown::infinite(infinite), } } From 618282e1af9d11438d9d8be28d8f5ce17849d329 Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Tue, 21 Jul 2026 20:49:46 -0600 Subject: [PATCH 5/9] chore(compiler): add else-if ladder compile timing example Wall-clock harness for synthetic ladders and --program files, plus --env-width/--env-depth/--env-seed-fields to seed a spine+stubs ExternalEnv Kind for more realistic compile timing. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- examples/else_if_ladder_compile.rs | 357 +++++++++++++++++++++++++++++ 1 file changed, 357 insertions(+) create mode 100644 examples/else_if_ladder_compile.rs diff --git a/examples/else_if_ladder_compile.rs b/examples/else_if_ladder_compile.rs new file mode 100644 index 0000000000..2333c411a1 --- /dev/null +++ b/examples/else_if_ladder_compile.rs @@ -0,0 +1,357 @@ +//! VRL compile timing: synthetic else-if ladder and/or a real program file. +//! +//! ```text +//! cargo run --release --example else_if_ladder_compile -- \ +//! --arms 10,20,40,60 --fields 40 +//! +//! cargo run --release --example else_if_ladder_compile -- \ +//! --program PATH --env-width 8 --env-depth 4 --warmup 2 --repeat 7 +//! ``` +//! +//! `--fields` is assigns per synthetic ladder arm. +//! `--env-width` / `--env-depth` / `--env-seed-fields` shape the starting event `Kind` +//! (incoming schema), independent of the program text. + +use std::collections::BTreeMap; +use std::env; +use std::fs; +use std::path::PathBuf; +use std::process; +use std::time::Instant; + +use vrl::compiler::CompileConfig; +use vrl::compiler::state::ExternalEnv; +use vrl::value::Kind; +use vrl::value::kind::{Collection, Field}; + +fn usage() -> ! { + eprintln!( + "Usage: + else_if_ladder_compile [--arms N,N,...] [--fields F] [env opts] [--warmup W] [--repeat R] + else_if_ladder_compile --program PATH [env opts] [--warmup W] [--repeat R] + + --arms comma-separated else-if arm counts (default: 10,20,40,60,80) + --fields assigns per synthetic ladder arm (default: 40) + --program time compile of a VRL source file (skips synthetic ladder) + --env-width W known siblings per spine level (default: 0 = ExternalEnv::default()) + --env-depth D spine nesting depth when width > 0 (default: 1) + --env-seed-fields N max typed known slots along spine+stubs (default: all) + --warmup discarded compiles before timing (default: 1) + --repeat timed compiles; report min/median/max ms (default: 3) + + Env Kind shape (width W, depth D): one spine path `.n.n.…` of length D; at each + level, W-1 sibling stubs `.s0…`. Cost O(D*W). Types cycle a fixed palette by index. +" + ); + process::exit(2); +} + +fn parse_csv_usize(s: &str) -> Result, String> { + s.split(',') + .map(|p| { + p.trim() + .parse::() + .map_err(|_| format!("invalid usize in list: {p:?}")) + }) + .collect() +} + +struct Args { + arms: Vec, + fields: usize, + warmup: usize, + repeat: usize, + program: Option, + env_width: usize, + env_depth: usize, + /// `None` = type every stub/leaf slot. + env_seed_fields: Option, +} + +fn parse_args() -> Args { + let mut arms = vec![10, 20, 40, 60, 80]; + let mut fields = 40; + let mut warmup = 1; + let mut repeat = 3; + let mut program = None; + let mut env_width = 0; + let mut env_depth = 1; + let mut env_seed_fields = None; + + let mut argv = env::args().skip(1); + while let Some(arg) = argv.next() { + match arg.as_str() { + "--arms" => { + let v = argv.next().unwrap_or_else(|| usage()); + arms = parse_csv_usize(&v).unwrap_or_else(|e| { + eprintln!("{e}"); + usage(); + }); + } + "--fields" => { + let v = argv.next().unwrap_or_else(|| usage()); + fields = v.parse().unwrap_or_else(|_| usage()); + } + "--program" => { + let v = argv.next().unwrap_or_else(|| usage()); + program = Some(PathBuf::from(v)); + } + "--env-width" => { + let v = argv.next().unwrap_or_else(|| usage()); + env_width = v.parse().unwrap_or_else(|_| usage()); + } + "--env-depth" => { + let v = argv.next().unwrap_or_else(|| usage()); + env_depth = v.parse().unwrap_or_else(|_| usage()); + } + "--env-seed-fields" => { + let v = argv.next().unwrap_or_else(|| usage()); + env_seed_fields = Some(v.parse().unwrap_or_else(|_| usage())); + } + "--warmup" => { + let v = argv.next().unwrap_or_else(|| usage()); + warmup = v.parse().unwrap_or_else(|_| usage()); + } + "--repeat" => { + let v = argv.next().unwrap_or_else(|| usage()); + repeat = v.parse().unwrap_or_else(|_| usage()); + } + "-h" | "--help" => usage(), + other => { + eprintln!("unknown arg: {other}"); + usage(); + } + } + } + + if program.is_none() && (arms.is_empty() || fields == 0) { + eprintln!("--arms/--fields must be non-empty, or pass --program"); + usage(); + } + if repeat == 0 { + eprintln!("--repeat must be > 0"); + usage(); + } + if env_width > 0 && env_depth == 0 { + eprintln!("--env-depth must be > 0 when --env-width > 0"); + usage(); + } + + Args { + arms, + fields, + warmup, + repeat, + program, + env_width, + env_depth, + env_seed_fields, + } +} + +/// Deterministic leaf kinds for seeded env fields. +fn palette_kind(index: usize) -> Kind { + match index % 6 { + 0 => Kind::bytes(), + 1 => Kind::integer(), + 2 => Kind::float(), + 3 => Kind::boolean(), + 4 => Kind::null(), + _ => Kind::timestamp(), + } +} + +fn take_typed(counter: &mut usize, limit: usize) -> Option { + if *counter >= limit { + return None; + } + let kind = palette_kind(*counter); + *counter += 1; + Some(kind) +} + +/// Spine `.n.n.…` of length `depth`, with `width - 1` sibling stubs `.s0…` at each level. +/// Unknown fields remain `any`. Slot count is O(depth * width). +fn build_env_event_kind(width: usize, depth: usize, seed_limit: usize) -> Kind { + fn level(width: usize, depth_left: usize, counter: &mut usize, limit: usize) -> Kind { + let mut known: BTreeMap = BTreeMap::new(); + let stubs = width.saturating_sub(1); + for s in 0..stubs { + if let Some(kind) = take_typed(counter, limit) { + known.insert(Field::from(format!("s{s}")), kind); + } + } + if depth_left <= 1 { + if let Some(kind) = take_typed(counter, limit) { + known.insert(Field::from("n"), kind); + } + } else { + known.insert(Field::from("n"), level(width, depth_left - 1, counter, limit)); + } + Kind::object(Collection::from_parts(known, Kind::any())) + } + + level(width, depth, &mut 0, seed_limit) +} + +fn build_external(args: &Args) -> ExternalEnv { + if args.env_width == 0 { + return ExternalEnv::default(); + } + let seed_limit = args.env_seed_fields.unwrap_or(usize::MAX); + let event = build_env_event_kind(args.env_width, args.env_depth, seed_limit); + ExternalEnv::new_with_kind(event, Kind::object(Collection::any())) +} + +fn env_label(args: &Args) -> String { + if args.env_width == 0 { + return "env=default".to_owned(); + } + let seed = match args.env_seed_fields { + None => "all".to_owned(), + Some(n) => n.to_string(), + }; + format!( + "env-width={} env-depth={} env-seed-fields={seed}", + args.env_width, args.env_depth + ) +} + +/// Nested `if / else if` ladder with the same field set on every arm. +fn build_ladder(arm_count: usize, fields_per_arm: usize) -> String { + let mut out = String::with_capacity(arm_count * (80 + fields_per_arm * 40)); + out.push_str("eid = int!(.eid)\n"); + + for i in 0..arm_count { + if i == 0 { + out.push_str(&format!("if eid == {i} {{\n")); + } else { + out.push_str(&format!("}} else if eid == {i} {{\n")); + } + for f in 0..fields_per_arm { + out.push_str(&format!(" ._itl.f{f} = \"a{i}_f{f}\"\n")); + } + out.push_str(" ._itl.class = \"NOTABLE\"\n"); + out.push_str(&format!(" ._itl.arm = {i}\n")); + } + out.push_str("} else {\n"); + for f in 0..fields_per_arm { + out.push_str(&format!(" ._itl.f{f} = \"else_f{f}\"\n")); + } + out.push_str(" ._itl.class = \"CONTEXT\"\n"); + out.push_str(" ._itl.arm = -1\n"); + out.push_str("}\n"); + out.push_str(".\n"); + out +} + +fn compile_once(src: &str, fns: &[Box], external: &ExternalEnv) { + match vrl::compiler::compile_with_external(src, fns, external, CompileConfig::default()) { + Ok(_) => {} + Err(diags) => { + eprintln!("compile failed:\n{diags:?}"); + process::exit(1); + } + } +} + +fn median_ms(samples: &mut [f64]) -> f64 { + samples.sort_by(|a, b| a.partial_cmp(b).unwrap()); + let n = samples.len(); + if n % 2 == 1 { + samples[n / 2] + } else { + (samples[n / 2 - 1] + samples[n / 2]) / 2.0 + } +} + +fn time_src( + label: &str, + src: &str, + warmup: usize, + repeat: usize, + fns: &[Box], + external: &ExternalEnv, +) { + let src_bytes = src.len(); + let else_ifs = src.matches("else if").count(); + + for _ in 0..warmup { + compile_once(src, fns, external); + } + + let mut samples = Vec::with_capacity(repeat); + for _ in 0..repeat { + let t0 = Instant::now(); + compile_once(src, fns, external); + samples.push(t0.elapsed().as_secs_f64() * 1000.0); + } + + let min = samples.iter().cloned().fold(f64::INFINITY, f64::min); + let max = samples.iter().cloned().fold(f64::NEG_INFINITY, f64::max); + let med = median_ms(&mut samples); + + println!( + "{label} src_bytes={src_bytes} else_if≈{else_ifs} warmup={warmup} repeat={repeat}" + ); + println!(" min_ms={min:.1} med_ms={med:.1} max_ms={max:.1}"); +} + +fn main() { + let args = parse_args(); + let fns = vrl::stdlib::all(); + let external = build_external(&args); + let env = env_label(&args); + + if let Some(path) = &args.program { + let src = fs::read_to_string(path).unwrap_or_else(|e| { + eprintln!("read {}: {e}", path.display()); + process::exit(1); + }); + time_src( + &format!("program {} {env}", path.display()), + &src, + args.warmup, + args.repeat, + &fns, + &external, + ); + return; + } + + println!( + "else-if ladder compile (fields/arm={}, {env}, warmup={}, repeat={})", + args.fields, args.warmup, args.repeat + ); + println!( + "{:>6} {:>10} {:>10} {:>10} {:>10} {:>12}", + "arms", "src_bytes", "min_ms", "med_ms", "max_ms", "ms/arm^2" + ); + + for &n in &args.arms { + let src = build_ladder(n, args.fields); + let src_bytes = src.len(); + + for _ in 0..args.warmup { + compile_once(&src, &fns, &external); + } + + let mut samples = Vec::with_capacity(args.repeat); + for _ in 0..args.repeat { + let t0 = Instant::now(); + compile_once(&src, &fns, &external); + samples.push(t0.elapsed().as_secs_f64() * 1000.0); + } + + let min = samples.iter().cloned().fold(f64::INFINITY, f64::min); + let max = samples.iter().cloned().fold(f64::NEG_INFINITY, f64::max); + let med = median_ms(&mut samples); + let per_n2 = if n > 0 { + med / ((n as f64) * (n as f64)) + } else { + 0.0 + }; + + println!("{n:>6} {src_bytes:>10} {min:>10.1} {med:>10.1} {max:>10.1} {per_n2:>12.4}"); + } +} From 76838b148a44cd249a374264ee81fb0782e34c7b Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Tue, 21 Jul 2026 20:49:46 -0600 Subject: [PATCH 6/9] perf(compiler): share LocalEnv bindings via Arc TypeState forks (if arms, compile_expr snapshots) clone locals cheaply until make_mut; ~2.7x faster compile on a large --program workload in the ladder harness. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- src/compiler/state.rs | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/compiler/state.rs b/src/compiler/state.rs index c2cb804306..a9e0f1fb24 100644 --- a/src/compiler/state.rs +++ b/src/compiler/state.rs @@ -1,9 +1,33 @@ use crate::path::PathPrefix; use crate::value::{Kind, Value}; use std::collections::{HashMap, hash_map::Entry}; +use std::ops::Deref; +use std::sync::Arc; use super::{TypeDef, parser::ast::Ident, type_def::Details, value::Collection}; +/// Shared local bindings: `TypeState` clones share until a write via [`Self::make_mut`]. +#[derive(Debug, Clone, Default, PartialEq)] +pub(crate) struct SharedBindings(Arc>); + +impl SharedBindings { + fn make_mut(&mut self) -> &mut HashMap { + Arc::make_mut(&mut self.0) + } + + fn into_map(self) -> HashMap { + Arc::try_unwrap(self.0).unwrap_or_else(|arc| (*arc).clone()) + } +} + +impl Deref for SharedBindings { + type Target = HashMap; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + #[derive(Debug, Clone)] pub struct TypeInfo { pub state: TypeState, @@ -54,7 +78,7 @@ impl TypeState { /// Local environment, limited to a given scope. #[derive(Debug, Default, Clone, PartialEq)] pub struct LocalEnv { - pub(crate) bindings: HashMap, + pub(crate) bindings: SharedBindings, } impl LocalEnv { @@ -67,21 +91,21 @@ impl LocalEnv { } pub(crate) fn variable_mut(&mut self, ident: &Ident) -> Option<&mut Details> { - self.bindings.get_mut(ident) + self.bindings.make_mut().get_mut(ident) } pub(crate) fn insert_variable(&mut self, ident: Ident, details: Details) { - self.bindings.insert(ident, details); + self.bindings.make_mut().insert(ident, details); } pub(crate) fn remove_variable(&mut self, ident: &Ident) -> Option
{ - self.bindings.remove(ident) + self.bindings.make_mut().remove(ident) } /// Any state the child scope modified that was part of the parent is copied to the parent scope pub(crate) fn apply_child_scope(mut self, child: Self) -> Self { - for (ident, child_details) in child.bindings { - if let Some(self_details) = self.bindings.get_mut(&ident) { + for (ident, child_details) in child.bindings.into_map() { + if let Some(self_details) = self.bindings.make_mut().get_mut(&ident) { *self_details = child_details; } } @@ -93,11 +117,12 @@ impl LocalEnv { /// where different `LocalEnv`'s can be created, and the result is decided at runtime. /// The compile-time type must be the union of the options. pub(crate) fn merge(mut self, other: Self) -> Self { - for (ident, other_details) in other.bindings { - if let Some(self_details) = self.bindings.get_mut(&ident) { + for (ident, other_details) in other.bindings.into_map() { + let bindings = self.bindings.make_mut(); + if let Some(self_details) = bindings.get_mut(&ident) { *self_details = self_details.clone().merge(other_details); } else { - self.bindings.insert(ident, other_details); + bindings.insert(ident, other_details); } } self From c5e9b03427095ae93997d3cce7e0d302c88242ea Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Thu, 23 Jul 2026 13:56:30 -0600 Subject: [PATCH 7/9] chore(compiler): changelog for PR 1865 Signed-off-by: Klondike Dragon --- changelog.d/1865.enhancement.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/1865.enhancement.md diff --git a/changelog.d/1865.enhancement.md b/changelog.d/1865.enhancement.md new file mode 100644 index 0000000000..84c786732c --- /dev/null +++ b/changelog.d/1865.enhancement.md @@ -0,0 +1,3 @@ +Improve VRL compile time for programs with deep `else if` chains (`O(n^2)` to `O(n)` complexity over number of arms). Locals assigned in an `else if` **predicate** now remain visible after the chain (same as assignments in the first `if` predicate). + +authors: klondikedragon From abe4e9fc7c196708f35fb65f022f25484d85372e Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Thu, 23 Jul 2026 14:45:36 -0600 Subject: [PATCH 8/9] fix(compiler): strip later else-if pred locals after chain Flattened typecheck treated later-predicate bindings as definite after the chain. Re-apply apply_child_scope from post-arm0 locals and expand if/else-if suite coverage. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon --- changelog.d/1865.enhancement.md | 2 +- .../else_braced_if_predicate_local_escape.vrl | 23 ++++ .../else_if_body_local_escape.vrl | 22 ++++ .../else_if_body_local_in_later_predicate.vrl | 22 ++++ .../else_if_predicate_local_escape.vrl | 18 ++- .../if_statement/else_if_predicate_locals.vrl | 124 ++++++++++++++++++ .../else_if_third_pred_local_escape.vrl | 25 ++++ .../else_multistmt_predicate_local_escape.vrl | 24 ++++ src/compiler/expression/if_statement.rs | 19 ++- 9 files changed, 274 insertions(+), 5 deletions(-) create mode 100644 lib/tests/tests/expressions/if_statement/else_braced_if_predicate_local_escape.vrl create mode 100644 lib/tests/tests/expressions/if_statement/else_if_body_local_escape.vrl create mode 100644 lib/tests/tests/expressions/if_statement/else_if_body_local_in_later_predicate.vrl create mode 100644 lib/tests/tests/expressions/if_statement/else_if_predicate_locals.vrl create mode 100644 lib/tests/tests/expressions/if_statement/else_if_third_pred_local_escape.vrl create mode 100644 lib/tests/tests/expressions/if_statement/else_multistmt_predicate_local_escape.vrl diff --git a/changelog.d/1865.enhancement.md b/changelog.d/1865.enhancement.md index 84c786732c..c7ac73f4d0 100644 --- a/changelog.d/1865.enhancement.md +++ b/changelog.d/1865.enhancement.md @@ -1,3 +1,3 @@ -Improve VRL compile time for programs with deep `else if` chains (`O(n^2)` to `O(n)` complexity over number of arms). Locals assigned in an `else if` **predicate** now remain visible after the chain (same as assignments in the first `if` predicate). +Improve VRL compile time for programs with deep `else if` chains (`O(n^2)` to `O(n)` complexity over number of arms). authors: klondikedragon diff --git a/lib/tests/tests/expressions/if_statement/else_braced_if_predicate_local_escape.vrl b/lib/tests/tests/expressions/if_statement/else_braced_if_predicate_local_escape.vrl new file mode 100644 index 0000000000..a1389d581b --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_braced_if_predicate_local_escape.vrl @@ -0,0 +1,23 @@ +# result: +# +# error[E701]: call to undefined variable +# ┌─ :10:1 +# │ +# 10 │ x +# │ ^ +# │ │ +# │ undefined variable +# │ did you mean "null"? +# │ +# = see language documentation at https://vrl.dev +# = try your code in the VRL REPL, learn more at https://vrl.dev/examples + +# Sole-stmt else { if } peels to the same multi-arm shape as else if; lock peel + non-escape together. +if false { + null +} else { + if (x = 1; true) { + null + } +} +x diff --git a/lib/tests/tests/expressions/if_statement/else_if_body_local_escape.vrl b/lib/tests/tests/expressions/if_statement/else_if_body_local_escape.vrl new file mode 100644 index 0000000000..17b6f2d74d --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_if_body_local_escape.vrl @@ -0,0 +1,22 @@ +# result: +# +# error[E701]: call to undefined variable +# ┌─ :9:1 +# │ +# 9 │ x +# │ ^ +# │ │ +# │ undefined variable +# │ did you mean "null"? +# │ +# = see language documentation at https://vrl.dev +# = try your code in the VRL REPL, learn more at https://vrl.dev/examples + +# Else-if body-only local must not escape (merged arm state must not leak body bindings after flatten). +if false { + null +} else if (true) { + x = 1 + null +} +x diff --git a/lib/tests/tests/expressions/if_statement/else_if_body_local_in_later_predicate.vrl b/lib/tests/tests/expressions/if_statement/else_if_body_local_in_later_predicate.vrl new file mode 100644 index 0000000000..21851d5cb3 --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_if_body_local_in_later_predicate.vrl @@ -0,0 +1,22 @@ +# result: +# +# error[E701]: call to undefined variable +# ┌─ :6:12 +# │ +# 6 │ } else if (x > 0) { +# │ ^ +# │ │ +# │ undefined variable +# │ did you mean "null"? +# │ +# = see language documentation at https://vrl.dev +# = try your code in the VRL REPL, learn more at https://vrl.dev/examples + +# Body locals stay in arm scope (flatten control: fails typing the later pred, not via post-chain strip). +if false { + x = 1 + null +} else if (x > 0) { + null +} +null diff --git a/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl b/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl index 64acdf4515..2e2e66f241 100644 --- a/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl +++ b/lib/tests/tests/expressions/if_statement/else_if_predicate_local_escape.vrl @@ -1,7 +1,19 @@ -# result: 1 +# result: +# +# error[E701]: call to undefined variable +# ┌─ :9:1 +# │ +# 9 │ x +# │ ^ +# │ │ +# │ undefined variable +# │ did you mean "null"? +# │ +# = see language documentation at https://vrl.dev +# = try your code in the VRL REPL, learn more at https://vrl.dev/examples -# Locals assigned in an else-if predicate are visible after the chain -# (same as the first if predicate; flatten peels nested else { if }). +# Locals first assigned in an else-if predicate must not escape the chain +# (same as nested else { if } Block scope before flatten). if false { null } else if (x = 1; true) { diff --git a/lib/tests/tests/expressions/if_statement/else_if_predicate_locals.vrl b/lib/tests/tests/expressions/if_statement/else_if_predicate_locals.vrl new file mode 100644 index 0000000000..6902d07074 --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_if_predicate_locals.vrl @@ -0,0 +1,124 @@ +# result: true + +# Flattened else-if local semantics (compile-success cases). +# Complements if_predicate_local_escape / else_if_predicate_local_escape +# and body-assign coverage in if_else_local_assignment / if_local_assignment. + +# Later predicate reads a local introduced in an earlier predicate. +r_pred = if false { + null +} else if (x = 5; x < 0) { + x +} else if (y = 6; y > x) { + y +} else { + 10 +} +assert!(r_pred == 6) + +# Later arm body reads a local introduced in an earlier predicate. +r_body = if false { + null +} else if (x = 5; false) { + null +} else if (true) { + x +} else { + 10 +} +assert!(r_body == 5) + +# Final else reads a local introduced in a prior predicate +# (all predicates ran and were false). +r_else = if false { + null +} else if (x = 5; false) { + null +} else { + x +} +assert!(r_else == 5) + +# Third arm body uses locals from two earlier predicates. +r_third = if false { + null +} else if (x = 1; false) { + null +} else if (y = 2; false) { + null +} else if (true) { + x + y +} else { + 0 +} +assert!(r_third == 3) + +# Peeled sole-stmt else { if } matches else if for cross-arm predicate locals. +r_braced = if false { + null +} else { + if (x = 5; x < 0) { + x + } else if (y = 6; y > x) { + y + } else { + 10 + } +} +assert!(r_braced == 6) + +# Multi-stmt else (not peeled): predicate local usable later in the same else block. +r_mid_else = if false { + null +} else { + if (x = 7; false) { + null + } + x +} +assert!(r_mid_else == 7) + +# First-predicate local remains visible after an else-if chain +# (first predicate always runs; also guards against stripping too early). +if (z = 1; false) { + null +} else if (false) { + null +} else { + null +} +assert!(z == 1) + +# Pre-existing local reassigned in else-if predicate: different type, arm taken. +b = 1 +if false { + null +} else if (b = 0.5; true) { + null +} +assert!(b == 0.5) +assert!(type_def(b) == {"integer": true, "float": true}) + +# Pre-existing local reassigned in else-if predicate: different type, arm not taken. +# c == 1: skipped predicate is not evaluated at runtime. +# type_def union: that predicate is still typechecked (side effects in the type env). +c = 1 +if true { + null +} else if (c = "x"; true) { + null +} +assert!(c == 1) +assert!(type_def(c) == {"integer": true, "bytes": true}) + +# Pre-existing local updated in first pred and again in else-if pred. +e = 1 +if (e = 2; false) { + null +} else if (e = "hi"; true) { + null +} +assert!(e == "hi") +assert!(type_def(e) == {"integer": true, "bytes": true}) + +true diff --git a/lib/tests/tests/expressions/if_statement/else_if_third_pred_local_escape.vrl b/lib/tests/tests/expressions/if_statement/else_if_third_pred_local_escape.vrl new file mode 100644 index 0000000000..b7386b6676 --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_if_third_pred_local_escape.vrl @@ -0,0 +1,25 @@ +# result: +# +# error[E701]: call to undefined variable +# ┌─ :12:1 +# │ +# 12 │ x +# │ ^ +# │ │ +# │ undefined variable +# │ did you mean "null"? +# │ +# = see language documentation at https://vrl.dev +# = try your code in the VRL REPL, learn more at https://vrl.dev/examples + +# Third-or-later pred local must not escape, with a final else (2nd-arm/no-else: else_if_predicate_local_escape). +if false { + null +} else if (false) { + null +} else if (x = 1; true) { + null +} else { + null +} +x diff --git a/lib/tests/tests/expressions/if_statement/else_multistmt_predicate_local_escape.vrl b/lib/tests/tests/expressions/if_statement/else_multistmt_predicate_local_escape.vrl new file mode 100644 index 0000000000..cb52cc6fd2 --- /dev/null +++ b/lib/tests/tests/expressions/if_statement/else_multistmt_predicate_local_escape.vrl @@ -0,0 +1,24 @@ +# result: +# +# error[E701]: call to undefined variable +# ┌─ :11:1 +# │ +# 11 │ x +# │ ^ +# │ │ +# │ undefined variable +# │ did you mean "null"? +# │ +# = see language documentation at https://vrl.dev +# = try your code in the VRL REPL, learn more at https://vrl.dev/examples + +# Multi-stmt else is not peeled: mid-else x OK; after outer if fails via Block scope (not flatten strip). +if false { + null +} else { + if (x = 1; false) { + null + } + x +} +x diff --git a/src/compiler/expression/if_statement.rs b/src/compiler/expression/if_statement.rs index 06a71dbe7d..fa0ee81ad3 100644 --- a/src/compiler/expression/if_statement.rs +++ b/src/compiler/expression/if_statement.rs @@ -56,11 +56,18 @@ impl Expression for IfStatement { let mut returns = Kind::never(); let mut arm_states: Vec = Vec::with_capacity(self.arms.len()); let mut result_def: Option = None; + // Locals present after arm 0's predicate (inbound + first-pred assigns). + // Used below to drop bindings first introduced by later predicates. + let mut locals_after_first_predicate = None; for arm in &self.arms { let predicate_info = arm.predicate.apply_type_info(&mut running); returns.merge_keep(predicate_info.returns().clone(), false); + if locals_after_first_predicate.is_none() { + locals_after_first_predicate = Some(running.local.clone()); + } + let arm_info = arm.block.type_info(&running); result_def = Some(match result_def { None => arm_info.result, @@ -71,7 +78,7 @@ impl Expression for IfStatement { let mut result = result_def.expect("at least one arm"); - let final_state = if let Some(else_block) = &self.else_block { + let mut final_state = if let Some(else_block) = &self.else_block { let else_info = else_block.type_info(&running); result = result.union(else_info.result); @@ -87,6 +94,16 @@ impl Expression for IfStatement { .fold(running, |acc, arm_state| acc.merge(arm_state)) }; + // Flattened else-if peels parser `else { if }` wrappers. Those Blocks + // used `apply_child_scope`, so locals first assigned in a later + // predicate did not escape the chain. Re-apply that rule: keep/merge + // updates to locals that already existed after arm 0's predicate; + // drop bindings introduced only by later predicates (they are not + // definite when an earlier arm matched). + if let Some(baseline) = locals_after_first_predicate { + final_state.local = baseline.apply_child_scope(final_state.local); + } + result.returns_mut().merge_keep(returns, false); TypeInfo::new(final_state, result) } From 799f72623a04596fa95bc836782088e7a5351cb2 Mon Sep 17 00:00:00 2001 From: Klondike Dragon Date: Thu, 23 Jul 2026 16:02:38 -0600 Subject: [PATCH 9/9] chore(compiler) clippy and fmt cleanup Signed-off-by: Klondike Dragon --- examples/else_if_ladder_compile.rs | 7 ++++++- src/compiler/compiler.rs | 14 ++++++++------ src/compiler/expression/if_statement.rs | 9 +++++---- src/value/kind/collection.rs | 2 +- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/examples/else_if_ladder_compile.rs b/examples/else_if_ladder_compile.rs index 2333c411a1..821382c0ce 100644 --- a/examples/else_if_ladder_compile.rs +++ b/examples/else_if_ladder_compile.rs @@ -12,6 +12,8 @@ //! `--env-width` / `--env-depth` / `--env-seed-fields` shape the starting event `Kind` //! (incoming schema), independent of the program text. +#![allow(clippy::print_stdout, clippy::print_stderr)] + use std::collections::BTreeMap; use std::env; use std::fs; @@ -186,7 +188,10 @@ fn build_env_event_kind(width: usize, depth: usize, seed_limit: usize) -> Kind { known.insert(Field::from("n"), kind); } } else { - known.insert(Field::from("n"), level(width, depth_left - 1, counter, limit)); + known.insert( + Field::from("n"), + level(width, depth_left - 1, counter, limit), + ); } Kind::object(Collection::from_parts(known, Kind::any())) } diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 784ee67c1c..0a52b90486 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -4,8 +4,8 @@ use crate::compiler::{ CompileConfig, Function, Program, TypeDef, expression::{ Abort, Array, Assignment, Block, Container, Expr, Expression, FunctionArgument, - FunctionCall, Group, IfArm, IfStatement, Literal, Noop, Not, Object, Op, Predicate, Query, Return, - Target, Unary, Variable, assignment, function_call, literal, predicate, query, + FunctionCall, Group, IfArm, IfStatement, Literal, Noop, Not, Object, Op, Predicate, Query, + Return, Target, Unary, Variable, assignment, function_call, literal, predicate, query, }, parser::ast::RootExpr, program::ProgramInfo, @@ -952,6 +952,11 @@ impl<'a> Compiler<'a> { } } +type FlattenedElseIfChain = ( + Vec<(Node, Node)>, + Option>, +); + /// Peel parser `else { if ... }` nesting into a flat list of arms. /// /// The VRL parser builds `else if` as `else` wrapping a block that contains a @@ -961,10 +966,7 @@ fn flatten_ast_else_if_chain( predicate: Node, if_node: Node, mut else_node: Option>, -) -> ( - Vec<(Node, Node)>, - Option>, -) { +) -> FlattenedElseIfChain { let mut arms = vec![(predicate, if_node)]; loop { diff --git a/src/compiler/expression/if_statement.rs b/src/compiler/expression/if_statement.rs index fa0ee81ad3..ab1de85aae 100644 --- a/src/compiler/expression/if_statement.rs +++ b/src/compiler/expression/if_statement.rs @@ -27,6 +27,9 @@ pub struct IfStatement { } impl IfStatement { + /// # Panics + /// + /// Panics if `arms` is empty. #[must_use] pub fn new(arms: Vec, else_block: Option) -> Self { assert!(!arms.is_empty(), "if statement requires at least one arm"); @@ -84,14 +87,12 @@ impl Expression for IfStatement { arm_states .into_iter() - .fold(else_info.state, |acc, arm_state| acc.merge(arm_state)) + .fold(else_info.state, TypeState::merge) } else { // All predicates false → null, and state is the post-predicate state // merged with every arm (same as nested `if` without `else`). result = result.or_null(); - arm_states - .into_iter() - .fold(running, |acc, arm_state| acc.merge(arm_state)) + arm_states.into_iter().fold(running, TypeState::merge) }; // Flattened else-if peels parser `else { if }` wrappers. Those Blocks diff --git a/src/value/kind/collection.rs b/src/value/kind/collection.rs index 182c0bd93d..3e2f46ed67 100644 --- a/src/value/kind/collection.rs +++ b/src/value/kind/collection.rs @@ -78,7 +78,7 @@ where BTreeMap: PartialOrd, { fn partial_cmp(&self, other: &Self) -> Option { - match self.known.deref().partial_cmp(other.known.deref()) { + match (*self.known).partial_cmp(&*other.known) { Some(Ordering::Equal) => self.unknown.partial_cmp(&other.unknown), non_eq => non_eq, }