Skip to content

feat(libexpr): mark start of infinite recursion in error traces - #16296

Merged
xokdvium merged 1 commit into
NixOS:masterfrom
roberth:feat-infinite-recursion-mark-start
Aug 15, 2026
Merged

feat(libexpr): mark start of infinite recursion in error traces#16296
xokdvium merged 1 commit into
NixOS:masterfrom
roberth:feat-infinite-recursion-mark-start

Conversation

@roberth

@roberth roberth commented Aug 12, 2026

Copy link
Copy Markdown
Member

Motivation

Trace useful. Found a need for this again when debugging the tracing cache (STF)

When Nix reports a trace for cyclic expression, it will now let you discern the cycle from the code that leads up to the cycle.

Context

Assisted-By: Claude Code (Claude Opus 4.7)
Assisted-By: Claude Code (Claude Opus 5)


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@roberth
roberth requested a review from edolstra as a code owner August 12, 2026 21:42
@github-actions github-actions Bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Aug 12, 2026
@roberth
roberth requested review from xokdvium and removed request for edolstra August 12, 2026 21:42
Comment thread src/libutil/include/nix/util/error.hh Outdated
Comment thread src/libutil/error.cc Outdated
@roberth
roberth force-pushed the feat-infinite-recursion-mark-start branch 3 times, most recently from a844e7c to ad80d38 Compare August 13, 2026 09:21
Comment thread src/libutil/error.cc Outdated
@roberth
roberth force-pushed the feat-infinite-recursion-mark-start branch from ad80d38 to 13bf395 Compare August 13, 2026 12:01
Comment thread doc/manual/rl-next/infinite-recursion-start.md Outdated
Assisted-By: Claude Code (Claude Opus 4.7)
Assisted-By: Claude Code (Claude Opus 5)
@roberth
roberth force-pushed the feat-infinite-recursion-mark-start branch from 13bf395 to 82c4f82 Compare August 13, 2026 12:18
Comment on lines +74 to +77
template<typename... Args>
explicit InfiniteRecursionError(EvalState & state, const Value * v, const Args &... args)
: CloneableError(state, args...)
, v(v)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
template<typename... Args>
explicit InfiniteRecursionError(EvalState & state, const Value * v, const Args &... args)
: CloneableError(state, args...)
, v(v)
template<typename... Args>
explicit InfiniteRecursionError(EvalState & state, const Value * v, Args &&... args)
: CloneableError(state, std::forward<Args>(args)...)
, v(v)

But it probably doesn't matter much.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't do anything atm. Would take 4 more changes:

# Link Signature File
1 EvalState::error<T> const Args &... args eval.hh:488
2 EvalErrorBuilder<T> ctor const Args &... args eval-error.hh:129
3 InfiniteRecursionError ctor const Args &... args eval-error.hh:75
4 EvalBaseError ctor const Args &... formatArgs eval-error.hh:39
5 BaseError ctor Args &&... argsHintFmt(...) error.hh:142 ✅ already forwards

Also errors are supposedly not in the hot path (tryEval aside)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's still good to do locally, even if we are not forwarding everything up the call stack.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can improve this in a follow-up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already asked Claude to do it #16309, if it's any help :)

@xokdvium
xokdvium added this pull request to the merge queue Aug 15, 2026
Merged via the queue into NixOS:master with commit a0651be Aug 15, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation with-tests Issues related to testing. PRs with tests have some priority

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants