Merge pull request #10592 from hercules-ci/builtins-warn

Add `builtins.warn`

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).

src/libexpr/eval-error.cc

@@ -70,15 +70,17 @@ EvalErrorBuilder<T>::addTrace(PosIdx pos, std::string_view formatString, const A
     return *this;
 }
 
+template<class T>
+EvalErrorBuilder<T> & EvalErrorBuilder<T>::setIsFromExpr()
+{
+    error.err.isFromExpr = true;
+    return *this;
+}
+
 template<class T>
 void EvalErrorBuilder<T>::debugThrow()
 {
-    if (error.state.debugRepl && !error.state.debugTraces.empty()) {
-        const DebugTrace & last = error.state.debugTraces.front();
-        const Env * env = &last.env;
-        const Expr * expr = &last.expr;
-        error.state.runDebugRepl(&error, *env, *expr);
-    }
+    error.state.runDebugRepl(&error);
 
     // `EvalState` is the only class that can construct an `EvalErrorBuilder`,
     // and it does so in dynamic storage. This is the final method called on
@@ -90,6 +92,7 @@ void EvalErrorBuilder<T>::debugThrow()
     throw error;
 }
 
+template class EvalErrorBuilder<EvalBaseError>;
 template class EvalErrorBuilder<EvalError>;
 template class EvalErrorBuilder<AssertionError>;
 template class EvalErrorBuilder<ThrownError>;

src/libexpr/eval-error.hh

@@ -15,27 +15,39 @@ class EvalState;
 template<class T>
 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<class T>
     friend class EvalErrorBuilder;
 public:
     EvalState & state;
 
-    EvalError(EvalState & state, ErrorInfo && errorInfo)
+    EvalBaseError(EvalState & state, ErrorInfo && errorInfo)
         : Error(errorInfo)
         , state(state)
     {
     }
 
     template<typename... Args>
-    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<T> & addTrace(PosIdx pos, HintFmt hint);
 
+    [[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & setIsFromExpr();
+
     template<typename... Args>
     [[nodiscard, gnu::noinline]] EvalErrorBuilder<T> &
     addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs);

src/libexpr/eval-settings.cc

@@ -48,6 +48,10 @@ EvalSettings::EvalSettings()
 {
     auto var = getEnv("NIX_PATH");
     if (var) nixPath = parseNixPath(*var);
+
+    var = getEnv("NIX_ABORT_ON_WARN");
+    if (var && (var == "1" || var == "yes" || var == "true"))
+        builtinsAbortOnWarn = true;
 }
 
 Strings EvalSettings::getDefaultNixPath()

src/libexpr/eval-settings.hh

@@ -158,13 +158,39 @@ struct EvalSettings : Config
 
     Setting<bool> builtinsTraceDebugger{this, false, "debugger-on-trace",
         R"(
-          If set to true and the `--debugger` flag is given,
-          [`builtins.trace`](@docroot@/language/builtins.md#builtins-trace) will
-          enter the debugger like
-          [`builtins.break`](@docroot@/language/builtins.md#builtins-break).
+          If set to true and the `--debugger` flag is given, the following functions
+          will enter the debugger like [`builtins.break`](@docroot@/language/builtins.md#builtins-break).
+
+          * [`builtins.trace`](@docroot@/language/builtins.md#builtins-trace)
+          * [`builtins.traceVerbose`](@docroot@/language/builtins.md#builtins-traceVerbose)
+            if [`trace-verbose`](#conf-trace-verbose) is set to true.
+          * [`builtins.warn`](@docroot@/language/builtins.md#builtins-warn)
 
           This is useful for debugging warnings in third-party Nix code.
         )"};
+
+    Setting<bool> builtinsDebuggerOnWarn{this, false, "debugger-on-warn",
+        R"(
+          If set to true and the `--debugger` flag is given, [`builtins.warn`](@docroot@/language/builtins.md#builtins-warn)
+          will enter the debugger like [`builtins.break`](@docroot@/language/builtins.md#builtins-break).
+
+          This is useful for debugging warnings in third-party Nix code.
+
+          Use [`debugger-on-trace`](#conf-debugger-on-trace) to also enter the debugger on legacy warnings that are logged with [`builtins.trace`](@docroot@/language/builtins.md#builtins-trace).
+        )"};
+
+    Setting<bool> builtinsAbortOnWarn{this, false, "abort-on-warn",
+        R"(
+          If set to true, [`builtins.warn`](@docroot@/language/builtins.md#builtins-warn) will throw an error when logging a warning.
+
+          This will give you a stack trace that leads to the location of the warning.
+
+          This is useful for finding information about warnings in third-party Nix code when you can not start the interactive debugger, such as when Nix is called from a non-interactive script. See [`debugger-on-warn`](#conf-debugger-on-warn).
+
+          Currently, a stack trace can only be produced when the debugger is enabled, or when evaluation is aborted.
+
+          This option can be enabled by setting `NIX_ABORT_ON_WARN=1` in the environment.
+        )"};
 };
 
 extern EvalSettings evalSettings;

src/libexpr/eval.cc

@@ -785,6 +785,24 @@ public:
     }
 };
 
+bool EvalState::canDebug()
+{
+    return debugRepl && !debugTraces.empty();
+}
+
+void EvalState::runDebugRepl(const Error * error)
+{
+    if (!canDebug())
+        return;
+
+    assert(!debugTraces.empty());
+    const DebugTrace & last = debugTraces.front();
+    const Env & env = last.env;
+    const Expr & expr = last.expr;
+
+    runDebugRepl(error, env, expr);
+}
+
 void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & expr)
 {
     // Make sure we have a debugger to run and we're not already in a debugger.

src/libexpr/eval.hh

@@ -276,6 +276,18 @@ public:
             return std::shared_ptr<const StaticEnv>();;
     }
 
+    /** Whether a debug repl can be started. If `false`, `runDebugRepl(error)` will return without starting a repl. */
+    bool canDebug();
+
+    /** Use front of `debugTraces`; see `runDebugRepl(error,env,expr)` */
+    void runDebugRepl(const Error * error);
+
+    /**
+     * Run a debug repl with the given error, environment and expression.
+     * @param error The error to debug, may be nullptr.
+     * @param env The environment to debug, matching the expression.
+     * @param expr The expression to debug, matching the environment.
+     */
     void runDebugRepl(const Error * error, const Env & env, const Expr & expr);
 
     template<class T, typename... Args>

src/libexpr/primops.cc

@@ -780,15 +780,14 @@ static RegisterPrimOp primop_break({
     )",
     .fun = [](EvalState & state, const PosIdx pos, Value * * args, Value & v)
     {
-        if (state.debugRepl && !state.debugTraces.empty()) {
+        if (state.canDebug()) {
             auto error = Error(ErrorInfo {
                 .level = lvlInfo,
                 .msg = HintFmt("breakpoint reached"),
                 .pos = state.positions[pos],
             });
 
-            auto & dt = state.debugTraces.front();
-            state.runDebugRepl(&error, dt.env, dt.expr);
+            state.runDebugRepl(&error);
         }
 
         // Return the value we were passed.
@@ -807,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<Abort>("evaluation aborted with the following error message: '%1%'", s).debugThrow();
+        state.error<Abort>("evaluation aborted with the following error message: '%1%'", s).setIsFromExpr().debugThrow();
     }
 });
 
@@ -826,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<ThrownError>(s).debugThrow();
+      state.error<ThrownError>(s).setIsFromExpr().debugThrow();
     }
 });
 
@@ -1018,9 +1017,8 @@ static void prim_trace(EvalState & state, const PosIdx pos, Value * * args, Valu
         printError("trace: %1%", args[0]->string_view());
     else
         printError("trace: %1%", ValuePrinter(state, *args[0]));
-    if (evalSettings.builtinsTraceDebugger && state.debugRepl && !state.debugTraces.empty()) {
-        const DebugTrace & last = state.debugTraces.front();
-        state.runDebugRepl(nullptr, last.env, last.expr);
+    if (evalSettings.builtinsTraceDebugger) {
+        state.runDebugRepl(nullptr);
     }
     state.forceValue(*args[1], pos);
     v = *args[1];
@@ -1043,6 +1041,55 @@ static RegisterPrimOp primop_trace({
     .fun = prim_trace,
 });
 
+static void prim_warn(EvalState & state, const PosIdx pos, Value * * args, Value & v)
+{
+    // We only accept a string argument for now. The use case for pretty printing a value is covered by `trace`.
+    // By rejecting non-strings we allow future versions to add more features without breaking existing code.
+    auto msgStr = state.forceString(*args[0], pos, "while evaluating the first argument; the message passed to builtins.warn");
+
+    {
+        BaseError msg(std::string{msgStr});
+        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<EvalBaseError>("aborting to reveal stack trace of warning, as abort-on-warn is set").setIsFromExpr().debugThrow();
+    }
+    if (evalSettings.builtinsTraceDebugger || evalSettings.builtinsDebuggerOnWarn) {
+        state.runDebugRepl(nullptr);
+    }
+    state.forceValue(*args[1], pos);
+    v = *args[1];
+}
+
+static RegisterPrimOp primop_warn({
+    .name = "__warn",
+    .args = {"e1", "e2"},
+    .doc = R"(
+      Evaluate *e1*, which must be a string and print iton standard error as a warning.
+      Then return *e2*.
+      This function is useful for non-critical situations where attention is advisable.
+
+      If the
+      [`debugger-on-trace`](@docroot@/command-ref/conf-file.md#conf-debugger-on-trace)
+      or [`debugger-on-warn`](@docroot@/command-ref/conf-file.md#conf-debugger-on-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,
+});
+
 
 /* Takes two arguments and evaluates to the second one. Used as the
  * builtins.traceVerbose implementation when --trace-verbose is not enabled

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: {

src/libutil/error.hh

@@ -89,6 +89,11 @@ struct ErrorInfo {
     HintFmt msg;
     std::shared_ptr<Pos> pos;
     std::list<Trace> 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.

tests/functional/lang.sh

@@ -36,6 +36,15 @@ nix-instantiate --eval -E 'let x = builtins.trace { x = x; } true; in x' \
 nix-instantiate --eval -E 'let x = { repeating = x; tracing = builtins.trace x true; }; in x.tracing'\
   2>&1 | grepQuiet -F 'trace: { repeating = «repeated»; tracing = «potential infinite recursion»; }'
 
+nix-instantiate --eval -E 'builtins.warn "Hello" 123' 2>&1 | grepQuiet 'warning: Hello'
+nix-instantiate --eval -E 'builtins.addErrorContext "while doing ${"something"} interesting" (builtins.warn "Hello" 123)' 2>/dev/null | grepQuiet 123
+
+# warn does not accept non-strings for now
+expectStderr 1 nix-instantiate --eval -E 'let x = builtins.warn { x = x; } true; in x' \
+  | grepQuiet "expected a string but found a set"
+expectStderr 1 nix-instantiate --eval --abort-on-warn -E 'builtins.warn "Hello" 123' | grepQuiet Hello
+NIX_ABORT_ON_WARN=1 expectStderr 1 nix-instantiate --eval -E 'builtins.addErrorContext "while doing ${"something"} interesting" (builtins.warn "Hello" 123)' | grepQuiet "while doing something interesting"
+
 set +x
 
 badDiff=0