Restore ambiguous value printer for nix-instantiate

The Nix team has requested that this output format remain unchanged.
I've added a warning to the man page explaining that `nix-instantiate
--eval` output will not parse correctly in many situations.

doc/manual/src/command-ref/nix-instantiate.md

@@ -35,13 +35,50 @@ standard input.
 
   - `--parse`\
     Just parse the input files, and print their abstract syntax trees on
-    standard output in ATerm format.
+    standard output as a Nix expression.
 
   - `--eval`\
     Just parse and evaluate the input files, and print the resulting
     values on standard output. No instantiation of store derivations
     takes place.
 
+    > **Warning**
+    >
+    > This option produces ambiguous output which is not suitable for machine
+    > consumption. For example, these two Nix expressions print the same result
+    > despite having different types:
+    >
+    > ```console
+    > $ nix-instantiate --eval --expr '{ a = {}; }'
+    > { a = <CODE>; }
+    > $ nix-instantiate --eval --expr '{ a = <CODE>; }'
+    > { a = <CODE>; }
+    > ```
+    >
+    > For human-readable output, `nix eval` (experimental) is more informative:
+    >
+    > ```console
+    > $ nix-instantiate --eval --expr 'a: a'
+    > <LAMBDA>
+    > $ nix eval --expr 'a: a'
+    > «lambda @ «string»:1:1»
+    > ```
+    >
+    > For machine-readable output, the `--xml` option produces unambiguous
+    > output:
+    >
+    > ```console
+    > $ nix-instantiate --eval --xml --expr '{ foo = <CODE>; }'
+    > <?xml version='1.0' encoding='utf-8'?>
+    > <expr>
+    >   <attrs>
+    >     <attr column="3" line="1" name="foo">
+    >       <unevaluated />
+    >     </attr>
+    >   </attrs>
+    > </expr>
+    > ```
+
   - `--find-file`\
     Look up the given files in Nix’s search path (as specified by the
     `NIX_PATH` environment variable). If found, print the corresponding
@@ -61,11 +98,11 @@ standard input.
 
   - `--json`\
     When used with `--eval`, print the resulting value as an JSON
-    representation of the abstract syntax tree rather than as an ATerm.
+    representation of the abstract syntax tree rather than as a Nix expression.
 
   - `--xml`\
     When used with `--eval`, print the resulting value as an XML
-    representation of the abstract syntax tree rather than as an ATerm.
+    representation of the abstract syntax tree rather than as a Nix expression.
     The schema is the same as that used by the [`toXML`
     built-in](../language/builtins.md).
 
@@ -133,28 +170,29 @@ $ nix-instantiate --eval --xml --expr '1 + 2'
 The difference between non-strict and strict evaluation:
 
 ```console
-$ nix-instantiate --eval --xml --expr 'rec { x = "foo"; y = x; }'
+$ nix-instantiate --eval --xml --expr '{ x = {}; }'
-...
+<?xml version='1.0' encoding='utf-8'?>
-  <attr name="x">
+<expr>
-    <string value="foo" />
+  <attrs>
-  </attr>
+    <attr column="3" line="1" name="x">
-  <attr name="y">
       <unevaluated />
     </attr>
-...
+  </attrs>
+</expr>
 ```
 
 Note that `y` is left unevaluated (the XML representation doesn’t
 attempt to show non-normal forms).
 
 ```console
-$ nix-instantiate --eval --xml --strict --expr 'rec { x = "foo"; y = x; }'
+$ nix-instantiate --eval --xml --strict --expr '{ x = {}; }'
-...
+<?xml version='1.0' encoding='utf-8'?>
-  <attr name="x">
+<expr>
-    <string value="foo" />
+  <attrs>
+    <attr column="3" line="1" name="x">
+      <attrs>
+      </attrs>
     </attr>
-  <attr name="y">
+  </attrs>
-    <string value="foo" />
+</expr>
-  </attr>
-...
 ```

src/libexpr/print-ambiguous.cc

@@ -0,0 +1,100 @@
+#include "print-ambiguous.hh"
+#include "print.hh"
+#include "signals.hh"
+
+namespace nix {
+
+// See: https://github.com/NixOS/nix/issues/9730
+void printAmbiguous(
+    Value &v,
+    const SymbolTable &symbols,
+    std::ostream &str,
+    std::set<const void *> *seen,
+    int depth)
+{
+    checkInterrupt();
+
+    if (depth <= 0) {
+        str << "«too deep»";
+        return;
+    }
+    switch (v.type()) {
+    case nInt:
+        str << v.integer;
+        break;
+    case nBool:
+        printLiteralBool(str, v.boolean);
+        break;
+    case nString:
+        printLiteralString(str, v.string_view());
+        break;
+    case nPath:
+        str << v.path().to_string(); // !!! escaping?
+        break;
+    case nNull:
+        str << "null";
+        break;
+    case nAttrs: {
+        if (seen && !v.attrs->empty() && !seen->insert(v.attrs).second)
+            str << "«repeated»";
+        else {
+            str << "{ ";
+            for (auto & i : v.attrs->lexicographicOrder(symbols)) {
+                str << symbols[i->name] << " = ";
+                printAmbiguous(*i->value, symbols, str, seen, depth - 1);
+                str << "; ";
+            }
+            str << "}";
+        }
+        break;
+    }
+    case nList:
+        if (seen && v.listSize() && !seen->insert(v.listElems()).second)
+            str << "«repeated»";
+        else {
+            str << "[ ";
+            for (auto v2 : v.listItems()) {
+                if (v2)
+                    printAmbiguous(*v2, symbols, str, seen, depth - 1);
+                else
+                    str << "(nullptr)";
+                str << " ";
+            }
+            str << "]";
+        }
+        break;
+    case nThunk:
+        if (!v.isBlackhole()) {
+            str << "<CODE>";
+        } else {
+            // Although we know for sure that it's going to be an infinite recursion
+            // when this value is accessed _in the current context_, it's likely
+            // that the user will misinterpret a simpler «infinite recursion» output
+            // as a definitive statement about the value, while in fact it may be
+            // a valid value after `builtins.trace` and perhaps some other steps
+            // have completed.
+            str << "«potential infinite recursion»";
+        }
+        break;
+    case nFunction:
+        if (v.isLambda()) {
+            str << "<LAMBDA>";
+        } else if (v.isPrimOp()) {
+            str << "<PRIMOP>";
+        } else if (v.isPrimOpApp()) {
+            str << "<PRIMOP-APP>";
+        }
+        break;
+    case nExternal:
+        str << *v.external;
+        break;
+    case nFloat:
+        str << v.fpoint;
+        break;
+    default:
+        printError("Nix evaluator internal error: printAmbiguous: invalid value type");
+        abort();
+    }
+}
+
+}

src/libexpr/print-ambiguous.hh

@@ -0,0 +1,24 @@
+#pragma once
+
+#include "value.hh"
+
+namespace nix {
+
+/**
+ * Print a value in the deprecated format used by `nix-instantiate --eval` and
+ * `nix-env` (for manifests).
+ *
+ * This output can't be changed because it's part of the `nix-instantiate` API,
+ * but it produces ambiguous output; unevaluated thunks and lambdas (and a few
+ * other types) are printed as Nix path syntax like `<CODE>`.
+ *
+ * See: https://github.com/NixOS/nix/issues/9730
+ */
+void printAmbiguous(
+    Value &v,
+    const SymbolTable &symbols,
+    std::ostream &str,
+    std::set<const void *> *seen,
+    int depth);
+
+}

src/nix-env/user-env.cc

@@ -108,8 +108,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
        environment. */
     auto manifestFile = ({
         std::ostringstream str;
-        std::set<const void *> seen;
+        printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits<int>::max());
-        printAmbiguous(manifest, state.symbols, str, &seen, std::numeric_limits<int>::max());
         // TODO with C++20 we can use str.view() instead and avoid copy.
         std::string str2 = str.str();
         StringSource source { str2 };

src/nix-instantiate/nix-instantiate.cc

@@ -1,9 +1,11 @@
 #include "globals.hh"
+#include "print-ambiguous.hh"
 #include "shared.hh"
 #include "eval.hh"
 #include "eval-inline.hh"
 #include "get-drvs.hh"
 #include "attr-path.hh"
+#include "signals.hh"
 #include "value-to-xml.hh"
 #include "value-to-json.hh"
 #include "store-api.hh"
@@ -24,7 +26,6 @@ static int rootNr = 0;
 
 enum OutputKind { okPlain, okXML, okJSON };
 
-
 void processExpr(EvalState & state, const Strings & attrPaths,
     bool parseOnly, bool strict, Bindings & autoArgs,
     bool evalOnly, OutputKind output, bool location, Expr * e)
@@ -56,7 +57,8 @@ void processExpr(EvalState & state, const Strings & attrPaths,
                 std::cout << std::endl;
             } else {
                 if (strict) state.forceValueDeep(vRes);
-                vRes.print(state, std::cout);
+                std::set<const void *> seen;
+                printAmbiguous(vRes, state.symbols, std::cout, &seen, std::numeric_limits<int>::max());
                 std::cout << std::endl;
             }
         } else {

tests/functional/lang/eval-okay-print.exp

@@ -1 +1 @@
-[ null «primop toString» «partially applied primop deepSeq» «lambda @ /pwd/lang/eval-okay-print.nix:1:61» [ [ «repeated» ] ] ]
+[ null <PRIMOP> <PRIMOP-APP> <LAMBDA> [ [ «repeated» ] ] ]