Merge pull request #9370 from hercules-ci/add-value-types

refactor: Add `Value` types, use `std::span` for list iteration

Author: tomberek

src/libexpr/value.hh

@@ -3,6 +3,7 @@
 
 #include <cassert>
 #include <climits>
+#include <span>
 
 #include "symbol-table.hh"
 #include "value/context.hh"
@@ -158,64 +159,72 @@ public:
     inline bool isPrimOp() const { return internalType == tPrimOp; };
     inline bool isPrimOpApp() const { return internalType == tPrimOpApp; };
 
-    union
-    {
-        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
+     *
+     *   "--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.
+
+     * 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.
+     */
+    struct StringWithContext {
+        const char * c_str;
+        const char * * context; // must be in sorted order
+    };
 
-        /**
-         * 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
+    struct Path {
+        InputAccessor * accessor;
+        const char * path;
+    };
 
-         *   "--with-freetype2-library=" + freetype + "/lib"
+    struct ClosureThunk {
+        Env * env;
+        Expr * expr;
+    };
 
-         * 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.
+    struct FunctionApplicationThunk {
+        Value * left, * right;
+    };
 
-         * 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.
+    struct Lambda {
+        Env * env;
+        ExprLambda * fun;
+    };
 
-         * For canonicity, the store paths should be in sorted order.
-         */
-        struct {
-            const char * c_str;
-            const char * * context; // must be in sorted order
-        } string;
+    union
+    {
+        NixInt integer;
+        bool boolean;
 
-        struct {
-            InputAccessor * accessor;
-            const char * path;
-        } _path;
+        StringWithContext string;
+
+        Path _path;
 
         Bindings * attrs;
         struct {
             size_t size;
             Value * * elems;
         } bigList;
         Value * smallList[2];
-        struct {
-            Env * env;
-            Expr * expr;
-        } thunk;
-        struct {
-            Value * left, * right;
-        } app;
-        struct {
-            Env * env;
-            ExprLambda * fun;
-        } lambda;
+        ClosureThunk thunk;
+        FunctionApplicationThunk app;
+        Lambda lambda;
         PrimOp * primOp;
-        struct {
-            Value * left, * right;
-        } primOpApp;
+        FunctionApplicationThunk primOpApp;
         ExternalValueBase * external;
         NixFloat fpoint;
     };
@@ -387,7 +396,13 @@ public:
         return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems;
     }
 
-    const Value * const * listElems() const
+    std::span<Value * const> listItems() const
+    {
+        assert(isList());
+        return std::span<Value * const>(listElems(), listSize());
+    }
+
+    Value * const * listElems() const
     {
         return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems;
     }
@@ -406,34 +421,6 @@ public:
      */
     bool isTrivial() const;
 
-    auto listItems()
-    {
-        struct ListIterable
-        {
-            typedef Value * const * iterator;
-            iterator _begin, _end;
-            iterator begin() const { return _begin; }
-            iterator end() const { return _end; }
-        };
-        assert(isList());
-        auto begin = listElems();
-        return ListIterable { begin, begin + listSize() };
-    }
-
-    auto listItems() const
-    {
-        struct ConstListIterable
-        {
-            typedef const Value * const * iterator;
-            iterator _begin, _end;
-            iterator begin() const { return _begin; }
-            iterator end() const { return _end; }
-        };
-        assert(isList());
-        auto begin = listElems();
-        return ConstListIterable { begin, begin + listSize() };
-    }
-
     SourcePath path() const
     {
         assert(internalType == tPath);

src/nix-env/nix-env.cc

@@ -172,7 +172,7 @@ static void loadSourceExpr(EvalState & state, const SourcePath & path, Value & v
        directory). */
     else if (st.type == InputAccessor::tDirectory) {
         auto attrs = state.buildBindings(maxAttrs);
-        attrs.alloc("_combineChannels").mkList(0);
+        state.mkList(attrs.alloc("_combineChannels"), 0);
         StringSet seen;
         getAllExprs(state, path, seen, attrs);
         v.mkAttrs(attrs);