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 <sternenseemann@systemli.org>

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<Builtin> {
     vec![
         Builtin::new("add", 2, |mut args, _| {

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<OpCode>,

tvix/eval/src/compiler/mod.rs

@@ -433,12 +433,13 @@ impl Compiler<'_, '_> {
         }
     }
 
-    // Compile list literals into equivalent bytecode. List
+    /// Compile list literals into equivalent bytecode. List
-    // construction is fairly simple, consisting of pushing code for
+    /// construction is fairly simple, consisting of pushing code for
-    // each literal element and an instruction with the element count.
+    /// 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.
+    /// 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.
+    /// Compile attribute set literals into equivalent bytecode.
-    //
+    ///
-    // This is complicated by a number of features specific to Nix
+    /// This is complicated by a number of features specific to Nix
-    // attribute sets, most importantly:
+    /// attribute sets, most importantly:
-    //
+    ///
-    // 1. Keys can be dynamically constructed through interpolation.
+    /// 1. Keys can be dynamically constructed through interpolation.
-    // 2. Keys can refer to nested attribute sets.
+    /// 2. Keys can refer to nested attribute sets.
-    // 3. Attribute sets can (optionally) be recursive.
+    /// 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.
+    /// Compile conditional expressions using jumping instructions in the VM.
-    //
+    ///
-    //                        ┌────────────────────┐
+    /// ```notrust
-    //                        │ 0  [ conditional ] │
+    ///                        ┌────────────────────┐
-    //                        │ 1   JUMP_IF_FALSE →┼─┐
+    ///                        │ 0  [ conditional ] │
-    //                        │ 2  [  main body  ] │ │ Jump to else body if
+    ///                        │ 1   JUMP_IF_FALSE →┼─┐
-    //                       ┌┼─3─←     JUMP       │ │ condition is false.
+    ///                        │ 2  [  main body  ] │ │ Jump to else body if
-    //  Jump over else body  ││ 4  [  else body  ]←┼─┘
+    ///                       ┌┼─3─←     JUMP       │ │ condition is false.
-    //  if condition is true.└┼─5─→     ...        │
+    ///  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<I: Iterator<Item = ast::Inherit>>(
         &mut self,
         slot: LocalIdx,
@@ -714,11 +717,11 @@ impl Compiler<'_, '_> {
         }
     }
 
-    // Compile a standard `let ...; in ...` statement.
+    /// Compile a standard `let ...; in ...` statement.
-    //
+    ///
-    // Unless in a non-standard scope, the encountered values are
+    /// Unless in a non-standard scope, the encountered values are
-    // simply pushed on the stack and their indices noted in the
+    /// simply pushed on the stack and their indices noted in the
-    // entries vector.
+    /// 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
+    /// Compile `with` expressions by emitting instructions that
-    // pop/remove the indices of attribute sets that are implicitly in
+    /// pop/remove the indices of attribute sets that are implicitly
-    // scope through `with` on the "with-stack".
+    /// 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
+    /// Normalises identifier fragments into a single string vector
-    // `let`-expressions; fails if fragments requiring dynamic computation
+    /// for `let`-expressions; fails if fragments requiring dynamic
-    // are encountered.
+    /// computation are encountered.
     fn normalise_ident_path<I: Iterator<Item = ast::Attr>>(
         &self,
         path: I,

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
+    /// Identifier of this local. This is always a statically known
-    // value (Nix does not allow dynamic identifier names in locals),
+    /// value (Nix does not allow dynamic identifier names in locals),
-    // or a "phantom" value not accessible by users.
+    /// 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
+    /// Does this local need to be finalised after the enclosing scope
-    // is completely constructed?
+    /// is completely constructed?
     pub needs_finaliser: bool,
 }
 
@@ -135,18 +135,18 @@ pub struct Scope {
     pub locals: Vec<Local>,
     pub upvalues: Vec<Upvalue>,
 
-    // 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
+    /// Users are allowed to override globally defined symbols like
-    // `true`, `false` or `null` in scopes. We call this "scope
+    /// `true`, `false` or `null` in scopes. We call this "scope
-    // poisoning", as it requires runtime resolution of those tokens.
+    /// poisoning", as it requires runtime resolution of those tokens.
-    //
+    ///
-    // To support this efficiently, the depth at which a poisoning
+    /// To support this efficiently, the depth at which a poisoning
-    // occured is tracked here.
+    /// occured is tracked here.
     poisoned_tokens: HashMap<&'static str, usize>,
 }
 

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<rnix::parser::ParseError>),
 
     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
+    /// An error occured while forcing a thunk, and needs to be
-    // up.
+    /// chained up.
     ThunkForce(Box<Error>),
 }
 

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

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
+/// In Nix, name/value attribute pairs are frequently constructed from
-// literals. This particular case should avoid allocation of a map,
+/// literals. This particular case should avoid allocation of a map,
-// additional heap values etc. and use the optimised `KV` variant
+/// additional heap values etc. and use the optimised `KV` variant
-// instead.
+/// instead.
-//
+///
-// `slice` is the top of the stack from which the attrset is being
+/// ```norust
-// constructed, e.g.
+/// `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
+///   slice: [ "value" 5 "name" "foo" ]
-//   stack:   3       2 1      0
+///   index:   0       1 2      3
+///   stack:   3       2 1      0
+/// ```
 fn attempt_optimise_kv(slice: &mut [Value]) -> Option<NixAttrs> {
     let (name_idx, value_idx) = {
         match (&slice[2], &slice[0]) {
@@ -340,8 +343,8 @@ fn attempt_optimise_kv(slice: &mut [Value]) -> Option<NixAttrs> {
     }))
 }
 
-// Set an attribute on an in-construction attribute set, while
+/// Set an attribute on an in-construction attribute set, while
-// checking against duplicate keys.
+/// 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
+/// Set a nested attribute inside of an attribute set, throwing a
-// duplicate key error if a non-hashmap entry already exists on the
+/// duplicate key error if a non-hashmap entry already exists on the
-// path.
+/// path.
-//
+///
-// There is some optimisation potential for this simple implementation
+/// There is some optimisation potential for this simple implementation
-// if it becomes a problem.
+/// if it becomes a problem.
 fn set_nested_attr(
     attrs: &mut NixAttrs,
     key: NixString,

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<Value>,
 }
 

tvix/eval/src/value/string.rs

@@ -72,12 +72,12 @@ impl NixString {
         }
     }
 
-    // Return a displayable representation of the string as an
+    /// Return a displayable representation of the string as an
-    // identifier.
+    /// identifier.
-    //
+    ///
-    // This is used when printing out strings used as e.g. attribute
+    /// This is used when printing out strings used as e.g. attribute
-    // set keys, as those are only escaped in the presence of special
+    /// set keys, as those are only escaped in the presence of special
-    // characters.
+    /// characters.
     pub fn ident_str(&self) -> Cow<str> {
         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
+/// Escape a Nix string for display, as most user-visible representation
-// are escaped strings.
+/// are escaped strings.
-//
+///
-// Note that this does not add the outer pair of surrounding quotes.
+/// Note that this does not add the outer pair of surrounding quotes.
 fn nix_escape_string(input: &str) -> Cow<str> {
     let mut iter = input.chars().enumerate().peekable();
 

tvix/eval/src/vm.rs

@@ -30,8 +30,8 @@ pub struct VM<'o> {
     frames: Vec<CallFrame>,
     stack: Vec<Value>,
 
-    // Stack indices of attribute sets from which variables should be
+    /// Stack indices of attribute sets from which variables should be
-    // dynamically resolved (`with`).
+    /// dynamically resolved (`with`).
     with_stack: Vec<usize>,
 
     observer: &'o mut dyn Observer,
@@ -560,12 +560,12 @@ impl<'o> VM<'o> {
         }
     }
 
-    // Construct runtime representation of an attr path (essentially
+    /// Construct runtime representation of an attr path (essentially
-    // just a list of strings).
+    /// just a list of strings).
-    //
+    ///
-    // The difference to the list construction operation is that this
+    /// The difference to the list construction operation is that this
-    // forces all elements into strings, as attribute set keys are
+    /// forces all elements into strings, as attribute set keys are
-    // required to be strict in Nix.
+    /// 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
+    /// Interpolate string fragments by popping the specified number of
-    // fragments of the stack, evaluating them to strings, and pushing
+    /// fragments of the stack, evaluating them to strings, and pushing
-    // the concatenated result string back on the stack.
+    /// the concatenated result string back on the stack.
     fn run_interpolate(&mut self, count: usize) -> EvalResult<()> {
         let mut out = String::new();