From f5cddcfc093d9b9ac2692e25aad6cdc6f431c688 Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Wed, 27 Nov 2024 02:09:08 +0100
Subject: [PATCH] libcmd: make Repl instances proper scopes

there's no reason to ever *own* a repl instance. everything either wants
to run a repl and receive the result of that run, or run a repl and then
totally ignore the result. allowing non-transient repl instances doesn't
do anything useful for us, unless making ownership analysis hard counts.

this also fixes a bug in which a repl could ignore the store it was told
to use during construction, using the one returned by openStore instead.
this never showed up in lix because openStore() was always passed anyway

Change-Id: If2b1ad24ab74377340199b36af1dd629d7ce3f25
---
 lix/libcmd/repl.cc | 57 ++++++++++++++++++----------------------------
 lix/libcmd/repl.hh | 28 ++++++++++++-----------
 lix/nix/repl.cc    | 10 +-------
 3 files changed, 38 insertions(+), 57 deletions(-)

diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc
index a9c059c37..8a4547e34 100644
--- a/lix/libcmd/repl.cc
+++ b/lix/libcmd/repl.cc
@@ -1092,43 +1092,30 @@ Value * NixRepl::evalFile(SourcePath & path)
     return result;
 }
 
-
-std::unique_ptr<AbstractNixRepl> AbstractNixRepl::create(
-   const SearchPath & searchPath, nix::ref<Store> store, ref<EvalState> state,
-   std::function<AnnotatedValues()> getValues)
+ReplExitStatus AbstractNixRepl::run(
+    const SearchPath & searchPath,
+    nix::ref<Store> store,
+    ref<EvalState> state,
+    std::function<AnnotatedValues()> getValues,
+    const ValMap & extraEnv,
+    Bindings * autoArgs
+)
 {
-    return std::make_unique<NixRepl>(
-        searchPath,
-        openStore(),
-        state,
-        getValues
+    NixRepl repl(searchPath, store, state, getValues);
+
+    repl.autoArgs = autoArgs;
+    repl.initEnv();
+    for (auto & [name, value] : extraEnv) {
+        repl.addVarToScope(repl.state->symbols.create(name), *value);
+    }
+    return repl.mainLoop();
+}
+
+ReplExitStatus AbstractNixRepl::runSimple(ref<EvalState> evalState, const ValMap & extraEnv)
+{
+    return run(
+        {}, openStore(), evalState, [] {  return AnnotatedValues{}; }, extraEnv, nullptr
     );
 }
 
-
-ReplExitStatus AbstractNixRepl::runSimple(
-    ref<EvalState> evalState,
-    const ValMap & extraEnv)
-{
-    auto getValues = [&]()->NixRepl::AnnotatedValues{
-        NixRepl::AnnotatedValues values;
-        return values;
-    };
-    SearchPath searchPath = {};
-    auto repl = std::make_unique<NixRepl>(
-            searchPath,
-            openStore(),
-            evalState,
-            getValues
-        );
-
-    repl->initEnv();
-
-    // add 'extra' vars.
-    for (auto & [name, value] : extraEnv)
-        repl->addVarToScope(repl->state->symbols.create(name), *value);
-
-    return repl->mainLoop();
-}
-
 }
diff --git a/lix/libcmd/repl.hh b/lix/libcmd/repl.hh
index fba582930..12b3198a2 100644
--- a/lix/libcmd/repl.hh
+++ b/lix/libcmd/repl.hh
@@ -7,6 +7,21 @@ namespace nix {
 
 struct AbstractNixRepl
 {
+    typedef std::vector<std::pair<Value*,std::string>> AnnotatedValues;
+
+    static ReplExitStatus
+    run(const SearchPath & searchPath,
+        nix::ref<Store> store,
+        ref<EvalState> state,
+        std::function<AnnotatedValues()> getValues,
+        const ValMap & extraEnv,
+        Bindings * autoArgs);
+
+    static ReplExitStatus runSimple(
+        ref<EvalState> evalState,
+        const ValMap & extraEnv);
+
+protected:
     ref<EvalState> state;
     Bindings * autoArgs;
 
@@ -14,19 +29,6 @@ struct AbstractNixRepl
         : state(state)
     { }
 
-    virtual ~AbstractNixRepl()
-    { }
-
-    typedef std::vector<std::pair<Value*,std::string>> AnnotatedValues;
-
-    static std::unique_ptr<AbstractNixRepl> create(
-        const SearchPath & searchPath, nix::ref<Store> store, ref<EvalState> state,
-        std::function<AnnotatedValues()> getValues);
-
-    static ReplExitStatus runSimple(
-        ref<EvalState> evalState,
-        const ValMap & extraEnv);
-
     virtual void initEnv() = 0;
 
     virtual ReplExitStatus mainLoop() = 0;
diff --git a/lix/nix/repl.cc b/lix/nix/repl.cc
index b74d01c49..186c10b51 100644
--- a/lix/nix/repl.cc
+++ b/lix/nix/repl.cc
@@ -86,15 +86,7 @@ struct CmdRepl : RawInstallablesCommand
             }
             return values;
         };
-        auto repl = AbstractNixRepl::create(
-            searchPath,
-            openStore(),
-            state,
-            getValues
-        );
-        repl->autoArgs = getAutoArgs(*repl->state);
-        repl->initEnv();
-        repl->mainLoop();
+        AbstractNixRepl::run(searchPath, openStore(), state, getValues, {}, getAutoArgs(*state));
     }
 };