From 2eb59c34b531f03a85f67b9246ccaf0ff5fcad23 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 03:02:02 +0100 Subject: [PATCH 1/7] Value: extract Value::StringWithContext --- src/libexpr/value.hh | 54 +++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 191cc30bad7..0f8cef418fc 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -158,37 +158,39 @@ public: inline bool isPrimOp() const { return internalType == tPrimOp; }; inline bool isPrimOpApp() const { return internalType == tPrimOpApp; }; + /** + * 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 + }; + 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 { - const char * c_str; - const char * * context; // must be in sorted order - } string; + StringWithContext string; struct { InputAccessor * accessor; From d8ff5cfe8eba34c8b4b5cc53f3b40cd3dfd84224 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 03:03:37 +0100 Subject: [PATCH 2/7] Value: extract Value::Path --- src/libexpr/value.hh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 0f8cef418fc..34f99499748 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -185,6 +185,11 @@ public: const char * * context; // must be in sorted order }; + struct Path { + InputAccessor * accessor; + const char * path; + }; + union { NixInt integer; @@ -192,10 +197,7 @@ public: StringWithContext string; - struct { - InputAccessor * accessor; - const char * path; - } _path; + Path _path; Bindings * attrs; struct { From b55203e874f8e4b2fc5289129efba791937c23d0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 03:06:04 +0100 Subject: [PATCH 3/7] Value: extract Value::ClosureThunk --- src/libexpr/value.hh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 34f99499748..4c51c52e4b4 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -190,6 +190,11 @@ public: const char * path; }; + struct ClosureThunk { + Env * env; + Expr * expr; + }; + union { NixInt integer; @@ -205,10 +210,7 @@ public: Value * * elems; } bigList; Value * smallList[2]; - struct { - Env * env; - Expr * expr; - } thunk; + ClosureThunk thunk; struct { Value * left, * right; } app; From 6af1d9f7b94da454252b62f0cfff4ce800c5a46b Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 03:06:55 +0100 Subject: [PATCH 4/7] Value: extract Value::FunctionApplicationThunk --- src/libexpr/value.hh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 4c51c52e4b4..cfb3f527679 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -195,6 +195,10 @@ public: Expr * expr; }; + struct FunctionApplicationThunk { + Value * left, * right; + }; + union { NixInt integer; @@ -211,17 +215,13 @@ public: } bigList; Value * smallList[2]; ClosureThunk thunk; - struct { - Value * left, * right; - } app; + FunctionApplicationThunk app; struct { Env * env; ExprLambda * fun; } lambda; PrimOp * primOp; - struct { - Value * left, * right; - } primOpApp; + FunctionApplicationThunk primOpApp; ExternalValueBase * external; NixFloat fpoint; }; From 7055c6528532fdd3b0ce9b8f5282b002fc011470 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 03:07:32 +0100 Subject: [PATCH 5/7] Value: extract Value::Lambda --- src/libexpr/value.hh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index cfb3f527679..93ccdbc2e9e 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -199,6 +199,11 @@ public: Value * left, * right; }; + struct Lambda { + Env * env; + ExprLambda * fun; + }; + union { NixInt integer; @@ -216,10 +221,7 @@ public: Value * smallList[2]; ClosureThunk thunk; FunctionApplicationThunk app; - struct { - Env * env; - ExprLambda * fun; - } lambda; + Lambda lambda; PrimOp * primOp; FunctionApplicationThunk primOpApp; ExternalValueBase * external; From 260c6147625e95e4772ccdee80d6463d242c7b64 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 03:31:45 +0100 Subject: [PATCH 6/7] Value: use std::span, change use of const **`Value` and `const`** These two deserve some explanation. We'll get to lists later. Values can normally be thought of as immutable, except they are are also the vehicle for call by need, which must be implemented using mutation. This circumstance makes a `const Value` a rather useless thing: - If it's a thunk, you can't evaluate it, except by copying, but that would not be call by need. - If it's not a thunk, you know the type, so the method that acquired it for you should have returned something more specific, such as a `const Bindings &` (which actually does make sense because that's an immutable span of pointers to mutable `Value`s. - If you don't care about the type yet, you might establish the convention that `const Value` means `deepSeq`-ed data, but this is hardly useful and not actually as safe as you would supposedly want to trust it to be - just convention. **Lists** `std::span` is a tuple of pointer and size - just what we need. We don't return them as `const Value`, because considering the first bullet point we discussed before, we'd have to force all the list values, which isn't what we want. So what we end up with is a nice representation of a list in weak head normal form: the spine is immutable, but the items may need some evaluation later. --- src/libexpr/value.hh | 37 ++++++++----------------------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 93ccdbc2e9e..bcff8ae55b7 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -3,6 +3,7 @@ #include #include +#include #include "symbol-table.hh" #include "value/context.hh" @@ -395,7 +396,13 @@ public: return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems; } - const Value * const * listElems() const + std::span listItems() const + { + assert(isList()); + return std::span(listElems(), listSize()); + } + + Value * const * listElems() const { return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems; } @@ -414,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); From 121665f3773bc46ca6df0dda6f66b1a86e7d9e72 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 12 Nov 2023 17:51:09 +0100 Subject: [PATCH 7/7] nix-env: Use state.mkList, required for correct stats --- src/nix-env/nix-env.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 213a20d933c..ab1d8f71317 100644 --- a/src/nix-env/nix-env.cc +++ b/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);