From dd8b91eebc0d31c9f8016609b36d89f58d8c4d19 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 5 May 2022 12:29:14 +0200 Subject: [PATCH] Style fixes In particular, use std::make_shared and enumerate(). Also renamed some fields to fit naming conventions. --- src/libcmd/command.cc | 28 +++--- src/libcmd/command.hh | 3 +- src/libcmd/repl.cc | 85 ++++++---------- src/libexpr/eval.cc | 217 +++++++++++++++++++---------------------- src/libexpr/eval.hh | 33 +++---- src/libexpr/nixexpr.cc | 101 +++++++++---------- src/libexpr/nixexpr.hh | 45 ++++----- src/libexpr/primops.cc | 40 +++----- 8 files changed, 247 insertions(+), 305 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 56d529461..12cd5ed83 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -121,25 +121,23 @@ ref EvalCommand::getEvalState() if (startReplOnEvalErrors) debuggerHook = [evalState{ref(evalState)}](const Error * error, const Env & env, const Expr & expr) { auto dts = - error && expr.getPos() ? - std::unique_ptr( - 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( + *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 vm(mapStaticEnvBindings(evalState->symbols, *expr.staticenv.get(), env)); + if (expr.staticEnv) { + auto vm = mapStaticEnvBindings(evalState->symbols, *expr.staticEnv.get(), env); runRepl(evalState, expr, *vm); } }; diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index db4d4c023..354877bc5 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -275,6 +275,7 @@ void printClosureDiff( void runRepl( ref evalState, - const Expr &expr, + const Expr & expr, const std::map & extraEnv); + } diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index b94831064..c35f29a2f 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -51,7 +51,7 @@ struct NixRepl ref 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 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 & 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 & 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(*(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({}); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 10dba69e7..ca0eec6e3 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -466,7 +466,7 @@ EvalState::EvalState( , env1AllocCache(std::make_shared(nullptr)) #endif , baseEnv(allocEnv(128)) - , staticBaseEnv(new StaticEnv(false, 0)) + , staticBaseEnv{std::make_shared(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::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 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(); 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 makeDebugTraceStacker(EvalState &state, Expr &expr, Env &env, - std::optional pos, const char * s, const std::string & s2) +static std::unique_ptr makeDebugTraceStacker( + EvalState & state, + Expr & expr, + Env & env, + std::optional pos, + const char * s, + const std::string & s2) { - return std::unique_ptr( - new DebugTraceStacker( - state, - DebugTrace - {.pos = pos, - .expr = expr, - .env = env, - .hint = hintfmt(s, s2), - .is_error = false - })); + return std::make_unique(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); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 2e7df13fc..f3852e248 100644 --- a/src/libexpr/eval.hh +++ b/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 mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & env); void copyContext(const Value & v, PathSet & context); @@ -75,10 +75,10 @@ std::shared_ptr makeRegexCache(); struct DebugTrace { std::optional 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'. */ diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 5624c4780..213cf93fa 100644 --- a/src/libexpr/nixexpr.cc +++ b/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 &env) +void Expr::bindVars(const EvalState & es, const std::shared_ptr & env) { abort(); } -void ExprInt::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprInt::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env;} + +void ExprFloat::bindVars(const EvalState & es, const std::shared_ptr & env) +{ + if (debuggerHook) + staticEnv = env; } -void ExprFloat::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprString::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env; } -void ExprString::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprPath::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env; } -void ExprPath::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprVar::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; -} - -void ExprVar::bindVars(const EvalState & es, const std::shared_ptr &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_ptrlevel = withLevel; } -void ExprSelect::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprSelect::bindVars(const EvalState & es, const std::shared_ptr & 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_ptrbindVars(es, env); } -void ExprOpHasAttr::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprOpHasAttr::bindVars(const EvalState & es, const std::shared_ptr & 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_ptrbindVars(es, env); } -void ExprAttrs::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprAttrs::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env; if (recursive) { - auto newEnv = std::shared_ptr(new StaticEnv(false, env.get(), recursive ? attrs.size() : 0)); + auto newEnv = std::make_shared(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 &env) +void ExprList::bindVars(const EvalState & es, const std::shared_ptr & 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 &env) +void ExprLambda::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env; - auto newEnv = std::shared_ptr( - new StaticEnv( - false, env.get(), - (hasFormals() ? formals->formals.size() : 0) + - (!arg ? 0 : 1))); + auto newEnv = std::make_shared( + 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_ptrbindVars(es, newEnv); } -void ExprCall::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprCall::bindVars(const EvalState & es, const std::shared_ptr & 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 &env) +void ExprLet::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env; - auto newEnv = std::shared_ptr(new StaticEnv(false, env.get(), attrs->attrs.size())); + auto newEnv = std::make_shared(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_ptrbindVars(es, newEnv); } -void ExprWith::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprWith::bindVars(const EvalState & es, const std::shared_ptr & 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_ptrbindVars(es, env); - auto newEnv = std::shared_ptr(new StaticEnv(true, env.get())); + auto newEnv = std::make_shared(true, env.get()); body->bindVars(es, newEnv); } -void ExprIf::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprIf::bindVars(const EvalState & es, const std::shared_ptr & 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 &env) +void ExprAssert::bindVars(const EvalState & es, const std::shared_ptr & 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 &env) +void ExprOpNot::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; + staticEnv = env; e->bindVars(es, env); } -void ExprConcatStrings::bindVars(const EvalState & es, const std::shared_ptr &env) +void ExprConcatStrings::bindVars(const EvalState & es, const std::shared_ptr & 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 &env) +void ExprPos::bindVars(const EvalState & es, const std::shared_ptr & env) { if (debuggerHook) - staticenv = env; - + staticEnv = env; } diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 82fff6dcf..c4a509f31 100644 --- a/src/libexpr/nixexpr.hh +++ b/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 staticenv; - virtual const PosIdx getPos() const = 0; + std::shared_ptr 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 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 && 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 > * es; ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector > * 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 }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 41476abe3..27976dc74 100644 --- a/src/libexpr/primops.cc +++ b/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(new StaticEnv(false, state.staticBaseEnv.get(), vScope->attrs->size())); + auto staticEnv = std::make_shared(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]);