From f97d3753a13f0ff916d83dbea4fe7dae7194f903 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 9 Sep 2019 17:34:38 +0200 Subject: Require flake.nix to be an attrset (not a non-trivial thunk) --- src/libexpr/value.hh | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index e1ec87d3b..bdf2cdde1 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -170,6 +170,11 @@ struct Value { return type == tList1 ? 1 : type == tList2 ? 2 : bigList.size; } + + /* Check whether forcing this value requires a trivial amount of + computation. In particular, function applications are + non-trivial. */ + bool isTrivial() const; }; -- cgit v1.2.3 From 50f13b06fb1b2f50a97323c000d1094d090f08ea Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 29 Jun 2020 19:08:37 +0200 Subject: EvalCache: Store string contexts --- src/libexpr/value.hh | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 1a0738241..fe11bb2ed 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -171,6 +171,8 @@ struct Value computation. In particular, function applications are non-trivial. */ bool isTrivial() const; + + std::vector> getContext(); }; -- cgit v1.2.3 From fa307875e961a616a049206645a651a76a050a79 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 11 Dec 2020 23:32:45 +0100 Subject: Introduce NormalType for the normal type of a Value This will be useful to abstract over the ValueType implementation details Make use of it already to replace the showType(ValueType) function --- src/libexpr/value.hh | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index fe11bb2ed..833af0f3d 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -29,6 +29,22 @@ typedef enum { tFloat } ValueType; +// 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, + nFloat, + nBool, + nString, + nPath, + nNull, + nAttrs, + nList, + nFunction, + nExternal +} NormalType; class Bindings; struct Env; @@ -147,6 +163,26 @@ struct Value NixFloat fpoint; }; + // Returns the normal type of a Value. This only returns nThunk if the + // Value hasn't been forceValue'd + inline NormalType normalType() const + { + switch (type) { + case tInt: return nInt; + case tBool: return nBool; + case tString: return nString; + case tPath: return nPath; + case tNull: return nNull; + case tAttrs: return nAttrs; + case tList1: case tList2: case tListN: return nList; + case tLambda: case tPrimOp: case tPrimOpApp: return nFunction; + case tExternal: return nExternal; + case tFloat: return nFloat; + case tThunk: case tApp: case tBlackhole: return nThunk; + } + abort(); + } + bool isList() const { return type == tList1 || type == tList2 || type == tListN; -- cgit v1.2.3 From 9f056f7afdb85b8c3bd59638197e356f269129b2 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sat, 12 Dec 2020 00:19:05 +0100 Subject: Introduce Value type setters and make use of them --- src/libexpr/value.hh | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 833af0f3d..0995dcd7b 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -107,6 +107,25 @@ std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); struct Value { ValueType type; + + inline void setInt() { type = tInt; }; + inline void setBool() { type = tBool; }; + inline void setString() { type = tString; }; + inline void setPath() { type = tPath; }; + inline void setNull() { type = tNull; }; + inline void setAttrs() { type = tAttrs; }; + inline void setList1() { type = tList1; }; + inline void setList2() { type = tList2; }; + inline void setListN() { type = tListN; }; + inline void setThunk() { type = tThunk; }; + inline void setApp() { type = tApp; }; + inline void setLambda() { type = tLambda; }; + inline void setBlackhole() { type = tBlackhole; }; + inline void setPrimOp() { type = tPrimOp; }; + inline void setPrimOpApp() { type = tPrimOpApp; }; + inline void setExternal() { type = tExternal; }; + inline void setFloat() { type = tFloat; }; + union { NixInt integer; @@ -223,7 +242,7 @@ static inline void clearValue(Value & v) static inline void mkInt(Value & v, NixInt n) { clearValue(v); - v.type = tInt; + v.setInt(); v.integer = n; } @@ -231,7 +250,7 @@ static inline void mkInt(Value & v, NixInt n) static inline void mkFloat(Value & v, NixFloat n) { clearValue(v); - v.type = tFloat; + v.setFloat(); v.fpoint = n; } @@ -239,7 +258,7 @@ static inline void mkFloat(Value & v, NixFloat n) static inline void mkBool(Value & v, bool b) { clearValue(v); - v.type = tBool; + v.setBool(); v.boolean = b; } @@ -247,13 +266,13 @@ static inline void mkBool(Value & v, bool b) static inline void mkNull(Value & v) { clearValue(v); - v.type = tNull; + v.setNull(); } static inline void mkApp(Value & v, Value & left, Value & right) { - v.type = tApp; + v.setApp(); v.app.left = &left; v.app.right = &right; } @@ -261,7 +280,7 @@ static inline void mkApp(Value & v, Value & left, Value & right) static inline void mkPrimOpApp(Value & v, Value & left, Value & right) { - v.type = tPrimOpApp; + v.setPrimOpApp(); v.app.left = &left; v.app.right = &right; } @@ -269,7 +288,7 @@ static inline void mkPrimOpApp(Value & v, Value & left, Value & right) static inline void mkStringNoCopy(Value & v, const char * s) { - v.type = tString; + v.setString(); v.string.s = s; v.string.context = 0; } @@ -287,7 +306,7 @@ void mkString(Value & v, const char * s); static inline void mkPathNoCopy(Value & v, const char * s) { clearValue(v); - v.type = tPath; + v.setPath(); v.path = s; } -- cgit v1.2.3 From bf9890396731a2bbe4f04a49684dee463d818906 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sat, 12 Dec 2020 02:15:11 +0100 Subject: Add ValueType checking functions for types that have the same NormalType --- src/libexpr/value.hh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 0995dcd7b..e743da9c3 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -126,6 +126,20 @@ struct Value inline void setExternal() { type = tExternal; }; inline void setFloat() { type = tFloat; }; + // Functions needed to distinguish the type + // These should be removed eventually, by putting the functionality that's + // needed by callers into methods of this type + + // normalType() == nThunk + inline bool isThunk() const { return type == tThunk; }; + inline bool isApp() const { return type == tApp; }; + inline bool isBlackhole() const { return type == tBlackhole; }; + + // normalType() == nFunction + inline bool isLambda() const { return type == tLambda; }; + inline bool isPrimOp() const { return type == tPrimOp; }; + inline bool isPrimOpApp() const { return type == tPrimOpApp; }; + union { NixInt integer; -- cgit v1.2.3 From 730b152b190135adef2f53c7a80cfd1111d37ead Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sat, 12 Dec 2020 02:22:58 +0100 Subject: Make Value::type private This is an implementation detail and shouldn't be used. Use normalType() and the various is functions instead --- src/libexpr/value.hh | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index e743da9c3..4050d7e4b 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -106,8 +106,14 @@ std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); struct Value { +private: ValueType type; +friend std::string showType(const Value & v); +friend void printValue(std::ostream & str, std::set & active, const Value & v); + +public: + inline void setInt() { type = tInt; }; inline void setBool() { type = tBool; }; inline void setString() { type = tString; }; -- cgit v1.2.3 From d67e02919c7f941615407dfd14cfdab6a28c4c26 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 17 Dec 2020 14:42:52 +0100 Subject: Rename ValueType -> InternalType, NormalType -> ValueType And Value::type to Value::internalType, such that type() can be used in the next commit to get the new ValueType --- src/libexpr/value.hh | 64 ++++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 4050d7e4b..8b312bf03 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -27,7 +27,7 @@ typedef enum { tPrimOpApp, tExternal, tFloat -} ValueType; +} InternalType; // This type abstracts over all actual value types in the language, // grouping together implementation details like tList*, different function @@ -44,7 +44,7 @@ typedef enum { nList, nFunction, nExternal -} NormalType; +} ValueType; class Bindings; struct Env; @@ -107,44 +107,44 @@ std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); struct Value { private: - ValueType type; + InternalType internalType; friend std::string showType(const Value & v); friend void printValue(std::ostream & str, std::set & active, const Value & v); public: - inline void setInt() { type = tInt; }; - inline void setBool() { type = tBool; }; - inline void setString() { type = tString; }; - inline void setPath() { type = tPath; }; - inline void setNull() { type = tNull; }; - inline void setAttrs() { type = tAttrs; }; - inline void setList1() { type = tList1; }; - inline void setList2() { type = tList2; }; - inline void setListN() { type = tListN; }; - inline void setThunk() { type = tThunk; }; - inline void setApp() { type = tApp; }; - inline void setLambda() { type = tLambda; }; - inline void setBlackhole() { type = tBlackhole; }; - inline void setPrimOp() { type = tPrimOp; }; - inline void setPrimOpApp() { type = tPrimOpApp; }; - inline void setExternal() { type = tExternal; }; - inline void setFloat() { type = tFloat; }; + inline void setInt() { internalType = tInt; }; + inline void setBool() { internalType = tBool; }; + inline void setString() { internalType = tString; }; + inline void setPath() { internalType = tPath; }; + inline void setNull() { internalType = tNull; }; + inline void setAttrs() { internalType = tAttrs; }; + inline void setList1() { internalType = tList1; }; + inline void setList2() { internalType = tList2; }; + inline void setListN() { internalType = tListN; }; + inline void setThunk() { internalType = tThunk; }; + inline void setApp() { internalType = tApp; }; + inline void setLambda() { internalType = tLambda; }; + inline void setBlackhole() { internalType = tBlackhole; }; + inline void setPrimOp() { internalType = tPrimOp; }; + inline void setPrimOpApp() { internalType = tPrimOpApp; }; + inline void setExternal() { internalType = tExternal; }; + inline void setFloat() { internalType = tFloat; }; // Functions needed to distinguish the type // These should be removed eventually, by putting the functionality that's // needed by callers into methods of this type // normalType() == nThunk - inline bool isThunk() const { return type == tThunk; }; - inline bool isApp() const { return type == tApp; }; - inline bool isBlackhole() const { return type == tBlackhole; }; + inline bool isThunk() const { return internalType == tThunk; }; + inline bool isApp() const { return internalType == tApp; }; + inline bool isBlackhole() const { return internalType == tBlackhole; }; // normalType() == nFunction - inline bool isLambda() const { return type == tLambda; }; - inline bool isPrimOp() const { return type == tPrimOp; }; - inline bool isPrimOpApp() const { return type == tPrimOpApp; }; + inline bool isLambda() const { return internalType == tLambda; }; + inline bool isPrimOp() const { return internalType == tPrimOp; }; + inline bool isPrimOpApp() const { return internalType == tPrimOpApp; }; union { @@ -204,9 +204,9 @@ public: // Returns the normal type of a Value. This only returns nThunk if the // Value hasn't been forceValue'd - inline NormalType normalType() const + inline ValueType normalType() const { - switch (type) { + switch (internalType) { case tInt: return nInt; case tBool: return nBool; case tString: return nString; @@ -224,22 +224,22 @@ public: bool isList() const { - return type == tList1 || type == tList2 || type == tListN; + return internalType == tList1 || internalType == tList2 || internalType == tListN; } Value * * listElems() { - return type == tList1 || type == tList2 ? smallList : bigList.elems; + return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems; } const Value * const * listElems() const { - return type == tList1 || type == tList2 ? smallList : bigList.elems; + return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems; } size_t listSize() const { - return type == tList1 ? 1 : type == tList2 ? 2 : bigList.size; + return internalType == tList1 ? 1 : internalType == tList2 ? 2 : bigList.size; } /* Check whether forcing this value requires a trivial amount of -- cgit v1.2.3 From 12e65078ef5c511196c9e48f7fdf71f6c0e5c89f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 17 Dec 2020 14:45:45 +0100 Subject: Rename Value::normalType() -> Value::type() --- src/libexpr/value.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 8b312bf03..61ea1d64b 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -136,12 +136,12 @@ public: // These should be removed eventually, by putting the functionality that's // needed by callers into methods of this type - // normalType() == nThunk + // type() == nThunk inline bool isThunk() const { return internalType == tThunk; }; inline bool isApp() const { return internalType == tApp; }; inline bool isBlackhole() const { return internalType == tBlackhole; }; - // normalType() == nFunction + // type() == nFunction inline bool isLambda() const { return internalType == tLambda; }; inline bool isPrimOp() const { return internalType == tPrimOp; }; inline bool isPrimOpApp() const { return internalType == tPrimOpApp; }; @@ -204,7 +204,7 @@ public: // Returns the normal type of a Value. This only returns nThunk if the // Value hasn't been forceValue'd - inline ValueType normalType() const + inline ValueType type() const { switch (internalType) { case tInt: return nInt; -- cgit v1.2.3 From b70d22baca3e8826392b61aa53955c6da74b8724 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 18 Dec 2020 14:38:49 +0100 Subject: Replace Value type setters with mk* functions Move clearValue inside Value mkInt instead of setInt mkBool instead of setBool mkString instead of setString mkPath instead of setPath mkNull instead of setNull mkAttrs instead of setAttrs mkList instead of setList* mkThunk instead of setThunk mkApp instead of setApp mkLambda instead of setLambda mkBlackhole instead of setBlackhole mkPrimOp instead of setPrimOp mkPrimOpApp instead of setPrimOpApp mkExternal instead of setExternal mkFloat instead of setFloat Add note that the static mk* function should be removed eventually --- src/libexpr/value.hh | 193 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 124 insertions(+), 69 deletions(-) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 61ea1d64b..b317c1898 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -114,24 +114,6 @@ friend void printValue(std::ostream & str, std::set & active, con public: - inline void setInt() { internalType = tInt; }; - inline void setBool() { internalType = tBool; }; - inline void setString() { internalType = tString; }; - inline void setPath() { internalType = tPath; }; - inline void setNull() { internalType = tNull; }; - inline void setAttrs() { internalType = tAttrs; }; - inline void setList1() { internalType = tList1; }; - inline void setList2() { internalType = tList2; }; - inline void setListN() { internalType = tListN; }; - inline void setThunk() { internalType = tThunk; }; - inline void setApp() { internalType = tApp; }; - inline void setLambda() { internalType = tLambda; }; - inline void setBlackhole() { internalType = tBlackhole; }; - inline void setPrimOp() { internalType = tPrimOp; }; - inline void setPrimOpApp() { internalType = tPrimOpApp; }; - inline void setExternal() { internalType = tExternal; }; - inline void setFloat() { internalType = tFloat; }; - // Functions needed to distinguish the type // These should be removed eventually, by putting the functionality that's // needed by callers into methods of this type @@ -222,6 +204,123 @@ 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. */ + inline void clearValue() + { + app.left = app.right = 0; + } + + inline void mkInt(NixInt n) + { + clearValue(); + internalType = tInt; + integer = n; + } + + inline void mkBool(bool b) + { + clearValue(); + internalType = tBool; + boolean = b; + } + + inline void mkString(const char * s, const char * * context = 0) + { + internalType = tString; + string.s = s; + string.context = context; + } + + inline void mkPath(const char * s) + { + clearValue(); + internalType = tPath; + path = s; + } + + inline void mkNull() + { + clearValue(); + internalType = tNull; + } + + inline void mkAttrs(Bindings * a) + { + clearValue(); + internalType = tAttrs; + attrs = a; + } + + inline void mkList(size_t size) + { + clearValue(); + if (size == 1) + internalType = tList1; + else if (size == 2) + internalType = tList2; + else { + internalType = tListN; + bigList.size = size; + } + } + + inline void mkThunk(Env * e, Expr * ex) + { + internalType = tThunk; + thunk.env = e; + thunk.expr = ex; + } + + inline void mkApp(Value * l, Value * r) + { + internalType = tApp; + app.left = l; + app.right = r; + } + + inline void mkLambda(Env * e, ExprLambda * f) + { + internalType = tLambda; + lambda.env = e; + lambda.fun = f; + } + + inline void mkBlackhole() + { + internalType = tBlackhole; + // Value will be overridden anyways + } + + inline void mkPrimOp(PrimOp * p) + { + clearValue(); + internalType = tPrimOp; + primOp = p; + } + + + inline void mkPrimOpApp(Value * l, Value * r) + { + internalType = tPrimOpApp; + app.left = l; + app.right = r; + } + + inline void mkExternal(ExternalValueBase * e) + { + clearValue(); + internalType = tExternal; + external = e; + } + + inline void mkFloat(NixFloat n) + { + clearValue(); + internalType = tFloat; + fpoint = n; + } + bool isList() const { return internalType == tList1 || internalType == tList2 || internalType == tListN; @@ -251,86 +350,42 @@ public: }; -/* After overwriting an app node, be sure to clear pointers in the - Value to ensure that the target isn't kept alive unnecessarily. */ -static inline void clearValue(Value & v) -{ - v.app.left = v.app.right = 0; -} - +// TODO: Remove these static functions, replace call sites with v.mk* instead static inline void mkInt(Value & v, NixInt n) { - clearValue(v); - v.setInt(); - v.integer = n; + v.mkInt(n); } - static inline void mkFloat(Value & v, NixFloat n) { - clearValue(v); - v.setFloat(); - v.fpoint = n; + v.mkFloat(n); } - static inline void mkBool(Value & v, bool b) { - clearValue(v); - v.setBool(); - v.boolean = b; + v.mkBool(b); } - static inline void mkNull(Value & v) { - clearValue(v); - v.setNull(); + v.mkNull(); } - static inline void mkApp(Value & v, Value & left, Value & right) { - v.setApp(); - v.app.left = &left; - v.app.right = &right; + v.mkApp(&left, &right); } - -static inline void mkPrimOpApp(Value & v, Value & left, Value & right) -{ - v.setPrimOpApp(); - v.app.left = &left; - v.app.right = &right; -} - - -static inline void mkStringNoCopy(Value & v, const char * s) -{ - v.setString(); - v.string.s = s; - v.string.context = 0; -} - - static inline void mkString(Value & v, const Symbol & s) { - mkStringNoCopy(v, ((const string &) s).c_str()); + v.mkString(((const string &) s).c_str()); } void mkString(Value & v, const char * s); -static inline void mkPathNoCopy(Value & v, const char * s) -{ - clearValue(v); - v.setPath(); - v.path = s; -} - - void mkPath(Value & v, const char * s); -- cgit v1.2.3 From 7c76964daa0d1ca07fd609f5eb28b51afd1246b7 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Fri, 8 Jan 2021 22:27:00 +0100 Subject: libexpr: misc improvements for proper error position When working on some more complex Nix code, there are sometimes rather unhelpful or misleading error messages, especially if coerce-errors are thrown. This patch is a first steps towards improving that. I'm happy to file more changes after that, but I'd like to gather some feedback first. To summarize, this patch does the following things: * Attrsets (a.k.a. `Bindings` in `libexpr`) now have a `Pos`. This is helpful e.g. to identify which attribute-set in `listToAttrs` is invalid. * The `Value`-struct has a new method named `determinePos` which tries to guess the position of a value and falls back to a default if that's not possible. This can be used to provide better messages if a coercion fails. * The new `determinePos`-API is used by `builtins.concatMap` now. With that change, Nix shows the exact position in the error where a wrong value was returned by the lambda. To make sure it's still obvious that `concatMap` is the problem, another stack-frame was added. * The changes described above can be added to every other `primop`, but first I'd like to get some feedback about the overall approach. --- src/libexpr/value.hh | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/libexpr/value.hh') diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index b317c1898..a1f131f9e 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -341,6 +341,8 @@ public: return internalType == tList1 ? 1 : internalType == tList2 ? 2 : bigList.size; } + Pos determinePos(const Pos &pos) const; + /* Check whether forcing this value requires a trivial amount of computation. In particular, function applications are non-trivial. */ -- cgit v1.2.3