From 985afeeb4d92f878b785f7906dd34679e7ba7688 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 27 Nov 2024 02:09:08 +0100 Subject: [PATCH] libexpr: allocate debug state only when debugger is active Change-Id: Id30f388264c5d1e472e3bdce5078db3914f3b475 --- lix/libcmd/command.cc | 2 +- lix/libcmd/repl.cc | 41 +++++++++++---------- lix/libexpr/eval-error.cc | 21 ++++++----- lix/libexpr/eval.cc | 22 ++++++------ lix/libexpr/eval.hh | 10 ++++-- lix/libexpr/nixexpr.cc | 76 +++++++++++++++++++-------------------- lix/libexpr/primops.cc | 33 +++++++++-------- 7 files changed, 108 insertions(+), 97 deletions(-) diff --git a/lix/libcmd/command.cc b/lix/libcmd/command.cc index f02c3bbb5..10b5a6472 100644 --- a/lix/libcmd/command.cc +++ b/lix/libcmd/command.cc @@ -118,7 +118,7 @@ ref EvalCommand::getEvalState() evalState->repair = repair; if (startReplOnEvalErrors) { - evalState->debug.repl = &AbstractNixRepl::runSimple; + evalState->debug = std::make_unique(&AbstractNixRepl::runSimple); }; } return ref(evalState); diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 2b9eaa122..599086118 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -285,7 +285,7 @@ ReplExitStatus NixRepl::mainLoop() { if (isFirstRepl) { std::string_view debuggerNotice = ""; - if (state.debug.inDebugger) { + if (state.debug && state.debug->inDebugger) { debuggerNotice = " debugger"; } notice("Lix %1%%2%\nType :? for help.", nixVersion, debuggerNotice); @@ -310,7 +310,9 @@ ReplExitStatus NixRepl::mainLoop() // number of chars as the prompt. if (!interacter->getLine(input, input.empty() ? ReplPromptType::ReplPrompt : ReplPromptType::ContinuationPrompt)) { // Ctrl-D should exit the debugger. - state.debug.stop = false; + if (state.debug) { + state.debug->stop = false; + } logger->cout(""); // TODO: Should Ctrl-D exit just the current debugger session or // the entire program? @@ -366,7 +368,7 @@ StringSet NixRepl::completePrefix(const std::string & prefix) } } - if (state.debug.inDebugger) { + if (state.debug && state.debug->inDebugger) { for (auto const & colonCmd : this->DEBUG_COMMANDS) { if (colonCmd.starts_with(prefix)) { completions.insert(std::string(colonCmd)); @@ -416,9 +418,8 @@ StringSet NixRepl::completePrefix(const std::string & prefix) } } else { /* Temporarily disable the debugger, to avoid re-entering readline. */ - auto debug_repl = state.debug.repl; - state.debug.repl = nullptr; - Finally restoreDebug([&]() { state.debug.repl = debug_repl; }); + auto debug = std::move(state.debug); + Finally restoreDebug([&]() { state.debug = std::move(debug); }); try { /* This is an expression that should evaluate to an attribute set. Evaluate it to get the names of the @@ -485,7 +486,7 @@ void NixRepl::loadDebugTraceEnv(DebugTrace & dt) { initEnv(); - auto se = state.debug.staticEnvFor(dt.expr); + auto se = state.debug->staticEnvFor(dt.expr); if (se) { auto vm = mapStaticEnvBindings(state.symbols, *se.get(), dt.env); @@ -541,7 +542,7 @@ ProcessLineResult NixRepl::processLine(std::string line) << " errors\n" << " :?, :help Brings up this help menu\n" ; - if (state.debug.inDebugger) { + if (state.debug && state.debug->inDebugger) { std::cout << "\n" << " Debug mode commands\n" @@ -556,15 +557,15 @@ ProcessLineResult NixRepl::processLine(std::string line) } - else if (state.debug.inDebugger && (command == ":bt" || command == ":backtrace")) { - for (const auto & [idx, i] : enumerate(state.debug.traces)) { + else if (state.debug && state.debug->inDebugger && (command == ":bt" || command == ":backtrace")) { + for (const auto & [idx, i] : enumerate(state.debug->traces)) { std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": "; showDebugTrace(std::cout, state.positions, i); } } - else if (state.debug.inDebugger && (command == ":env")) { - for (const auto & [idx, i] : enumerate(state.debug.traces)) { + else if (state.debug && state.debug->inDebugger && (command == ":env")) { + for (const auto & [idx, i] : enumerate(state.debug->traces)) { if (idx == debugTraceIndex) { printEnvBindings(state, i.expr, i.env); break; @@ -572,13 +573,13 @@ ProcessLineResult NixRepl::processLine(std::string line) } } - else if (state.debug.inDebugger && (command == ":st")) { + else if (state.debug && state.debug->inDebugger && (command == ":st")) { try { // change the DebugTrace index. debugTraceIndex = stoi(arg); } catch (...) { } - for (const auto & [idx, i] : enumerate(state.debug.traces)) { + for (const auto & [idx, i] : enumerate(state.debug->traces)) { if (idx == debugTraceIndex) { std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": "; showDebugTrace(std::cout, state.positions, i); @@ -590,15 +591,15 @@ ProcessLineResult NixRepl::processLine(std::string line) } } - else if (state.debug.inDebugger && (command == ":s" || command == ":step")) { + else if (state.debug && state.debug->inDebugger && (command == ":s" || command == ":step")) { // set flag to stop at next DebugTrace; exit repl. - state.debug.stop = true; + state.debug->stop = true; return ProcessLineResult::Continue; } - else if (state.debug.inDebugger && (command == ":c" || command == ":continue")) { + else if (state.debug && state.debug->inDebugger && (command == ":c" || command == ":continue")) { // set flag to run to next breakpoint or end of program; exit repl. - state.debug.stop = false; + state.debug->stop = false; return ProcessLineResult::Continue; } @@ -771,7 +772,9 @@ ProcessLineResult NixRepl::processLine(std::string line) } else if (command == ":q" || command == ":quit") { - state.debug.stop = false; + if (state.debug) { + state.debug->stop = false; + } return ProcessLineResult::Quit; } diff --git a/lix/libexpr/eval-error.cc b/lix/libexpr/eval-error.cc index 1fa43e81e..d32c5734d 100644 --- a/lix/libexpr/eval-error.cc +++ b/lix/libexpr/eval-error.cc @@ -45,12 +45,15 @@ EvalErrorBuilder & EvalErrorBuilder::withFrame(const Env & env, const Expr // NOTE: This is abusing side-effects. // TODO: check compatibility with nested debugger calls. // TODO: What side-effects?? - error.state.debug.traces.push_front(DebugTrace{ - .pos = error.state.positions[expr.getPos()], - .expr = expr, - .env = env, - .hint = HintFmt("Fake frame for debugging purposes"), - .isError = true}); + if (error.state.debug) { + error.state.debug->traces.push_front(DebugTrace{ + .pos = error.state.positions[expr.getPos()], + .expr = expr, + .env = env, + .hint = HintFmt("Fake frame for debugging purposes"), + .isError = true + }); + } return *this; } @@ -74,11 +77,11 @@ EvalErrorBuilder::addTrace(PosIdx pos, std::string_view formatString, const A template void EvalErrorBuilder::debugThrow() { - if (error.state.debug.repl && !error.state.debug.traces.empty()) { - const DebugTrace & last = error.state.debug.traces.front(); + if (error.state.debug && !error.state.debug->traces.empty()) { + const DebugTrace & last = error.state.debug->traces.front(); const Env * env = &last.env; const Expr * expr = &last.expr; - error.state.debug.runDebugRepl(error.state, &error, *env, *expr); + error.state.debug->runDebugRepl(error.state, &error, *env, *expr); } // `EvalState` is the only class that can construct an `EvalErrorBuilder`, diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 7f9eefe74..81b907aea 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -587,7 +587,7 @@ void printEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & void printEnvBindings(const EvalState &es, const Expr & expr, const Env & env) { // just print the names for now - auto se = es.debug.staticEnvFor(expr); + auto se = es.debug->staticEnvFor(expr); if (se) printEnvBindings(es.symbols, *se, env, 0); } @@ -641,7 +641,7 @@ void DebugState::runDebugRepl( ) { // Make sure we have a debugger to run and we're not already in a debugger. - if (!repl || inDebugger) + if (inDebugger) return; auto dts = @@ -717,9 +717,9 @@ DebugTraceStacker::DebugTraceStacker(EvalState & evalState, DebugTrace t) : evalState(evalState) , trace(std::move(t)) { - evalState.debug.traces.push_front(trace); - if (evalState.debug.stop && evalState.debug.repl) - evalState.debug.runDebugRepl(evalState, nullptr, trace.env, trace.expr); + evalState.debug->traces.push_front(trace); + if (evalState.debug->stop && evalState.debug->repl) + evalState.debug->runDebugRepl(evalState, nullptr, trace.env, trace.expr); } inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) @@ -954,7 +954,7 @@ void EvalState::cacheFile( fileParseCache[resolvedPath] = e; try { - auto dts = debug.repl + auto dts = debug ? makeDebugTraceStacker( *this, *e, @@ -1172,7 +1172,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v) *i.second.chooseByKind(&env2, &env, inheritEnv)); } - auto dts = state.debug.repl + auto dts = state.debug ? makeDebugTraceStacker( state, *this, @@ -1257,7 +1257,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) } try { - auto dts = state.debug.repl + auto dts = state.debug ? makeDebugTraceStacker( state, *this, @@ -1575,7 +1575,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & /* Evaluate the body. */ try { - auto dts = debug.repl + auto dts = debug ? makeDebugTraceStacker( *this, *lambda.body, env2, positions[lambda.pos], "while calling %s", @@ -1714,7 +1714,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & void ExprCall::eval(EvalState & state, Env & env, Value & v) { - auto dts = state.debug.repl + auto dts = state.debug ? makeDebugTraceStacker( state, *this, @@ -2087,7 +2087,7 @@ void EvalState::forceValueDeep(Value & v) for (auto & i : *v.attrs) try { // If the value is a thunk, we're evaling. Otherwise no trace necessary. - auto dts = debug.repl && i.value->isThunk() + auto dts = debug && i.value->isThunk() ? makeDebugTraceStacker(*this, *i.value->thunk.expr, *i.value->thunk.env, positions[i.pos], "while evaluating the attribute '%1%'", symbols[i.name]) : nullptr; diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index 26a43f08b..50075961d 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -161,6 +161,12 @@ struct DebugState std::map> exprEnvs; int trylevel = 0; + explicit DebugState(std::function repl) + : repl(repl) + { + assert(repl); + } + void runDebugRepl( EvalState & evalState, const EvalError * error, const Env & env, const Expr & expr ); @@ -224,7 +230,7 @@ public: RootValue vCallFlake = nullptr; RootValue vImportedDrvToDerivation = nullptr; - DebugState debug; + std::unique_ptr debug; template [[nodiscard, gnu::noinline]] @@ -759,7 +765,7 @@ struct DebugTraceStacker { DebugTraceStacker(EvalState & evalState, DebugTrace t); ~DebugTraceStacker() { - evalState.debug.traces.pop_front(); + evalState.debug->traces.pop_front(); } EvalState & evalState; DebugTrace trace; diff --git a/lix/libexpr/nixexpr.cc b/lix/libexpr/nixexpr.cc index 16a87b51a..c78b0605e 100644 --- a/lix/libexpr/nixexpr.cc +++ b/lix/libexpr/nixexpr.cc @@ -302,32 +302,32 @@ void Expr::bindVars(EvalState & es, const std::shared_ptr & env void ExprInt::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); } void ExprFloat::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); } void ExprString::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); } void ExprPath::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); } void ExprVar::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); fromWith = nullptr; @@ -364,14 +364,14 @@ void ExprVar::bindVars(EvalState & es, const std::shared_ptr & void ExprInheritFrom::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); } void ExprSelect::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); e->bindVars(es, env); if (def) def->bindVars(es, env); @@ -382,8 +382,8 @@ void ExprSelect::bindVars(EvalState & es, const std::shared_ptr void ExprOpHasAttr::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); e->bindVars(es, env); for (auto & i : attrPath) @@ -414,8 +414,8 @@ std::shared_ptr ExprAttrs::bindInheritSources( void ExprAttrs::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); if (recursive) { auto newEnv = [&] () -> std::shared_ptr { @@ -453,8 +453,8 @@ void ExprAttrs::bindVars(EvalState & es, const std::shared_ptr void ExprList::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); for (auto & i : elems) i->bindVars(es, env); @@ -462,8 +462,8 @@ void ExprList::bindVars(EvalState & es, const std::shared_ptr & void ExprLambda::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); auto newEnv = std::make_shared( nullptr, env.get(), @@ -489,8 +489,8 @@ void ExprLambda::bindVars(EvalState & es, const std::shared_ptr void ExprCall::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); fun->bindVars(es, env); for (auto & e : args) @@ -514,16 +514,16 @@ void ExprLet::bindVars(EvalState & es, const std::shared_ptr & for (auto & i : attrs->attrs) i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv)); - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); body->bindVars(es, newEnv); } void ExprWith::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); parentWith = nullptr; for (auto * e = env.get(); e && !parentWith; e = e->up) @@ -548,8 +548,8 @@ void ExprWith::bindVars(EvalState & es, const std::shared_ptr & void ExprIf::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); cond->bindVars(es, env); then->bindVars(es, env); @@ -558,8 +558,8 @@ void ExprIf::bindVars(EvalState & es, const std::shared_ptr & e void ExprAssert::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); cond->bindVars(es, env); body->bindVars(es, env); @@ -567,16 +567,16 @@ void ExprAssert::bindVars(EvalState & es, const std::shared_ptr void ExprOpNot::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); e->bindVars(es, env); } void ExprConcatStrings::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); for (auto & i : this->es) i.second->bindVars(es, env); @@ -584,8 +584,8 @@ void ExprConcatStrings::bindVars(EvalState & es, const std::shared_ptr & env) { - if (es.debug.repl) - es.debug.exprEnvs.insert(std::make_pair(this, env)); + if (es.debug) + es.debug->exprEnvs.insert(std::make_pair(this, env)); } diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index b6662c638..c59903469 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -566,15 +566,15 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a static void prim_break(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - if (state.debug.repl && !state.debug.traces.empty()) { + if (state.debug && !state.debug->traces.empty()) { auto error = EvalError(state, ErrorInfo { .level = lvlInfo, .msg = HintFmt("breakpoint reached"), .pos = state.positions[pos], }); - auto & dt = state.debug.traces.front(); - state.debug.runDebugRepl(state, &error, dt.env, dt.expr); + auto & dt = state.debug->traces.front(); + state.debug->runDebugRepl(state, &error, dt.env, dt.expr); } // Return the value we were passed. @@ -637,15 +637,14 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value * * args, Va { auto attrs = state.buildBindings(2); - /* increment state.trylevel, and decrement it when this function returns. */ - MaintainCount trylevel(state.debug.trylevel); - - std::function savedDebugRepl; - if (state.debug.repl && evalSettings.ignoreExceptionsDuringTry) - { - /* to prevent starting the repl from exceptions withing a tryEval, null it. */ - savedDebugRepl = state.debug.repl; - state.debug.repl = nullptr; + std::optional> trylevel; + std::unique_ptr savedDebug; + if (state.debug) { + trylevel.emplace(state.debug->trylevel); + if (evalSettings.ignoreExceptionsDuringTry) { + /* to prevent starting the repl from exceptions withing a tryEval, null it. */ + savedDebug = std::move(state.debug); + } } try { @@ -658,8 +657,8 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value * * args, Va } // restore the debugRepl pointer if we saved it earlier. - if (savedDebugRepl) - state.debug.repl = savedDebugRepl; + if (savedDebug) + state.debug = std::move(savedDebug); v.mkAttrs(attrs); } @@ -697,9 +696,9 @@ static void prim_trace(EvalState & state, const PosIdx pos, Value * * args, Valu printError("trace: %1%", args[0]->string.s); else printError("trace: %1%", ValuePrinter(state, *args[0])); - if (evalSettings.builtinsTraceDebugger && state.debug.repl && !state.debug.traces.empty()) { - const DebugTrace & last = state.debug.traces.front(); - state.debug.runDebugRepl(state, nullptr, last.env, last.expr); + if (evalSettings.builtinsTraceDebugger && state.debug && !state.debug->traces.empty()) { + const DebugTrace & last = state.debug->traces.front(); + state.debug->runDebugRepl(state, nullptr, last.env, last.expr); } state.forceValue(*args[1], pos); v = *args[1];