1 files changed, 35 insertions, 24 deletions

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 193358161..5bf161cc0 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -63,9 +63,15 @@ static char * dupString(const char * s)
 }
 
 
-static char * dupStringWithLen(const char * s, size_t size)
+// When there's no need to write to the string, we can optimize away empty
+// string allocations.
+// This function handles makeImmutableStringWithLen(null, 0) by returning the
+// empty string.
+static const char * makeImmutableStringWithLen(const char * s, size_t size)
 {
     char * t;
+    if (size == 0)
+        return "";
 #if HAVE_BOEHMGC
     t = GC_STRNDUP(s, size);
 #else
@@ -75,6 +81,10 @@ static char * dupStringWithLen(const char * s, size_t size)
     return t;
 }
 
+static inline const char * makeImmutableString(std::string_view s) {
+    return makeImmutableStringWithLen(s.data(), s.size());
+}
+
 
 RootValue allocRootValue(Value * v)
 {
@@ -86,15 +96,10 @@ RootValue allocRootValue(Value * v)
 }
 
 
-void printValue(std::ostream & str, std::set<const Value *> & active, const Value & v)
+void printValue(std::ostream & str, std::set<const void *> & seen, const Value & v)
 {
     checkInterrupt();
 
-    if (!active.insert(&v).second) {
-        str << "<CYCLE>";
-        return;
-    }
-
     switch (v.internalType) {
     case tInt:
         str << v.integer;
@@ -120,24 +125,32 @@ void printValue(std::ostream & str, std::set<const Value *> & active, const Valu
         str << "null";
         break;
     case tAttrs: {
-        str << "{ ";
-        for (auto & i : v.attrs->lexicographicOrder()) {
-            str << i->name << " = ";
-            printValue(str, active, *i->value);
-            str << "; ";
+        if (!v.attrs->empty() && !seen.insert(v.attrs).second)
+            str << "<REPEAT>";
+        else {
+            str << "{ ";
+            for (auto & i : v.attrs->lexicographicOrder()) {
+                str << i->name << " = ";
+                printValue(str, seen, *i->value);
+                str << "; ";
+            }
+            str << "}";
         }
-        str << "}";
         break;
     }
     case tList1:
     case tList2:
     case tListN:
-        str << "[ ";
-        for (auto v2 : v.listItems()) {
-            printValue(str, active, *v2);
-            str << " ";
+        if (v.listSize() && !seen.insert(v.listElems()).second)
+            str << "<REPEAT>";
+        else {
+            str << "[ ";
+            for (auto v2 : v.listItems()) {
+                printValue(str, seen, *v2);
+                str << " ";
+            }
+            str << "]";
         }
-        str << "]";
         break;
     case tThunk:
     case tApp:
@@ -161,15 +174,13 @@ void printValue(std::ostream & str, std::set<const Value *> & active, const Valu
     default:
         abort();
     }
-
-    active.erase(&v);
 }
 
 
 std::ostream & operator << (std::ostream & str, const Value & v)
 {
-    std::set<const Value *> active;
-    printValue(str, active, v);
+    std::set<const void *> seen;
+    printValue(str, seen, v);
     return str;
 }
 
@@ -804,7 +815,7 @@ LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, con
 
 void Value::mkString(std::string_view s)
 {
-    mkString(dupStringWithLen(s.data(), s.size()));
+    mkString(makeImmutableString(s));
 }
 
 
@@ -835,7 +846,7 @@ void Value::mkStringMove(const char * s, const PathSet & context)
 
 void Value::mkPath(std::string_view s)
 {
-    mkPath(dupStringWithLen(s.data(), s.size()));
+    mkPath(makeImmutableString(s));
 }