refactor(treewide): modern C++ idioms

src/libcmd/command.cc

@@ -1,4 +1,5 @@
 #include <algorithm>
+#include <ranges>
 #include <nlohmann/json.hpp>
 
 #include "nix/cmd/command.hh"
@@ -233,7 +234,7 @@ void StorePathsCommand::run(ref<Store> store, BuiltPaths && allPaths, BuiltPaths
             storePaths.insert(p);
 
     auto sorted = store->topoSortPaths(storePaths);
-    std::reverse(sorted.begin(), sorted.end());
+    std::ranges::reverse(sorted);
 
     run(store, std::move(sorted));
 }
@@ -408,7 +409,7 @@ void createOutLinks(const std::filesystem::path & outLink, const BuiltPaths & bu
     }
 }
 
-void MixOutLinkBase::createOutLinksMaybe(const std::vector<BuiltPathWithResult> & buildables, ref<Store> & store)
+void MixOutLinkBase::createOutLinksMaybe(std::span<const BuiltPathWithResult> buildables, ref<Store> & store)
 {
     if (outLink != "")
         if (auto store2 = store.dynamic_pointer_cast<LocalFSStore>())

src/libcmd/include/nix/cmd/command.hh

@@ -8,6 +8,7 @@
 #include "nix/flake/lockfile.hh"
 
 #include <optional>
+#include <span>
 
 namespace nix {
 
@@ -398,7 +399,7 @@ struct MixOutLinkBase : virtual Args
     {
     }
 
-    void createOutLinksMaybe(const std::vector<BuiltPathWithResult> & buildables, ref<Store> & store);
+    void createOutLinksMaybe(std::span<const BuiltPathWithResult> buildables, ref<Store> & store);
 };
 
 /** `--out-link`, `--no-link`, `createOutLinksMaybe` */

src/libcmd/include/nix/cmd/installables.hh

@@ -9,6 +9,7 @@
 #include "nix/store/build-result.hh"
 
 #include <optional>
+#include <span>
 
 namespace nix {
 
@@ -86,7 +87,7 @@ struct BuiltPathWithResult
     std::optional<BuildResult> result;
 };
 
-BuiltPaths toBuiltPaths(const std::vector<BuiltPathWithResult> & builtPathsWithResult);
+BuiltPaths toBuiltPaths(std::span<const BuiltPathWithResult> builtPathsWithResult);
 
 /**
  * Shorthand, for less typing and helping us keep the choice of

src/libcmd/installable-flake.cc

@@ -78,7 +78,7 @@ InstallableFlake::InstallableFlake(
 
 DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
 {
-    Activity act(*logger, lvlTalkative, actUnknown, fmt("evaluating derivation '%s'", what()));
+    Activity act(*logger, Verbosity::Talkative, ActivityType::Unknown, fmt("evaluating derivation '%s'", what()));
 
     auto attr = getCursor(*state);
 

src/libcmd/installables.cc

@@ -584,7 +584,7 @@ static void throwBuildErrors(std::vector<KeyedBuildResult> & buildResults, const
                 if (!failedResult->second->errorMsg.empty()) {
                     logError(
                         ErrorInfo{
-                            .level = lvlError,
+                            .level = Verbosity::Error,
                             .msg = failedResult->second->errorMsg,
                         });
                 }
@@ -623,7 +623,7 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
 
     case Realise::Nothing:
     case Realise::Derivation:
-        printMissing(store, pathsToBuild, lvlError);
+        printMissing(store, pathsToBuild, Verbosity::Error);
 
         for (auto & path : pathsToBuild) {
             for (auto & aux : backmap[path]) {
@@ -653,7 +653,7 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
 
     case Realise::Outputs: {
         if (settings.printMissing)
-            printMissing(store, pathsToBuild, lvlInfo);
+            printMissing(store, pathsToBuild, Verbosity::Info);
 
         auto buildResults = store->buildPathsWithResults(pathsToBuild, bMode, evalStore);
         throwBuildErrors(buildResults, *store);
@@ -854,7 +854,7 @@ void BuiltPathsCommand::applyDefaultInstallables(std::vector<std::string> & rawI
         rawInstallables.push_back(".");
 }
 
-BuiltPaths toBuiltPaths(const std::vector<BuiltPathWithResult> & builtPathsWithResult)
+BuiltPaths toBuiltPaths(std::span<const BuiltPathWithResult> builtPathsWithResult)
 {
     BuiltPaths res;
     for (auto & i : builtPathsWithResult)

src/libcmd/repl.cc

@@ -219,9 +219,9 @@ ReplExitStatus NixRepl::mainLoop()
         } catch (IncompleteReplExpr &) {
             continue;
         } catch (Error & e) {
-            printMsg(lvlError, e.msg());
+            printMsg(Verbosity::Error, e.msg());
         } catch (Interrupted & e) {
-            printMsg(lvlError, e.msg());
+            printMsg(Verbosity::Error, e.msg());
         }
 
         // We handled the current input fully, so we should clear it

src/libexpr-tests/value/print.cc

@@ -19,42 +19,42 @@ struct ValuePrintingTests : LibExprTest
     }
 };
 
-TEST_F(ValuePrintingTests, tInt)
+TEST_F(ValuePrintingTests, printInt)
 {
     Value vInt;
     vInt.mkInt(10);
     test(vInt, "10");
 }
 
-TEST_F(ValuePrintingTests, tBool)
+TEST_F(ValuePrintingTests, printBool)
 {
     Value vBool;
     vBool.mkBool(true);
     test(vBool, "true");
 }
 
-TEST_F(ValuePrintingTests, tString)
+TEST_F(ValuePrintingTests, printString)
 {
     Value vString;
     vString.mkStringNoCopy("some-string"_sds);
     test(vString, "\"some-string\"");
 }
 
-TEST_F(ValuePrintingTests, tPath)
+TEST_F(ValuePrintingTests, printPath)
 {
     Value vPath;
     vPath.mkStringNoCopy("/foo"_sds);
     test(vPath, "\"/foo\"");
 }
 
-TEST_F(ValuePrintingTests, tNull)
+TEST_F(ValuePrintingTests, printNull)
 {
     Value vNull;
     vNull.mkNull();
     test(vNull, "null");
 }
 
-TEST_F(ValuePrintingTests, tAttrs)
+TEST_F(ValuePrintingTests, printAttrs)
 {
     Value vOne;
     vOne.mkInt(1);

src/libexpr/eval-cache.cc

@@ -8,6 +8,8 @@
 // Need specialization involving `SymbolStr` just in this one module.
 #include "nix/util/strings-inline.hh"
 
+#include <ranges>
+
 namespace nix::eval_cache {
 
 CachedEvalError::CachedEvalError(ref<AttrCursor> cursor, Symbol attr)
@@ -511,7 +513,7 @@ ref<AttrCursor> AttrCursor::getAttr(std::string_view name)
     return getAttr(root->state.symbols.create(name));
 }
 
-OrSuggestions<ref<AttrCursor>> AttrCursor::findAlongAttrPath(const std::vector<Symbol> & attrPath)
+OrSuggestions<ref<AttrCursor>> AttrCursor::findAlongAttrPath(std::span<const Symbol> attrPath)
 {
     auto res = shared_from_this();
     for (auto & attr : attrPath) {
@@ -685,7 +687,7 @@ std::vector<Symbol> AttrCursor::getAttrs()
     std::vector<Symbol> attrs;
     for (auto & attr : *getValue().attrs())
         attrs.push_back(attr.name);
-    std::sort(attrs.begin(), attrs.end(), [&](Symbol a, Symbol b) {
+    std::ranges::sort(attrs, [&](Symbol a, Symbol b) {
         std::string_view sa = root->state.symbols[a], sb = root->state.symbols[b];
         return sa < sb;
     });

src/libexpr/eval.cc

@@ -165,17 +165,17 @@ std::string showType(const Value & v)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wswitch-enum"
     switch (v.getInternalType()) {
-    case tString:
+    case InternalType::String:
         return v.context() ? "a string with context" : "a string";
-    case tPrimOp:
+    case InternalType::PrimOp:
         return fmt("the built-in function '%s'", std::string(v.primOp()->name));
-    case tPrimOpApp:
+    case InternalType::PrimOpApp:
         return fmt("the partially applied built-in function '%s'", v.primOpAppPrimOp()->name);
-    case tExternal:
+    case InternalType::External:
         return v.external()->showType();
-    case tThunk:
+    case InternalType::Thunk:
         return v.isBlackhole() ? "a black hole" : "a thunk";
-    case tApp:
+    case InternalType::App:
         return "a function application";
     default:
         return std::string(showType(v.type()));
@@ -189,11 +189,11 @@ PosIdx Value::determinePos(const PosIdx pos) const
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wswitch-enum"
     switch (getInternalType()) {
-    case tAttrs:
+    case InternalType::Attrs:
         return attrs()->pos;
-    case tLambda:
+    case InternalType::Lambda:
         return lambda().fun->pos;
-    case tApp:
+    case InternalType::App:
         return app().left->determinePos(pos);
     default:
         return pos;
@@ -203,8 +203,8 @@ PosIdx Value::determinePos(const PosIdx pos) const
 
 bool Value::isTrivial() const
 {
-    return !isa<tApp, tPrimOpApp>()
-           && (!isa<tThunk>()
+    return !isa<InternalType::App, InternalType::PrimOpApp>()
+           && (!isa<InternalType::Thunk>()
                || (dynamic_cast<ExprAttrs *>(thunk().expr) && ((ExprAttrs *) thunk().expr)->dynamicAttrs->empty())
                || dynamic_cast<ExprLambda *>(thunk().expr) || dynamic_cast<ExprList *>(thunk().expr));
 }
@@ -645,8 +645,8 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
 void printStaticEnvBindings(const SymbolTable & st, const StaticEnv & se)
 {
     std::cout << ANSI_MAGENTA;
-    for (auto & i : se.vars)
-        std::cout << st[i.first] << " ";
+    for (auto & [symbol, _] : se.vars)
+        std::cout << st[symbol] << " ";
     std::cout << ANSI_NORMAL;
     std::cout << std::endl;
 }
@@ -682,9 +682,9 @@ void printEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env &
         std::cout << ANSI_MAGENTA;
         // 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] << " ";
+        for (auto & [symbol, _] : se.vars)
+            if (!hasPrefix(st[symbol], "__"))
+                std::cout << st[symbol] << " ";
         std::cout << ANSI_NORMAL;
         std::cout << std::endl;
         if (se.isWith)
@@ -715,8 +715,8 @@ void mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const En
                 vm.insert_or_assign(std::string(st[j.name]), j.value);
         } else {
             // iterate through staticenv bindings and add them.
-            for (auto & i : se.vars)
-                vm.insert_or_assign(std::string(st[i.first]), env.values[i.second]);
+            for (auto & [symbol, displ] : se.vars)
+                vm.insert_or_assign(std::string(st[symbol]), env.values[displ]);
         }
     }
 }
@@ -795,7 +795,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr &
     if (error) {
         printError("%s\n", error->what());
 
-        if (trylevel > 0 && error->info().level != lvlInfo)
+        if (trylevel > 0 && error->info().level != Verbosity::Info)
             printError(
                 "This exception occurred in a 'tryEval' call. Use " ANSI_GREEN "--ignore-try" ANSI_NORMAL
                 " to skip these.\n");
@@ -1637,7 +1637,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             size_t argsLeft = vCur.primOp()->arity;
 
             if (args.size() < argsLeft) {
-                /* We don't have enough arguments, so create a tPrimOpApp chain. */
+                /* We don't have enough arguments, so create a InternalType::PrimOpApp chain. */
                 makeAppChain();
                 return;
             } else {
@@ -1673,7 +1673,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             auto argsLeft = arity - argsDone;
 
             if (args.size() < argsLeft) {
-                /* We still don't have enough arguments, so extend the tPrimOpApp chain. */
+                /* We still don't have enough arguments, so extend the InternalType::PrimOpApp chain. */
                 makeAppChain();
                 return;
             } else {
@@ -2509,7 +2509,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
             repair);
         allowPath(dstPath);
         srcToStore->try_emplace(path, dstPath);
-        printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(dstPath));
+        printMsg(Verbosity::Chatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(dstPath));
         return dstPath;
     }();
 
@@ -3133,7 +3133,7 @@ Expr * EvalState::parseStdin()
     // NOTE this method (and parseExprFromString) must take care to *fully copy* their
     // input into their respective Pos::Origin until the parser stops overwriting its
     // input data.
-    // Activity act(*logger, lvlTalkative, "parsing standard input");
+    // Activity act(*logger, Verbosity::Talkative, "parsing standard input");
     auto buffer = drainFD(0);
     // drainFD should have left some extra space for terminators
     buffer.append("\0\0", 2);

src/libexpr/function-trace.cc

@@ -8,15 +8,15 @@ void FunctionCallTrace::preFunctionCallHook(
 {
     auto duration = std::chrono::high_resolution_clock::now().time_since_epoch();
     auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(duration);
-    printMsg(lvlInfo, "function-trace entered %1% at %2%", state.positions[pos], ns.count());
+    printMsg(Verbosity::Info, "function-trace entered %1% at %2%", state.positions[pos], ns.count());
 }
 
 void FunctionCallTrace::postFunctionCallHook(
     EvalState & state, const Value & v, std::span<Value *> args, const PosIdx pos)
 {
     auto duration = std::chrono::high_resolution_clock::now().time_since_epoch();
     auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(duration);
-    printMsg(lvlInfo, "function-trace exited %1% at %2%", state.positions[pos], ns.count());
+    printMsg(Verbosity::Info, "function-trace exited %1% at %2%", state.positions[pos], ns.count());
 }
 
 } // namespace nix

src/libexpr/include/nix/expr/eval-cache.hh

@@ -6,6 +6,7 @@
 #include "nix/expr/eval.hh"
 
 #include <functional>
+#include <span>
 #include <variant>
 
 namespace nix::eval_cache {
@@ -146,7 +147,7 @@ public:
      * Get an attribute along a chain of attrsets. Note that this does
      * not auto-call functors or functions.
      */
-    OrSuggestions<ref<AttrCursor>> findAlongAttrPath(const std::vector<Symbol> & attrPath);
+    OrSuggestions<ref<AttrCursor>> findAlongAttrPath(std::span<const Symbol> attrPath);
 
     std::string getString();
 

src/libexpr/include/nix/expr/eval.hh

@@ -663,9 +663,9 @@ public:
     /**
      * Force `v`, and then verify that it has the expected type.
      */
-    NixInt forceInt(Value & v, const PosIdx pos, std::string_view errorCtx);
-    NixFloat forceFloat(Value & v, const PosIdx pos, std::string_view errorCtx);
-    bool forceBool(Value & v, const PosIdx pos, std::string_view errorCtx);
+    [[nodiscard]] NixInt forceInt(Value & v, const PosIdx pos, std::string_view errorCtx);
+    [[nodiscard]] NixFloat forceFloat(Value & v, const PosIdx pos, std::string_view errorCtx);
+    [[nodiscard]] bool forceBool(Value & v, const PosIdx pos, std::string_view errorCtx);
 
     void forceAttrs(Value & v, const PosIdx pos, std::string_view errorCtx);
 
@@ -703,7 +703,7 @@ public:
      * @return true iff the value `v` denotes a derivation (i.e. a
      * set with attribute `type = "derivation"`).
      */
-    bool isDerivation(Value & v);
+    [[nodiscard]] bool isDerivation(Value & v);
 
     std::optional<std::string> tryAttrsToString(
         const PosIdx pos, Value & v, NixStringContext & context, bool coerceMore = false, bool copyToStore = true);
@@ -895,7 +895,7 @@ public:
      */
     void assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx);
 
-    bool isFunctor(const Value & fun) const;
+    [[nodiscard]] bool isFunctor(const Value & fun) const;
 
     void callFunction(Value & fun, std::span<Value *> args, Value & vRes, const PosIdx pos);