Merge remote-tracking branch 'upstream/master' into source-path

22 files changed, 655 insertions, 266 deletions

diff --git a/src/libexpr/attr-path.hh b/src/libexpr/attr-path.hh
index dee811fe1..eb00ffb93 100644
--- a/src/libexpr/attr-path.hh
+++ b/src/libexpr/attr-path.hh
@@ -17,7 +17,9 @@ std::pair<Value *, PosIdx> findAlongAttrPath(
     Bindings & autoArgs,
     Value & vIn);
 
-/* Heuristic to find the filename and lineno or a nix value. */
+/**
+ * Heuristic to find the filename and lineno or a nix value.
+ */
 std::pair<SourcePath, uint32_t> findPackageFilename(EvalState & state, Value & v, std::string what);
 
 std::vector<Symbol> parseAttrPath(EvalState & state, std::string_view s);
diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh
index 3fe54408b..31215f880 100644
--- a/src/libexpr/attr-set.hh
+++ b/src/libexpr/attr-set.hh
@@ -13,7 +13,9 @@ namespace nix {
 class EvalState;
 struct Value;
 
-/* Map one attribute name to its value. */
+/**
+ * Map one attribute name to its value.
+ */
 struct Attr
 {
     /* the placement of `name` and `pos` in this struct is important.
@@ -37,10 +39,12 @@ static_assert(sizeof(Attr) == 2 * sizeof(uint32_t) + sizeof(Value *),
     "avoid introducing any padding into Attr if at all possible, and do not "
     "introduce new fields that need not be present for almost every instance.");
 
-/* Bindings contains all the attributes of an attribute set. It is defined
-   by its size and its capacity, the capacity being the number of Attr
-   elements allocated after this structure, while the size corresponds to
-   the number of elements already inserted in this structure. */
+/**
+ * Bindings contains all the attributes of an attribute set. It is defined
+ * by its size and its capacity, the capacity being the number of Attr
+ * elements allocated after this structure, while the size corresponds to
+ * the number of elements already inserted in this structure.
+ */
 class Bindings
 {
 public:
@@ -95,7 +99,9 @@ public:
 
     size_t capacity() { return capacity_; }
 
-    /* Returns the attributes in lexicographically sorted order. */
+    /**
+     * Returns the attributes in lexicographically sorted order.
+     */
     std::vector<const Attr *> lexicographicOrder(const SymbolTable & symbols) const
     {
         std::vector<const Attr *> res;
@@ -112,9 +118,11 @@ public:
     friend class EvalState;
 };
 
-/* A wrapper around Bindings that ensures that its always in sorted
-   order at the end. The only way to consume a BindingsBuilder is to
-   call finish(), which sorts the bindings. */
+/**
+ * A wrapper around Bindings that ensures that its always in sorted
+ * order at the end. The only way to consume a BindingsBuilder is to
+ * call finish(), which sorts the bindings.
+ */
 class BindingsBuilder
 {
     Bindings * bindings;
diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh
index c90882edc..46c4999c8 100644
--- a/src/libexpr/eval-cache.hh
+++ b/src/libexpr/eval-cache.hh
@@ -110,8 +110,10 @@ public:
 
     ref<AttrCursor> getAttr(std::string_view name);
 
-    /* Get an attribute along a chain of attrsets. Note that this does
-       not auto-call functors or functions. */
+    /**
+     * Get an attribute along a chain of attrsets. Note that this does
+     * not auto-call functors or functions.
+     */
     OrSuggestions<ref<AttrCursor>> findAlongAttrPath(const std::vector<Symbol> & attrPath, bool force = false);
 
     std::string getString();
@@ -130,7 +132,9 @@ public:
 
     Value & forceValue();
 
-    /* Force creation of the .drv file in the Nix store. */
+    /**
+     * Force creation of the .drv file in the Nix store.
+     */
     StorePath forceDerivation();
 };
 
diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh
index f8ddd2acc..a988fa40c 100644
--- a/src/libexpr/eval-inline.hh
+++ b/src/libexpr/eval-inline.hh
@@ -5,7 +5,9 @@
 
 namespace nix {
 
-/* Note: Various places expect the allocated memory to be zeroed. */
+/**
+ * Note: Various places expect the allocated memory to be zeroed.
+ */
 [[gnu::always_inline]]
 inline void * allocBytes(size_t n)
 {
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index dec123b69..d6daf2727 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -9,6 +9,7 @@
 #include "filetransfer.hh"
 #include "function-trace.hh"
 #include "profiles.hh"
+#include "print.hh"
 
 #include <algorithm>
 #include <chrono>
@@ -104,18 +105,10 @@ void Value::print(const SymbolTable & symbols, std::ostream & str,
         str << integer;
         break;
     case tBool:
-        str << (boolean ? "true" : "false");
+        printLiteralBool(str, boolean);
         break;
     case tString:
-        str << "\"";
-        for (const char * i = string.s; *i; i++)
-            if (*i == '\"' || *i == '\\') str << "\\" << *i;
-            else if (*i == '\n') str << "\\n";
-            else if (*i == '\r') str << "\\r";
-            else if (*i == '\t') str << "\\t";
-            else if (*i == '$' && *(i+1) == '{') str << "\\" << *i;
-            else str << *i;
-        str << "\"";
+        printLiteralString(str, string.s);
         break;
     case tPath:
         str << path().to_string(); // !!! escaping?
@@ -173,7 +166,17 @@ void Value::print(const SymbolTable & symbols, std::ostream & str,
     case tFloat:
         str << fpoint;
         break;
+    case tBlackhole:
+        // Although we know for sure that it's going to be an infinite recursion
+        // when this value is accessed _in the current context_, it's likely
+        // that the user will misinterpret a simpler «infinite recursion» output
+        // as a definitive statement about the value, while in fact it may be
+        // a valid value after `builtins.trace` and perhaps some other steps
+        // have completed.
+        str << "«potential infinite recursion»";
+        break;
     default:
+        printError("Nix evaluator internal error: Value::print(): invalid value type %1%", internalType);
         abort();
     }
 }
@@ -229,6 +232,9 @@ std::string_view showType(ValueType type)
 
 std::string showType(const Value & v)
 {
+    // Allow selecting a subset of enum values
+    #pragma GCC diagnostic push
+    #pragma GCC diagnostic ignored "-Wswitch-enum"
     switch (v.internalType) {
         case tString: return v.string.context ? "a string with context" : "a string";
         case tPrimOp:
@@ -242,16 +248,21 @@ std::string showType(const Value & v)
     default:
         return std::string(showType(v.type()));
     }
+    #pragma GCC diagnostic pop
 }
 
 PosIdx Value::determinePos(const PosIdx pos) const
 {
+    // Allow selecting a subset of enum values
+    #pragma GCC diagnostic push
+    #pragma GCC diagnostic ignored "-Wswitch-enum"
     switch (internalType) {
         case tAttrs: return attrs->pos;
         case tLambda: return lambda.fun->pos;
         case tApp: return app.left->determinePos(pos);
         default: return pos;
     }
+    #pragma GCC diagnostic pop
 }
 
 bool Value::isTrivial() const
@@ -326,6 +337,22 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
     }
 }
 
+#if HAVE_BOEHMGC
+/* Disable GC while this object lives. Used by CoroutineContext.
+ *
+ * Boehm keeps a count of GC_disable() and GC_enable() calls,
+ * and only enables GC when the count matches.
+ */
+class BoehmDisableGC {
+public:
+    BoehmDisableGC() {
+        GC_disable();
+    };
+    ~BoehmDisableGC() {
+        GC_enable();
+    };
+};
+#endif
 
 static bool gcInitialised = false;
 
@@ -350,6 +377,15 @@ void initGC()
 
     StackAllocator::defaultAllocator = &boehmGCStackAllocator;
 
+
+#if NIX_BOEHM_PATCH_VERSION != 1
+    printTalkative("Unpatched BoehmGC, disabling GC inside coroutines");
+    /* Used to disable GC when entering coroutines on macOS */
+    create_coro_gc_hook = []() -> std::shared_ptr<void> {
+        return std::make_shared<BoehmDisableGC>();
+    };
+#endif
+
     /* Set the initial heap size to something fairly big (25% of
        physical RAM, up to a maximum of 384 MiB) so that in most cases
        we don't need to garbage collect at all.  (Collection has a
@@ -2334,6 +2370,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
         case nFloat:
             return v1.fpoint == v2.fpoint;
 
+        case nThunk: // Must not be left by forceValue
         default:
             error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow<EvalError>();
     }
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index b9578321f..dea8b4a3a 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -44,7 +44,10 @@ struct PrimOp
 struct Env
 {
     Env * up;
-    unsigned short prevWith:14; // nr of levels up to next `with' environment
+    /**
+     * Number of of levels up to next `with` environment
+     */
+    unsigned short prevWith:14;
     enum { Plain = 0, HasWithExpr, HasWithAttrs } type:2;
     Value * values[0];
 };
@@ -66,7 +69,9 @@ typedef std::pair<std::string, std::string> SearchPathElem;
 typedef std::list<SearchPathElem> SearchPath;
 
 
-/* Initialise the Boehm GC, if applicable. */
+/**
+ * Initialise the Boehm GC, if applicable.
+ */
 void initGC();
 
 
@@ -138,28 +143,38 @@ public:
         sPrefix,
         sOutputSpecified;
 
-    /* If set, force copying files to the Nix store even if they
-       already exist there. */
+    /**
+     * If set, force copying files to the Nix store even if they
+     * already exist there.
+     */
     RepairFlag repair;
 
-    /* The allowed filesystem paths in restricted or pure evaluation
-       mode. */
+    /**
+     * The allowed filesystem paths in restricted or pure evaluation
+     * mode.
+     */
     std::optional<PathSet> allowedPaths;
 
     Bindings emptyBindings;
 
     const SourcePath derivationInternal;
 
-    /* Store used to materialise .drv files. */
+    /**
+     * Store used to materialise .drv files.
+     */
     const ref<Store> store;
 
-    /* Store used to build stuff. */
+    /**
+     * Store used to build stuff.
+     */
     const ref<Store> buildStore;
 
     RootValue vCallFlake = nullptr;
     RootValue vImportedDrvToDerivation = nullptr;
 
-    /* Debugger */
+    /**
+     * Debugger
+     */
     void (* debugRepl)(ref<EvalState> es, const ValMap & extraEnv);
     bool debugStop;
     bool debugQuit;
@@ -218,7 +233,9 @@ private:
        paths. */
     std::map<SourcePath, StorePath> srcToStore;
 
-    /* A cache from path names to parse trees. */
+    /**
+     * A cache from path names to parse trees.
+     */
 #if HAVE_BOEHMGC
     typedef std::map<SourcePath, Expr *, std::less<SourcePath>, traceable_allocator<std::pair<const SourcePath, Expr *>>> FileParseCache;
 #else
@@ -226,7 +243,9 @@ private:
 #endif
     FileParseCache fileParseCache;
 
-    /* A cache from path names to values. */
+    /**
+     * A cache from path names to values.
+     */
 #if HAVE_BOEHMGC
     typedef std::map<SourcePath, Value, std::less<SourcePath>, traceable_allocator<std::pair<const SourcePath, Value>>> FileEvalCache;
 #else
@@ -238,17 +257,25 @@ private:
 
     std::map<std::string, std::pair<bool, std::string>> searchPathResolved;
 
-    /* Cache used by checkSourcePath(). */
+    /**
+     * Cache used by checkSourcePath().
+     */
     std::unordered_map<Path, SourcePath> resolvedPaths;
 
-    /* Cache used by prim_match(). */
+    /**
+     * Cache used by prim_match().
+     */
     std::shared_ptr<RegexCache> regexCache;
 
 #if HAVE_BOEHMGC
-    /* Allocation cache for GC'd Value objects. */
+    /**
+     * Allocation cache for GC'd Value objects.
+     */
     std::shared_ptr<void *> valueAllocCache;
 
-    /* Allocation cache for size-1 Env objects. */
+    /**
+     * Allocation cache for size-1 Env objects.
+     */
     std::shared_ptr<void *> env1AllocCache;
 #endif
 
@@ -270,47 +297,65 @@ public:
      */
     SourcePath rootPath(CanonPath path);
 
-    /* Allow access to a path. */
+    /**
+     * Allow access to a path.
+     */
     void allowPath(const Path & path);
 
-    /* Allow access to a store path. Note that this gets remapped to
-       the real store path if `store` is a chroot store. */
+    /**
+     * Allow access to a store path. Note that this gets remapped to
+     * the real store path if `store` is a chroot store.
+     */
     void allowPath(const StorePath & storePath);
 
-    /* Allow access to a store path and return it as a string. */
+    /**
+     * Allow access to a store path and return it as a string.
+     */
     void allowAndSetStorePathString(const StorePath & storePath, Value & v);
 
-    /* Check whether access to a path is allowed and throw an error if
-       not. Otherwise return the canonicalised path. */
+    /**
+     * Check whether access to a path is allowed and throw an error if
+     * not. Otherwise return the canonicalised path.
+     */
     SourcePath checkSourcePath(const SourcePath & path);
 
     void checkURI(const std::string & uri);
 
-    /* When using a diverted store and 'path' is in the Nix store, map
-       'path' to the diverted location (e.g. /nix/store/foo is mapped
-       to /home/alice/my-nix/nix/store/foo). However, this is only
-       done if the context is not empty, since otherwise we're
-       probably trying to read from the actual /nix/store. This is
-       intended to distinguish between import-from-derivation and
-       sources stored in the actual /nix/store. */
+    /**
+     * When using a diverted store and 'path' is in the Nix store, map
+     * 'path' to the diverted location (e.g. /nix/store/foo is mapped
+     * to /home/alice/my-nix/nix/store/foo). However, this is only
+     * done if the context is not empty, since otherwise we're
+     * probably trying to read from the actual /nix/store. This is
+     * intended to distinguish between import-from-derivation and
+     * sources stored in the actual /nix/store.
+     */
     Path toRealPath(const Path & path, const PathSet & context);
 
-    /* Parse a Nix expression from the specified file. */
+    /**
+     * Parse a Nix expression from the specified file.
+     */
     Expr * parseExprFromFile(const SourcePath & path);
     Expr * parseExprFromFile(const SourcePath & path, std::shared_ptr<StaticEnv> & staticEnv);
 
-    /* Parse a Nix expression from the specified string. */
+    /**
+     * Parse a Nix expression from the specified string.
+     */
     Expr * parseExprFromString(std::string s, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv);
     Expr * parseExprFromString(std::string s, const SourcePath & basePath);
 
     Expr * parseStdin();
 
-    /* Evaluate an expression read from the given file to normal
-       form. Optionally enforce that the top-level expression is
-       trivial (i.e. doesn't require arbitrary computation). */
+    /**
+     * Evaluate an expression read from the given file to normal
+     * form. Optionally enforce that the top-level expression is
+     * trivial (i.e. doesn't require arbitrary computation).
+     */
     void evalFile(const SourcePath & path, Value & v, bool mustBeTrivial = false);
 
-    /* Like `evalFile`, but with an already parsed expression. */
+    /**
+     * Like `evalFile`, but with an already parsed expression.
+     */
     void cacheFile(
         const SourcePath & path,
         const SourcePath & resolvedPath,
@@ -320,37 +365,52 @@ public:
 
     void resetFileCache();
 
-    /* Look up a file in the search path. */
+    /**
+     * Look up a file in the search path.
+     */
     SourcePath findFile(const std::string_view path);
     SourcePath findFile(SearchPath & searchPath, const std::string_view path, const PosIdx pos = noPos);
 
-    /* If the specified search path element is a URI, download it. */
+    /**
+     * If the specified search path element is a URI, download it.
+     */
     std::pair<bool, std::string> resolveSearchPathElem(const SearchPathElem & elem);
 
-    /* Evaluate an expression to normal form, storing the result in
-       value `v'. */
+    /**
+     * Evaluate an expression to normal form
+     *
+     * @param [out] v The resulting is stored here.
+     */
     void eval(Expr * e, Value & v);
 
-    /* Evaluation the expression, then verify that it has the expected
-       type. */
+    /**
+     * Evaluation the expression, then verify that it has the expected
+     * type.
+     */
     inline bool evalBool(Env & env, Expr * e);
     inline bool evalBool(Env & env, Expr * e, const PosIdx pos, std::string_view errorCtx);
     inline void evalAttrs(Env & env, Expr * e, Value & v, const PosIdx pos, std::string_view errorCtx);
 
-    /* If `v' is a thunk, enter it and overwrite `v' with the result
-       of the evaluation of the thunk.  If `v' is a delayed function
-       application, call the function and overwrite `v' with the
-       result.  Otherwise, this is a no-op. */
+    /**
+     * If `v` is a thunk, enter it and overwrite `v` with the result
+     * of the evaluation of the thunk.  If `v` is a delayed function
+     * application, call the function and overwrite `v` with the
+     * result.  Otherwise, this is a no-op.
+     */
     inline void forceValue(Value & v, const PosIdx pos);
 
     template <typename Callable>
     inline void forceValue(Value & v, Callable getPos);
 
-    /* Force a value, then recursively force list elements and
-       attributes. */
+    /**
+     * Force a value, then recursively force list elements and
+     * attributes.
+     */
     void forceValueDeep(Value & v);
 
-    /* Force `v', and then verify that it has the expected type. */
+    /**
+     * Force `v`, and then verify that it has the expected type.
+     */
     NixInt forceInt(Value & v, const PosIdx pos, std::string_view errorCtx);
     NixFloat forceFloat(Value & v, const PosIdx pos, std::string_view errorCtx);
     bool forceBool(Value & v, const PosIdx pos, std::string_view errorCtx);
@@ -361,7 +421,10 @@ public:
     inline void forceAttrs(Value & v, Callable getPos, std::string_view errorCtx);
 
     inline void forceList(Value & v, const PosIdx pos, std::string_view errorCtx);
-    void forceFunction(Value & v, const PosIdx pos, std::string_view errorCtx); // either lambda or primop
+    /**
+     * @param v either lambda or primop
+     */
+    void forceFunction(Value & v, const PosIdx pos, std::string_view errorCtx);
     std::string_view forceString(Value & v, const PosIdx pos, std::string_view errorCtx);
     std::string_view forceString(Value & v, PathSet & context, const PosIdx pos, std::string_view errorCtx);
     std::string_view forceStringNoCtx(Value & v, const PosIdx pos, std::string_view errorCtx);
@@ -372,17 +435,23 @@ public:
     void addErrorTrace(Error & e, const PosIdx pos, const char * s, const std::string & s2, bool frame = false) const;
 
 public:
-    /* Return true iff the value `v' denotes a derivation (i.e. a
-       set with attribute `type = "derivation"'). */
+    /**
+     * @return true iff the value `v` denotes a derivation (i.e. a
+     * set with attribute `type = "derivation"`).
+     */
     bool isDerivation(Value & v);
 
     std::optional<std::string> tryAttrsToString(const PosIdx pos, Value & v,
         PathSet & context, bool coerceMore = false, bool copyToStore = true);
 
-    /* String coercion.  Converts strings, paths and derivations to a
-       string.  If `coerceMore' is set, also converts nulls, integers,
-       booleans and lists to a string.  If `copyToStore' is set,
-       referenced paths are copied to the Nix store as a side effect. */
+    /**
+     * String coercion.
+     *
+     * Converts strings, paths and derivations to a
+     * string.  If `coerceMore` is set, also converts nulls, integers,
+     * booleans and lists to a string.  If `copyToStore` is set,
+     * referenced paths are copied to the Nix store as a side effect.
+     */
     BackedStringView coerceToString(const PosIdx pos, Value & v, PathSet & context,
         std::string_view errorCtx,
         bool coerceMore = false, bool copyToStore = true,
@@ -390,21 +459,31 @@ public:
 
     StorePath copyPathToStore(PathSet & context, const SourcePath & path);
 
-    /* Path coercion.  Converts strings, paths and derivations to a
-       path.  The result is guaranteed to be a canonicalised, absolute
-       path.  Nothing is copied to the store. */
+    /**
+     * Path coercion.
+     *
+     * Converts strings, paths and derivations to a
+     * path.  The result is guaranteed to be a canonicalised, absolute
+     * path.  Nothing is copied to the store.
+     */
     SourcePath coerceToPath(const PosIdx pos, Value & v, PathSet & context, std::string_view errorCtx);
 
-    /* Like coerceToPath, but the result must be a store path. */
+    /**
+     * Like coerceToPath, but the result must be a store path.
+     */
     StorePath coerceToStorePath(const PosIdx pos, Value & v, PathSet & context, std::string_view errorCtx);
 
 public:
 
-    /* The base environment, containing the builtin functions and
-       values. */
+    /**
+     * The base environment, containing the builtin functions and
+     * values.
+     */
     Env & baseEnv;
 
-    /* The same, but used during parsing to resolve variables. */
+    /**
+     * The same, but used during parsing to resolve variables.
+     */
     std::shared_ptr<StaticEnv> staticBaseEnv; // !!! should be private
 
 private:
@@ -454,8 +533,10 @@ private:
 
 public:
 
-    /* Do a deep equality test between two values.  That is, list
-       elements and attributes are compared recursively. */
+    /**
+     * Do a deep equality test between two values.  That is, list
+     * elements and attributes are compared recursively.
+     */
     bool eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx);
 
     bool isFunctor(Value & fun);
@@ -469,11 +550,15 @@ public:
         callFunction(fun, 1, args, vRes, pos);
     }
 
-    /* Automatically call a function for which each argument has a
-       default value or has a binding in the `args' map. */
+    /**
+     * Automatically call a function for which each argument has a
+     * default value or has a binding in the `args` map.
+     */
     void autoCallFunction(Bindings & args, Value & fun, Value & res);
 
-    /* Allocation primitives. */
+    /**
+     * Allocation primitives.
+     */
     inline Value * allocValue();
     inline Env & allocEnv(size_t size);
 
@@ -493,10 +578,13 @@ public:
 
     void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx);
 
-    /* Print statistics. */
+    /**
+     * Print statistics.
+     */
     void printStats();
 
-    /* Realise the given context, and return a mapping from the placeholders
+    /**
+     * Realise the given context, and return a mapping from the placeholders
      * used to construct the associated value to their final store path
      */
     [[nodiscard]] StringMap realiseContext(const PathSet & context);
@@ -556,11 +644,15 @@ struct DebugTraceStacker {
     DebugTrace trace;
 };
 
-/* Return a string representing the type of the value `v'. */
+/**
+ * @return A string representing the type of the value `v`.
+ */
 std::string_view showType(ValueType type);
 std::string showType(const Value & v);
 
-/* If `path' refers to a directory, then append "/default.nix". */
+/**
+ * If `path` refers to a directory, then append "/default.nix".
+ */
 SourcePath resolveExprPath(const SourcePath & path);
 
 struct InvalidPathError : EvalError
diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc
index 4c571fd7d..ccf868361 100644
--- a/src/libexpr/flake/flake.cc
+++ b/src/libexpr/flake/flake.cc
@@ -125,6 +125,9 @@ static FlakeInput parseFlakeInput(EvalState & state,
                 follows.insert(follows.begin(), lockRootPath.begin(), lockRootPath.end());
                 input.follows = follows;
             } else {
+                // Allow selecting a subset of enum values
+                #pragma GCC diagnostic push
+                #pragma GCC diagnostic ignored "-Wswitch-enum"
                 switch (attr.value->type()) {
                     case nString:
                         attrs.emplace(state.symbols[attr.name], attr.value->string.s);
@@ -139,6 +142,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
                         throw TypeError("flake input attribute '%s' is %s while a string, Boolean, or integer is expected",
                             state.symbols[attr.name], showType(*attr.value));
                 }
+                #pragma GCC diagnostic pop
             }
         } catch (Error & e) {
             e.addTrace(
@@ -334,10 +338,14 @@ LockedFlake lockFlake(
     }
 
     try {
+        if (!fetchSettings.allowDirty && lockFlags.referenceLockFilePath) {
+            throw Error("reference lock file was provided, but the `allow-dirty` setting is set to false");
+        }
 
         // FIXME: symlink attack
         auto oldLockFile = LockFile::read(
-            flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock");
+            lockFlags.referenceLockFilePath.value_or(
+                flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock"));
 
         debug("old lock file: %s", oldLockFile);
 
@@ -619,13 +627,20 @@ LockedFlake lockFlake(
 
         debug("new lock file: %s", newLockFile);
 
+        auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
+        auto sourcePath = topRef.input.getSourcePath();
+        auto outputLockFilePath = sourcePath ? std::optional{*sourcePath + "/" + relPath} : std::nullopt;
+        if (lockFlags.outputLockFilePath) {
+            outputLockFilePath = lockFlags.outputLockFilePath;
+        }
+
         /* Check whether we need to / can write the new lock file. */
-        if (!(newLockFile == oldLockFile)) {
+        if (newLockFile != oldLockFile || lockFlags.outputLockFilePath) {
 
             auto diff = LockFile::diff(oldLockFile, newLockFile);
 
             if (lockFlags.writeLockFile) {
-                if (auto sourcePath = topRef.input.getSourcePath()) {
+                if (outputLockFilePath) {
                     if (auto unlockedInput = newLockFile.isUnlocked()) {
                         if (fetchSettings.warnDirty)
                             warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput);
@@ -633,25 +648,24 @@ LockedFlake lockFlake(
                         if (!lockFlags.updateLockFile)
                             throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef);
 
-                        auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
-
-                        auto path = *sourcePath + "/" + relPath;
-
-                        bool lockFileExists = pathExists(path);
+                        bool lockFileExists = pathExists(*outputLockFilePath);
 
                         if (lockFileExists) {
                             auto s = chomp(diff);
                             if (s.empty())
-                                warn("updating lock file '%s'", path);
+                                warn("updating lock file '%s'", *outputLockFilePath);
                             else
-                                warn("updating lock file '%s':\n%s", path, s);
+                                warn("updating lock file '%s':\n%s", *outputLockFilePath, s);
                         } else
-                            warn("creating lock file '%s'", path);
+                            warn("creating lock file '%s'", *outputLockFilePath);
 
-                        newLockFile.write(path);
+                        newLockFile.write(*outputLockFilePath);
 
                         std::optional<std::string> commitMessage = std::nullopt;
                         if (lockFlags.commitLockFile) {
+                            if (lockFlags.outputLockFilePath) {
+                                throw Error("--commit-lock-file and --output-lock-file are currently incompatible");
+                            }
                             std::string cm;
 
                             cm = fetchSettings.commitLockFileSummary.get();
diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh
index 3cb39d766..c1d1b71e5 100644
--- a/src/libexpr/flake/flake.hh
+++ b/src/libexpr/flake/flake.hh
@@ -18,7 +18,8 @@ struct FlakeInput;
 
 typedef std::map<FlakeId, FlakeInput> FlakeInputs;
 
-/* FlakeInput is the 'Flake'-level parsed form of the "input" entries
+/**
+ * FlakeInput is the 'Flake'-level parsed form of the "input" entries
  * in the flake file.
  *
  * A FlakeInput is normally constructed by the 'parseFlakeInput'
@@ -42,7 +43,12 @@ typedef std::map<FlakeId, FlakeInput> FlakeInputs;
 struct FlakeInput
 {
     std::optional<FlakeRef> ref;
-    bool isFlake = true;  // true = process flake to get outputs, false = (fetched) static source path
+    /**
+     * true = process flake to get outputs
+     *
+     * false = (fetched) static source path
+     */
+    bool isFlake = true;
     std::optional<InputPath> follows;
     FlakeInputs overrides;
 };
@@ -56,23 +62,42 @@ struct ConfigFile
     void apply();
 };
 
-/* The contents of a flake.nix file. */
+/**
+ * The contents of a flake.nix file.
+ */
 struct Flake
 {
-    FlakeRef originalRef; // the original flake specification (by the user)
-    FlakeRef resolvedRef; // registry references and caching resolved to the specific underlying flake
-    FlakeRef lockedRef; // the specific local store result of invoking the fetcher
-    bool forceDirty = false; // pretend that 'lockedRef' is dirty
+    /**
+     * The original flake specification (by the user)
+     */
+    FlakeRef originalRef;
+    /**
+     * registry references and caching resolved to the specific underlying flake
+     */
+    FlakeRef resolvedRef;
+    /**
+     * the specific local store result of invoking the fetcher
+     */
+    FlakeRef lockedRef;
+    /**
+     * pretend that 'lockedRef' is dirty
+     */
+    bool forceDirty = false;
     std::optional<std::string> description;
     std::shared_ptr<const fetchers::Tree> sourceInfo;
     FlakeInputs inputs;
-    ConfigFile config; // 'nixConfig' attribute
+    /**
+     * 'nixConfig' attribute
+     */
+    ConfigFile config;
     ~Flake();
 };
 
 Flake getFlake(EvalState & state, const FlakeRef & flakeRef, bool allowLookup);
 
-/* Fingerprint of a locked flake; used as a cache key. */
+/**
+ * Fingerprint of a locked flake; used as a cache key.
+ */
 typedef Hash Fingerprint;
 
 struct LockedFlake
@@ -85,44 +110,72 @@ struct LockedFlake
 
 struct LockFlags
 {
-    /* Whether to ignore the existing lock file, creating a new one
-       from scratch. */
+    /**
+     * Whether to ignore the existing lock file, creating a new one
+     * from scratch.
+     */
     bool recreateLockFile = false;
 
-    /* Whether to update the lock file at all. If set to false, if any
-       change to the lock file is needed (e.g. when an input has been
-       added to flake.nix), you get a fatal error. */
+    /**
+     * Whether to update the lock file at all. If set to false, if any
+     * change to the lock file is needed (e.g. when an input has been
+     * added to flake.nix), you get a fatal error.
+     */
     bool updateLockFile = true;
 
-    /* Whether to write the lock file to disk. If set to true, if the
-       any changes to the lock file are needed and the flake is not
-       writable (i.e. is not a local Git working tree or similar), you
-       get a fatal error. If set to false, Nix will use the modified
-       lock file in memory only, without writing it to disk. */
+    /**
+     * Whether to write the lock file to disk. If set to true, if the
+     * any changes to the lock file are needed and the flake is not
+     * writable (i.e. is not a local Git working tree or similar), you
+     * get a fatal error. If set to false, Nix will use the modified
+     * lock file in memory only, without writing it to disk.
+     */
     bool writeLockFile = true;
 
-    /* Whether to use the registries to lookup indirect flake
-       references like 'nixpkgs'. */
+    /**
+     * Whether to use the registries to lookup indirect flake
+     * references like 'nixpkgs'.
+     */
     std::optional<bool> useRegistries = std::nullopt;
 
-    /* Whether to apply flake's nixConfig attribute to the configuration */
+    /**
+     * Whether to apply flake's nixConfig attribute to the configuration
+     */
 
     bool applyNixConfig = false;
 
-    /* Whether unlocked flake references (i.e. those without a Git
-       revision or similar) without a corresponding lock are
-       allowed. Unlocked flake references with a lock are always
-       allowed. */
+    /**
+     * Whether unlocked flake references (i.e. those without a Git
+     * revision or similar) without a corresponding lock are
+     * allowed. Unlocked flake references with a lock are always
+     * allowed.
+     */
     bool allowUnlocked = true;
 
-    /* Whether to commit changes to flake.lock. */
+    /**
+     * Whether to commit changes to flake.lock.
+     */
     bool commitLockFile = false;
 
-    /* Flake inputs to be overridden. */
+    /**
+     * The path to a lock file to read instead of the `flake.lock` file in the top-level flake
+     */
+    std::optional<std::string> referenceLockFilePath;
+
+    /**
+     * The path to a lock file to write to instead of the `flake.lock` file in the top-level flake
+     */
+    std::optional<Path> outputLockFilePath;
+
+    /**
+     * Flake inputs to be overridden.
+     */
     std::map<InputPath, FlakeRef> inputOverrides;
 
-    /* Flake inputs to be updated. This means that any existing lock
-       for those inputs will be ignored. */
+    /**
+     * Flake inputs to be updated. This means that any existing lock
+     * for those inputs will be ignored.
+     */
     std::set<InputPath> inputUpdates;
 };
 
diff --git a/src/libexpr/flake/flakeref.hh b/src/libexpr/flake/flakeref.hh
index 23d19adb1..a7c9208c0 100644
--- a/src/libexpr/flake/flakeref.hh
+++ b/src/libexpr/flake/flakeref.hh
@@ -14,7 +14,8 @@ class Store;
 
 typedef std::string FlakeId;
 
-/* A flake reference specifies how to fetch a flake or raw source
+/**
+ * A flake reference specifies how to fetch a flake or raw source
  * (e.g. from a Git repository).  It is created from a URL-like syntax
  * (e.g. 'github:NixOS/patchelf'), an attrset representation (e.g. '{
  * type="github"; owner = "NixOS"; repo = "patchelf"; }'), or a local
@@ -33,14 +34,17 @@ typedef std::string FlakeId;
  * be lazy), but the fetcher can be invoked at any time via the
  * FlakeRef to ensure the store is populated with this input.
  */
-
 struct FlakeRef
 {
-    /* Fetcher-specific representation of the input, sufficient to
-       perform the fetch operation. */
+    /**
+     * Fetcher-specific representation of the input, sufficient to
+     * perform the fetch operation.
+     */
     fetchers::Input input;
 
-    /* sub-path within the fetched input that represents this input */
+    /**
+     * sub-path within the fetched input that represents this input
+     */
     Path subdir;
 
     bool operator==(const FlakeRef & other) const;
diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc
index a74e68c9c..ba2fd46f0 100644
--- a/src/libexpr/flake/lockfile.cc
+++ b/src/libexpr/flake/lockfile.cc
@@ -234,6 +234,11 @@ bool LockFile::operator ==(const LockFile & other) const
     return toJSON() == other.toJSON();
 }
 
+bool LockFile::operator !=(const LockFile & other) const
+{
+    return !(*this == other);
+}
+
 InputPath parseInputPath(std::string_view s)
 {
     InputPath path;
diff --git a/src/libexpr/flake/lockfile.hh b/src/libexpr/flake/lockfile.hh
index 6512509c5..ba4c0c848 100644
--- a/src/libexpr/flake/lockfile.hh
+++ b/src/libexpr/flake/lockfile.hh
@@ -16,9 +16,11 @@ typedef std::vector<FlakeId> InputPath;
 
 struct LockedNode;
 
-/* A node in the lock file. It has outgoing edges to other nodes (its
-   inputs). Only the root node has this type; all other nodes have
-   type LockedNode. */
+/**
+ * A node in the lock file. It has outgoing edges to other nodes (its
+ * inputs). Only the root node has this type; all other nodes have
+ * type LockedNode.
+ */
 struct Node : std::enable_shared_from_this<Node>
 {
     typedef std::variant<ref<LockedNode>, InputPath> Edge;
@@ -28,7 +30,9 @@ struct Node : std::enable_shared_from_this<Node>
     virtual ~Node() { }
 };
 
-/* A non-root node in the lock file. */
+/**
+ * A non-root node in the lock file.
+ */
 struct LockedNode : Node
 {
     FlakeRef lockedRef, originalRef;
@@ -63,10 +67,15 @@ struct LockFile
 
     void write(const Path & path) const;
 
-    /* Check whether this lock file has any unlocked inputs. */
+    /**
+     * Check whether this lock file has any unlocked inputs.
+     */
     std::optional<FlakeRef> isUnlocked() const;
 
     bool operator ==(const LockFile & other) const;
+    // Needed for old gcc versions that don't synthesize it (like gcc 8.2.2
+    // that is still the default on aarch64-linux)
+    bool operator !=(const LockFile & other) const;
 
     std::shared_ptr<Node> findInput(const InputPath & path);
 
@@ -74,7 +83,9 @@ struct LockFile
 
     static std::string diff(const LockFile & oldLocks, const LockFile & newLocks);
 
-    /* Check that every 'follows' input target exists. */
+    /**
+     * Check that every 'follows' input target exists.
+     */
     void check();
 };
 
diff --git a/src/libexpr/get-drvs.hh b/src/libexpr/get-drvs.hh
index 51ef7782a..584d64ac1 100644
--- a/src/libexpr/get-drvs.hh
+++ b/src/libexpr/get-drvs.hh
@@ -26,7 +26,10 @@ private:
     mutable std::string outputName;
     Outputs outputs;
 
-    bool failed = false; // set if we get an AssertionError
+    /**
+     * Set if we get an AssertionError
+     */
+    bool failed = false;
 
     Bindings * attrs = nullptr, * meta = nullptr;
 
@@ -35,7 +38,10 @@ private:
     bool checkMeta(Value & v);
 
 public:
-    std::string attrPath; /* path towards the derivation */
+    /**
+     * path towards the derivation
+     */
+    std::string attrPath;
 
     DrvInfo(EvalState & state) : state(&state) { };
     DrvInfo(EvalState & state, std::string attrPath, Bindings * attrs);
@@ -47,8 +53,10 @@ public:
     StorePath requireDrvPath() const;
     StorePath queryOutPath() const;
     std::string queryOutputName() const;
-    /** Return the unordered map of output names to (optional) output paths.
-     * The "outputs to install" are determined by `meta.outputsToInstall`. */
+    /**
+     * Return the unordered map of output names to (optional) output paths.
+     * The "outputs to install" are determined by `meta.outputsToInstall`.
+     */
     Outputs queryOutputs(bool withPaths = true, bool onlyOutputsToInstall = false);
 
     StringSet queryMetaNames();
@@ -80,8 +88,10 @@ typedef std::list<DrvInfo> DrvInfos;
 #endif
 
 
-/* If value `v' denotes a derivation, return a DrvInfo object
-   describing it. Otherwise return nothing. */
+/**
+ * If value `v` denotes a derivation, return a DrvInfo object
+ * describing it. Otherwise return nothing.
+ */
 std::optional<DrvInfo> getDerivation(EvalState & state,
     Value & v, bool ignoreAssertionFailures);
 
diff --git a/src/libexpr/local.mk b/src/libexpr/local.mk
index 2171e769b..d243b9cec 100644
--- a/src/libexpr/local.mk
+++ b/src/libexpr/local.mk
@@ -46,3 +46,5 @@ $(foreach i, $(wildcard src/libexpr/flake/*.hh), \
 $(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh
 
 $(d)/flake/flake.cc: $(d)/flake/call-flake.nix.gen.hh
+
+src/libexpr/primops/fromTOML.o:	ERROR_SWITCH_ENUM =
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 2c9d5754e..4566a1388 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -3,6 +3,7 @@
 #include "eval.hh"
 #include "symbol-table.hh"
 #include "util.hh"
+#include "print.hh"
 
 #include <cstdlib>
 
@@ -60,45 +61,12 @@ Pos::operator std::shared_ptr<AbstractPos>() const
     return pos;
 }
 
-/* Displaying abstract syntax trees. */
-
-static void showString(std::ostream & str, std::string_view s)
-{
-    str << '"';
-    for (auto c : s)
-        if (c == '"' || c == '\\' || c == '$') str << "\\" << c;
-        else if (c == '\n') str << "\\n";
-        else if (c == '\r') str << "\\r";
-        else if (c == '\t') str << "\\t";
-        else str << c;
-    str << '"';
-}
-
+// FIXME: remove, because *symbols* are abstract and do not have a single
+//        textual representation; see printIdentifier()
 std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
 {
     std::string_view s = symbol;
-
-    if (s.empty())
-        str << "\"\"";
-    else if (s == "if") // FIXME: handle other keywords
-        str << '"' << s << '"';
-    else {
-        char c = s[0];
-        if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) {
-            showString(str, s);
-            return str;
-        }
-        for (auto c : s)
-            if (!((c >= 'a' && c <= 'z') ||
-                  (c >= 'A' && c <= 'Z') ||
-                  (c >= '0' && c <= '9') ||
-                  c == '_' || c == '\'' || c == '-')) {
-                showString(str, s);
-                return str;
-            }
-        str << s;
-    }
-    return str;
+    return printIdentifier(str, s);
 }
 
 void Expr::show(const SymbolTable & symbols, std::ostream & str) const
@@ -118,7 +86,7 @@ void ExprFloat::show(const SymbolTable & symbols, std::ostream & str) const
 
 void ExprString::show(const SymbolTable & symbols, std::ostream & str) const
 {
-    showString(str, s);
+    printLiteralString(str, s);
 }
 
 void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index d70280582..5ca3d1fa6 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -22,7 +22,9 @@ MakeError(UndefinedVarError, Error);
 MakeError(MissingArgumentError, EvalError);
 MakeError(RestrictedPathError, Error);
 
-/* Position objects. */
+/**
+ * Position objects.
+ */
 struct Pos
 {
     uint32_t line;
@@ -133,7 +135,9 @@ class EvalState;
 struct StaticEnv;
 
 
-/* An attribute path is a sequence of attribute names. */
+/**
+ * An attribute path is a sequence of attribute names.
+ */
 struct AttrName
 {
     Symbol symbol;
@@ -213,11 +217,11 @@ struct ExprVar : Expr
        or function argument) or from a "with". */
     bool fromWith;
 
-    /* In the former case, the value is obtained by going `level'
+    /* In the former case, the value is obtained by going `level`
        levels up from the current environment and getting the
-       `displ'th value in that environment.  In the latter case, the
-       value is obtained by getting the attribute named `name' from
-       the set stored in the environment that is `level' levels up
+       `displ`th value in that environment.  In the latter case, the
+       value is obtained by getting the attribute named `name` from
+       the set stored in the environment that is `level` levels up
        from the current one.*/
     Level level;
     Displacement displ;
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 7c34e93ad..60bf45147 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -578,6 +578,9 @@ struct CompareValues
                 return v1->integer < v2->fpoint;
             if (v1->type() != v2->type())
                 state.error("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow<EvalError>();
+            // Allow selecting a subset of enum values
+            #pragma GCC diagnostic push
+            #pragma GCC diagnostic ignored "-Wswitch-enum"
             switch (v1->type()) {
                 case nInt:
                     return v1->integer < v2->integer;
@@ -600,6 +603,7 @@ struct CompareValues
                     }
                 default:
                     state.error("cannot compare %s with %s; values of that type are incomparable", showType(*v1), showType(*v2)).debugThrow<EvalError>();
+            #pragma GCC diagnostic pop
             }
         } catch (Error & e) {
             if (!errorCtx.empty())
diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh
index 1c5ce219f..4ae73fe1f 100644
--- a/src/libexpr/primops.hh
+++ b/src/libexpr/primops.hh
@@ -23,9 +23,11 @@ struct RegisterPrimOp
     typedef std::vector<Info> PrimOps;
     static PrimOps * primOps;
 
-    /* You can register a constant by passing an arity of 0. fun
-       will get called during EvalState initialization, so there
-       may be primops not yet added and builtins is not yet sorted. */
+    /**
+     * You can register a constant by passing an arity of 0. fun
+     * will get called during EvalState initialization, so there
+     * may be primops not yet added and builtins is not yet sorted.
+     */
     RegisterPrimOp(
         std::string name,
         size_t arity,
@@ -38,10 +40,14 @@ struct RegisterPrimOp
    may wish to use them in limited contexts without globally enabling
    them. */
 
-/* Load a ValueInitializer from a DSO and return whatever it initializes */
+/**
+ * Load a ValueInitializer from a DSO and return whatever it initializes
+ */
 void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Value & v);
 
-/* Execute a program and parse its output */
+/**
+ * Execute a program and parse its output
+ */
 void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v);
 
 }
diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc
new file mode 100644
index 000000000..d08672cfc
--- /dev/null
+++ b/src/libexpr/print.cc
@@ -0,0 +1,78 @@
+#include "print.hh"
+
+namespace nix {
+
+std::ostream &
+printLiteralString(std::ostream & str, const std::string_view string)
+{
+    str << "\"";
+    for (auto i = string.begin(); i != string.end(); ++i) {
+        if (*i == '\"' || *i == '\\') str << "\\" << *i;
+        else if (*i == '\n') str << "\\n";
+        else if (*i == '\r') str << "\\r";
+        else if (*i == '\t') str << "\\t";
+        else if (*i == '$' && *(i+1) == '{') str << "\\" << *i;
+        else str << *i;
+    }
+    str << "\"";
+    return str;
+}
+
+std::ostream &
+printLiteralBool(std::ostream & str, bool boolean)
+{
+    str << (boolean ? "true" : "false");
+    return str;
+}
+
+std::ostream &
+printIdentifier(std::ostream & str, std::string_view s) {
+    if (s.empty())
+        str << "\"\"";
+    else if (s == "if") // FIXME: handle other keywords
+        str << '"' << s << '"';
+    else {
+        char c = s[0];
+        if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) {
+            printLiteralString(str, s);
+            return str;
+        }
+        for (auto c : s)
+            if (!((c >= 'a' && c <= 'z') ||
+                  (c >= 'A' && c <= 'Z') ||
+                  (c >= '0' && c <= '9') ||
+                  c == '_' || c == '\'' || c == '-')) {
+                printLiteralString(str, s);
+                return str;
+            }
+        str << s;
+    }
+    return str;
+}
+
+// FIXME: keywords
+static bool isVarName(std::string_view s)
+{
+    if (s.size() == 0) return false;
+    char c = s[0];
+    if ((c >= '0' && c <= '9') || c == '-' || c == '\'') return false;
+    for (auto & i : s)
+        if (!((i >= 'a' && i <= 'z') ||
+              (i >= 'A' && i <= 'Z') ||
+              (i >= '0' && i <= '9') ||
+              i == '_' || i == '-' || i == '\''))
+            return false;
+    return true;
+}
+
+std::ostream &
+printAttributeName(std::ostream & str, std::string_view name) {
+    if (isVarName(name))
+        str << name;
+    else
+        printLiteralString(str, name);
+    return str;
+}
+
+
+}
diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh
new file mode 100644
index 000000000..f9cfc3964
--- /dev/null
+++ b/src/libexpr/print.hh
@@ -0,0 +1,48 @@
+#pragma once
+/**
+ * @file
+ * @brief Common printing functions for the Nix language
+ *
+ * While most types come with their own methods for printing, they share some
+ * functions that are placed here.
+ */
+
+#include <iostream>
+
+namespace nix {
+    /**
+     * Print a string as a Nix string literal.
+     *
+     * Quotes and fairly minimal escaping are added.
+     *
+     * @param s The logical string
+     */
+    std::ostream & printLiteralString(std::ostream & o, std::string_view s);
+    inline std::ostream & printLiteralString(std::ostream & o, const char * s) {
+        return printLiteralString(o, std::string_view(s));
+    }
+    inline std::ostream & printLiteralString(std::ostream & o, const std::string & s) {
+        return printLiteralString(o, std::string_view(s));
+    }
+
+    /** Print `true` or `false`. */
+    std::ostream & printLiteralBool(std::ostream & o, bool b);
+
+    /**
+     * Print a string as an attribute name in the Nix expression language syntax.
+     *
+     * Prints a quoted string if necessary.
+     */
+    std::ostream & printAttributeName(std::ostream & o, std::string_view s);
+
+    /**
+     * Print a string as an identifier in the Nix expression language syntax.
+     *
+     * FIXME: "identifier" is ambiguous. Identifiers do not have a single
+     *        textual representation. They can be used in variable references,
+     *        let bindings, left-hand sides or attribute names in a select
+     *        expression, or something else entirely, like JSON. Use one of the
+     *        `print*` functions instead.
+     */
+    std::ostream & printIdentifier(std::ostream & o, std::string_view s);
+}
diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh
index c97a0a2db..967a186dd 100644
--- a/src/libexpr/symbol-table.hh
+++ b/src/libexpr/symbol-table.hh
@@ -10,15 +10,11 @@
 
 namespace nix {
 
-/* Symbol table used by the parser and evaluator to represent and look
-   up identifiers and attributes efficiently.  SymbolTable::create()
-   converts a string into a symbol.  Symbols have the property that
-   they can be compared efficiently (using an equality test),
-   because the symbol table stores only one copy of each string. */
-
-/* This class mainly exists to give us an operator<< for ostreams. We could also
-   return plain strings from SymbolTable, but then we'd have to wrap every
-   instance of a symbol that is fmt()ed, which is inconvenient and error-prone. */
+/**
+ * This class mainly exists to give us an operator<< for ostreams. We could also
+ * return plain strings from SymbolTable, but then we'd have to wrap every
+ * instance of a symbol that is fmt()ed, which is inconvenient and error-prone.
+ */
 class SymbolStr
 {
     friend class SymbolTable;
@@ -47,6 +43,11 @@ public:
     friend std::ostream & operator <<(std::ostream & os, const SymbolStr & symbol);
 };
 
+/**
+ * Symbols have the property that they can be compared efficiently
+ * (using an equality test), because the symbol table stores only one
+ * copy of each string.
+ */
 class Symbol
 {
     friend class SymbolTable;
@@ -66,6 +67,10 @@ public:
     bool operator!=(const Symbol other) const { return id != other.id; }
 };
 
+/**
+ * Symbol table used by the parser and evaluator to represent and look
+ * up identifiers and attributes efficiently.
+ */
 class SymbolTable
 {
 private:
@@ -74,6 +79,9 @@ private:
 
 public:
 
+    /**
+     * converts a string into a symbol.
+     */
     Symbol create(std::string_view s)
     {
         // Most symbols are looked up more than once, so we trade off insertion performance
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index eb6f56d07..d524c4869 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -37,9 +37,11 @@ typedef enum {
     tFloat
 } InternalType;
 
-// This type abstracts over all actual value types in the language,
-// grouping together implementation details like tList*, different function
-// types, and types in non-normal form (so thunks and co.)
+/**
+ * This type abstracts over all actual value types in the language,
+ * grouping together implementation details like tList*, different function
+ * types, and types in non-normal form (so thunks and co.)
+ */
 typedef enum {
     nThunk,
     nInt,
@@ -71,38 +73,51 @@ class XMLWriter;
 typedef int64_t NixInt;
 typedef double NixFloat;
 
-/* External values must descend from ExternalValueBase, so that
+/**
+ * External values must descend from ExternalValueBase, so that
  * type-agnostic nix functions (e.g. showType) can be implemented
  */
 class ExternalValueBase
 {
     friend std::ostream & operator << (std::ostream & str, const ExternalValueBase & v);
     protected:
-    /* Print out the value */
+    /**
+     * Print out the value
+     */
     virtual std::ostream & print(std::ostream & str) const = 0;
 
     public:
-    /* Return a simple string describing the type */
+    /**
+     * Return a simple string describing the type
+     */
     virtual std::string showType() const = 0;
 
-    /* Return a string to be used in builtins.typeOf */
+    /**
+     * Return a string to be used in builtins.typeOf
+     */
     virtual std::string typeOf() const = 0;
 
-    /* Coerce the value to a string. Defaults to uncoercable, i.e. throws an
+    /**
+     * Coerce the value to a string. Defaults to uncoercable, i.e. throws an
      * error.
      */
     virtual std::string coerceToString(const Pos & pos, PathSet & context, bool copyMore, bool copyToStore) const;
 
-    /* Compare to another value of the same type. Defaults to uncomparable,
+    /**
+     * Compare to another value of the same type. Defaults to uncomparable,
      * i.e. always false.
      */
     virtual bool operator ==(const ExternalValueBase & b) const;
 
-    /* Print the value as JSON. Defaults to unconvertable, i.e. throws an error */
+    /**
+     * Print the value as JSON. Defaults to unconvertable, i.e. throws an error
+     */
     virtual nlohmann::json printValueAsJSON(EvalState & state, bool strict,
         PathSet & context, bool copyToStore = true) const;
 
-    /* Print the value as XML. Defaults to unevaluated */
+    /**
+     * Print the value as XML. Defaults to unevaluated
+     */
     virtual void printValueAsXML(EvalState & state, bool strict, bool location,
         XMLWriter & doc, PathSet & context, PathSet & drvsSeen,
         const PosIdx pos) const;
@@ -147,26 +162,28 @@ public:
         NixInt integer;
         bool boolean;
 
-        /* Strings in the evaluator carry a so-called `context' which
-           is a list of strings representing store paths.  This is to
-           allow users to write things like
+        /**
+         * Strings in the evaluator carry a so-called `context` which
+         * is a list of strings representing store paths.  This is to
+         * allow users to write things like
 
-             "--with-freetype2-library=" + freetype + "/lib"
+         *   "--with-freetype2-library=" + freetype + "/lib"
 
-           where `freetype' is a derivation (or a source to be copied
-           to the store).  If we just concatenated the strings without
-           keeping track of the referenced store paths, then if the
-           string is used as a derivation attribute, the derivation
-           will not have the correct dependencies in its inputDrvs and
-           inputSrcs.
+         * where `freetype` is a derivation (or a source to be copied
+         * to the store).  If we just concatenated the strings without
+         * keeping track of the referenced store paths, then if the
+         * string is used as a derivation attribute, the derivation
+         * will not have the correct dependencies in its inputDrvs and
+         * inputSrcs.
 
-           The semantics of the context is as follows: when a string
-           with context C is used as a derivation attribute, then the
-           derivations in C will be added to the inputDrvs of the
-           derivation, and the other store paths in C will be added to
-           the inputSrcs of the derivations.
+         * The semantics of the context is as follows: when a string
+         * with context C is used as a derivation attribute, then the
+         * derivations in C will be added to the inputDrvs of the
+         * derivation, and the other store paths in C will be added to
+         * the inputSrcs of the derivations.
 
-           For canonicity, the store paths should be in sorted order. */
+         * For canonicity, the store paths should be in sorted order.
+         */
         struct {
             const char * s;
             const char * * context; // must be in sorted order
@@ -198,8 +215,10 @@ public:
         NixFloat fpoint;
     };
 
-    // Returns the normal type of a Value. This only returns nThunk if the
-    // Value hasn't been forceValue'd
+    /**
+     * Returns the normal type of a Value. This only returns nThunk if
+     * the Value hasn't been forceValue'd
+     */
     inline ValueType type() const
     {
         switch (internalType) {
@@ -218,8 +237,10 @@ public:
         abort();
     }
 
-    /* After overwriting an app node, be sure to clear pointers in the
-       Value to ensure that the target isn't kept alive unnecessarily. */
+    /**
+     * After overwriting an app node, be sure to clear pointers in the
+     * Value to ensure that the target isn't kept alive unnecessarily.
+     */
     inline void clearValue()
     {
         app.left = app.right = 0;
@@ -372,9 +393,11 @@ public:
 
     PosIdx determinePos(const PosIdx pos) const;
 
-    /* Check whether forcing this value requires a trivial amount of
-       computation. In particular, function applications are
-       non-trivial. */
+    /**
+     * Check whether forcing this value requires a trivial amount of
+     * computation. In particular, function applications are
+     * non-trivial.
+     */
     bool isTrivial() const;
 
     NixStringContext getContext(const Store &);
@@ -432,7 +455,9 @@ typedef std::map<Symbol, ValueVector> ValueVectorMap;
 #endif
 
 
-/* A value allocated in traceable memory. */
+/**
+ * A value allocated in traceable memory.
+ */
 typedef std::shared_ptr<Value *> RootValue;
 
 RootValue allocRootValue(Value * v);
diff --git a/src/libexpr/value/context.hh b/src/libexpr/value/context.hh
index d467b4f1d..8719602d8 100644
--- a/src/libexpr/value/context.hh
+++ b/src/libexpr/value/context.hh
@@ -28,34 +28,37 @@ public:
 
 class Store;
 
-/* Plain opaque path to some store object.
-
-   Encoded as just the path: ‘<path>’.
-*/
+/**
+ * Plain opaque path to some store object.
+ *
+ * Encoded as just the path: ‘<path>’.
+ */
 struct NixStringContextElem_Opaque {
     StorePath path;
 
     GENERATE_CMP(NixStringContextElem_Opaque, me->path);
 };
 
-/* Path to a derivation and its entire build closure.
-
-   The path doesn't just refer to derivation itself and its closure, but
-   also all outputs of all derivations in that closure (including the
-   root derivation).
-
-   Encoded in the form ‘=<drvPath>’.
-*/
+/**
+ * Path to a derivation and its entire build closure.
+ *
+ * The path doesn't just refer to derivation itself and its closure, but
+ * also all outputs of all derivations in that closure (including the
+ * root derivation).
+ *
+ * Encoded in the form ‘=<drvPath>’.
+ */
 struct NixStringContextElem_DrvDeep {
     StorePath drvPath;
 
     GENERATE_CMP(NixStringContextElem_DrvDeep, me->drvPath);
 };
 
-/* Derivation output.
-
-   Encoded in the form ‘!<output>!<drvPath>’.
-*/
+/**
+ * Derivation output.
+ *
+ * Encoded in the form ‘!<output>!<drvPath>’.
+ */
 struct NixStringContextElem_Built {
     StorePath drvPath;
     std::string output;
@@ -84,11 +87,12 @@ struct NixStringContextElem : _NixStringContextElem_Raw {
         return static_cast<Raw &>(*this);
     }
 
-    /* Decode a context string, one of:
-       - ‘<path>’
-       - ‘=<path>’
-       - ‘!<name>!<path>’
-      */
+    /**
+     * Decode a context string, one of:
+     * - ‘<path>’
+     * - ‘=<path>’
+     * - ‘!<name>!<path>’
+     */
     static NixStringContextElem parse(const Store & store, std::string_view s);
     std::string to_string(const Store & store) const;
 };