fix(tvix/eval): update identifier quoting to match cppnix 2.17 r/6589

In cppnix 2.17, commit b72bc4a972fe568744d98b89d63adcd504cb586c, the
libexpr pretty-printing routine was fixed so that it would no longer
pretty-print attrsets with keywords in their attrnames incorrectly.

This commit implements the corresponding fix for tvix, fixes our
tests to work with cppnix>=2.17 oracles, and expands our test cases
to cover all the keywords.

Change-Id: I4b51389cd3a9c44babc8ab2a84b383b7b0b116ca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9283
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI

13 files changed, 38 insertions, 6 deletions

diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.exp
index 074f5f07f5..9800c675fc 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.exp
@@ -1 +1 @@
-{ "'quoted'" = false; "-20°" = false; "2normal" = false; "45 44 43-'3 2 1" = false; "9front" = false; Very2Normal = true; VeryNormal = true; _'12 = true; "_'12.5" = false; __internal = true; _internal = true; abort = true; assert = true; "attr.path" = false; false = true; foldl' = true; normal = true; normal2 = true; null = true; or = true; throw = true; true = true; x = true; x' = true; x'' = true; "😀" = false; }
+{ "'quoted'" = false; "-20°" = false; "2normal" = false; "45 44 43-'3 2 1" = false; "9front" = false; Very2Normal = true; VeryNormal = true; _'12 = true; "_'12.5" = false; __internal = true; _internal = true; abort = true; "assert" = false; "attr.path" = false; "else" = false; false = true; foldl' = true; "if" = false; "in" = false; "inherit" = false; "let" = false; normal = true; normal2 = true; null = true; or = true; "rec" = false; "then" = false; throw = true; true = true; "with" = false; x = true; x' = true; x'' = true; "😀" = false; }
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.nix
index 8f9aa28238..58af3d6d16 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-identifier-formatting.nix
@@ -1,3 +1,6 @@
+# Note: the attribute values in this set aren't just dummies!  They
+# are booleans which indicate whether or not the corresponding
+# attrname is valid without quotification.
 {
   __internal = true;
   _internal = true;
@@ -15,7 +18,7 @@
   false = true;
   null = true;
   or = true;
-  "assert" = true; # -ish
+  "assert" = false;
   throw = true;
   abort = true;
 
@@ -27,4 +30,13 @@
   "'quoted'" = false;
   "_'12.5" = false;
   "😀" = false;
+
+  "if" = false;
+  "then" = false;
+  "else" = false;
+  "with" = false;
+  "let" = false;
+  "in" = false;
+  "rec" = false;
+  "inherit" = false;
 }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-assert.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-assert.nix
new file mode 100644
index 0000000000..575b1af588
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-assert.nix
@@ -0,0 +1 @@
+{ "assert" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-else.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-else.nix
new file mode 100644
index 0000000000..7601f14b32
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-else.nix
@@ -0,0 +1 @@
+{ "else" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-if.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-if.nix
new file mode 100644
index 0000000000..1c391fc9a3
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-if.nix
@@ -0,0 +1 @@
+{ "if" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-in.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-in.nix
new file mode 100644
index 0000000000..b4f238651d
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-in.nix
@@ -0,0 +1 @@
+{ "in" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-inherit.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-inherit.nix
new file mode 100644
index 0000000000..e62ed32b04
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-inherit.nix
@@ -0,0 +1 @@
+{ "inherit" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-let.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-let.nix
new file mode 100644
index 0000000000..196ec7cc88
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-let.nix
@@ -0,0 +1 @@
+{ "let" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-rec.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-rec.nix
new file mode 100644
index 0000000000..d2c4f93a2a
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-rec.nix
@@ -0,0 +1 @@
+{ "rec" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-then.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-then.nix
new file mode 100644
index 0000000000..f2af8d6970
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-then.nix
@@ -0,0 +1 @@
+{ "then" = true; }
diff --git a/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-with.nix b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-with.nix
new file mode 100644
index 0000000000..cbcfa970c2
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-with.nix
@@ -0,0 +1 @@
+{ "with" = true; }
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index 2649e00f08..8ffbc2a532 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -135,7 +135,7 @@ impl NixString {
             // A borrowed string is unchanged and can be returned as
             // is.
             Cow::Borrowed(_) => {
-                if is_valid_nix_identifier(&escaped) {
+                if is_valid_nix_identifier(&escaped) && !is_keyword(&escaped) {
                     escaped
                 } else {
                     Cow::Owned(format!("\"{}\"", escaped))
@@ -171,6 +171,17 @@ fn nix_escape_char(ch: char, next: Option<&char>) -> Option<&'static str> {
     }
 }
 
+/// Return true if this string is a keyword -- character strings
+/// which lexically match the "identifier" production but are not
+/// parsed as identifiers.  See also cppnix commit
+/// b72bc4a972fe568744d98b89d63adcd504cb586c.
+fn is_keyword(s: &str) -> bool {
+    match s {
+        "if" | "then" | "else" | "assert" | "with" | "let" | "in" | "rec" | "inherit" => true,
+        _ => false,
+    }
+}
+
 /// Return true if this string can be used as an identifier in Nix.
 fn is_valid_nix_identifier(s: &str) -> bool {
     // adapted from rnix-parser's tokenizer.rs
diff --git a/tvix/verify-lang-tests/default.nix b/tvix/verify-lang-tests/default.nix
index caf7235b8b..3c3fcb53fd 100644
--- a/tvix/verify-lang-tests/default.nix
+++ b/tvix/verify-lang-tests/default.nix
@@ -70,9 +70,9 @@ let
     "eval-okay-readFileType.nix" = [ nix ];
     # builtins.fromTOML gains support for timestamps in Nix 2.16
     "eval-okay-fromTOML-timestamps.nix" = [ nix ];
-    # identifier formatting seems to have changed in Nix 2.17
-    # TODO: figure out why, this is just to get the bump in cl/9125 working.
-    "eval-okay-identifier-formatting.nix" = [ nix_latest ];
+    # identifier formatting changed in Nix 2.17 due to cppnix commit
+    # b72bc4a972fe568744d98b89d63adcd504cb586c
+    "eval-okay-identifier-formatting.nix" = [ nix ];
 
     # TODO(sterni): support diffing working directory and home relative paths
     # like C++ Nix test suite (using string replacement).