Introduce a "failed" value type

In the multithreaded evaluator, it's possible for multiple threads to
wait on the same thunk. If evaluation of the thunk results in an
exception, the waiting threads shouldn't try to re-force the thunk.
Instead, they should rethrow the same exception, without duplicating
any work.

Therefore, there is now a new value type `tFailed` that stores an
std::exception_ptr. If evaluation of a thunk/app results in an
exception, `forceValue()` overwrites the value with a `tFailed`. If
`forceValue()` encounters a `tFailed`, it rethrows the exception. So
you normally never need to check for failed values (since forcing them
causes a rethrow).

Author: edolstra

src/libexpr-c/nix_api_value.cc

@@ -177,6 +177,8 @@ ValueType nix_get_type(nix_c_context * context, const nix_value * value)
         switch (v.type()) {
         case nThunk:
             return NIX_TYPE_THUNK;
+        case nFailed:
+            return NIX_TYPE_FAILED;
         case nInt:
             return NIX_TYPE_INT;
         case nFloat:

src/libexpr-c/nix_api_value.h

@@ -32,7 +32,8 @@ typedef enum {
     NIX_TYPE_ATTRS,
     NIX_TYPE_LIST,
     NIX_TYPE_FUNCTION,
-    NIX_TYPE_EXTERNAL
+    NIX_TYPE_EXTERNAL,
+    NIX_TYPE_FAILED,
 } ValueType;
 
 // forward declarations

src/libexpr/eval.cc

@@ -124,6 +124,8 @@ std::string_view showType(ValueType type, bool withArticle)
         return WA("a", "float");
     case nThunk:
         return WA("a", "thunk");
+    case nFailed:
+        return WA("a", "failure");
     }
     unreachable();
 }
@@ -2693,8 +2695,11 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st
         }
         return;
 
-    case nThunk: // Must not be left by forceValue
-        assert(false);
+    // Cannot be returned by forceValue().
+    case nThunk:
+    case nFailed:
+        unreachable();
+
     default: // Note that we pass compiler flags that should make `default:` unreachable.
         // Also note that this probably ran after `eqValues`, which implements
         // the same logic more efficiently (without having to unwind stacks),
@@ -2786,8 +2791,11 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
         // !!!
         return v1.fpoint() == v2.fpoint();
 
-    case nThunk: // Must not be left by forceValue
-        assert(false);
+    // Cannot be returned by forceValue().
+    case nThunk:
+    case nFailed:
+        unreachable();
+
     default: // Note that we pass compiler flags that should make `default:` unreachable.
         error<EvalError>("eqValues: cannot compare %1% with %2%", showType(v1), showType(v2))
             .withTrace(pos, errorCtx)

src/libexpr/include/nix/expr/eval-inline.hh

@@ -97,12 +97,19 @@ void EvalState::forceValue(Value & v, const PosIdx pos)
             else
                 ExprBlackHole::throwInfiniteRecursionError(*this, v);
         } catch (...) {
-            v.mkThunk(env, expr);
             tryFixupBlackHolePos(v, pos);
+            v.mkFailed();
             throw;
         }
-    } else if (v.isApp())
-        callFunction(*v.app().left, *v.app().right, v, pos);
+    } else if (v.isApp()) {
+        try {
+            callFunction(*v.app().left, *v.app().right, v, pos);
+        } catch (...) {
+            v.mkFailed();
+            throw;
+        }
+    } else if (v.isFailed())
+        std::rethrow_exception(v.failed()->ex);
 }
 
 [[gnu::always_inline]]

src/libexpr/include/nix/expr/value.hh

@@ -35,6 +35,7 @@ typedef enum {
     tBool,
     tNull,
     tFloat,
+    tFailed,
     tExternal,
     tPrimOp,
     tAttrs,
@@ -57,6 +58,7 @@ typedef enum {
  */
 typedef enum {
     nThunk,
+    nFailed,
     nInt,
     nFloat,
     nBool,
@@ -265,6 +267,11 @@ struct ValueBase
         size_t size;
         Value * const * elems;
     };
+
+    struct Failed : gc
+    {
+        std::exception_ptr ex;
+    };
 };
 
 template<typename T>
@@ -291,6 +298,7 @@ struct PayloadTypeToInternalType
     MACRO(PrimOp *, primOp, tPrimOp)                                \
     MACRO(ValueBase::PrimOpApplicationThunk, primOpApp, tPrimOpApp) \
     MACRO(ExternalValueBase *, external, tExternal)                 \
+    MACRO(ValueBase::Failed *, failed, tFailed)                     \
     MACRO(NixFloat, fpoint, tFloat)
 
 #define NIX_VALUE_PAYLOAD_TYPE(T, FIELD_NAME, DISCRIMINATOR) \
@@ -594,6 +602,11 @@ protected:
         path.path = std::bit_cast<const char *>(payload[1]);
     }
 
+    void getStorage(Failed *& failed) const noexcept
+    {
+        failed = std::bit_cast<Failed *>(payload[1]);
+    }
+
     void setStorage(NixInt integer) noexcept
     {
         setSingleDWordPayload<tInt>(integer.value);
@@ -643,6 +656,11 @@ protected:
     {
         setUntaggablePayload<pdPath>(path.accessor, path.path);
     }
+
+    void setStorage(Failed * failed) noexcept
+    {
+        setSingleDWordPayload<tFailed>(std::bit_cast<PackedPointer>(failed));
+    }
 };
 
 /**
@@ -865,30 +883,35 @@ public:
     inline bool isThunk() const
     {
         return isa<tThunk>();
-    };
+    }
 
     inline bool isApp() const
     {
         return isa<tApp>();
-    };
+    }
 
     inline bool isBlackhole() const;
 
     // type() == nFunction
     inline bool isLambda() const
     {
         return isa<tLambda>();
-    };
+    }
 
     inline bool isPrimOp() const
     {
         return isa<tPrimOp>();
-    };
+    }
 
     inline bool isPrimOpApp() const
     {
         return isa<tPrimOpApp>();
-    };
+    }
+
+    inline bool isFailed() const
+    {
+        return isa<tFailed>();
+    }
 
     /**
      * Returns the normal type of a Value. This only returns nThunk if
@@ -925,6 +948,8 @@ public:
             return nExternal;
         case tFloat:
             return nFloat;
+        case tFailed:
+            return nFailed;
         case tThunk:
         case tApp:
             return nThunk;
@@ -1039,6 +1064,11 @@ public:
         setStorage(n);
     }
 
+    inline void mkFailed() noexcept
+    {
+        setStorage(new Value::Failed{.ex = std::current_exception()});
+    }
+
     bool isList() const noexcept
     {
         return isa<tListSmall, tListN>();
@@ -1142,6 +1172,11 @@ public:
     {
         return getStorage<Path>().accessor;
     }
+
+    Failed * failed() const noexcept
+    {
+        return getStorage<Failed *>();
+    }
 };
 
 extern ExprBlackHole eBlackHole;

src/libexpr/primops.cc

@@ -515,6 +515,7 @@ static void prim_typeOf(EvalState & state, const PosIdx pos, Value ** args, Valu
         v.mkStringNoCopy("float");
         break;
     case nThunk:
+    case nFailed:
         unreachable();
     }
 }

src/libexpr/print-ambiguous.cc

@@ -75,6 +75,9 @@ void printAmbiguous(
             str << "«potential infinite recursion»";
         }
         break;
+    case nFailed:
+        str << "«failed»";
+        break;
     case nFunction:
         if (v.isLambda()) {
             str << "<LAMBDA>";

src/libexpr/print.cc

@@ -508,6 +508,11 @@ class Printer
         }
     }
 
+    void printFailed(Value & v)
+    {
+        output << "«failed»";
+    }
+
     void printExternal(Value & v)
     {
         v.external()->print(output);
@@ -583,6 +588,10 @@ class Printer
                 printThunk(v);
                 break;
 
+            case nFailed:
+                printFailed(v);
+                break;
+
             case nExternal:
                 printExternal(v);
                 break;

src/libexpr/value-to-json.cc

@@ -96,6 +96,7 @@ json printValueAsJSON(
         break;
 
     case nThunk:
+    case nFailed:
     case nFunction:
         state.error<TypeError>("cannot convert %1% to JSON", showType(v)).atPos(v.determinePos(pos)).debugThrow();
     }

src/libexpr/value-to-xml.cc

@@ -170,6 +170,11 @@ static void printValueAsXML(
 
     case nThunk:
         doc.writeEmptyElement("unevaluated");
+        break;
+
+    case nFailed:
+        doc.writeEmptyElement("failed");
+        break;
     }
 }