From 92566210097bf301e7f6651c161115a51598f20d Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Fri, 8 Sep 2023 23:38:28 -0700 Subject: fix(tvix/eval): update identifier quoting to match cppnix 2.17 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 Reviewed-by: sterni Tested-by: BuildkiteCI --- .../tests/tvix_tests/eval-okay-identifier-formatting.exp | 2 +- .../tests/tvix_tests/eval-okay-identifier-formatting.nix | 14 +++++++++++++- .../tests/tvix_tests/identity-quoted-attrname-assert.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-else.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-if.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-in.nix | 1 + .../tests/tvix_tests/identity-quoted-attrname-inherit.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-let.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-rec.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-then.nix | 1 + .../src/tests/tvix_tests/identity-quoted-attrname-with.nix | 1 + tvix/eval/src/value/string.rs | 13 ++++++++++++- tvix/verify-lang-tests/default.nix | 6 +++--- 13 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-assert.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-else.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-if.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-in.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-inherit.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-let.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-rec.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-then.nix create mode 100644 tvix/eval/src/tests/tvix_tests/identity-quoted-attrname-with.nix (limited to 'tvix') 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 074f5f07f5f0..9800c675fc91 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 8f9aa2823801..58af3d6d16ab 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 000000000000..575b1af5883d --- /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 000000000000..7601f14b32d1 --- /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 000000000000..1c391fc9a355 --- /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 000000000000..b4f238651dc6 --- /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 000000000000..e62ed32b0435 --- /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 000000000000..196ec7cc887f --- /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 000000000000..d2c4f93a2ae1 --- /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 000000000000..f2af8d69701e --- /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 000000000000..cbcfa970c289 --- /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 2649e00f0830..8ffbc2a5325c 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 caf7235b8bf2..3c3fcb53fd76 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). -- cgit 1.4.1