From 2d6f1ddbb5f2ae2f3153785d7a4e5321c85495fd Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 28 Nov 2019 13:52:42 +0100
Subject: [PATCH] Remove builtins.valueSize

Fixes #3246.
---
 src/libexpr/eval.cc    | 91 ------------------------------------------
 src/libexpr/eval.hh    |  1 -
 src/libexpr/primops.cc |  8 ----
 src/libexpr/value.hh   |  9 -----
 4 files changed, 109 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 3a9b7a3c0..b89a67b19 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -614,13 +614,9 @@ Value * EvalState::allocValue()
 
 Env & EvalState::allocEnv(size_t size)
 {
-    if (size > std::numeric_limits<decltype(Env::size)>::max())
-        throw Error("environment size %d is too big", size);
-
     nrEnvs++;
     nrValuesInEnvs += size;
     Env * env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *));
-    env->size = (decltype(Env::size)) size;
     env->type = Env::Plain;
 
     /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */
@@ -1868,93 +1864,6 @@ void EvalState::printStats()
 }
 
 
-size_t valueSize(Value & v)
-{
-    std::set<const void *> seen;
-
-    auto doString = [&](const char * s) -> size_t {
-        if (!seen.insert(s).second) return 0;
-        return strlen(s) + 1;
-    };
-
-    std::function<size_t(Value & v)> doValue;
-    std::function<size_t(Env & v)> doEnv;
-
-    doValue = [&](Value & v) -> size_t {
-        if (!seen.insert(&v).second) return 0;
-
-        size_t sz = sizeof(Value);
-
-        switch (v.type) {
-        case tString:
-            sz += doString(v.string.s);
-            if (v.string.context)
-                for (const char * * p = v.string.context; *p; ++p)
-                    sz += doString(*p);
-            break;
-        case tPath:
-            sz += doString(v.path);
-            break;
-        case tAttrs:
-            if (seen.insert(v.attrs).second) {
-                sz += sizeof(Bindings) + sizeof(Attr) * v.attrs->capacity();
-                for (auto & i : *v.attrs)
-                    sz += doValue(*i.value);
-            }
-            break;
-        case tList1:
-        case tList2:
-        case tListN:
-            if (seen.insert(v.listElems()).second) {
-                sz += v.listSize() * sizeof(Value *);
-                for (size_t n = 0; n < v.listSize(); ++n)
-                    sz += doValue(*v.listElems()[n]);
-            }
-            break;
-        case tThunk:
-            sz += doEnv(*v.thunk.env);
-            break;
-        case tApp:
-            sz += doValue(*v.app.left);
-            sz += doValue(*v.app.right);
-            break;
-        case tLambda:
-            sz += doEnv(*v.lambda.env);
-            break;
-        case tPrimOpApp:
-            sz += doValue(*v.primOpApp.left);
-            sz += doValue(*v.primOpApp.right);
-            break;
-        case tExternal:
-            if (!seen.insert(v.external).second) break;
-            sz += v.external->valueSize(seen);
-            break;
-        default:
-            ;
-        }
-
-        return sz;
-    };
-
-    doEnv = [&](Env & env) -> size_t {
-        if (!seen.insert(&env).second) return 0;
-
-        size_t sz = sizeof(Env) + sizeof(Value *) * env.size;
-
-        if (env.type != Env::HasWithExpr)
-            for (size_t i = 0; i < env.size; ++i)
-                if (env.values[i])
-                    sz += doValue(*env.values[i]);
-
-        if (env.up) sz += doEnv(*env.up);
-
-        return sz;
-    };
-
-    return doValue(v);
-}
-
-
 string ExternalValueBase::coerceToString(const Pos & pos, PathSet & context, bool copyMore, bool copyToStore) const
 {
     throw TypeError(format("cannot coerce %1% to a string, at %2%") %
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 9d075a48a..419b703fc 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -37,7 +37,6 @@ struct PrimOp
 struct Env
 {
     Env * up;
-    unsigned short size; // used by ‘valueSize’
     unsigned short prevWith:14; // nr of levels up to next `with' environment
     enum { Plain = 0, HasWithExpr, HasWithAttrs } type:2;
     Value * values[0];
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 2cc03fe61..d693a3b20 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -506,13 +506,6 @@ static void prim_trace(EvalState & state, const Pos & pos, Value * * args, Value
 }
 
 
-void prim_valueSize(EvalState & state, const Pos & pos, Value * * args, Value & v)
-{
-    /* We're not forcing the argument on purpose. */
-    mkInt(v, valueSize(*args[0]));
-}
-
-
 /*************************************************************
  * Derivations
  *************************************************************/
@@ -2206,7 +2199,6 @@ void EvalState::createBaseEnv()
 
     // Debugging
     addPrimOp("__trace", 2, prim_trace);
-    addPrimOp("__valueSize", 1, prim_valueSize);
 
     // Paths
     addPrimOp("__toPath", 1, prim_toPath);
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 6743d7fd8..689373873 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -62,9 +62,6 @@ class ExternalValueBase
     /* Return a string to be used in builtins.typeOf */
     virtual string typeOf() const = 0;
 
-    /* How much space does this value take up */
-    virtual size_t valueSize(std::set<const void *> & seen) const = 0;
-
     /* Coerce the value to a string. Defaults to uncoercable, i.e. throws an
      * error
      */
@@ -255,12 +252,6 @@ static inline void mkPathNoCopy(Value & v, const char * s)
 void mkPath(Value & v, const char * s);
 
 
-/* Compute the size in bytes of the given value, including all values
-   and environments reachable from it. Static expressions (Exprs) are
-   not included. */
-size_t valueSize(Value & v);
-
-
 #if HAVE_BOEHMGC
 typedef std::vector<Value *, gc_allocator<Value *> > ValueVector;
 typedef std::map<Symbol, Value *, std::less<Symbol>, gc_allocator<std::pair<const Symbol, Value *> > > ValueMap;