From 70b10362247ef563ef4cad02709ec2bdb5564206 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 22 May 2024 12:51:46 +0200 Subject: [PATCH] builtins.warn: Use new EvalBaseError + "evaluation warning" --- doc/manual/rl-next/builtins-warn.md | 10 ++++++++++ src/libexpr/eval-error.cc | 8 ++++++++ src/libexpr/eval-error.hh | 20 +++++++++++++++++--- src/libexpr/primops.cc | 12 +++++++++--- src/libutil/error.cc | 5 ++++- src/libutil/error.hh | 5 +++++ 6 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 doc/manual/rl-next/builtins-warn.md diff --git a/doc/manual/rl-next/builtins-warn.md b/doc/manual/rl-next/builtins-warn.md new file mode 100644 index 000000000..0f7a6ebba --- /dev/null +++ b/doc/manual/rl-next/builtins-warn.md @@ -0,0 +1,10 @@ +--- +synopsis: "New builtin: `builtins.warn`" +issues: 306026 +prs: 10592 +--- + +`builtins.warn` behaves like `builtins.trace "warning: ${msg}`, has an accurate log level, and is controlled by the options +[`debugger-on-trace`](@docroot@/command-ref/conf-file.md#conf-debugger-on-trace), +[`debugger-on-warn`](@docroot@/command-ref/conf-file.md#conf-debugger-on-warn) and +[`abort-on-warn`](@docroot@/command-ref/conf-file.md#conf-abort-on-warn). diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc index 5a0c67514..bd84e0428 100644 --- a/src/libexpr/eval-error.cc +++ b/src/libexpr/eval-error.cc @@ -70,6 +70,13 @@ EvalErrorBuilder::addTrace(PosIdx pos, std::string_view formatString, const A return *this; } +template +EvalErrorBuilder & EvalErrorBuilder::setIsFromExpr() +{ + error.err.isFromExpr = true; + return *this; +} + template void EvalErrorBuilder::debugThrow() { @@ -85,6 +92,7 @@ void EvalErrorBuilder::debugThrow() throw error; } +template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; diff --git a/src/libexpr/eval-error.hh b/src/libexpr/eval-error.hh index 27407eb6e..fe48e054b 100644 --- a/src/libexpr/eval-error.hh +++ b/src/libexpr/eval-error.hh @@ -15,27 +15,39 @@ class EvalState; template class EvalErrorBuilder; -class EvalError : public Error +/** + * Base class for all errors that occur during evaluation. + * + * Most subclasses should inherit from `EvalError` instead of this class. + */ +class EvalBaseError : public Error { template friend class EvalErrorBuilder; public: EvalState & state; - EvalError(EvalState & state, ErrorInfo && errorInfo) + EvalBaseError(EvalState & state, ErrorInfo && errorInfo) : Error(errorInfo) , state(state) { } template - explicit EvalError(EvalState & state, const std::string & formatString, const Args &... formatArgs) + explicit EvalBaseError(EvalState & state, const std::string & formatString, const Args &... formatArgs) : Error(formatString, formatArgs...) , state(state) { } }; +/** + * `EvalError` is the base class for almost all errors that occur during evaluation. + * + * All instances of `EvalError` should show a degree of purity that allows them to be + * cached in pure mode. This means that they should not depend on the configuration or the overall environment. + */ +MakeError(EvalError, EvalBaseError); MakeError(ParseError, Error); MakeError(AssertionError, EvalError); MakeError(ThrownError, AssertionError); @@ -90,6 +102,8 @@ public: [[nodiscard, gnu::noinline]] EvalErrorBuilder & addTrace(PosIdx pos, HintFmt hint); + [[nodiscard, gnu::noinline]] EvalErrorBuilder & setIsFromExpr(); + template [[nodiscard, gnu::noinline]] EvalErrorBuilder & addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9741e3177..22d7f188f 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -806,7 +806,7 @@ static RegisterPrimOp primop_abort({ NixStringContext context; auto s = state.coerceToString(pos, *args[0], context, "while evaluating the error message passed to builtins.abort").toOwned(); - state.error("evaluation aborted with the following error message: '%1%'", s).debugThrow(); + state.error("evaluation aborted with the following error message: '%1%'", s).setIsFromExpr().debugThrow(); } }); @@ -825,7 +825,7 @@ static RegisterPrimOp primop_throw({ NixStringContext context; auto s = state.coerceToString(pos, *args[0], context, "while evaluating the error message passed to builtin.throw").toOwned(); - state.error(s).debugThrow(); + state.error(s).setIsFromExpr().debugThrow(); } }); @@ -1052,12 +1052,13 @@ static void prim_warn(EvalState & state, const PosIdx pos, Value * * args, Value msg.atPos(state.positions[pos]); auto info = msg.info(); info.level = lvlWarn; + info.isFromExpr = true; logWarning(info); } if (evalSettings.builtinsAbortOnWarn) { // Not an EvalError or subclass, which would cause the error to be stored in the eval cache. - state.error("aborting to reveal stack trace of warning, as abort-on-warn is set").debugThrow(); + state.error("aborting to reveal stack trace of warning, as abort-on-warn is set").setIsFromExpr().debugThrow(); } if (evalSettings.builtinsTraceDebugger || evalSettings.builtinsDebuggerOnWarn) { state.runDebugRepl(nullptr); @@ -1080,6 +1081,11 @@ static RegisterPrimOp primop_warn({ option is set to `true` and the `--debugger` flag is given, the interactive debugger will be started when `warn` is called (like [`break`](@docroot@/language/builtins.md#builtins-break)). + + If the + [`abort-on-warn`](@docroot@/command-ref/conf-file.md#conf-abort-on-warn) + option is set, the evaluation will be aborted after the warning is printed. + This is useful to reveal the stack trace of the warning, when the context is non-interactive and a debugger can not be launched. )", .fun = prim_warn, }); diff --git a/src/libutil/error.cc b/src/libutil/error.cc index fd4f4efd1..e01f06448 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -240,7 +240,10 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s break; } case Verbosity::lvlWarn: { - prefix = ANSI_WARNING "warning"; + if (einfo.isFromExpr) + prefix = ANSI_WARNING "evaluation warning"; + else + prefix = ANSI_WARNING "warning"; break; } case Verbosity::lvlInfo: { diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 87d181c94..269000016 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -89,6 +89,11 @@ struct ErrorInfo { HintFmt msg; std::shared_ptr pos; std::list traces; + /** + * Some messages are generated directly by expressions; notably `builtins.warn`, `abort`, `throw`. + * These may be rendered differently, so that users can distinguish them. + */ + bool isFromExpr = false; /** * Exit status.