From 06909f182151cf2332a14909e8b772ab4648854e Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Mon, 5 Sep 2022 01:30:58 +0300 Subject: fix(tvix/eval): fix doc comment syntax where applicable As pointed out by grfn on cl/6091 Change-Id: I28308577b7cf99dffb4a4fd3cc8783eb9ab4d0d6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6460 Tested-by: BuildkiteCI Reviewed-by: sterni --- tvix/eval/src/builtins/mod.rs | 3 ++ tvix/eval/src/chunk.rs | 3 ++ tvix/eval/src/compiler/mod.rs | 75 +++++++++++++++++++++-------------------- tvix/eval/src/compiler/scope.rs | 34 +++++++++---------- tvix/eval/src/errors.rs | 20 +++++------ tvix/eval/src/opcode.rs | 10 +++--- tvix/eval/src/value/attrs.rs | 43 ++++++++++++----------- tvix/eval/src/value/builtin.rs | 2 +- tvix/eval/src/value/string.rs | 20 +++++------ tvix/eval/src/vm.rs | 22 ++++++------ 10 files changed, 122 insertions(+), 110 deletions(-) diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 1fe5d4a83eb1..f08c17904584 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -36,6 +36,9 @@ macro_rules! force { }; } +/// Return all pure builtins, that is all builtins that do not rely on +/// I/O outside of the VM and which can be used in any contexts (e.g. +/// WASM). fn pure_builtins() -> Vec { vec![ Builtin::new("add", 2, |mut args, _| { diff --git a/tvix/eval/src/chunk.rs b/tvix/eval/src/chunk.rs index a085da571660..7c6e08638a65 100644 --- a/tvix/eval/src/chunk.rs +++ b/tvix/eval/src/chunk.rs @@ -26,6 +26,9 @@ struct SourceSpan { count: usize, } +/// A chunk is a representation of a sequence of bytecode +/// instructions, associated constants and additional metadata as +/// emitted by the compiler. #[derive(Clone, Debug, Default)] pub struct Chunk { pub code: Vec, diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 496f0aaf3207..314ae86d43e9 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -433,12 +433,13 @@ impl Compiler<'_, '_> { } } - // Compile list literals into equivalent bytecode. List - // construction is fairly simple, consisting of pushing code for - // each literal element and an instruction with the element count. - // - // The VM, after evaluating the code for each element, simply - // constructs the list from the given number of elements. + /// Compile list literals into equivalent bytecode. List + /// construction is fairly simple, consisting of pushing code for + /// each literal element and an instruction with the element + /// count. + /// + /// The VM, after evaluating the code for each element, simply + /// constructs the list from the given number of elements. fn compile_list(&mut self, slot: LocalIdx, node: ast::List) { let mut count = 0; @@ -450,14 +451,14 @@ impl Compiler<'_, '_> { self.push_op(OpCode::OpList(Count(count)), &node); } - // Compile attribute set literals into equivalent bytecode. - // - // This is complicated by a number of features specific to Nix - // attribute sets, most importantly: - // - // 1. Keys can be dynamically constructed through interpolation. - // 2. Keys can refer to nested attribute sets. - // 3. Attribute sets can (optionally) be recursive. + /// Compile attribute set literals into equivalent bytecode. + /// + /// This is complicated by a number of features specific to Nix + /// attribute sets, most importantly: + /// + /// 1. Keys can be dynamically constructed through interpolation. + /// 2. Keys can refer to nested attribute sets. + /// 3. Attribute sets can (optionally) be recursive. fn compile_attr_set(&mut self, slot: LocalIdx, node: ast::AttrSet) { if node.rec_token().is_some() { todo!("recursive attribute sets are not yet implemented") @@ -632,16 +633,18 @@ impl Compiler<'_, '_> { self.compile(slot, node.body().unwrap()); } - // Compile conditional expressions using jumping instructions in the VM. - // - // ┌────────────────────┐ - // │ 0 [ conditional ] │ - // │ 1 JUMP_IF_FALSE →┼─┐ - // │ 2 [ main body ] │ │ Jump to else body if - // ┌┼─3─← JUMP │ │ condition is false. - // Jump over else body ││ 4 [ else body ]←┼─┘ - // if condition is true.└┼─5─→ ... │ - // └────────────────────┘ + /// Compile conditional expressions using jumping instructions in the VM. + /// + /// ```notrust + /// ┌────────────────────┐ + /// │ 0 [ conditional ] │ + /// │ 1 JUMP_IF_FALSE →┼─┐ + /// │ 2 [ main body ] │ │ Jump to else body if + /// ┌┼─3─← JUMP │ │ condition is false. + /// Jump over else body ││ 4 [ else body ]←┼─┘ + /// if condition is true.└┼─5─→ ... │ + /// └────────────────────┘ + /// ``` fn compile_if_else(&mut self, slot: LocalIdx, node: ast::IfElse) { self.compile(slot, node.condition().unwrap()); self.emit_force(&node.condition().unwrap()); @@ -663,7 +666,7 @@ impl Compiler<'_, '_> { self.patch_jump(else_idx); // patch jump *over* else body } - // Compile an `inherit` node of a `let`-expression. + /// Compile an `inherit` node of a `let`-expression. fn compile_let_inherit>( &mut self, slot: LocalIdx, @@ -714,11 +717,11 @@ impl Compiler<'_, '_> { } } - // Compile a standard `let ...; in ...` statement. - // - // Unless in a non-standard scope, the encountered values are - // simply pushed on the stack and their indices noted in the - // entries vector. + /// Compile a standard `let ...; in ...` statement. + /// + /// Unless in a non-standard scope, the encountered values are + /// simply pushed on the stack and their indices noted in the + /// entries vector. fn compile_let_in(&mut self, slot: LocalIdx, node: ast::LetIn) { self.begin_scope(); @@ -837,9 +840,9 @@ impl Compiler<'_, '_> { }; } - // Compile `with` expressions by emitting instructions that - // pop/remove the indices of attribute sets that are implicitly in - // scope through `with` on the "with-stack". + /// Compile `with` expressions by emitting instructions that + /// pop/remove the indices of attribute sets that are implicitly + /// in scope through `with` on the "with-stack". fn compile_with(&mut self, slot: LocalIdx, node: ast::With) { self.begin_scope(); // TODO: Detect if the namespace is just an identifier, and @@ -1298,9 +1301,9 @@ impl Compiler<'_, '_> { } } - // Normalises identifier fragments into a single string vector for - // `let`-expressions; fails if fragments requiring dynamic computation - // are encountered. + /// Normalises identifier fragments into a single string vector + /// for `let`-expressions; fails if fragments requiring dynamic + /// computation are encountered. fn normalise_ident_path>( &self, path: I, diff --git a/tvix/eval/src/compiler/scope.rs b/tvix/eval/src/compiler/scope.rs index a7791b750af0..b6fa61e7759b 100644 --- a/tvix/eval/src/compiler/scope.rs +++ b/tvix/eval/src/compiler/scope.rs @@ -32,25 +32,25 @@ enum LocalName { /// Represents a single local already known to the compiler. #[derive(Debug)] pub struct Local { - // Identifier of this local. This is always a statically known - // value (Nix does not allow dynamic identifier names in locals), - // or a "phantom" value not accessible by users. + /// Identifier of this local. This is always a statically known + /// value (Nix does not allow dynamic identifier names in locals), + /// or a "phantom" value not accessible by users. name: LocalName, - // Source span at which this local was declared. + /// Source span at which this local was declared. pub span: codemap::Span, - // Scope depth of this local. + /// Scope depth of this local. pub depth: usize, - // Is this local initialised? + /// Is this local initialised? pub initialised: bool, - // Is this local known to have been used at all? + /// Is this local known to have been used at all? pub used: bool, - // Does this local need to be finalised after the enclosing scope - // is completely constructed? + /// Does this local need to be finalised after the enclosing scope + /// is completely constructed? pub needs_finaliser: bool, } @@ -135,18 +135,18 @@ pub struct Scope { pub locals: Vec, pub upvalues: Vec, - // How many scopes "deep" are these locals? + /// How many scopes "deep" are these locals? pub scope_depth: usize, - // Current size of the `with`-stack at runtime. + /// Current size of the `with`-stack at runtime. with_stack_size: usize, - // Users are allowed to override globally defined symbols like - // `true`, `false` or `null` in scopes. We call this "scope - // poisoning", as it requires runtime resolution of those tokens. - // - // To support this efficiently, the depth at which a poisoning - // occured is tracked here. + /// Users are allowed to override globally defined symbols like + /// `true`, `false` or `null` in scopes. We call this "scope + /// poisoning", as it requires runtime resolution of those tokens. + /// + /// To support this efficiently, the depth at which a poisoning + /// occured is tracked here. poisoned_tokens: HashMap<&'static str, usize>, } diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs index 5f1a24b54b4b..2b8ba8f9631f 100644 --- a/tvix/eval/src/errors.rs +++ b/tvix/eval/src/errors.rs @@ -20,37 +20,37 @@ pub enum ErrorKind { rhs: &'static str, }, - // Resolving a user-supplied path literal failed in some way. + /// Resolving a user-supplied path literal failed in some way. PathResolution(String), - // Dynamic keys are not allowed in let. + /// Dynamic keys are not allowed in let. DynamicKeyInLet(rnix::SyntaxNode), - // Unknown variable in statically known scope. + /// Unknown variable in statically known scope. UnknownStaticVariable, - // Unknown variable in dynamic scope (with, rec, ...). + /// Unknown variable in dynamic scope (with, rec, ...). UnknownDynamicVariable(String), - // User is defining the same variable twice at the same depth. + /// User is defining the same variable twice at the same depth. VariableAlreadyDefined(String), - // Attempt to call something that is not callable. + /// Attempt to call something that is not callable. NotCallable, - // Infinite recursion encountered while forcing thunks. + /// Infinite recursion encountered while forcing thunks. InfiniteRecursion, ParseErrors(Vec), AssertionFailed, - // These are user-generated errors through builtins. + /// These are user-generated errors through builtins. Throw(String), Abort(String), - // An error occured while forcing a thunk, and needs to be chained - // up. + /// An error occured while forcing a thunk, and needs to be + /// chained up. ThunkForce(Box), } diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs index 565c44a572de..42b8760a1ed4 100644 --- a/tvix/eval/src/opcode.rs +++ b/tvix/eval/src/opcode.rs @@ -35,10 +35,10 @@ pub struct Count(pub usize); #[warn(variant_size_differences)] #[derive(Clone, Copy, Debug)] pub enum OpCode { - // Push a constant onto the stack. + /// Push a constant onto the stack. OpConstant(ConstantIdx), - // Discard a value from the stack. + /// Discard a value from the stack. OpPop, // Push a literal value. @@ -93,13 +93,13 @@ pub enum OpCode { // Type assertion operators OpAssertBool, - // Access local identifiers with statically known positions. + /// Access local identifiers with statically known positions. OpGetLocal(StackIdx), - // Close scopes while leaving their expression value around. + /// Close scopes while leaving their expression value around. OpCloseScope(Count), // number of locals to pop - // Asserts stack top is a boolean, and true. + /// Asserts stack top is a boolean, and true. OpAssert, // Lambdas & closures diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index d122f9155d2c..fddf0b582ccb 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -150,7 +150,8 @@ impl PartialEq for NixAttrs { } impl NixAttrs { - // Update one attribute set with the values of the other. + /// Return an attribute set containing the merge of the two + /// provided sets. Keys from the `other` set have precedence. pub fn update(self, other: Self) -> Self { // Short-circuit on some optimal cases: match (&self.0, &other.0) { @@ -301,17 +302,19 @@ impl NixAttrs { } } -// In Nix, name/value attribute pairs are frequently constructed from -// literals. This particular case should avoid allocation of a map, -// additional heap values etc. and use the optimised `KV` variant -// instead. -// -// `slice` is the top of the stack from which the attrset is being -// constructed, e.g. -// -// slice: [ "value" 5 "name" "foo" ] -// index: 0 1 2 3 -// stack: 3 2 1 0 +/// In Nix, name/value attribute pairs are frequently constructed from +/// literals. This particular case should avoid allocation of a map, +/// additional heap values etc. and use the optimised `KV` variant +/// instead. +/// +/// ```norust +/// `slice` is the top of the stack from which the attrset is being +/// constructed, e.g. +/// +/// slice: [ "value" 5 "name" "foo" ] +/// index: 0 1 2 3 +/// stack: 3 2 1 0 +/// ``` fn attempt_optimise_kv(slice: &mut [Value]) -> Option { let (name_idx, value_idx) = { match (&slice[2], &slice[0]) { @@ -340,8 +343,8 @@ fn attempt_optimise_kv(slice: &mut [Value]) -> Option { })) } -// Set an attribute on an in-construction attribute set, while -// checking against duplicate keys. +/// Set an attribute on an in-construction attribute set, while +/// checking against duplicate keys. fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> Result<(), ErrorKind> { match attrs.0.map_mut().entry(key) { btree_map::Entry::Occupied(entry) => Err(ErrorKind::DuplicateAttrsKey { @@ -355,12 +358,12 @@ fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> Result<(), Er } } -// Set a nested attribute inside of an attribute set, throwing a -// duplicate key error if a non-hashmap entry already exists on the -// path. -// -// There is some optimisation potential for this simple implementation -// if it becomes a problem. +/// Set a nested attribute inside of an attribute set, throwing a +/// duplicate key error if a non-hashmap entry already exists on the +/// path. +/// +/// There is some optimisation potential for this simple implementation +/// if it becomes a problem. fn set_nested_attr( attrs: &mut NixAttrs, key: NixString, diff --git a/tvix/eval/src/value/builtin.rs b/tvix/eval/src/value/builtin.rs index 7572103ec96e..5d39a0a9f557 100644 --- a/tvix/eval/src/value/builtin.rs +++ b/tvix/eval/src/value/builtin.rs @@ -39,7 +39,7 @@ pub struct Builtin { arity: usize, func: BuiltinFn, - // Partially applied function arguments. + /// Partially applied function arguments. partials: Vec, } diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs index 095f87645cc5..aa542181f9b1 100644 --- a/tvix/eval/src/value/string.rs +++ b/tvix/eval/src/value/string.rs @@ -72,12 +72,12 @@ impl NixString { } } - // Return a displayable representation of the string as an - // identifier. - // - // This is used when printing out strings used as e.g. attribute - // set keys, as those are only escaped in the presence of special - // characters. + /// Return a displayable representation of the string as an + /// identifier. + /// + /// This is used when printing out strings used as e.g. attribute + /// set keys, as those are only escaped in the presence of special + /// characters. pub fn ident_str(&self) -> Cow { let escaped = nix_escape_string(self.as_str()); @@ -111,10 +111,10 @@ fn nix_escape_char(ch: char, next: Option<&char>) -> Option<&'static str> { } } -// Escape a Nix string for display, as most user-visible representation -// are escaped strings. -// -// Note that this does not add the outer pair of surrounding quotes. +/// Escape a Nix string for display, as most user-visible representation +/// are escaped strings. +/// +/// Note that this does not add the outer pair of surrounding quotes. fn nix_escape_string(input: &str) -> Cow { let mut iter = input.chars().enumerate().peekable(); diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index a7b9bf3e9206..89702a090fff 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -30,8 +30,8 @@ pub struct VM<'o> { frames: Vec, stack: Vec, - // Stack indices of attribute sets from which variables should be - // dynamically resolved (`with`). + /// Stack indices of attribute sets from which variables should be + /// dynamically resolved (`with`). with_stack: Vec, observer: &'o mut dyn Observer, @@ -560,12 +560,12 @@ impl<'o> VM<'o> { } } - // Construct runtime representation of an attr path (essentially - // just a list of strings). - // - // The difference to the list construction operation is that this - // forces all elements into strings, as attribute set keys are - // required to be strict in Nix. + /// Construct runtime representation of an attr path (essentially + /// just a list of strings). + /// + /// The difference to the list construction operation is that this + /// forces all elements into strings, as attribute set keys are + /// required to be strict in Nix. fn run_attr_path(&mut self, count: usize) -> EvalResult<()> { debug_assert!(count > 1, "AttrPath needs at least two fragments"); let mut path = Vec::with_capacity(count); @@ -588,9 +588,9 @@ impl<'o> VM<'o> { Ok(()) } - // Interpolate string fragments by popping the specified number of - // fragments of the stack, evaluating them to strings, and pushing - // the concatenated result string back on the stack. + /// Interpolate string fragments by popping the specified number of + /// fragments of the stack, evaluating them to strings, and pushing + /// the concatenated result string back on the stack. fn run_interpolate(&mut self, count: usize) -> EvalResult<()> { let mut out = String::new(); -- cgit 1.4.1