NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.86k stars 1.52k forks source link

`builtins.mergeMapAttrs` (or `builtins.concatMapAttrs`) #11887

Open roberth opened 1 week ago

roberth commented 1 week ago

Is your feature request related to a problem? Please describe.

Nixpkgs lib defines concatMapAttrs f attrs, which is a very versatile function. (To those it may concern: by virtue of being almost the monadic bind operation on attrsets. See here. Almost => no associativity due to attribute name collisions iirc)

Describe the solution you'd like

Add to builtins a primop that behaves like Nixpkgs' lib.concatMapAttrs. It could be builtins.concatMapAttrs or a different name, such as flatMap attrs or mergeMapAttrs. Its implementation will be slightly more efficient than that in lib, and lib may reuse it, so that it becomes a polyfill. The added efficiency comes from having no intermediate Nix lists, and may build the attrset in one go.

The current implementation in lib has the following behaviors, which could be optimized away (in order of increasing implementation work):

  1. performs more function calls (significant, since we're an interpreter, and our calls perform allocations)
  2. creates and sorts intermediate attrsets after each f call (except the final one which is the return value)
  3. creates and sorts intermediate attrsets returned by f

If we pick a new name, we could make attribute name collisions an error, which iirc would make it a lawful monad in all the successful cases. Errors are better than buggy behavior somewhere deep down in some expression, because how would you find the bug.

Describe alternatives you've considered

Additional context

Priorities

Add :+1: to issues you find important.

hsjobeki commented 4 days ago

Could you give some example how a function call would look like?

Same as concatMapAttrs ?

mergeMapAttrs 
  (name: value: { ${name + "-mapped"} = value * 2 ; } )
  { foo = 1; bar =2; nested.foo = 3; }
=>
{
  { foo-mapped = 2; bar-mapped = 4; nested-mapped.foo = 6; }
}

Would it make sense to accept an optional merge function.

mergeMapAttrs {
  mapFn =  (name: value: { ${name + "-mapped"} = value * 2 ; } );
  mergeFn = (name: values: merge name values) # defaults to an error function? 
  } 
  { foo = 1; bar =2; nested.foo = 3; }
=>
{
  { foo-mapped = 2; bar-mapped = 4; nested-mapped.foo = 6; }
}

would taking an attribute set as arguments add some benefit? in this case: default behavior that can be customized.

roberth commented 4 days ago

@hsjobeki

Same as concatMapAttrs?

Yes. I've updated the description.

accept an optional merge function.

Perhaps, but it has a slight cost, and not much, but non-zero benefit. See the alternatives section.

would taking an attribute set as arguments add some benefit? in this case: default behavior that can be customized.

In the current evaluator implementation, "argument attrsets" aren't special or optimized, whereas curried function application of positional arguments to primops is optimized. I'd like this to make no difference, but optimizing those calls is non-trivial, because we don't have static knowledge of the function, given an application Expr. So an attrset based call would have a slight overhead compared to entirely positional arguments.

hsjobeki commented 4 days ago

I have to admit i am a big fan of this Idea overall.

edolstra commented 1 day ago

Team meeting notes: