From 3b6169f87be45c77ec4b56d118a5e2c718ff3f2b Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@tweag.io>
Date: Fri, 22 Sep 2023 12:22:04 +0200
Subject: [PATCH] lib.lists.foldl': Make strict in the initial accumulator

To maintain backwards compatibility, this can't be changed in the Nix language.
We can however ensure that the version Nixpkgs has the more intuitive behavior.
---
 lib/attrsets.nix                                  |  2 +-
 lib/lists.nix                                     | 13 ++++++++++---
 lib/tests/misc.nix                                |  4 ++--
 nixos/doc/manual/release-notes/rl-2311.section.md |  3 +++
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/attrsets.nix b/lib/attrsets.nix
index 77e36d3271f7..11932c05dd29 100644
--- a/lib/attrsets.nix
+++ b/lib/attrsets.nix
@@ -392,7 +392,7 @@ rec {
       foldlAttrs :: ( a -> String -> b -> a ) -> a -> { ... :: b } -> a
   */
   foldlAttrs = f: init: set:
-    foldl'
+    builtins.foldl'
       (acc: name: f acc name set.${name})
       init
       (attrNames set);
diff --git a/lib/lists.nix b/lib/lists.nix
index 8c5099084bb5..3835e3ba69cb 100644
--- a/lib/lists.nix
+++ b/lib/lists.nix
@@ -90,9 +90,12 @@ rec {
     Reduce a list by applying a binary operator from left to right,
     starting with an initial accumulator.
 
-    After each application of the operator, the resulting value is evaluated.
+    Before each application of the operator, the accumulator value is evaluated.
     This behavior makes this function stricter than [`foldl`](#function-library-lib.lists.foldl).
 
+    Unlike [`builtins.foldl'`](https://nixos.org/manual/nix/unstable/language/builtins.html#builtins-foldl'),
+    the initial accumulator argument is evaluated before the first iteration.
+
     A call like
 
     ```nix
@@ -104,7 +107,7 @@ rec {
 
     ```nix
     let
-      acc₁   =                      op acc₀   x₀   ;
+      acc₁   = builtins.seq acc₀   (op acc₀   x₀  );
       acc₂   = builtins.seq acc₁   (op acc₁   x₁  );
       acc₃   = builtins.seq acc₂   (op acc₂   x₂  );
       ...
@@ -135,7 +138,11 @@ rec {
     # The list to fold
     list:
 
-    builtins.foldl' op acc list;
+    # The builtin `foldl'` is a bit lazier than one might expect.
+    # See https://github.com/NixOS/nix/pull/7158.
+    # In particular, the initial accumulator value is not forced before the first iteration starts.
+    builtins.seq acc
+      (builtins.foldl' op acc list);
 
   /* Map with index starting from 0
 
diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
index 50d615c5be38..d40d92049880 100644
--- a/lib/tests/misc.nix
+++ b/lib/tests/misc.nix
@@ -524,10 +524,10 @@ runTests {
     expected = [ 3 2 1 ];
   };
 
-  # The same as builtins.foldl', lib.foldl' doesn't evaluate the first accumulator strictly
+  # Compared to builtins.foldl', lib.foldl' evaluates the first accumulator strictly too
   testFoldl'StrictInitial = {
     expr = (builtins.tryEval (foldl' (acc: el: el) (throw "hello") [])).success;
-    expected = true;
+    expected = false;
   };
 
   # Make sure we don't get a stack overflow for large lists
diff --git a/nixos/doc/manual/release-notes/rl-2311.section.md b/nixos/doc/manual/release-notes/rl-2311.section.md
index cdb73fb49fa8..011fa84c96af 100644
--- a/nixos/doc/manual/release-notes/rl-2311.section.md
+++ b/nixos/doc/manual/release-notes/rl-2311.section.md
@@ -226,6 +226,9 @@
 
 - `networking.networkmanager.firewallBackend` was removed as NixOS is now using iptables-nftables-compat even when using iptables, therefore Networkmanager now uses the nftables backend unconditionally.
 
+- [`lib.lists.foldl'`](https://nixos.org/manual/nixpkgs/stable#function-library-lib.lists.foldl-prime) now always evaluates the initial accumulator argument first.
+  If you depend on the lazier behavior, consider using [`lib.lists.foldl`](https://nixos.org/manual/nixpkgs/stable#function-library-lib.lists.foldl) or [`builtins.foldl'`](https://nixos.org/manual/nix/stable/language/builtins.html#builtins-foldl') instead.
+
 - `rome` was removed because it is no longer maintained and is succeeded by `biome`.
 
 - The `services.mtr-exporter.target` has been removed in favor of `services.mtr-exporter.jobs` which allows specifying multiple targets.