Skip to content

libexpr: move the ExprLet::attrs allocations into the arena#14635

Merged
Ericson2314 merged 2 commits intoNixOS:masterfrom
Radvendii:alloc-exprlet-exprattrs
Nov 24, 2025
Merged

libexpr: move the ExprLet::attrs allocations into the arena#14635
Ericson2314 merged 2 commits intoNixOS:masterfrom
Radvendii:alloc-exprlet-exprattrs

Conversation

@Radvendii
Copy link
Contributor

In #14539, I missed that ExprLet has an ExprAttrs parameter that doesn't get recursed into during bindVars.

This PR plugs that leak, and also factors out the moving-to-different-allocator code into a separate function.

Motivation

Context


Add 👍 to pull requests you find important.

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

@Radvendii Radvendii requested a review from edolstra as a code owner November 24, 2025 20:01
I missed this because I assumed all Exprs were recursed into by
bindVars, but ExprLet's ExprAttrs field is not really its own AST node,
so it doesn't get recursed into.
@Radvendii Radvendii force-pushed the alloc-exprlet-exprattrs branch from 891038c to 60f0992 Compare November 24, 2025 20:43
@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 24, 2025
Merged via the queue into NixOS:master with commit 3bac0d7 Nov 24, 2025
16 checks passed
@edolstra edolstra mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants