From 2a8fe9a93837733e9dd9ed5c078734a35b203e14 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Fri, 2 Feb 2024 18:53:49 -0800 Subject: [PATCH 1/2] `:quit` in the debugger should quit the whole program --- src/libcmd/repl.cc | 63 ++++++++++++++++++++++++--------- src/libcmd/repl.hh | 4 +-- src/libexpr/eval.cc | 14 ++++++-- src/libexpr/eval.hh | 5 ++- src/libexpr/primops.cc | 11 +----- src/libexpr/repl-exit-status.hh | 20 +++++++++++ src/libmain/shared.cc | 2 -- src/libmain/shared.hh | 10 +----- src/libutil/exit.cc | 7 ++++ src/libutil/exit.hh | 19 ++++++++++ 10 files changed, 111 insertions(+), 44 deletions(-) create mode 100644 src/libexpr/repl-exit-status.hh create mode 100644 src/libutil/exit.cc create mode 100644 src/libutil/exit.hh diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 03602e170..e423df3fe 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -52,6 +52,27 @@ extern "C" { namespace nix { +/** + * Returned by `NixRepl::processLine`. + */ +enum class ProcessLineResult { + /** + * The user exited with `:quit`. The REPL should exit. The surrounding + * program or evaluation (e.g., if the REPL was acting as the debugger) + * should also exit. + */ + QuitAll, + /** + * The user exited with `:continue`. The REPL should exit, but the program + * should continue running. + */ + QuitOnce, + /** + * The user did not exit. The REPL should request another line of input. + */ + Continue, +}; + struct NixRepl : AbstractNixRepl #if HAVE_BOEHMGC @@ -75,13 +96,13 @@ struct NixRepl std::function getValues); virtual ~NixRepl(); - void mainLoop() override; + ReplExitStatus mainLoop() override; void initEnv() override; StringSet completePrefix(const std::string & prefix); bool getLine(std::string & input, const std::string & prompt); StorePath getDerivationPath(Value & v); - bool processLine(std::string line); + ProcessLineResult processLine(std::string line); void loadFile(const Path & path); void loadFlake(const std::string & flakeRef); @@ -246,7 +267,7 @@ static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positi static bool isFirstRepl = true; -void NixRepl::mainLoop() +ReplExitStatus NixRepl::mainLoop() { if (isFirstRepl) { std::string_view debuggerNotice = ""; @@ -287,15 +308,25 @@ void NixRepl::mainLoop() // 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> " : " ")) { - // ctrl-D should exit the debugger. + // Ctrl-D should exit the debugger. state->debugStop = false; - state->debugQuit = true; logger->cout(""); - break; + // TODO: Should Ctrl-D exit just the current debugger session or + // the entire program? + return ReplExitStatus::QuitAll; } logger->resume(); try { - if (!removeWhitespace(input).empty() && !processLine(input)) return; + switch (processLine(input)) { + case ProcessLineResult::QuitAll: + return ReplExitStatus::QuitAll; + case ProcessLineResult::QuitOnce: + return ReplExitStatus::Continue; + case ProcessLineResult::Continue: + break; + default: + abort(); + } } catch (ParseError & e) { if (e.msg().find("unexpected end of file") != std::string::npos) { // For parse errors on incomplete input, we continue waiting for the next line of @@ -483,10 +514,11 @@ void NixRepl::loadDebugTraceEnv(DebugTrace & dt) } } -bool NixRepl::processLine(std::string line) +ProcessLineResult NixRepl::processLine(std::string line) { line = trim(line); - if (line == "") return true; + if (line.empty()) + return ProcessLineResult::Continue; _isInterrupted = false; @@ -581,13 +613,13 @@ bool NixRepl::processLine(std::string line) else if (state->debugRepl && (command == ":s" || command == ":step")) { // set flag to stop at next DebugTrace; exit repl. state->debugStop = true; - return false; + return ProcessLineResult::QuitOnce; } else if (state->debugRepl && (command == ":c" || command == ":continue")) { // set flag to run to next breakpoint or end of program; exit repl. state->debugStop = false; - return false; + return ProcessLineResult::QuitOnce; } else if (command == ":a" || command == ":add") { @@ -730,8 +762,7 @@ bool NixRepl::processLine(std::string line) else if (command == ":q" || command == ":quit") { state->debugStop = false; - state->debugQuit = true; - return false; + return ProcessLineResult::QuitAll; } else if (command == ":doc") { @@ -792,7 +823,7 @@ bool NixRepl::processLine(std::string line) } } - return true; + return ProcessLineResult::Continue; } void NixRepl::loadFile(const Path & path) @@ -923,7 +954,7 @@ std::unique_ptr AbstractNixRepl::create( } -void AbstractNixRepl::runSimple( +ReplExitStatus AbstractNixRepl::runSimple( ref evalState, const ValMap & extraEnv) { @@ -945,7 +976,7 @@ void AbstractNixRepl::runSimple( for (auto & [name, value] : extraEnv) repl->addVarToScope(repl->state->symbols.create(name), *value); - repl->mainLoop(); + return repl->mainLoop(); } } diff --git a/src/libcmd/repl.hh b/src/libcmd/repl.hh index 6d88883fe..21aa8bfc7 100644 --- a/src/libcmd/repl.hh +++ b/src/libcmd/repl.hh @@ -28,13 +28,13 @@ struct AbstractNixRepl const SearchPath & searchPath, nix::ref store, ref state, std::function getValues); - static void runSimple( + static ReplExitStatus runSimple( ref evalState, const ValMap & extraEnv); virtual void initEnv() = 0; - virtual void mainLoop() = 0; + virtual ReplExitStatus mainLoop() = 0; }; } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 832c8369a..3de26bd1e 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -3,6 +3,7 @@ #include "hash.hh" #include "primops.hh" #include "print-options.hh" +#include "shared.hh" #include "types.hh" #include "util.hh" #include "store-api.hh" @@ -416,7 +417,6 @@ EvalState::EvalState( , buildStore(buildStore ? buildStore : store) , debugRepl(nullptr) , debugStop(false) - , debugQuit(false) , trylevel(0) , regexCache(makeRegexCache()) #if HAVE_BOEHMGC @@ -792,7 +792,17 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & auto se = getStaticEnv(expr); if (se) { auto vm = mapStaticEnvBindings(symbols, *se.get(), env); - (debugRepl)(ref(shared_from_this()), *vm); + auto exitStatus = (debugRepl)(ref(shared_from_this()), *vm); + switch (exitStatus) { + case ReplExitStatus::QuitAll: + if (error) + throw *error; + throw Exit(0); + case ReplExitStatus::Continue: + break; + default: + abort(); + } } } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 8e639a1fa..42fe0d3e4 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -11,6 +11,7 @@ #include "experimental-features.hh" #include "input-accessor.hh" #include "search-path.hh" +#include "repl-exit-status.hh" #include #include @@ -219,9 +220,8 @@ public: /** * Debugger */ - void (* debugRepl)(ref es, const ValMap & extraEnv); + ReplExitStatus (* debugRepl)(ref es, const ValMap & extraEnv); bool debugStop; - bool debugQuit; int trylevel; std::list debugTraces; std::map> exprEnvs; @@ -758,7 +758,6 @@ struct DebugTraceStacker { DebugTraceStacker(EvalState & evalState, DebugTrace t); ~DebugTraceStacker() { - // assert(evalState.debugTraces.front() == trace); evalState.debugTraces.pop_front(); } EvalState & evalState; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 731485133..5b3b2f11a 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -760,15 +760,6 @@ static RegisterPrimOp primop_break({ auto & dt = state.debugTraces.front(); state.runDebugRepl(&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 the debugger"), - .pos = nullptr, - }); - } } // Return the value we were passed. @@ -879,7 +870,7 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value * * args, Va /* increment state.trylevel, and decrement it when this function returns. */ MaintainCount trylevel(state.trylevel); - void (* savedDebugRepl)(ref es, const ValMap & extraEnv) = nullptr; + ReplExitStatus (* savedDebugRepl)(ref es, const ValMap & extraEnv) = nullptr; if (state.debugRepl && evalSettings.ignoreExceptionsDuringTry) { /* to prevent starting the repl from exceptions withing a tryEval, null it. */ diff --git a/src/libexpr/repl-exit-status.hh b/src/libexpr/repl-exit-status.hh new file mode 100644 index 000000000..08299ff61 --- /dev/null +++ b/src/libexpr/repl-exit-status.hh @@ -0,0 +1,20 @@ +#pragma once + +namespace nix { + +/** + * Exit status returned from the REPL. + */ +enum class ReplExitStatus { + /** + * The user exited with `:quit`. The program (e.g., if the REPL was acting + * as the debugger) should exit. + */ + QuitAll, + /** + * The user exited with `:continue`. The program should continue running. + */ + Continue, +}; + +} diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 7b9b3c5b5..7bced0aa4 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -408,6 +408,4 @@ PrintFreed::~PrintFreed() showBytes(results.bytesFreed)); } -Exit::~Exit() { } - } diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh index c68f6cd83..99c3dffab 100644 --- a/src/libmain/shared.hh +++ b/src/libmain/shared.hh @@ -7,6 +7,7 @@ #include "common-args.hh" #include "path.hh" #include "derived-path.hh" +#include "exit.hh" #include @@ -15,15 +16,6 @@ namespace nix { -class Exit : public std::exception -{ -public: - int status; - Exit() : status(0) { } - Exit(int status) : status(status) { } - virtual ~Exit(); -}; - int handleExceptions(const std::string & programName, std::function fun); /** diff --git a/src/libutil/exit.cc b/src/libutil/exit.cc new file mode 100644 index 000000000..73cd8b04e --- /dev/null +++ b/src/libutil/exit.cc @@ -0,0 +1,7 @@ +#include "exit.hh" + +namespace nix { + +Exit::~Exit() {} + +} diff --git a/src/libutil/exit.hh b/src/libutil/exit.hh new file mode 100644 index 000000000..55f33e62f --- /dev/null +++ b/src/libutil/exit.hh @@ -0,0 +1,19 @@ +#pragma once + +#include + +namespace nix { + +/** + * Exit the program with a given exit code. + */ +class Exit : public std::exception +{ +public: + int status; + Exit() : status(0) { } + explicit Exit(int status) : status(status) { } + virtual ~Exit(); +}; + +} From 8e71883e3f59100479e96aa1883ef52dbaa03fd3 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Tue, 20 Feb 2024 14:52:16 -0800 Subject: [PATCH 2/2] Rename `ProcessLineResult` variants --- src/libcmd/repl.cc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index e423df3fe..42ec0f709 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -61,16 +61,16 @@ enum class ProcessLineResult { * program or evaluation (e.g., if the REPL was acting as the debugger) * should also exit. */ - QuitAll, + Quit, /** * The user exited with `:continue`. The REPL should exit, but the program * should continue running. */ - QuitOnce, + Continue, /** * The user did not exit. The REPL should request another line of input. */ - Continue, + PromptAgain, }; struct NixRepl @@ -318,11 +318,11 @@ ReplExitStatus NixRepl::mainLoop() logger->resume(); try { switch (processLine(input)) { - case ProcessLineResult::QuitAll: + case ProcessLineResult::Quit: return ReplExitStatus::QuitAll; - case ProcessLineResult::QuitOnce: - return ReplExitStatus::Continue; case ProcessLineResult::Continue: + return ReplExitStatus::Continue; + case ProcessLineResult::PromptAgain: break; default: abort(); @@ -518,7 +518,7 @@ ProcessLineResult NixRepl::processLine(std::string line) { line = trim(line); if (line.empty()) - return ProcessLineResult::Continue; + return ProcessLineResult::PromptAgain; _isInterrupted = false; @@ -613,13 +613,13 @@ ProcessLineResult NixRepl::processLine(std::string line) else if (state->debugRepl && (command == ":s" || command == ":step")) { // set flag to stop at next DebugTrace; exit repl. state->debugStop = true; - return ProcessLineResult::QuitOnce; + return ProcessLineResult::Continue; } else if (state->debugRepl && (command == ":c" || command == ":continue")) { // set flag to run to next breakpoint or end of program; exit repl. state->debugStop = false; - return ProcessLineResult::QuitOnce; + return ProcessLineResult::Continue; } else if (command == ":a" || command == ":add") { @@ -762,7 +762,7 @@ ProcessLineResult NixRepl::processLine(std::string line) else if (command == ":q" || command == ":quit") { state->debugStop = false; - return ProcessLineResult::QuitAll; + return ProcessLineResult::Quit; } else if (command == ":doc") { @@ -823,7 +823,7 @@ ProcessLineResult NixRepl::processLine(std::string line) } } - return ProcessLineResult::Continue; + return ProcessLineResult::PromptAgain; } void NixRepl::loadFile(const Path & path)