From 2fa40c9de49e47c1f59dd32bf99227884bad3a2d Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 9 Nov 2025 18:43:18 +0100 Subject: [PATCH] libstore: bring back some pointer equality nonsense the change to shareable thunks also removed a few cases of pointer equality checks that allowed structures containing functions to be considered equal to other sets containing the same functions, even if the sets themselves were pointer-equal themselves. *so* busted. Change-Id: If87fdab658f9037ce2a654f69a9e3da6ae2f53e5 --- doc/manual/rl-next/ptr-eq.md | 48 ++++++++++ lix/libexpr/eval.cc | 4 +- .../eq/eval-okay-pointer-equality.out.exp | 1 + .../lang/eq/in-pointer-equality.nix | 96 +++++++++++++++++++ 4 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 doc/manual/rl-next/ptr-eq.md create mode 100644 tests/functional2/lang/eq/eval-okay-pointer-equality.out.exp create mode 100644 tests/functional2/lang/eq/in-pointer-equality.nix diff --git a/doc/manual/rl-next/ptr-eq.md b/doc/manual/rl-next/ptr-eq.md new file mode 100644 index 000000000..e6cab647f --- /dev/null +++ b/doc/manual/rl-next/ptr-eq.md @@ -0,0 +1,48 @@ +--- +synopsis: "Function equality semantics are more consistent, but still bad" +cls: [4556, 4244] +issues: [] +category: "Breaking Changes" +credits: [horrors] +--- + +Lix has inherited a historic misfeature from CppNix in the form of pointer +equality checks built into the `==` operator. These checks were originally +meant to optimize comparison for large sets, but they have the unfortunate +side effect of producing unexpected results when sets containing functions +are compared. **Lix 2.93 and earlier** behave as shown in the repl session + +``` +Lix 2.93.3 +Type :? for help. +nix-repl> f = x: x +Added f. + +nix-repl> f == f +false + +nix-repl> let s.f = f; in s.f == s.f +false + +nix-repl> # however! + { inherit f; } == { inherit f; } +true + +nix-repl> [ f ] == [ f ] +true + +nix-repl> # and, in another twist: + [ f ] == map f [ f ] +false +``` + +Nixpkgs relies on sets containing functions being comparable, so we cannot +simply deprecate this behavior. Due to changes to the object model used by +Lix ***all* comparisons above now evaluate to `true`**. This is considered +a breaking change because eval results may differ, but we also consider it +minor because the optimization is unsound (c.f. `let l = [NaN]; in l == l` +evaluates to `true` even though floating point `NaN` is incomparable). Lix +intends to remove this optimization altogether in the future, but until we +can do that we instead make it slightly less broken to allow other, *real* +optimizations. Function equality comparison remains **undefined behavior** +and should not be relied upon in Nixlang code that intends to be portable. diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index f1ba9e01c..d532d5da9 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -2680,9 +2680,9 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v return true; } - /* Functions are incomparable. */ + /* Functions are incomparable, except for identity (see note above about this nonsense). */ case nFunction: - return false; + return pointerEq(); case nExternal: if (pointerEq()) return true; diff --git a/tests/functional2/lang/eq/eval-okay-pointer-equality.out.exp b/tests/functional2/lang/eq/eval-okay-pointer-equality.out.exp new file mode 100644 index 000000000..8b06c6aec --- /dev/null +++ b/tests/functional2/lang/eq/eval-okay-pointer-equality.out.exp @@ -0,0 +1 @@ +{ functionsDirect = [ true false false true ]; functionsSelected = [ true false false true ]; lists = { bigList1 = { bigList1 = true; bigList1' = false; bigList2 = true; bigList2' = false; bigList3 = true; bigList3' = false; list1 = false; list1' = false; list2 = false; list2' = false; }; bigList1' = { bigList1 = false; bigList1' = true; bigList2 = false; bigList2' = true; bigList3 = false; bigList3' = true; list1 = false; list1' = false; list2 = false; list2' = false; }; bigList2 = { bigList1 = true; bigList1' = false; bigList2 = true; bigList2' = false; bigList3 = true; bigList3' = false; list1 = false; list1' = false; list2 = false; list2' = false; }; bigList2' = { bigList1 = false; bigList1' = true; bigList2 = false; bigList2' = true; bigList3 = false; bigList3' = true; list1 = false; list1' = false; list2 = false; list2' = false; }; bigList3 = { bigList1 = true; bigList1' = false; bigList2 = true; bigList2' = false; bigList3 = true; bigList3' = false; list1 = false; list1' = false; list2 = false; list2' = false; }; bigList3' = { bigList1 = false; bigList1' = true; bigList2 = false; bigList2' = true; bigList3 = false; bigList3' = true; list1 = false; list1' = false; list2 = false; list2' = false; }; list1 = { bigList1 = false; bigList1' = false; bigList2 = false; bigList2' = false; bigList3 = false; bigList3' = false; list1 = true; list1' = false; list2 = true; list2' = false; }; list1' = { bigList1 = false; bigList1' = false; bigList2 = false; bigList2' = false; bigList3 = false; bigList3' = false; list1 = false; list1' = true; list2 = false; list2' = true; }; list2 = { bigList1 = false; bigList1' = false; bigList2 = false; bigList2' = false; bigList3 = false; bigList3' = false; list1 = true; list1' = false; list2 = true; list2' = false; }; list2' = { bigList1 = false; bigList1' = false; bigList2 = false; bigList2' = false; bigList3 = false; bigList3' = false; list1 = false; list1' = true; list2 = false; list2' = true; }; }; nans = { list = { list = true; plain = false; set = false; }; plain = { list = false; plain = false; set = false; }; set = { list = false; plain = false; set = true; }; }; sets = { bigSet1 = { bigSet1 = true; bigSet1' = false; bigSet2 = true; bigSet2' = false; bigSet3 = true; bigSet3' = false; set1 = false; set1' = false; set2 = false; set2' = false; }; bigSet1' = { bigSet1 = false; bigSet1' = true; bigSet2 = false; bigSet2' = true; bigSet3 = false; bigSet3' = true; set1 = false; set1' = false; set2 = false; set2' = false; }; bigSet2 = { bigSet1 = true; bigSet1' = false; bigSet2 = true; bigSet2' = false; bigSet3 = true; bigSet3' = false; set1 = false; set1' = false; set2 = false; set2' = false; }; bigSet2' = { bigSet1 = false; bigSet1' = true; bigSet2 = false; bigSet2' = true; bigSet3 = false; bigSet3' = true; set1 = false; set1' = false; set2 = false; set2' = false; }; bigSet3 = { bigSet1 = true; bigSet1' = false; bigSet2 = true; bigSet2' = false; bigSet3 = true; bigSet3' = false; set1 = false; set1' = false; set2 = false; set2' = false; }; bigSet3' = { bigSet1 = false; bigSet1' = true; bigSet2 = false; bigSet2' = true; bigSet3 = false; bigSet3' = true; set1 = false; set1' = false; set2 = false; set2' = false; }; set1 = { bigSet1 = false; bigSet1' = false; bigSet2 = false; bigSet2' = false; bigSet3 = false; bigSet3' = false; set1 = true; set1' = false; set2 = true; set2' = false; }; set1' = { bigSet1 = false; bigSet1' = false; bigSet2 = false; bigSet2' = false; bigSet3 = false; bigSet3' = false; set1 = false; set1' = true; set2 = false; set2' = true; }; set2 = { bigSet1 = false; bigSet1' = false; bigSet2 = false; bigSet2' = false; bigSet3 = false; bigSet3' = false; set1 = true; set1' = false; set2 = true; set2' = false; }; set2' = { bigSet1 = false; bigSet1' = false; bigSet2 = false; bigSet2' = false; bigSet3 = false; bigSet3' = false; set1 = false; set1' = true; set2 = false; set2' = true; }; }; } diff --git a/tests/functional2/lang/eq/in-pointer-equality.nix b/tests/functional2/lang/eq/in-pointer-equality.nix new file mode 100644 index 000000000..d19c0b47c --- /dev/null +++ b/tests/functional2/lang/eq/in-pointer-equality.nix @@ -0,0 +1,96 @@ +let + f1 = x: x; + f2 = x: x; + + inf = let f = n: if n == 0 then 1.0 else 10 * f (n - 1); in f 1000; + nan = inf * 0; + + # all of these sets and lists are assumed to have different *addresses* as long as CSE does not occur + sets = rec { + set1 = { + f = f1; + }; + set1' = { + f = f2; + }; + set2 = { + f = f1; + }; + set2' = { + f = f2; + }; + bigSet1 = { + f = f1; + meow = true; + }; + bigSet1' = { + f = f2; + meow = true; + }; + bigSet2 = bigSet1 // { + meow = true; + }; + bigSet2' = bigSet1' // { + meow = true; + }; + bigSet3 = { + f = f1; + meow = true; + }; + bigSet3' = { + f = f2; + meow = true; + }; + }; + + lists = rec { + list1 = [ f1 ]; + list1' = [ f2 ]; + list2 = [ f1 ]; + list2' = [ f2 ]; + bigList1 = [ f1 true ]; + bigList1' = [ f2 true ]; + bigList2 = map (e: if builtins.isBool e then true else e) bigList1; + bigList2'= map (e: if builtins.isBool e then true else e) bigList1'; + bigList3 = [ f1 true ]; + bigList3' = [ f2 true ]; + }; + + nans = rec { + plain = nan; + list = [ nan ]; + set = { inherit nan; }; + }; + + compareAllPairs = cases: + let + names = builtins.attrNames cases; + in + builtins.listToAttrs (map + (n1: { + name = n1; + value = builtins.listToAttrs (map + (n2: { name = n2; value = cases.${n1} == cases.${n2}; }) + names); + }) + names); + + results = { + functionsDirect = [ + (f1 == f1) + (f1 == f2) + (f2 == f1) + (f2 == f2) + ]; + functionsSelected = [ + (sets.set1.f == sets.set1.f) + (sets.set1.f == sets.set1'.f) + (sets.set1'.f == sets.set1.f) + (sets.set1'.f == sets.set1'.f) + ]; + sets = compareAllPairs sets; + lists = compareAllPairs lists; + nans = compareAllPairs nans; + }; +in +builtins.deepSeq results results