Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function equality is broken #3371

Open
puckipedia opened this issue Feb 24, 2020 · 6 comments
Open

Function equality is broken #3371

puckipedia opened this issue Feb 24, 2020 · 6 comments
Labels
breaking Changes we can't make without breaking old expressions, changing hashes, etc bug language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@puckipedia
Copy link
Contributor

As foretold by the code,

/* Functions are incomparable. */

And this seems to be true. If we try to evaluate this:

let lambda = (a: a); in lambda == lambda

We get a nice false result, as expected. But, what if ... we put the lambda in an attrset?

let lambda = (a: a); in { inherit lambda; } == { inherit lambda; }

Now, all of a sudden, the two attrsets are equal! But this is impossible, as functions are incomparable!

The reason for this is a 2010 hack added to support "some old broken code that relies on pointer equality":

nix/src/libexpr/eval.cc

Lines 1689 to 1692 in 2e953b5

/* !!! Hack to support some old broken code that relies on pointer
equality tests between sets. (Specifically, builderDefs calls
uniqList on a list of sets.) Will remove this eventually. */
if (&v1 == &v2) return true;

@zimbatm zimbatm added the bug label Feb 24, 2020
@7c6f434c
Copy link
Member

Hm, as builderDefs do not exist in Nixpkgs anymore, and has been removed for a few years, I guess one could check whether there is a current use in the tree?

What does modules system listOf do when the same set is added multiple times?

@stale
Copy link

stale bot commented Jun 3, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 3, 2021
@andir
Copy link
Member

andir commented Nov 28, 2021

This is still an issue to this date. Just came across the same behaviour while trying to generate random numbers in pure eval...

@stale stale bot removed the stale label Nov 28, 2021
@stale
Copy link

stale bot commented Jul 11, 2022

I marked this as stale due to inactivity. → More info

@sternenseemann
Copy link
Member

I looked into this again to be able to reimplement it accurately and tried writing up a summary. I think this also accounts aspects of the behavior that were not well-known before (i.e. that it works with non-functions as well and the wrapping value need not be an attribute set).

Also I came to the conclusion that we can't remove it as the platform sets in nixpkgs need to be comparable and contain functions. Even if we remove this in nixpkgs, we'll loose the ability to evaluate nixpkgs revisions people will care about. Maybe an experimental feature to disable it down the line would be possible.

@sternenseemann
Copy link
Member

One (good?) thing is that pointer equality is based on the Value struct's location and not the location of an inner representation the struct has a pointer to. This means that when trying to (ab)use pointer equality, you are not able to in many cases, since Nix expressions create copies of Value structs all the time (e.g. select, function application, …, but identifier access does not):

let
  inherit (builtins) add;
  set = { inherit (builtins) add; };
in

[
  (set == set)                                                 # => true

  ({ inherit (set) add; } == { inherit (set) add; })           # => false
  ({ inherit (builtins) add; } == { inherit (builtins) add; }) # => false
  ({ inherit add; } == { inherit add; })                       # => true
]

See also #8462 (comment).

@roberth roberth added language The Nix expression language; parser, interpreter, primops, evaluation, etc breaking Changes we can't make without breaking old expressions, changing hashes, etc labels Jun 18, 2023
tvlbot pushed a commit to tvlfyi/tvix that referenced this issue Jul 2, 2023
I want to expand on the C++ Nix behavior, since it seems relevant to
note that a lot of operations in C++ Nix (like select) don't preserve
pointer equality (see
<NixOS/nix#3371 (comment)>).
It is especially so, as Tvix establishes pointer equality in a different
way and thus shows differing behavior. Therefore I want to additionally
document Tvix's current behavior and make it more explicit to what
extent nixpkgs needs pointer equality.

Change-Id: I9b4ba75dacb749c9fcbba4b9646c6b48bb57bbad
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8852
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changes we can't make without breaking old expressions, changing hashes, etc bug language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

No branches or pull requests

6 participants