Style fixes

In particular, use std::make_shared and enumerate(). Also renamed some
fields to fit naming conventions.

src/libcmd/command.cc

@@ -121,25 +121,23 @@ ref<EvalState> EvalCommand::getEvalState()
         if (startReplOnEvalErrors)
             debuggerHook = [evalState{ref<EvalState>(evalState)}](const Error * error, const Env & env, const Expr & expr) {
                 auto dts =
-                    error && expr.getPos() ?
-                          std::unique_ptr<DebugTraceStacker>(
-                              new DebugTraceStacker(
-                                  *evalState,
-                                  DebugTrace
-                                        {.pos = (error->info().errPos ? *error->info().errPos : evalState->positions[expr.getPos()]),
-                                         .expr = expr,
-                                         .env = env,
-                                         .hint = error->info().msg,
-                                         .is_error = true
-                                        }))
-                          : nullptr;
+                    error && expr.getPos()
+                    ? std::make_unique<DebugTraceStacker>(
+                        *evalState,
+                        DebugTrace {
+                            .pos = error->info().errPos ? *error->info().errPos : evalState->positions[expr.getPos()],
+                            .expr = expr,
+                            .env = env,
+                            .hint = error->info().msg,
+                            .isError = true
+                        })
+                    : nullptr;
 
                 if (error)
                     printError("%s\n\n" ANSI_BOLD "Starting REPL to allow you to inspect the current state of the evaluator.\n" ANSI_NORMAL, error->what());
 
-                if (expr.staticenv)
-                {
-                    std::unique_ptr<valmap> vm(mapStaticEnvBindings(evalState->symbols, *expr.staticenv.get(), env));
+                if (expr.staticEnv) {
+                    auto vm = mapStaticEnvBindings(evalState->symbols, *expr.staticEnv.get(), env);
                     runRepl(evalState, expr, *vm);
                 }
             };

src/libcmd/command.hh

@@ -275,6 +275,7 @@ void printClosureDiff(
 
 void runRepl(
     ref<EvalState> evalState,
-    const Expr &expr,
+    const Expr & expr,
     const std::map<std::string, Value *> & extraEnv);
+
 }

src/libcmd/repl.cc

@@ -51,7 +51,7 @@ struct NixRepl
     ref<EvalState> state;
     Bindings * autoArgs;
 
-    int debugTraceIndex;
+    size_t debugTraceIndex;
 
     Strings loadedFiles;
 
@@ -79,7 +79,7 @@ struct NixRepl
     void addVarToScope(const Symbol name, Value & v);
     Expr * parseString(std::string s);
     void evalString(std::string s, Value & v);
-    void loadDebugTraceEnv(DebugTrace &dt);
+    void loadDebugTraceEnv(DebugTrace & dt);
 
     typedef std::set<Value *> ValuesSeen;
     std::ostream & printValue(std::ostream & str, Value & v, unsigned int maxDepth);
@@ -203,15 +203,16 @@ namespace {
     }
 }
 
-std::ostream& showDebugTrace(std::ostream &out, const PosTable &positions, const DebugTrace &dt)
+static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positions, const DebugTrace & dt)
 {
-    if (dt.is_error)
+    if (dt.isError)
         out << ANSI_RED "error: " << ANSI_NORMAL;
     out << dt.hint.str() << "\n";
 
     // prefer direct pos, but if noPos then try the expr.
-    auto pos = (*dt.pos ? *dt.pos :
-       positions[(dt.expr.getPos() ? dt.expr.getPos() : noPos)]);
+    auto pos = *dt.pos
+        ? *dt.pos
+        : positions[dt.expr.getPos() ? dt.expr.getPos() : noPos];
 
     if (pos) {
         printAtPos(pos, out);
@@ -258,8 +259,7 @@ void NixRepl::mainLoop(const std::vector<std::string> & files)
     while (true) {
         // When continuing input from previous lines, don't print a prompt, just align to the same
         // number of chars as the prompt.
-        if (!getLine(input, input.empty() ? "nix-repl> " : "          "))
-        {
+        if (!getLine(input, input.empty() ? "nix-repl> " : "          ")) {
             // ctrl-D should exit the debugger.
             state->debugStop = false;
             state->debugQuit = true;
@@ -279,8 +279,8 @@ void NixRepl::mainLoop(const std::vector<std::string> & files)
             // in debugger mode, an EvalError should trigger another repl session.
             // when that session returns the exception will land here.  No need to show it again;
             // show the error for this repl session instead.
-            if (debuggerHook && !this->state->debugTraces.empty())
-                showDebugTrace(std::cout, this->state->positions, this->state->debugTraces.front());
+            if (debuggerHook && !state->debugTraces.empty())
+                showDebugTrace(std::cout, state->positions, state->debugTraces.front());
             else
                 printMsg(lvlError, e.msg());
         } catch (Error & e) {
@@ -437,22 +437,16 @@ StorePath NixRepl::getDerivationPath(Value & v) {
     return *drvPath;
 }
 
-void NixRepl::loadDebugTraceEnv(DebugTrace &dt)
+void NixRepl::loadDebugTraceEnv(DebugTrace & dt)
 {
-    if (dt.expr.staticenv)
-    {
-        initEnv();
+    initEnv();
 
-        auto vm = std::make_unique<valmap>(*(mapStaticEnvBindings(this->state->symbols, *dt.expr.staticenv.get(), dt.env)));
+    if (dt.expr.staticEnv) {
+        auto vm = mapStaticEnvBindings(state->symbols, *dt.expr.staticEnv.get(), dt.env);
 
         // add staticenv vars.
-        for (auto & [name, value] : *(vm.get())) {
-            this->addVarToScope(this->state->symbols.create(name), *value);
-        }
-    }
-    else
-    {
-        initEnv();
+        for (auto & [name, value] : *(vm.get()))
+            addVarToScope(state->symbols.create(name), *value);
     }
 }
 
@@ -509,45 +503,31 @@ bool NixRepl::processLine(std::string line)
 
     else if (command == ":d" || command == ":debug") {
         if (arg == "stack") {
-            int idx = 0;
-            for (auto iter = this->state->debugTraces.begin();
-                 iter !=  this->state->debugTraces.end();
-                 ++iter, ++idx) {
-                 std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": ";
-                 showDebugTrace(std::cout, this->state->positions, *iter);
+            for (const auto & [idx, i] : enumerate(state->debugTraces)) {
+                std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": ";
+                showDebugTrace(std::cout, state->positions, i);
             }
         } else if (arg == "env") {
-            int idx = 0;
-            for (auto iter = this->state->debugTraces.begin();
-                 iter !=  this->state->debugTraces.end();
-                 ++iter, ++idx) {
-                 if (idx == this->debugTraceIndex)
-                 {
-                     printEnvBindings(state->symbols,iter->expr, iter->env);
-                     break;
-                 }
+            for (const auto & [idx, i] : enumerate(state->debugTraces)) {
+                if (idx == debugTraceIndex) {
+                    printEnvBindings(state->symbols, i.expr, i.env);
+                    break;
+                }
             }
         }
-        else if (arg.compare(0,4,"show") == 0) {
+        else if (arg.compare(0, 4, "show") == 0) {
             try {
                 // change the DebugTrace index.
                 debugTraceIndex = stoi(arg.substr(4));
-            }
-            catch (...)
-            {
-            }
+            } catch (...) { }
 
-            int idx = 0;
-            for (auto iter = this->state->debugTraces.begin();
-                 iter !=  this->state->debugTraces.end();
-                 ++iter, ++idx) {
-                 if (idx == this->debugTraceIndex)
-                 {
+            for (const auto & [idx, i] : enumerate(state->debugTraces)) {
+                 if (idx == debugTraceIndex) {
                      std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": ";
-                     showDebugTrace(std::cout, this->state->positions, *iter);
+                     showDebugTrace(std::cout, state->positions, i);
                      std::cout << std::endl;
-                     printEnvBindings(state->symbols,iter->expr, iter->env);
-                     loadDebugTraceEnv(*iter);
+                     printEnvBindings(state->symbols, i.expr, i.env);
+                     loadDebugTraceEnv(i);
                      break;
                  }
             }
@@ -1032,9 +1012,8 @@ void runRepl(
     repl->initEnv();
 
     // add 'extra' vars.
-    for (auto & [name, value] : extraEnv) {
+    for (auto & [name, value] : extraEnv)
         repl->addVarToScope(repl->state->symbols.create(name), *value);
-    }
 
     repl->mainLoop({});
 }

src/libexpr/eval.cc

@@ -466,7 +466,7 @@ EvalState::EvalState(
     , env1AllocCache(std::make_shared<void *>(nullptr))
 #endif
     , baseEnv(allocEnv(128))
-    , staticBaseEnv(new StaticEnv(false, 0))
+    , staticBaseEnv{std::make_shared<StaticEnv>(false, nullptr)}
 {
     countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0";
 
@@ -524,7 +524,7 @@ void EvalState::allowPath(const StorePath & storePath)
         allowedPaths->insert(store->toRealPath(storePath));
 }
 
-void EvalState::allowAndSetStorePathString(const StorePath &storePath, Value & v)
+void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value & v)
 {
     allowPath(storePath);
 
@@ -714,22 +714,19 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
 
 
 // just for the current level of StaticEnv, not the whole chain.
-void printStaticEnvBindings(const SymbolTable &st, const StaticEnv &se)
+void printStaticEnvBindings(const SymbolTable & st, const StaticEnv & se)
 {
     std::cout << ANSI_MAGENTA;
-    for (auto i = se.vars.begin(); i != se.vars.end(); ++i)
-    {
-      std::cout << st[i->first] << " ";
-    }
+    for (auto & i : se.vars)
+        std::cout << st[i.first] << " ";
     std::cout << ANSI_NORMAL;
     std::cout << std::endl;
 }
 
 // just for the current level of Env, not the whole chain.
-void printWithBindings(const SymbolTable &st, const Env &env)
+void printWithBindings(const SymbolTable & st, const Env & env)
 {
-    if (env.type == Env::HasWithAttrs)
-    {
+    if (env.type == Env::HasWithAttrs) {
         std::cout << "with: ";
         std::cout << ANSI_MAGENTA;
         Bindings::iterator j = env.values[0]->attrs->begin();
@@ -742,7 +739,7 @@ void printWithBindings(const SymbolTable &st, const Env &env)
     }
 }
 
-void printEnvBindings(const SymbolTable &st, const StaticEnv &se, const Env &env, int lvl)
+void printEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & env, int lvl)
 {
     std::cout << "Env level " << lvl << std::endl;
 
@@ -752,16 +749,13 @@ void printEnvBindings(const SymbolTable &st, const StaticEnv &se, const Env &env
         printWithBindings(st, env);
         std::cout << std::endl;
         printEnvBindings(st, *se.up, *env.up, ++lvl);
-    }
-    else
-    {
+    } else {
         std::cout << ANSI_MAGENTA;
-        // for the top level, don't print the double underscore ones; they are in builtins.
-        for (auto i = se.vars.begin(); i != se.vars.end(); ++i)
-        {
-            if (((std::string)st[i->first]).substr(0,2) != "__")
-                std::cout << st[i->first] << " ";
-        }
+        // for the top level, don't print the double underscore ones;
+        // they are in builtins.
+        for (auto & i : se.vars)
+            if (!hasPrefix(st[i.first], "__"))
+                std::cout << st[i.first] << " ";
         std::cout << ANSI_NORMAL;
         std::cout << std::endl;
         printWithBindings(st, env);  // probably nothing there for the top level.
@@ -771,56 +765,50 @@ void printEnvBindings(const SymbolTable &st, const StaticEnv &se, const Env &env
 }
 
 // TODO: add accompanying env for With stuff.
-void printEnvBindings(const SymbolTable &st, const Expr &expr, const Env &env)
+void printEnvBindings(const SymbolTable & st, const Expr & expr, const Env & env)
 {
     // just print the names for now
-    if (expr.staticenv)
-    {
-      printEnvBindings(st, *expr.staticenv.get(), env, 0);
+    if (expr.staticEnv)
+        printEnvBindings(st, *expr.staticEnv.get(), env, 0);
+}
+
+void mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & env, valmap & vm)
+{
+    // add bindings for the next level up first, so that the bindings for this level
+    // override the higher levels.
+    // The top level bindings (builtins) are skipped since they are added for us by initEnv()
+    if (env.up && se.up) {
+        mapStaticEnvBindings(st, *se.up, *env.up, vm);
+
+        if (env.type == Env::HasWithAttrs) {
+            // add 'with' bindings.
+            Bindings::iterator j = env.values[0]->attrs->begin();
+            while (j != env.values[0]->attrs->end()) {
+                vm[st[j->name]] = j->value;
+                ++j;
+            }
+        } else {
+            // iterate through staticenv bindings and add them.
+            for (auto & i : se.vars)
+                vm[st[i.first]] = env.values[i.second];
+        }
     }
 }
 
-void mapStaticEnvBindings(const SymbolTable &st, const StaticEnv &se, const Env &env, valmap & vm)
+std::unique_ptr<valmap> mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & env)
 {
-  // add bindings for the next level up first, so that the bindings for this level
-  // override the higher levels.
-  // The top level bindings (builtins) are skipped since they are added for us by initEnv()
-  if (env.up && se.up) {
-      mapStaticEnvBindings(st, *se.up, *env.up, vm);
-
-      if (env.type == Env::HasWithAttrs)
-      {
-          // add 'with' bindings.
-          Bindings::iterator j = env.values[0]->attrs->begin();
-          while (j != env.values[0]->attrs->end()) {
-              vm[st[j->name]] = j->value;
-              ++j;
-          }
-      }
-      else
-      {
-          // iterate through staticenv bindings and add them.
-          for (auto iter = se.vars.begin(); iter != se.vars.end(); ++iter)
-          {
-              vm[st[iter->first]] = env.values[iter->second];
-          }
-      }
-  }
-}
-
-valmap * mapStaticEnvBindings(const SymbolTable &st,const StaticEnv &se, const Env &env)
-{
-    auto vm = new valmap();
+    auto vm = std::make_unique<valmap>();
     mapStaticEnvBindings(st, se, env, *vm);
     return vm;
 }
 
-
-void EvalState::debugLastTrace(Error & e) const {
-    // call this in the situation where Expr and Env are inaccessible.  The debugger will start in the last context
-    // that's in the DebugTrace stack.
+void EvalState::debugLastTrace(Error & e) const
+{
+    // Call this in the situation where Expr and Env are inaccessible.
+    // The debugger will start in the last context that's in the
+    // DebugTrace stack.
     if (debuggerHook && !debugTraces.empty()) {
-        const DebugTrace &last = debugTraces.front();
+        const DebugTrace & last = debugTraces.front();
         debuggerHook(&e, last.env, last.expr);
     }
 }
@@ -829,7 +817,7 @@ void EvalState::debugLastTrace(Error & e) const {
    of stack space, which is a real killer in the recursive
    evaluator.  So here are some helper functions for throwing
    exceptions. */
-void EvalState::throwEvalError(const PosIdx pos, const char * s, Env & env, Expr &expr) const
+void EvalState::throwEvalError(const PosIdx pos, const char * s, Env & env, Expr & expr) const
 {
     auto error = EvalError({
         .msg = hintfmt(s),
@@ -864,7 +852,7 @@ void EvalState::throwEvalError(const char * s, const std::string & s2) const
 }
 
 void EvalState::throwEvalError(const PosIdx pos, const Suggestions & suggestions, const char * s,
-    const std::string & s2, Env & env, Expr &expr) const
+    const std::string & s2, Env & env, Expr & expr) const
 {
     auto error = EvalError(ErrorInfo{
         .msg = hintfmt(s, s2),
@@ -890,7 +878,7 @@ void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::stri
     throw error;
 }
 
-void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::string & s2, Env & env, Expr &expr) const
+void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::string & s2, Env & env, Expr & expr) const
 {
     auto error = EvalError({
         .msg = hintfmt(s, s2),
@@ -900,7 +888,6 @@ void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::stri
     if (debuggerHook)
         debuggerHook(&error, env, expr);
 
-
     throw error;
 }
 
@@ -931,7 +918,7 @@ void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::stri
 }
 
 void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::string & s2,
-    const std::string & s3, Env & env, Expr &expr) const
+    const std::string & s3, Env & env, Expr & expr) const
 {
     auto error = EvalError({
         .msg = hintfmt(s, s2),
@@ -944,7 +931,7 @@ void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::stri
     throw error;
 }
 
-void EvalState::throwEvalError(const PosIdx p1, const char * s, const Symbol sym, const PosIdx p2, Env & env, Expr &expr) const
+void EvalState::throwEvalError(const PosIdx p1, const char * s, const Symbol sym, const PosIdx p2, Env & env, Expr & expr) const
 {
     // p1 is where the error occurred; p2 is a position mentioned in the message.
     auto error = EvalError({
@@ -970,7 +957,7 @@ void EvalState::throwTypeError(const PosIdx pos, const char * s, const Value & v
     throw error;
 }
 
-void EvalState::throwTypeError(const PosIdx pos, const char * s, const Value & v, Env & env, Expr &expr) const
+void EvalState::throwTypeError(const PosIdx pos, const char * s, const Value & v, Env & env, Expr & expr) const
 {
     auto error = TypeError({
         .msg = hintfmt(s, showType(v)),
@@ -1086,27 +1073,31 @@ void EvalState::addErrorTrace(Error & e, const PosIdx pos, const char * s, const
     e.addTrace(positions[pos], s, s2);
 }
 
-std::unique_ptr<DebugTraceStacker> makeDebugTraceStacker(EvalState &state, Expr &expr, Env &env,
-    std::optional<ErrPos> pos, const char * s, const std::string & s2)
+static std::unique_ptr<DebugTraceStacker> makeDebugTraceStacker(
+    EvalState & state,
+    Expr & expr,
+    Env & env,
+    std::optional<ErrPos> pos,
+    const char * s,
+    const std::string & s2)
 {
-  return std::unique_ptr<DebugTraceStacker>(
-      new DebugTraceStacker(
-          state,
-          DebugTrace
-                {.pos = pos,
-                 .expr = expr,
-                 .env = env,
-                 .hint = hintfmt(s, s2),
-                 .is_error = false
-                }));
+    return std::make_unique<DebugTraceStacker>(state,
+        DebugTrace {
+            .pos = pos,
+            .expr = expr,
+            .env = env,
+            .hint = hintfmt(s, s2),
+            .isError = false
+        });
 }
 
-DebugTraceStacker::DebugTraceStacker(EvalState &evalState, DebugTrace t)
-:evalState(evalState), trace(t)
+DebugTraceStacker::DebugTraceStacker(EvalState & evalState, DebugTrace t)
+    : evalState(evalState)
+    , trace(std::move(t))
 {
-    evalState.debugTraces.push_front(t);
+    evalState.debugTraces.push_front(trace);
     if (evalState.debugStop && debuggerHook)
-        debuggerHook(0, t.env, t.expr);
+        debuggerHook(nullptr, trace.env, trace.expr);
 }
 
 void Value::mkString(std::string_view s)
@@ -1303,15 +1294,14 @@ void EvalState::cacheFile(
     fileParseCache[resolvedPath] = e;
 
     try {
-        auto dts =
-            debuggerHook ?
-                makeDebugTraceStacker(
-                    *this,
-                    *e,
-                    this->baseEnv,
-                    (e->getPos() ? std::optional(ErrPos(positions[e->getPos()])) : std::nullopt),
-                    "while evaluating the file '%1%':", resolvedPath)
-                : nullptr;
+        auto dts = debuggerHook
+            ? makeDebugTraceStacker(
+                *this,
+                *e,
+                this->baseEnv,
+                e->getPos() ? std::optional(ErrPos(positions[e->getPos()])) : std::nullopt,
+                "while evaluating the file '%1%':", resolvedPath)
+            : nullptr;
 
         // Enforce that 'flake.nix' is a direct attrset, not a
         // computation.
@@ -1538,15 +1528,14 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
     e->eval(state, env, vTmp);
 
     try {
-        auto dts =
-            debuggerHook ?
-                makeDebugTraceStacker(
-                    state,
-                    *this,
-                    env,
-                    state.positions[pos2],
-                    "while evaluating the attribute '%1%'",
-                    showAttrPath(state, env, attrPath))
+        auto dts = debuggerHook
+            ? makeDebugTraceStacker(
+                state,
+                *this,
+                env,
+                state.positions[pos2],
+                "while evaluating the attribute '%1%'",
+                showAttrPath(state, env, attrPath))
             : nullptr;
 
         for (auto & i : attrPath) {
@@ -1708,14 +1697,14 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
 
             /* Evaluate the body. */
             try {
-                auto dts =
-                    debuggerHook ?
-                        makeDebugTraceStacker(*this, *lambda.body, env2, positions[lambda.pos],
-                                           "while evaluating %s",
-                                           (lambda.name
-                                               ? concatStrings("'", symbols[lambda.name], "'")
-                                               : "anonymous lambda"))
-                        : nullptr;
+                auto dts = debuggerHook
+                    ? makeDebugTraceStacker(
+                        *this, *lambda.body, env2, positions[lambda.pos],
+                        "while evaluating %s",
+                        lambda.name
+                        ? concatStrings("'", symbols[lambda.name], "'")
+                        : "anonymous lambda")
+                    : nullptr;
 
                 lambda.body->eval(*this, env2, vCur);
             } catch (Error & e) {
@@ -2135,14 +2124,10 @@ void EvalState::forceValueDeep(Value & v)
         if (v.type() == nAttrs) {
             for (auto & i : *v.attrs)
                 try {
-
-                    auto dts =
-                        debuggerHook ?
-                            // if the value is a thunk, we're evaling.  otherwise no trace necessary.
-                            (i.value->isThunk() ?
-                                makeDebugTraceStacker(*this, *v.thunk.expr, *v.thunk.env, positions[i.pos],
-                                                  "while evaluating the attribute '%1%'", symbols[i.name])
-                                : nullptr)
+                    // If the value is a thunk, we're evaling. Otherwise no trace necessary.
+                    auto dts = debuggerHook && i.value->isThunk()
+                        ? makeDebugTraceStacker(*this, *v.thunk.expr, *v.thunk.env, positions[i.pos],
+                            "while evaluating the attribute '%1%'", symbols[i.name])
                         : nullptr;
 
                     recurse(*i.value);

src/libexpr/eval.hh

@@ -25,8 +25,8 @@ enum RepairFlag : bool;
 
 typedef void (* PrimOpFun) (EvalState & state, const PosIdx pos, Value * * args, Value & v);
 
-void printEnvBindings(const SymbolTable &st, const Expr &expr, const Env &env);
-void printEnvBindings(const SymbolTable &st, const StaticEnv &se, const Env &env, int lvl = 0);
+void printEnvBindings(const SymbolTable & st, const Expr & expr, const Env & env);
+void printEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & env, int lvl = 0);
 
 struct PrimOp
 {
@@ -47,7 +47,7 @@ struct Env
     Value * values[0];
 };
 
-valmap * mapStaticEnvBindings(const SymbolTable &st, const StaticEnv &se, const Env &env);
+std::unique_ptr<valmap> mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & env);
 
 void copyContext(const Value & v, PathSet & context);
 
@@ -75,10 +75,10 @@ std::shared_ptr<RegexCache> makeRegexCache();
 
 struct DebugTrace {
     std::optional<ErrPos> pos;
-    const Expr &expr;
-    const Env &env;
+    const Expr & expr;
+    const Env & env;
     hintformat hint;
-    bool is_error;
+    bool isError;
 };
 
 class EvalState
@@ -297,7 +297,7 @@ public:
     void throwEvalError(const char * s, const std::string & s2, const std::string & s3) const;
     [[gnu::noinline, gnu::noreturn]]
     void throwEvalError(const PosIdx pos, const Suggestions & suggestions, const char * s, const std::string & s2,
-        Env & env, Expr &expr) const;
+        Env & env, Expr & expr) const;
     [[gnu::noinline, gnu::noreturn]]
     void throwEvalError(const PosIdx p1, const char * s, const Symbol sym, const PosIdx p2,
         Env & env, Expr & expr) const;
@@ -508,16 +508,15 @@ private:
     friend struct Value;
 };
 
-class DebugTraceStacker {
-    public:
-        DebugTraceStacker(EvalState &evalState, DebugTrace t);
-        ~DebugTraceStacker()
-        {
-            // assert(evalState.debugTraces.front() == trace);
-            evalState.debugTraces.pop_front();
-        }
-        EvalState &evalState;
-        DebugTrace trace;
+struct DebugTraceStacker {
+    DebugTraceStacker(EvalState & evalState, DebugTrace t);
+    ~DebugTraceStacker()
+    {
+        // assert(evalState.debugTraces.front() == trace);
+        evalState.debugTraces.pop_front();
+    }
+    EvalState & evalState;
+    DebugTrace trace;
 };
 
 /* Return a string representing the type of the value `v'. */

src/libexpr/nixexpr.cc

@@ -296,39 +296,38 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath)
 
 /* Computing levels/displacements for variables. */
 
-void Expr::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void Expr::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     abort();
 }
 
-void ExprInt::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprInt::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;}
+
+void ExprFloat::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
+{
+    if (debuggerHook)
+        staticEnv = env;
 }
 
-void ExprFloat::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprString::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 }
 
-void ExprString::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprPath::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 }
 
-void ExprPath::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprVar::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
-}
-
-void ExprVar::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
-{
-    if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     /* Check whether the variable appears in the environment.  If so,
        set its level and displacement. */
@@ -353,20 +352,18 @@ void ExprVar::bindVars(const EvalState & es, const std::shared_ptr<const StaticE
        enclosing `with'.  If there is no `with', then we can issue an
        "undefined variable" error now. */
     if (withLevel == -1)
-    {
         throw UndefinedVarError({
             .msg = hintfmt("undefined variable '%1%'", es.symbols[name]),
             .errPos = es.positions[pos]
         });
-    }
     fromWith = true;
     this->level = withLevel;
 }
 
-void ExprSelect::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprSelect::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     e->bindVars(es, env);
     if (def) def->bindVars(es, env);
@@ -375,10 +372,10 @@ void ExprSelect::bindVars(const EvalState & es, const std::shared_ptr<const Stat
             i.expr->bindVars(es, env);
 }
 
-void ExprOpHasAttr::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprOpHasAttr::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     e->bindVars(es, env);
     for (auto & i : attrPath)
@@ -386,13 +383,13 @@ void ExprOpHasAttr::bindVars(const EvalState & es, const std::shared_ptr<const S
             i.expr->bindVars(es, env);
 }
 
-void ExprAttrs::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprAttrs::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     if (recursive) {
-        auto newEnv = std::shared_ptr<StaticEnv>(new StaticEnv(false, env.get(), recursive ? attrs.size() : 0));
+        auto newEnv = std::make_shared<StaticEnv>(false, env.get(), recursive ? attrs.size() : 0);
 
         Displacement displ = 0;
         for (auto & i : attrs)
@@ -419,25 +416,24 @@ void ExprAttrs::bindVars(const EvalState & es, const std::shared_ptr<const Stati
     }
 }
 
-void ExprList::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprList::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     for (auto & i : elems)
         i->bindVars(es, env);
 }
 
-void ExprLambda::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprLambda::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
-    auto newEnv = std::shared_ptr<StaticEnv>(
-        new StaticEnv(
-                false, env.get(),
-                (hasFormals() ? formals->formals.size() : 0) +
-                (!arg ? 0 : 1)));
+    auto newEnv = std::make_shared<StaticEnv>(
+        false, env.get(),
+        (hasFormals() ? formals->formals.size() : 0) +
+        (!arg ? 0 : 1));
 
     Displacement displ = 0;
 
@@ -456,22 +452,22 @@ void ExprLambda::bindVars(const EvalState & es, const std::shared_ptr<const Stat
     body->bindVars(es, newEnv);
 }
 
-void ExprCall::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprCall::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     fun->bindVars(es, env);
     for (auto e : args)
         e->bindVars(es, env);
 }
 
-void ExprLet::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprLet::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
-    auto newEnv = std::shared_ptr<StaticEnv>(new StaticEnv(false, env.get(), attrs->attrs.size()));
+    auto newEnv = std::make_shared<StaticEnv>(false, env.get(), attrs->attrs.size());
 
     Displacement displ = 0;
     for (auto & i : attrs->attrs)
@@ -485,10 +481,10 @@ void ExprLet::bindVars(const EvalState & es, const std::shared_ptr<const StaticE
     body->bindVars(es, newEnv);
 }
 
-void ExprWith::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprWith::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     /* Does this `with' have an enclosing `with'?  If so, record its
        level so that `lookupVar' can look up variables in the previous
@@ -503,54 +499,53 @@ void ExprWith::bindVars(const EvalState & es, const std::shared_ptr<const Static
         }
 
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     attrs->bindVars(es, env);
-    auto newEnv = std::shared_ptr<StaticEnv>(new StaticEnv(true, env.get()));
+    auto newEnv = std::make_shared<StaticEnv>(true, env.get());
     body->bindVars(es, newEnv);
 }
 
-void ExprIf::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprIf::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     cond->bindVars(es, env);
     then->bindVars(es, env);
     else_->bindVars(es, env);
 }
 
-void ExprAssert::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprAssert::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     cond->bindVars(es, env);
     body->bindVars(es, env);
 }
 
-void ExprOpNot::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprOpNot::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     e->bindVars(es, env);
 }
 
-void ExprConcatStrings::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprConcatStrings::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
+        staticEnv = env;
 
     for (auto & i : *this->es)
         i.second->bindVars(es, env);
 }
 
-void ExprPos::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> &env)
+void ExprPos::bindVars(const EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
     if (debuggerHook)
-        staticenv = env;
-
+        staticEnv = env;
 }
 
 

src/libexpr/nixexpr.hh

@@ -148,8 +148,8 @@ struct Expr
     virtual void eval(EvalState & state, Env & env, Value & v);
     virtual Value * maybeThunk(EvalState & state, Env & env);
     virtual void setName(Symbol name);
-    std::shared_ptr<const StaticEnv> staticenv;
-    virtual const PosIdx getPos() const = 0;
+    std::shared_ptr<const StaticEnv> staticEnv;
+    virtual PosIdx getPos() const { return noPos; }
 };
 
 #define COMMON_METHODS \
@@ -163,7 +163,6 @@ struct ExprInt : Expr
     Value v;
     ExprInt(NixInt n) : n(n) { v.mkInt(n); };
     Value * maybeThunk(EvalState & state, Env & env);
-    const PosIdx getPos() const { return noPos; }
     COMMON_METHODS
 };
 
@@ -173,7 +172,6 @@ struct ExprFloat : Expr
     Value v;
     ExprFloat(NixFloat nf) : nf(nf) { v.mkFloat(nf); };
     Value * maybeThunk(EvalState & state, Env & env);
-    const PosIdx getPos() const { return noPos; }
     COMMON_METHODS
 };
 
@@ -183,7 +181,6 @@ struct ExprString : Expr
     Value v;
     ExprString(std::string s) : s(std::move(s)) { v.mkString(this->s.data()); };
     Value * maybeThunk(EvalState & state, Env & env);
-    const PosIdx getPos() const { return noPos; }
     COMMON_METHODS
 };
 
@@ -193,7 +190,6 @@ struct ExprPath : Expr
     Value v;
     ExprPath(std::string s) : s(std::move(s)) { v.mkPath(this->s.c_str()); };
     Value * maybeThunk(EvalState & state, Env & env);
-    const PosIdx getPos() const { return noPos; }
     COMMON_METHODS
 };
 
@@ -221,7 +217,7 @@ struct ExprVar : Expr
     ExprVar(Symbol name) : name(name) { };
     ExprVar(const PosIdx & pos, Symbol name) : pos(pos), name(name) { };
     Value * maybeThunk(EvalState & state, Env & env);
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -232,7 +228,7 @@ struct ExprSelect : Expr
     AttrPath attrPath;
     ExprSelect(const PosIdx & pos, Expr * e, const AttrPath & attrPath, Expr * def) : pos(pos), e(e), def(def), attrPath(attrPath) { };
     ExprSelect(const PosIdx & pos, Expr * e, Symbol name) : pos(pos), e(e), def(0) { attrPath.push_back(AttrName(name)); };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -241,7 +237,7 @@ struct ExprOpHasAttr : Expr
     Expr * e;
     AttrPath attrPath;
     ExprOpHasAttr(Expr * e, const AttrPath & attrPath) : e(e), attrPath(attrPath) { };
-    const PosIdx getPos() const { return e->getPos(); }
+    PosIdx getPos() const override { return e->getPos(); }
     COMMON_METHODS
 };
 
@@ -270,7 +266,7 @@ struct ExprAttrs : Expr
     DynamicAttrDefs dynamicAttrs;
     ExprAttrs(const PosIdx &pos) : recursive(false), pos(pos) { };
     ExprAttrs() : recursive(false) { };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -278,13 +274,12 @@ struct ExprList : Expr
 {
     std::vector<Expr *> elems;
     ExprList() { };
-    const PosIdx getPos() const
-      { if (elems.empty())
-            return noPos;
-        else
-            return elems.front()->getPos();
-      }
     COMMON_METHODS
+
+    PosIdx getPos() const override
+    {
+        return elems.empty() ? noPos : elems.front()->getPos();
+    }
 };
 
 struct Formal
@@ -337,7 +332,7 @@ struct ExprLambda : Expr
     void setName(Symbol name);
     std::string showNamePos(const EvalState & state) const;
     inline bool hasFormals() const { return formals != nullptr; }
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -349,7 +344,7 @@ struct ExprCall : Expr
     ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args)
         : fun(fun), args(args), pos(pos)
     { }
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -358,7 +353,6 @@ struct ExprLet : Expr
     ExprAttrs * attrs;
     Expr * body;
     ExprLet(ExprAttrs * attrs, Expr * body) : attrs(attrs), body(body) { };
-    const PosIdx getPos() const { return noPos; }
     COMMON_METHODS
 };
 
@@ -368,7 +362,7 @@ struct ExprWith : Expr
     Expr * attrs, * body;
     size_t prevWith;
     ExprWith(const PosIdx & pos, Expr * attrs, Expr * body) : pos(pos), attrs(attrs), body(body) { };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -377,7 +371,7 @@ struct ExprIf : Expr
     PosIdx pos;
     Expr * cond, * then, * else_;
     ExprIf(const PosIdx & pos, Expr * cond, Expr * then, Expr * else_) : pos(pos), cond(cond), then(then), else_(else_) { };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -386,7 +380,7 @@ struct ExprAssert : Expr
     PosIdx pos;
     Expr * cond, * body;
     ExprAssert(const PosIdx & pos, Expr * cond, Expr * body) : pos(pos), cond(cond), body(body) { };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -394,7 +388,6 @@ struct ExprOpNot : Expr
 {
     Expr * e;
     ExprOpNot(Expr * e) : e(e) { };
-    const PosIdx getPos() const { return noPos; }
     COMMON_METHODS
 };
 
@@ -414,7 +407,7 @@ struct ExprOpNot : Expr
             e1->bindVars(es, env); e2->bindVars(es, env);    \
         } \
         void eval(EvalState & state, Env & env, Value & v); \
-        const PosIdx getPos() const { return pos; } \
+        PosIdx getPos() const override { return pos; } \
     };
 
 MakeBinOp(ExprOpEq, "==")
@@ -432,7 +425,7 @@ struct ExprConcatStrings : Expr
     std::vector<std::pair<PosIdx, Expr *> > * es;
     ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector<std::pair<PosIdx, Expr *> > * es)
         : pos(pos), forceString(forceString), es(es) { };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 
@@ -440,7 +433,7 @@ struct ExprPos : Expr
 {
     PosIdx pos;
     ExprPos(const PosIdx & pos) : pos(pos) { };
-    const PosIdx getPos() const { return pos; }
+    PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };
 

src/libexpr/primops.cc

@@ -227,7 +227,7 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v
             Env * env = &state.allocEnv(vScope->attrs->size());
             env->up = &state.baseEnv;
 
-            auto staticEnv = std::shared_ptr<StaticEnv>(new StaticEnv(false, state.staticBaseEnv.get(), vScope->attrs->size()));
+            auto staticEnv = std::make_shared<StaticEnv>(false, state.staticBaseEnv.get(), vScope->attrs->size());
 
             unsigned int displ = 0;
             for (auto & attr : *vScope->attrs) {
@@ -329,8 +329,7 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu
     std::string sym(state.forceStringNoCtx(*args[1], pos));
 
     void *handle = dlopen(path.c_str(), RTLD_LAZY | RTLD_LOCAL);
-    if (!handle)
-    {
+    if (!handle) {
         auto e = EvalError("could not open '%1%': %2%", path, dlerror());
         state.debugLastTrace(e);
         throw e;
@@ -340,14 +339,11 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu
     ValueInitializer func = (ValueInitializer) dlsym(handle, sym.c_str());
     if(!func) {
         char *message = dlerror();
-        if (message)
-        {
+        if (message) {
             auto e = EvalError("could not load symbol '%1%' from '%2%': %3%", sym, path, message);
             state.debugLastTrace(e);
             throw e;
-        }
-        else
-        {
+        } else {
             auto e = EvalError("symbol '%1%' from '%2%' resolved to NULL when a function pointer was expected",
                 sym, path);
             state.debugLastTrace(e);
@@ -573,8 +569,7 @@ struct CompareValues
             return v1->fpoint < v2->integer;
         if (v1->type() == nInt && v2->type() == nFloat)
             return v1->integer < v2->fpoint;
-        if (v1->type() != v2->type())
-        {
+        if (v1->type() != v2->type()) {
             auto e = EvalError("cannot compare %1% with %2%", showType(*v1), showType(*v2));
             state.debugLastTrace(e);
             throw e;
@@ -599,12 +594,11 @@ struct CompareValues
                         return (*this)(v1->listElems()[i], v2->listElems()[i]);
                     }
                 }
-            default:
-                {
-                    auto e = EvalError("cannot compare %1% with %2%", showType(*v1), showType(*v2));
-                    state.debugLastTrace(e);
-                    throw e;
-                }
+            default: {
+                auto e = EvalError("cannot compare %1% with %2%", showType(*v1), showType(*v2));
+                state.debugLastTrace(e);
+                throw e;
+            }
         }
     }
 };
@@ -703,8 +697,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a
 
         Bindings::iterator key =
             e->attrs->find(state.sKey);
-        if (key == e->attrs->end())
-        {
+        if (key == e->attrs->end()) {
             auto e = EvalError({
                 .msg = hintfmt("attribute 'key' required"),
                 .errPos = state.positions[pos]
@@ -772,7 +765,7 @@ static RegisterPrimOp primop_break({
     .name = "break",
     .args = {"v"},
     .doc = R"(
-      In debug mode, pause Nix expression evaluation and enter the repl.
+      In debug mode (enabled using `--debugger`), pause Nix expression evaluation and enter the REPL.
     )",
     .fun = [](EvalState & state, const PosIdx pos, Value * * args, Value & v)
     {
@@ -783,16 +776,15 @@ static RegisterPrimOp primop_break({
             .msg = hintfmt("breakpoint reached; value was %1%", s),
             .errPos = state.positions[pos],
         });
-        if (debuggerHook && !state.debugTraces.empty())
-        {
-            auto &dt = state.debugTraces.front();
+        if (debuggerHook && !state.debugTraces.empty()) {
+            auto & dt = state.debugTraces.front();
             debuggerHook(&error, dt.env, dt.expr);
 
             if (state.debugQuit) {
                 // if the user elects to quit the repl, throw an exception.
                 throw Error(ErrorInfo{
                     .level = lvlInfo,
-                    .msg = hintfmt("quit from debugger"),
+                    .msg = hintfmt("quit the debugger"),
                     .errPos = state.positions[noPos],
                 });
             }
@@ -903,7 +895,7 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value * * args, Va
 {
     auto attrs = state.buildBindings(2);
     auto saveDebuggerHook = debuggerHook;
-    debuggerHook = 0;
+    debuggerHook = nullptr;
     try {
         state.forceValue(*args[0], pos);
         attrs.insert(state.sValue, args[0]);