Skip to content

libexpr: Add filterAttrs builtin#14753

Closed
not-ronjinger wants to merge 2 commits intoNixOS:masterfrom
not-ronjinger:add-filterattrs
Closed

libexpr: Add filterAttrs builtin#14753
not-ronjinger wants to merge 2 commits intoNixOS:masterfrom
not-ronjinger:add-filterattrs

Conversation

@not-ronjinger
Copy link

@not-ronjinger not-ronjinger commented Dec 9, 2025

Motivation

The implementation in nixpkgs is slightly inefficient:

filterAttrs = pred: set: removeAttrs set (filter (name: !pred name set.${name}) (attrNames set));

Namely, it has to create a temporary list of names attrNames set, then has to scan through the entries with filter, apply the predicate which needs to dereference the value, then pass it to removeAttrs, which then needs to do another scan of names which failed the predicate. It's likely as good as you can get without creating a specific builtin.

With a native builtin, we should be able to eliminate the need for generating the temporary list, the initial scan should be a bit faster as there is less indirection with looking up the value, and the need to do another scan for removeAttrs should be eliminated altogether.

Re-introducing this to nixpkgs/lib should be easy with a builtins ? filterAttrs check.

Context

Gotta go fast 🚀

@wolfgangwalther
Copy link
Contributor

This user was banned.

crertel added a commit to crertel/nix that referenced this pull request Dec 11, 2025
@Lillecarl
Copy link

Is it possible to reopen this after NixOS/org#229 got merged?

@philiptaron
Copy link
Contributor

I do want builtins.filterAttrs to land (the results in the DetSys PR are quite nice) but Jon's ban is not based on which specific person is enforcing it, Carl, as well you likely know.

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.

4 participants