assert: Report why values aren't equal

src/libexpr/eval-error.cc

@@ -92,6 +92,14 @@ void EvalErrorBuilder<T>::debugThrow()
  throw error;
 }
 
+template<class T>
+void EvalErrorBuilder<T>::panic()
+{
+ logError(error.info());
+ printError("This is a bug! An unexpected condition occurred, causing the Nix evaluator to have to stop. If you could share a reproducible example or a core dump, please open an issue at https://github.com/NixOS/nix/issues");
+ abort();
+}
+
 template class EvalErrorBuilder<EvalBaseError>;
 template class EvalErrorBuilder<EvalError>;
 template class EvalErrorBuilder<AssertionError>;

src/libexpr/eval-error.hh

@@ -112,6 +112,12 @@ public:
  * Delete the `EvalErrorBuilder` and throw the underlying exception.
  */
  [[gnu::noinline, gnu::noreturn]] void debugThrow();
+
+ /**
+ * A programming error or fatal condition occurred. Abort the process for core dump and debugging.
+ * This does not print a proper backtrace, because unwinding the stack is destructive.
+ */
+ [[gnu::noinline, gnu::noreturn]] void panic();
 };
 
 }

src/libexpr/eval.cc

@@ -1759,9 +1759,24 @@ void ExprIf::eval(EvalState & state, Env & env, Value & v)
 void ExprAssert::eval(EvalState & state, Env & env, Value & v)
 {
  if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) {
- std::ostringstream out;
- cond->show(state.symbols, out);
- state.error<AssertionError>("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow();
+ auto exprStr = ({
+ std::ostringstream out;
+ cond->show(state.symbols, out);
+ out.str();
+ });
+
+ if (auto eq = dynamic_cast<ExprOpEq *>(cond)) {
+ try {
+ Value v1; eq->e1->eval(state, env, v1);
+ Value v2; eq->e2->eval(state, env, v2);
+ state.assertEqValues(v1, v2, eq->pos, "in an equality assertion");
+ } catch (AssertionError & e) {
+ e.addTrace(state.positions[pos], "while evaluating the condition of the assertion '%s'", exprStr);
+ throw;
+ }
+ }
+
+ state.error<AssertionError>("assertion '%1%' failed", exprStr).atPos(pos).withFrame(env, *this).debugThrow();
  }
  body->eval(state, env, v);
 }
@@ -2418,6 +2433,214 @@ SingleDerivedPath EvalState::coerceToSingleDerivedPath(const PosIdx pos, Value &
 }
 
 
+
+// NOTE: This implementation must match eqValues!
+// We accept this burden because informative error messages for
+// `assert a == b; x` are critical for our users' testing UX.
+void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx)
+{
+ // This implementation must match eqValues.
+ forceValue(v1, pos);
+ forceValue(v2, pos);
+
+ if (&v1 == &v2)
+ return;
+
+ // Special case type-compatibility between float and int
+ if ((v1.type() == nInt || v1.type() == nFloat) && (v2.type() == nInt || v2.type() == nFloat)) {
+ if (eqValues(v1, v2, pos, errorCtx)) {
+ return;
+ } else {
+ error<AssertionError>(
+ "%s with value '%s' is not equal to %s with value '%s'",
+ showType(v1),
+ ValuePrinter(*this, v1, errorPrintOptions),
+ showType(v2),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ }
+
+ if (v1.type() != v2.type()) {
+ error<AssertionError>(
+ "%s of value '%s' is not equal to %s of value '%s'",
+ showType(v1),
+ ValuePrinter(*this, v1, errorPrintOptions),
+ showType(v2),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+
+ switch (v1.type()) {
+ case nInt:
+ if (v1.integer() != v2.integer()) {
+ error<AssertionError>("integer '%d' is not equal to integer '%d'", v1.integer(), v2.integer()).debugThrow();
+ }
+ return;
+
+ case nBool:
+ if (v1.boolean() != v2.boolean()) {
+ error<AssertionError>(
+ "boolean '%s' is not equal to boolean '%s'",
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ return;
+
+ case nString:
+ if (strcmp(v1.c_str(), v2.c_str()) != 0) {
+ error<AssertionError>(
+ "string '%s' is not equal to string '%s'",
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ return;
+
+ case nPath:
+ if (v1.payload.path.accessor != v2.payload.path.accessor) {
+ error<AssertionError>(
+ "path '%s' is not equal to path '%s' because their accessors are different",
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ if (strcmp(v1.payload.path.path, v2.payload.path.path) != 0) {
+ error<AssertionError>(
+ "path '%s' is not equal to path '%s'",
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ return;
+
+ case nNull:
+ return;
+
+ case nList:
+ if (v1.listSize() != v2.listSize()) {
+ error<AssertionError>(
+ "list of size '%d' is not equal to list of size '%d', left hand side is '%s', right hand side is '%s'",
+ v1.listSize(),
+ v2.listSize(),
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ for (size_t n = 0; n < v1.listSize(); ++n) {
+ try {
+ assertEqValues(*v1.listElems()[n], *v2.listElems()[n], pos, errorCtx);
+ } catch (Error & e) {
+ e.addTrace(positions[pos], "while comparing list element %d", n);
+ throw;
+ }
+ }
+ return;
+
+ case nAttrs: {
+ if (isDerivation(v1) && isDerivation(v2)) {
+ auto i = v1.attrs()->get(sOutPath);
+ auto j = v2.attrs()->get(sOutPath);
+ if (i && j) {
+ try {
+ assertEqValues(*i->value, *j->value, pos, errorCtx);
+ return;
+ } catch (Error & e) {
+ e.addTrace(positions[pos], "while comparing a derivation by its '%s' attribute", "outPath");
+ throw;
+ }
+ assert(false);
+ }
+ }
+
+ if (v1.attrs()->size() != v2.attrs()->size()) {
+ error<AssertionError>(
+ "attribute names of attribute set '%s' differs from attribute set '%s'",
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+
+ // Like normal comparison, we compare the attributes in non-deterministic Symbol index order.
+ // This function is called when eqValues has found a difference, so to reliably
+ // report about its result, we should follow in its literal footsteps and not
+ // try anything fancy that could lead to an error.
+ Bindings::const_iterator i, j;
+ for (i = v1.attrs()->begin(), j = v2.attrs()->begin(); i != v1.attrs()->end(); ++i, ++j) {
+ if (i->name != j->name) {
+ // A difference in a sorted list means that one attribute is not contained in the other, but we don't
+ // know which. Let's find out. Could use <, but this is more clear.
+ if (!v2.attrs()->get(i->name)) {
+ error<AssertionError>(
+ "attribute name '%s' is contained in '%s', but not in '%s'",
+ symbols[i->name],
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ if (!v1.attrs()->get(j->name)) {
+ error<AssertionError>(
+ "attribute name '%s' is missing in '%s', but is contained in '%s'",
+ symbols[j->name],
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ assert(false);
+ }
+ try {
+ assertEqValues(*i->value, *j->value, pos, errorCtx);
+ } catch (Error & e) {
+ // The order of traces is reversed, so this presents as
+ // where left hand side is
+ // at <pos>
+ // where right hand side is
+ // at <pos>
+ // while comparing attribute '<name>'
+ if (j->pos != noPos)
+ e.addTrace(positions[j->pos], "where right hand side is");
+ if (i->pos != noPos)
+ e.addTrace(positions[i->pos], "where left hand side is");
+ e.addTrace(positions[pos], "while comparing attribute '%s'", symbols[i->name]);
+ throw;
+ }
+ }
+ return;
+ }
+
+ case nFunction:
+ error<AssertionError>("distinct functions and immediate comparisons of identical functions compare as unequal")
+ .debugThrow();
+
+ case nExternal:
+ if (!(*v1.external() == *v2.external())) {
+ error<AssertionError>(
+ "external value '%s' is not equal to external value '%s'",
+ ValuePrinter(*this, v1, errorPrintOptions),
+ ValuePrinter(*this, v2, errorPrintOptions))
+ .debugThrow();
+ }
+ return;
+
+ case nFloat:
+ // !!!
+ if (!(v1.fpoint() == v2.fpoint())) {
+ error<AssertionError>("float '%f' is not equal to float '%f'", v1.fpoint(), v2.fpoint()).debugThrow();
+ }
+ return;
+
+ case nThunk: // Must not be left by forceValue
+ assert(false);
+ 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),
+ // so maybe `assertEqValues` and `eqValues` are out of sync. Check it for solutions.
+ error<EvalError>("assertEqValues: cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).panic();
+ }
+}
+
+// This implementation must match assertEqValues
 bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx)
 {
  forceValue(v1, pos);
@@ -2491,11 +2714,13 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
  return *v1.external() == *v2.external();
 
  case nFloat:
+ // !!!
  return v1.fpoint() == v2.fpoint();
 
  case nThunk: // Must not be left by forceValue
- default:
- error<EvalError>("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow();
+ assert(false);
+ 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).panic();
  }
 }
 

src/libexpr/eval.hh

@@ -644,6 +644,15 @@ public:
  */
  bool eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx);
 
+ /**
+ * Like `eqValues`, but throws an `AssertionError` if not equal.
+ *
+ * WARNING:
+ * Callers should call `eqValues` first and report if `assertEqValues` behaves
+ * incorrectly. (e.g. if it doesn't throw if eqValues returns false or vice versa)
+ */
+ void assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx);
+
  bool isFunctor(Value & fun);
 
  // FIXME: use std::span

tests/functional/lang/eval-fail-assert-equal-attrs-names-2.err.exp

@@ -0,0 +1,8 @@
+error:
+ … while evaluating the condition of the assertion '({ a = true; } == { a = true; b = true; })'
+ at /pwd/lang/eval-fail-assert-equal-attrs-names-2.nix:1:1:
+ 1| assert { a = true; } == { a = true; b = true; };
+ | ^
+ 2| throw "unreachable"
+
+ error: attribute names of attribute set '{ a = true; }' differs from attribute set '{ a = true; b = true; }'

tests/functional/lang/eval-fail-assert-equal-attrs-names-2.nix

@@ -0,0 +1,2 @@
+assert { a = true; } == { a = true; b = true; };
+throw "unreachable"

tests/functional/lang/eval-fail-assert-equal-attrs-names.err.exp

@@ -0,0 +1,8 @@
+error:
+ … while evaluating the condition of the assertion '({ a = true; b = true; } == { a = true; })'
+ at /pwd/lang/eval-fail-assert-equal-attrs-names.nix:1:1:
+ 1| assert { a = true; b = true; } == { a = true; };
+ | ^
+ 2| throw "unreachable"
+
+ error: attribute names of attribute set '{ a = true; b = true; }' differs from attribute set '{ a = true; }'

tests/functional/lang/eval-fail-assert-equal-attrs-names.nix

@@ -0,0 +1,2 @@
+assert { a = true; b = true; } == { a = true; };
+throw "unreachable"

tests/functional/lang/eval-fail-assert-equal-derivations-extra.err.exp

@@ -0,0 +1,26 @@
+error:
+ … while evaluating the condition of the assertion '({ foo = { outPath = "/nix/store/0"; type = "derivation"; }; } == { foo = { devious = true; outPath = "/nix/store/1"; type = "derivation"; }; })'
+ at /pwd/lang/eval-fail-assert-equal-derivations-extra.nix:1:1:
+ 1| assert
+ | ^
+ 2| { foo = { type = "derivation"; outPath = "/nix/store/0"; }; }
+
+ … while comparing attribute 'foo'
+
+ … where left hand side is
+ at /pwd/lang/eval-fail-assert-equal-derivations-extra.nix:2:5:
+ 1| assert
+ 2| { foo = { type = "derivation"; outPath = "/nix/store/0"; }; }
+ | ^
+ 3| ==
+
+ … where right hand side is
+ at /pwd/lang/eval-fail-assert-equal-derivations-extra.nix:4:5:
+ 3| ==
+ 4| { foo = { type = "derivation"; outPath = "/nix/store/1"; devious = true; }; };
+ | ^
+ 5| throw "unreachable"
+
+ … while comparing a derivation by its 'outPath' attribute
+
+ error: string '"/nix/store/0"' is not equal to string '"/nix/store/1"'

tests/functional/lang/eval-fail-assert-equal-derivations-extra.nix

@@ -0,0 +1,5 @@
+assert
+ { foo = { type = "derivation"; outPath = "/nix/store/0"; }; }
+ ==
+ { foo = { type = "derivation"; outPath = "/nix/store/1"; devious = true; }; };
+throw "unreachable"

tests/functional/lang/eval-fail-assert-equal-derivations.err.exp

@@ -0,0 +1,26 @@
+error:
+ … while evaluating the condition of the assertion '({ foo = { ignored = (abort "not ignored"); outPath = "/nix/store/0"; type = "derivation"; }; } == { foo = { ignored = (abort "not ignored"); outPath = "/nix/store/1"; type = "derivation"; }; })'
+ at /pwd/lang/eval-fail-assert-equal-derivations.nix:1:1:
+ 1| assert
+ | ^
+ 2| { foo = { type = "derivation"; outPath = "/nix/store/0"; ignored = abort "not ignored"; }; }
+
+ … while comparing attribute 'foo'
+
+ … where left hand side is
+ at /pwd/lang/eval-fail-assert-equal-derivations.nix:2:5:
+ 1| assert
+ 2| { foo = { type = "derivation"; outPath = "/nix/store/0"; ignored = abort "not ignored"; }; }
+ | ^
+ 3| ==
+
+ … where right hand side is
+ at /pwd/lang/eval-fail-assert-equal-derivations.nix:4:5:
+ 3| ==
+ 4| { foo = { type = "derivation"; outPath = "/nix/store/1"; ignored = abort "not ignored"; }; };
+ | ^
+ 5| throw "unreachable"
+
+ … while comparing a derivation by its 'outPath' attribute
+
+ error: string '"/nix/store/0"' is not equal to string '"/nix/store/1"'

tests/functional/lang/eval-fail-assert-equal-derivations.nix

@@ -0,0 +1,5 @@
+assert
+ { foo = { type = "derivation"; outPath = "/nix/store/0"; ignored = abort "not ignored"; }; }
+ ==
+ { foo = { type = "derivation"; outPath = "/nix/store/1"; ignored = abort "not ignored"; }; };
+throw "unreachable"