Fix getAttrPos after mapAttrs

[Onyad]

Motivation

Fix position information loss in mapAttrs that breaks unsafeGetAttrPos.

Context

• mapAttrs wasn't preserving attribute positions
• Now passes original positions through (i.pos)
• Added test verifying position preservation

Add 👍 to pull requests you find important.

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

[xokdvium]

Does this fix #13166?

[roberth]

I guess we're deciding that the attribute position is about the attribute name more so than its value.
Otherwise, the position should be on line 6

key: value: value
            ^

The chosen interpretation of the attribute position seems more useful, so I think it's the right choice.

Another way to look at it is that an attribute is a metadata-carrying one-item container, whose metadata does not change. Then mapAttrs can be seen as a bunch of functional, pointwise attribute "updates".

[Onyad]

As far as I understand the CI error, it’s caused by this line in nixpkgs. I don’t understand the underlying reasons for doing it this way, nor whether my PR is unacceptable and breaks nixpkgs.

If this change is indeed valid, I’m not entirely sure how I should proceed to update nixpkgs and Nix.

[Onyad]

Does this fix #13166?

I've researched the issue a bit, it seems to me that the problem is not in the sequence of applying the // operator, but in the fact that the filterAttrs function calls removeAttrs, which has a similar error as with mapAttrs, the position from the original attrset is not entered in this line(This should be the third parameter of emplace_back, since it is the 3rd parameter of the constructor Attr).

Apparently this is not an isolated error as I thought, I'll try to run through all the builtin functions that interact with attrset in one way or another.

[roberth]

the CI error

This is, I believe, why it's unsafe in the name unsafeGetAttrPos.
Something based on listToAttrs seems like a better solution for that helper function. It seems highly unlikely that we'll make that magically preserve positions.
I'm pretty sure this implementation was chosen for its simplicity, not some particularly deep reasons, having reviewed that code, but let's double check with @lf- - what do you think about the test and the change here?

[lf-]

the CI error

This is, I believe, why it's unsafe in the name unsafeGetAttrPos.

Yes, it's because it's a derivation stability footgun.

Something based on listToAttrs seems like a better solution for that helper function. It seems highly unlikely that we'll make that magically preserve positions.

Yeah, this whole mechanism definitely isn't a good one and is kind of a hack. Really you want to think about the way to write the position stuff as a nixpkgs level API question.

I'm pretty sure this implementation was chosen for its simplicity, not some particularly deep reasons

I think this is a correct read, it's a best effort position mechanism. There were likely serious performance considerations involved in how these decisions were made also, nix-re.pl implements positions on every type, but nix only does in attrsets themselves, functions, and attrset attributes.

[roberth]

I guess we're deciding that the attribute position is about the attribute name more so than its value.

This is rather significant. If we decide this is the rule, other constructs should follow. Candidates:

• zipAttrsWith for the case where only a single input attrset contributed to the returned attribute
  • (+) more info is better
  • (+) consistent with mapAttrs in terms of not choosing the lambda body position (i.e. position of the attribute value)
• inherit (foo) a; could produce an attribute a with foo.a's position
  • (+) makes authors move documentation closer to definitions
  • (-) not exactly syntax sugar for a = foo.a; as we have told before
  • (+) does not affect "safe" value semantics model (which does not include position info)
  • (+) makes existing choice of syntax actually useful
  • (-) change isn't always fun
  • I believe the (+) weigh heavier, especially the documentation DX

Are we ok with these consequences?

quite significant; asking for opinions

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2025-08-20-nix-team-meeting-minutes-242-241/68245/1