From d826427f022308f9ec2ee8f0ec9f7df85089fdf7 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 29 Jan 2024 06:19:23 +0100 Subject: [PATCH] normalize formal order on ExprLambda::show we already normalize attr order to lexicographic, doing the same for formals makes sense. doubly so because the order of formals would otherwise depend on the context of the expression, which is not quite as useful as one might expect. (cherry picked from commit 4147ecfb1c51f3fe3b4adcbd4e753fd487dab645) Change-Id: I3fd0dbdef3ac7447a3a03ff20bb514a0d0f23fb1 --- doc/manual/rl-next/formal-order.md | 7 +++++++ src/libexpr/nixexpr.cc | 5 ++++- tests/functional/lang/parse-okay-subversion.exp | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 doc/manual/rl-next/formal-order.md diff --git a/doc/manual/rl-next/formal-order.md b/doc/manual/rl-next/formal-order.md new file mode 100644 index 000000000..12628e318 --- /dev/null +++ b/doc/manual/rl-next/formal-order.md @@ -0,0 +1,7 @@ +--- +synopsis: consistent order of lambda formals in printed expressions +prs: 9874 +--- + +Always print lambda formals in lexicographic order rather than the internal, creation-time based symbol order. +This makes printed formals independent of the context they appear in. diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 3bcd35fef..c591916e3 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -147,7 +147,10 @@ void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const if (hasFormals()) { str << "{ "; bool first = true; - for (auto & i : formals->formals) { + // the natural Symbol ordering is by creation time, which can lead to the + // same expression being printed in two different ways depending on its + // context. always use lexicographic ordering to avoid this. + for (auto & i : formals->lexicographicOrder(symbols)) { if (first) first = false; else str << ", "; str << symbols[i.name]; if (i.def) { diff --git a/tests/functional/lang/parse-okay-subversion.exp b/tests/functional/lang/parse-okay-subversion.exp index 2303932c4..32fbba3c5 100644 --- a/tests/functional/lang/parse-okay-subversion.exp +++ b/tests/functional/lang/parse-okay-subversion.exp @@ -1 +1 @@ -({ fetchurl, localServer ? false, httpServer ? false, sslSupport ? false, pythonBindings ? false, javaSwigBindings ? false, javahlBindings ? false, stdenv, openssl ? null, httpd ? null, db4 ? null, expat, swig ? null, j2sdk ? null }: assert (expat != null); assert (localServer -> (db4 != null)); assert (httpServer -> ((httpd != null) && ((httpd).expat == expat))); assert (sslSupport -> ((openssl != null) && (httpServer -> ((httpd).openssl == openssl)))); assert (pythonBindings -> ((swig != null) && (swig).pythonSupport)); assert (javaSwigBindings -> ((swig != null) && (swig).javaSupport)); assert (javahlBindings -> (j2sdk != null)); ((stdenv).mkDerivation { inherit expat httpServer javaSwigBindings javahlBindings localServer pythonBindings sslSupport; builder = /foo/bar; db4 = (if localServer then db4 else null); httpd = (if httpServer then httpd else null); j2sdk = (if javaSwigBindings then (swig).j2sdk else (if javahlBindings then j2sdk else null)); name = "subversion-1.1.1"; openssl = (if sslSupport then openssl else null); patches = (if javahlBindings then [ (/javahl.patch) ] else [ ]); python = (if pythonBindings then (swig).python else null); src = (fetchurl { md5 = "a180c3fe91680389c210c99def54d9e0"; url = "http://subversion.tigris.org/tarballs/subversion-1.1.1.tar.bz2"; }); swig = (if (pythonBindings || javaSwigBindings) then swig else null); })) +({ db4 ? null, expat, fetchurl, httpServer ? false, httpd ? null, j2sdk ? null, javaSwigBindings ? false, javahlBindings ? false, localServer ? false, openssl ? null, pythonBindings ? false, sslSupport ? false, stdenv, swig ? null }: assert (expat != null); assert (localServer -> (db4 != null)); assert (httpServer -> ((httpd != null) && ((httpd).expat == expat))); assert (sslSupport -> ((openssl != null) && (httpServer -> ((httpd).openssl == openssl)))); assert (pythonBindings -> ((swig != null) && (swig).pythonSupport)); assert (javaSwigBindings -> ((swig != null) && (swig).javaSupport)); assert (javahlBindings -> (j2sdk != null)); ((stdenv).mkDerivation { inherit expat httpServer javaSwigBindings javahlBindings localServer pythonBindings sslSupport; builder = /foo/bar; db4 = (if localServer then db4 else null); httpd = (if httpServer then httpd else null); j2sdk = (if javaSwigBindings then (swig).j2sdk else (if javahlBindings then j2sdk else null)); name = "subversion-1.1.1"; openssl = (if sslSupport then openssl else null); patches = (if javahlBindings then [ (/javahl.patch) ] else [ ]); python = (if pythonBindings then (swig).python else null); src = (fetchurl { md5 = "a180c3fe91680389c210c99def54d9e0"; url = "http://subversion.tigris.org/tarballs/subversion-1.1.1.tar.bz2"; }); swig = (if (pythonBindings || javaSwigBindings) then swig else null); }))