diff --git a/core/src/bytecode/ast/compat.rs b/core/src/bytecode/ast/compat.rs index f4361696f5..8192384f24 100644 --- a/core/src/bytecode/ast/compat.rs +++ b/core/src/bytecode/ast/compat.rs @@ -540,6 +540,7 @@ impl From<&term::UnaryOp> for PrimOp { ignore_not_exported: *ignore_not_exported, }, term::UnaryOp::RecordEmptyWithTail => PrimOp::RecordEmptyWithTail, + term::UnaryOp::RecordFreeze => PrimOp::RecordFreeze, term::UnaryOp::Trace => PrimOp::Trace, term::UnaryOp::LabelPushDiag => PrimOp::LabelPushDiag, #[cfg(feature = "nix-experimental")] @@ -1081,6 +1082,7 @@ impl FromAst for TermPrimOp { ignore_not_exported: *ignore_not_exported, }), PrimOp::RecordEmptyWithTail => TermPrimOp::Unary(term::UnaryOp::RecordEmptyWithTail), + PrimOp::RecordFreeze => TermPrimOp::Unary(term::UnaryOp::RecordFreeze), PrimOp::Trace => TermPrimOp::Unary(term::UnaryOp::Trace), PrimOp::LabelPushDiag => TermPrimOp::Unary(term::UnaryOp::LabelPushDiag), PrimOp::EnumGetArg => TermPrimOp::Unary(term::UnaryOp::EnumGetArg), diff --git a/core/src/bytecode/ast/primop.rs b/core/src/bytecode/ast/primop.rs index bb45ce9a77..5825f82e7e 100644 --- a/core/src/bytecode/ast/primop.rs +++ b/core/src/bytecode/ast/primop.rs @@ -335,6 +335,15 @@ pub enum PrimOp { /// 1. The model record to take the tail from. RecordEmptyWithTail, + /// Freezes a recursive record to make it a static dictionary. Apply all pending lazy contracts + /// (and flush them), and remove all dependency information, so that the value of the fields is + /// fixed in time and subsequent overrides will only impact the overriden field. + /// + /// # Arguments + /// + /// 1. The record to freeze. + RecordFreeze, + /// Print a message when encountered during evaluation and proceed with the evaluation of the /// argument on the top of the stack. Operationally the same as the identity function /// @@ -964,6 +973,7 @@ impl fmt::Display for PrimOp { StringFindAll => write!(f, "string/find_all"), Force { .. } => write!(f, "force"), RecordEmptyWithTail => write!(f, "record/empty_with_tail"), + RecordFreeze => write!(f, "record/freeze"), Trace => write!(f, "trace"), LabelPushDiag => write!(f, "label/push_diag"), @@ -1091,6 +1101,7 @@ impl PrimOp { | StringFindAll | Force { .. } | RecordEmptyWithTail + | RecordFreeze | Trace | LabelPushDiag | EnumGetArg diff --git a/core/src/error/mod.rs b/core/src/error/mod.rs index 7d7cacf686..6d9fe14e18 100644 --- a/core/src/error/mod.rs +++ b/core/src/error/mod.rs @@ -251,7 +251,8 @@ pub enum IllegalPolymorphicTailAction { FieldAccess { field: String }, Map, Merge, - RecordRemove { field: String }, + FieldRemove { field: String }, + Freeze, } impl IllegalPolymorphicTailAction { @@ -264,9 +265,10 @@ impl IllegalPolymorphicTailAction { } Map => "cannot map over a record sealed by a polymorphic contract".to_owned(), Merge => "cannot merge a record sealed by a polymorphic contract".to_owned(), - RecordRemove { field } => { + FieldRemove { field } => { format!("cannot remove field `{field}` sealed by a polymorphic contract") } + Freeze => "cannot freeze a record sealed by a polymorphic contract".to_owned(), } } } diff --git a/core/src/eval/operation.rs b/core/src/eval/operation.rs index 0eeaa1ba3c..4e1a8f8c64 100644 --- a/core/src/eval/operation.rs +++ b/core/src/eval/operation.rs @@ -645,9 +645,18 @@ impl VirtualMachine { missing_field_err.into_eval_err(pos, pos_op) })?; + // By construction, mapping freezes the record. We set the frozen flag so + // that operations that require the record to be frozen don't have to + // perform the work again. + let attrs = record.attrs.frozen(); + Ok(Closure { body: RichTerm::new( - Term::Record(RecordData { fields, ..record }), + Term::Record(RecordData { + fields, + attrs, + ..record + }), pos_op_inh, ), env: Environment::new(), @@ -1181,6 +1190,73 @@ impl VirtualMachine { } _ => mk_type_error!("Record"), }), + UnaryOp::RecordFreeze => match_sharedterm!(match (t) { + Term::Record(record) => { + let mut record = record; + + if record.attrs.frozen { + // A frozen record shouldn't have a polymorphic tail + debug_assert!(record.sealed_tail.is_none()); + + return Ok(Closure { + body: RichTerm::new(Term::Record(record), pos), + env, + }); + } + + // It's not clear what the semantics of freezing a record with a sealed tail + // would be, as their might be dependencies between the sealed part and the + // unsealed part. Merging is disallowed on records with tail, so we disallow + // freezing as well. + if let Some(record::SealedTail { label, .. }) = record.sealed_tail { + return Err(EvalError::IllegalPolymorphicTailAccess { + action: IllegalPolymorphicTailAction::Freeze, + evaluated_arg: label.get_evaluated_arg(&self.cache), + label, + call_stack: std::mem::take(&mut self.call_stack), + }); + } + + let fields = record + .fields + .into_iter() + .map(|(id, field)| { + let value = field.value.map(|value| { + let pos = value.pos; + RuntimeContract::apply_all( + value, + field.pending_contracts.into_iter(), + pos, + ) + }); + + let field = Field { + value, + pending_contracts: Vec::new(), + ..field + } + .closurize(&mut self.cache, env.clone()); + + (id, field) + }) + .collect(); + + let attrs = record.attrs.frozen(); + + Ok(Closure { + body: RichTerm::new( + Term::Record(RecordData { + fields, + attrs, + sealed_tail: None, + }), + pos_op.into_inherited(), + ), + env, + }) + } + _ => mk_type_error!("Record"), + }), UnaryOp::Trace => { if let Term::Str(s) = &*t { let _ = writeln!(self.trace, "std.trace: {s}"); @@ -2250,6 +2326,7 @@ impl VirtualMachine { )) } _ => Ok(Closure { + // Insertion preserves the frozenness body: Term::Record(RecordData { fields, ..record }).into(), env: env2, }), @@ -2282,7 +2359,7 @@ impl VirtualMachine { match record.sealed_tail.as_ref() { Some(t) if t.has_dyn_field(&id) => { Err(EvalError::IllegalPolymorphicTailAccess { - action: IllegalPolymorphicTailAction::RecordRemove { + action: IllegalPolymorphicTailAction::FieldRemove { field: id.to_string(), }, evaluated_arg: t.label.get_evaluated_arg(&self.cache), @@ -2307,6 +2384,7 @@ impl VirtualMachine { } else { Ok(Closure { body: RichTerm::new( + // Removal preserves the frozenness Term::Record(RecordData { fields, ..record }), pos_op_inh, ), @@ -2745,6 +2823,11 @@ impl VirtualMachine { // documentation let mut record_data = record_data; + // Applying a lazy contract unfreezes a record, as frozen record are + // expected to have all their contracts applied and thus an empty list of + // pending contracts. + record_data.attrs.frozen = false; + let mut contract_at_field = |id: LocIdent| { let pos = contract_term.pos; mk_app!( diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop index c093b25a71..6facb1d51a 100644 --- a/core/src/parser/grammar.lalrpop +++ b/core/src/parser/grammar.lalrpop @@ -1170,6 +1170,7 @@ UOp: PrimOp = { // "op rec_force" => PrimOp::RecForce, // "op rec_default" => PrimOp::RecDefault, "record/empty_with_tail" => PrimOp::RecordEmptyWithTail, + "record/freeze" => PrimOp::RecordFreeze, "trace" => PrimOp::Trace, "label/push_diag" => PrimOp::LabelPushDiag, "eval_nix" =>? { @@ -1617,6 +1618,7 @@ extern { "record/split_pair" => Token::Normal(NormalToken::RecordSplitPair), "record/disjoint_merge" => Token::Normal(NormalToken::RecordDisjointMerge), "record/merge_contract" => Token::Normal(NormalToken::RecordMergeContract), + "record/freeze" => Token::Normal(NormalToken::RecordFreeze), "array/map" => Token::Normal(NormalToken::ArrayMap), "array/generate" => Token::Normal(NormalToken::ArrayGen), "array/at" => Token::Normal(NormalToken::ArrayAt), diff --git a/core/src/parser/lexer.rs b/core/src/parser/lexer.rs index 305252f02c..12eba63410 100644 --- a/core/src/parser/lexer.rs +++ b/core/src/parser/lexer.rs @@ -311,6 +311,8 @@ pub enum NormalToken<'input> { RecordDisjointMerge, #[token("%record/merge_contract%")] RecordMergeContract, + #[token("%record/freeze%")] + RecordFreeze, #[token("merge")] Merge, diff --git a/core/src/term/mod.rs b/core/src/term/mod.rs index 1d115b723a..364b756fdb 100644 --- a/core/src/term/mod.rs +++ b/core/src/term/mod.rs @@ -1538,6 +1538,11 @@ pub enum UnaryOp { /// function that preserves the sealed polymorphic tail of its argument. RecordEmptyWithTail, + /// Freezes a recursive record to make it a static dictionary. Apply all pending lazy contracts + /// (and flush them), and remove all dependency information, so that the value of the fields is + /// fixed in time and subsequent overrides will only impact the overriden field. + RecordFreeze, + /// Print a message when encountered during evaluation and proceed with the evaluation of the /// argument on the top of the stack. Operationally the same as the identity function Trace, @@ -1655,6 +1660,7 @@ impl fmt::Display for UnaryOp { RecDefault => write!(f, "rec_default"), RecForce => write!(f, "rec_force"), RecordEmptyWithTail => write!(f, "record/empty_with_tail"), + RecordFreeze => write!(f, "record/freeze"), Trace => write!(f, "trace"), LabelPushDiag => write!(f, "label/push_diag"), diff --git a/core/src/term/record.rs b/core/src/term/record.rs index beed017b4b..f941a81cb7 100644 --- a/core/src/term/record.rs +++ b/core/src/term/record.rs @@ -23,6 +23,20 @@ pub struct RecordAttrs { /// be closurized by construction. In the meantime, while we need to cope with a unique AST /// across the whole pipeline, we use this flag. pub closurized: bool, + /// If the record has been frozen. + /// + /// A recursive record is frozen when all the lazy contracts are applied to their corresponding + /// fields and flushed from the lazy contracts list. The values of the fields are computed but + /// all dependencies are erased. That is, we turn a recursive, overridable record into a static + /// dictionary. The information about field dependencies is lost and future overriding won't + /// update reverse dependencies. + /// + /// Like `closurized`, we store this information for performance reason: freezing is expensive + /// (linear in the number of fields of the record), and we might need to do it on every + /// dictionary operation such as `insert`, `remove`, etc. (see + /// [#1877](https://github.com/tweag/nickel/issues/1877)). This flags avoid repeated, useless + /// freezing. + pub frozen: bool, } impl RecordAttrs { @@ -30,11 +44,17 @@ impl RecordAttrs { Self::default() } - /// Set the `closurized` flag to true and return the updated attributes. + /// Sets the `closurized` flag to true and return the updated attributes. pub fn closurized(mut self) -> Self { self.closurized = true; self } + + /// Sets the `frozen` flag to true and return the updated attributes. + pub fn frozen(mut self) -> Self { + self.frozen = true; + self + } } impl Combine for RecordAttrs { @@ -42,6 +62,7 @@ impl Combine for RecordAttrs { RecordAttrs { open: left.open || right.open, closurized: left.closurized && right.closurized, + frozen: left.frozen && right.frozen, } } } diff --git a/core/src/typecheck/operation.rs b/core/src/typecheck/operation.rs index dfad995573..d8cd5b3354 100644 --- a/core/src/typecheck/operation.rs +++ b/core/src/typecheck/operation.rs @@ -231,7 +231,11 @@ pub fn get_uop_type( (ty.clone(), ty) } UnaryOp::RecordEmptyWithTail => (mk_uniftype::dynamic(), mk_uniftype::dynamic()), - + // forall a. { _ : a} -> { _ : a } + UnaryOp::RecordFreeze => { + let dict = mk_uniftype::dict(state.table.fresh_type_uvar(var_level)); + (dict.clone(), dict) + } // forall a. Str -> a -> a UnaryOp::Trace => { let ty = state.table.fresh_type_uvar(var_level); diff --git a/core/stdlib/std.ncl b/core/stdlib/std.ncl index 8b9eb3d016..df35ae1571 100644 --- a/core/stdlib/std.ncl +++ b/core/stdlib/std.ncl @@ -3317,6 +3317,44 @@ record |> fields |> std.array.length, + + freeze + : forall a. { _ : a } -> { _ : a } + | doc m%" + Freezes a recursive record: + + - The values of recursive fields are frozen at the current values, and + the dependencies are forgotten. **This means that subsequent overrides + through merging will only affect the overriden fields, but will no + longer update reverse dependencies**. + - All lazy pending contracts accumulated through merging and annotations + will be applied and flushed. **This means that the contracts of this + record won't propagate through future merges anymore after freezing**. + + Note that the documentation and the priority of the fields aren't + affected. + + `freeze` is used internally in the stdlib when applying dictionary + operations, such as `std.record.insert` or `std.record.remove`. Indeed, + interleaving dictionary operations and recursive records can lean to + surprising and unintuitive results. The Nickel stdlib tries to avoid + using records in both recursive and dictionary modes at the same time. + + Freezing is idempotent: freezing an already frozen record has no effect. + + # Examples + + ```nickel multiline + let frozen = std.record.freeze { x = "old", y = x } in + frozen & { x | force = "new" } + # => { x = "new", y = "old" } + + let frozen = std.record.freeze { x | Number = 1 } in + frozen & { x | force = "bypass number contract" } + # => { x = "bypass number contract" } + ``` + "% + = fun record => %record/freeze% record, }, string = { diff --git a/core/tests/integration/inputs/records/freezing.ncl b/core/tests/integration/inputs/records/freezing.ncl new file mode 100644 index 0000000000..7af461cda6 --- /dev/null +++ b/core/tests/integration/inputs/records/freezing.ncl @@ -0,0 +1,14 @@ +# test.type = 'pass' +[ + (%record/freeze% { x = 1, y = x }) & { x | force = 2 } == { x = 2, y = 1 }, + (%record/freeze% { x | default = 1, y = x }) & { x = 2 } == { x = 2, y = 1 }, + (%record/freeze% { x | default = 1, y = x }) + & (%record/freeze% { x = 2, z = x }) + & { x | force = 3 } == { x = 3, y = 1, z = 2 }, + + # freezing, as record mapping, flushes pending contracts and make them not + # propagate anymore + (%record/freeze% {x | String = "a"}) & {x | force = 1} == {x = 1}, + + ] +|> std.test.assert_all diff --git a/core/tests/integration/inputs/records/merge_unfreezes.ncl b/core/tests/integration/inputs/records/merge_unfreezes.ncl new file mode 100644 index 0000000000..752bd2acdd --- /dev/null +++ b/core/tests/integration/inputs/records/merge_unfreezes.ncl @@ -0,0 +1,12 @@ +# test.type = 'error' +# +# [test.metadata] +# error = 'EvalError::BlameError' + +# Test that even if freezing makes the initial `String` contract to not +# propagate, it doesn't prevent other contracts coming from unfrozen records to +# propagate. +%force% ((%record/freeze% { x | String = "a" }) +& { x | priority 1 | Number = 2 } +& { x | priority 2 = false }) +