From 90b5c0a1a67c31a3f2553fef110417e4ba1b635d Mon Sep 17 00:00:00 2001
From: pennae <github@quasiparticle.net>
Date: Sat, 5 Mar 2022 19:26:36 +0100
Subject: [PATCH] turn primop names into strings

we don't *need* symbols here. the only advantage they have over strings is
making call-counting slightly faster, but that's a diagnostic feature and thus
needn't be optimized.

this also fixes a move bug that previously didn't show up: PrimOp structs were
accessed after being moved from, which technically invalidates them. previously
the names remained valid because Symbol copies on move, but strings are
invalidated. we now copy the entire primop struct instead of moving since primop
registration happen once and are not performance-sensitive.
---
 src/libexpr/eval.cc    | 14 +++++++-------
 src/libexpr/eval.hh    |  6 +++---
 src/libexpr/primops.cc |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index b87e06ef5..a5eb2e473 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -645,14 +645,14 @@ Value * EvalState::addPrimOp(const std::string & name,
        the primop to a dummy value. */
     if (arity == 0) {
         auto vPrimOp = allocValue();
-        vPrimOp->mkPrimOp(new PrimOp { .fun = primOp, .arity = 1, .name = sym });
+        vPrimOp->mkPrimOp(new PrimOp { .fun = primOp, .arity = 1, .name = name2 });
         Value v;
         v.mkApp(vPrimOp, vPrimOp);
         return addConstant(name, v);
     }
 
     Value * v = allocValue();
-    v->mkPrimOp(new PrimOp { .fun = primOp, .arity = arity, .name = sym });
+    v->mkPrimOp(new PrimOp { .fun = primOp, .arity = arity, .name = name2 });
     staticBaseEnv.vars.emplace_back(symbols.create(name), baseEnvDispl);
     baseEnv.values[baseEnvDispl++] = v;
     baseEnv.values[0]->attrs->push_back(Attr(sym, v));
@@ -667,21 +667,21 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
     if (primOp.arity == 0) {
         primOp.arity = 1;
         auto vPrimOp = allocValue();
-        vPrimOp->mkPrimOp(new PrimOp(std::move(primOp)));
+        vPrimOp->mkPrimOp(new PrimOp(primOp));
         Value v;
         v.mkApp(vPrimOp, vPrimOp);
         return addConstant(primOp.name, v);
     }
 
-    Symbol envName = primOp.name;
+    Symbol envName = symbols.create(primOp.name);
     if (hasPrefix(primOp.name, "__"))
-        primOp.name = symbols.create(std::string(primOp.name, 2));
+        primOp.name = primOp.name.substr(2);
 
     Value * v = allocValue();
-    v->mkPrimOp(new PrimOp(std::move(primOp)));
+    v->mkPrimOp(new PrimOp(primOp));
     staticBaseEnv.vars.emplace_back(envName, baseEnvDispl);
     baseEnv.values[baseEnvDispl++] = v;
-    baseEnv.values[0]->attrs->push_back(Attr(primOp.name, v));
+    baseEnv.values[0]->attrs->push_back(Attr(symbols.create(primOp.name), v));
     return v;
 }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 7ed376e8d..7c39e3704 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -30,7 +30,7 @@ struct PrimOp
 {
     PrimOpFun fun;
     size_t arity;
-    Symbol name;
+    std::string name;
     std::vector<std::string> args;
     const char * doc = nullptr;
 };
@@ -305,7 +305,7 @@ public:
     struct Doc
     {
         Pos pos;
-        std::optional<Symbol> name;
+        std::optional<std::string> name;
         size_t arity;
         std::vector<std::string> args;
         const char * doc;
@@ -391,7 +391,7 @@ private:
 
     bool countCalls;
 
-    typedef std::map<Symbol, size_t> PrimOpCalls;
+    typedef std::map<std::string, size_t> PrimOpCalls;
     PrimOpCalls primOpCalls;
 
     typedef std::map<ExprLambda *, size_t> FunctionCalls;
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 73817dbdd..3ebc2f1df 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -3906,7 +3906,7 @@ void EvalState::createBaseEnv()
                 addPrimOp({
                     .fun = primOp.fun,
                     .arity = std::max(primOp.args.size(), primOp.arity),
-                    .name = symbols.create(primOp.name),
+                    .name = primOp.name,
                     .args = primOp.args,
                     .doc = primOp.doc,
                 });