Repository: cedar-policy/cedar
Pull request #1387 (2024-12-20 19:47:53)

Improve substitution
## Description of changes

Adds a new function `reauthorize_with_iter` to `PartialResponse`, that takes an `IntoIterator` instead of the `HashMap<SmolString, RestrictedExpression>`. This has two advantages:
- The SmolStr, which is an implementation detail of the cedar-policy-core crate is not exposed to the callers, instead &str is used. This in in line with the apparent policy of the api in cedar-policy.
- `reauthorize` immediately consumes the hashmap, the new api removes the need to allocate it if the remappings are  already available in some other format.

I'm considering to add one that takes a Fn(&str) -> Option<Value> too, that one would remove the need to iterate them all.
In fact the whole substitution infrastructure in -core could be using such a function, but the current reauthorize API is not (easily) implementable in those terms, because of the fallibility of conversion from RestrictedExpression to Value.

However, I believe the most common case will be when the caller provides literal types in the substitutions: If we make a specialized method that takes only valid values, the need to evaluate the 'restricted expression' disappears.
But I suppose ast::Value shoudn't be exposed in the public API, instead EvalResult or a similar wrapper would be needed?

## Issue #, if available

## Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

- [x] A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

- [x] Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that [`cedar-spec`](https://github.com/cedar-policy/cedar-spec) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

I confirm that [`docs.cedarpolicy.com`](https://docs.cedarpolicy.com/) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar language specification.


shaobo-he-aws: LGTM. Nit: I prefer the new API to be named like `reauthorize_with_bindings`.
B-Lorentz: > LGTM. Nit: I prefer the new API to be named like `reauthorize_with_bindings`.
I can do that. `with_iter` seems to suggest some 'deeper' reliance on iteration than it being just the form the bindings are supplied. But I think `with_bindings` is also suboptimal, since the old `reauthorize` also uses bindings just as much, just takes them as a map. Can we deprecate the old method?
 


shaobo-he-aws: > Can we deprecate the old method?

Yes, we theoretically can. You can mark it and other team members can weigh in when they review it.

Patch:
diff --git a/.gitignore b/.gitignore
index a7504906..be975cd6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,6 +5,10 @@
 # Don't check in the Emacs temp files
 *~
 
+# Don't check in common editors configs
+.vscode
+.zed
+
 # Don't check in test framework files
 .attach_pid*
 
diff --git a/Cargo.lock b/Cargo.lock
index dbdb44df..b90336ba 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -98,9 +98,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.94"
+version = "1.0.95"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c1fd03a028ef38ba2276dce7e33fcd6369c158a1bca17946c4b1b701891c1ff7"
+checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04"
 
 [[package]]
 name = "arbitrary"
@@ -184,30 +184,15 @@ version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3a8241f3ebb85c056b509d4327ad0358fbbba6ffb340bf388f26350aeda225b1"
 
-[[package]]
-name = "bit-set"
-version = "0.5.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0700ddab506f33b20a03b13996eccd309a48e5ff77d0d95926aa0210fb4e95f1"
-dependencies = [
- "bit-vec 0.6.3",
-]
-
 [[package]]
 name = "bit-set"
 version = "0.8.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3"
 dependencies = [
- "bit-vec 0.8.0",
+ "bit-vec",
 ]
 
-[[package]]
-name = "bit-vec"
-version = "0.6.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb"
-
 [[package]]
 name = "bit-vec"
 version = "0.8.0"
@@ -287,9 +272,9 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5"
 
 [[package]]
 name = "cc"
-version = "1.2.4"
+version = "1.2.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9157bbaa6b165880c27a4293a474c91cdcf265cc68cc829bf10be0964a391caf"
+checksum = "8d6dbb628b8f8555f86d0323c2eb39e3ec81901f4b83e091db8a6a76d316a333"
 dependencies = [
  "shlex",
 ]
@@ -320,7 +305,7 @@ dependencies = [
  "serde_json",
  "serde_with",
  "smol_str",
- "thiserror 2.0.7",
+ "thiserror 2.0.9",
  "tsify",
  "wasm-bindgen",
 ]
@@ -345,7 +330,7 @@ dependencies = [
  "serde",
  "serde_json",
  "tempfile",
- "thiserror 2.0.7",
+ "thiserror 2.0.9",
 ]
 
 [[package]]
@@ -374,7 +359,7 @@ dependencies = [
  "serde_with",
  "smol_str",
  "stacker",
- "thiserror 2.0.7",
+ "thiserror 2.0.9",
  "tsify",
  "wasm-bindgen",
 ]
@@ -417,7 +402,7 @@ dependencies = [
  "similar-asserts",
  "smol_str",
  "stacker",
- "thiserror 2.0.7",
+ "thiserror 2.0.9",
  "tsify",
  "unicode-security",
  "wasm-bindgen",
@@ -559,14 +544,14 @@ checksum = "5b63caa9aa9397e2d9480a9b13673856c78d8ac123288526c37d7839f2a86990"
 
 [[package]]
 name = "console"
-version = "0.15.8"
+version = "0.15.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0e1f83fc076bd6dd27517eacdf25fef6c4dfe5f1d7448bafaaf3a26f13b5e4eb"
+checksum = "ea3c6ecd8059b57859df5c69830340ed3c41d30e3da0c1cbed90a96ac853041b"
 dependencies = [
  "encode_unicode",
- "lazy_static",
  "libc",
- "windows-sys 0.52.0",
+ "once_cell",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -814,9 +799,9 @@ dependencies = [
 
 [[package]]
 name = "encode_unicode"
-version = "0.3.6"
+version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f"
+checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0"
 
 [[package]]
 name = "enum-ordinalize"
@@ -868,9 +853,9 @@ checksum = "0ce7134b9999ecaf8bcd65542e436736ef32ddca1b3e06094cb6ec5755203b80"
 
 [[package]]
 name = "float-cmp"
-version = "0.9.0"
+version = "0.10.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4"
+checksum = "b09cf3155332e944990140d967ff5eceb70df778b34f77d8075db46e4704e6d8"
 dependencies = [
  "num-traits",
 ]
@@ -1014,9 +999,9 @@ checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f"
 
 [[package]]
 name = "glob"
-version = "0.3.1"
+version = "0.3.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b"
+checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2"
 
 [[package]]
 name = "globset"
@@ -1376,7 +1361,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "06093b57658c723a21da679530e061a8c25340fa5a6f98e313b542268c7e2a1f"
 dependencies = [
  "ascii-canvas",
- "bit-set 0.8.0",
+ "bit-set",
  "ena",
  "itertools 0.13.0",
  "lalrpop-util",
@@ -1409,15 +1394,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe"
 
 [[package]]
 name = "libc"
-version = "0.2.168"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5aaeb2981e0606ca11d79718f8bb01164f1d6ed75080182d3abf017e6d244b6d"
-
-[[package]]
-name = "libm"
-version = "0.2.11"
+version = "0.2.169"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8355be11b20d696c8f18f6cc018c4e372165b1fa8126cef092399c9951984ffa"
+checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a"
 
 [[package]]
 name = "linked-hash-map"
@@ -1537,9 +1516,9 @@ dependencies = [
 
 [[package]]
 name = "miniz_oxide"
-version = "0.8.0"
+version = "0.8.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e2d80299ef12ff69b16a84bb182e3b9df68b5a91574d3d4fa6e41b65deec4df1"
+checksum = "4ffbe83022cedc1d264172192511ae958937694cd57ce297164951b8b3568394"
 dependencies = [
  "adler2",
 ]
@@ -1587,14 +1566,13 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841"
 dependencies = [
  "autocfg",
- "libm",
 ]
 
 [[package]]
 name = "object"
-version = "0.36.5"
+version = "0.36.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "aedf0a2d09c573ed1d8d85b30c119153926a2b36dce0ab28322c09a117a4683e"
+checksum = "62948e14d923ea95ea2c7c86c71013138b66525b86bdc08d2dcc262bdb497b87"
 dependencies = [
  "memchr",
 ]
@@ -1734,9 +1712,9 @@ checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c"
 
 [[package]]
 name = "predicates"
-version = "3.1.2"
+version = "3.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7e9086cc7640c29a356d1a29fd134380bee9d8f79a17410aa76e7ad295f42c97"
+checksum = "a5d19ee57562043d37e82899fade9a22ebab7be9cef5026b07fda9cdd4293573"
 dependencies = [
  "anstyle",
  "difflib",
@@ -1748,15 +1726,15 @@ dependencies = [
 
 [[package]]
 name = "predicates-core"
-version = "1.0.8"
+version = "1.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ae8177bee8e75d6846599c6b9ff679ed51e882816914eec639944d7c9aa11931"
+checksum = "727e462b119fe9c93fd0eb1429a5f7647394014cf3c04ab2c0350eeb09095ffa"
 
 [[package]]
 name = "predicates-tree"
-version = "1.0.11"
+version = "1.0.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "41b740d195ed3166cd147c8047ec98db0e22ec019eb8eeb76d343b795304fb13"
+checksum = "72dd2d6d381dfb73a193c7fca536518d7caee39fc8503f74e7dc0be0531b425c"
 dependencies = [
  "predicates-core",
  "termtree",
@@ -1803,12 +1781,12 @@ dependencies = [
 
 [[package]]
 name = "proptest"
-version = "1.5.0"
+version = "1.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b4c2511913b88df1637da85cc8d96ec8e43a3f8bb8ccb71ee1ac240d6f3df58d"
+checksum = "14cae93065090804185d3b75f0bf93b8eeda30c7a9b4a33d3bdb3988d6229e50"
 dependencies = [
- "bit-set 0.5.3",
- "bit-vec 0.6.3",
+ "bit-set",
+ "bit-vec",
  "bitflags",
  "lazy_static",
  "num-traits",
@@ -1890,9 +1868,9 @@ checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0"
 
 [[package]]
 name = "quote"
-version = "1.0.37"
+version = "1.0.38"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b5b9d34b8991d19d98081b46eacdd8eb58c6f2b201139f7c5f643cc155a633af"
+checksum = "0e4dccaaaf89514f546c693ddc140f729f958c247918a13380cccc6078391acc"
 dependencies = [
  "proc-macro2",
 ]
@@ -2095,9 +2073,9 @@ dependencies = [
 
 [[package]]
 name = "rustversion"
-version = "1.0.18"
+version = "1.0.19"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0e819f2bc632f285be6d7cd36e25940d45b2391dd6d9b939e79de557f7014248"
+checksum = "f7c45b9784283f1b2e7fb61b42047c2fd678ef0960d4f6f1eba131594cc369d4"
 
 [[package]]
 name = "rusty-fork"
@@ -2149,9 +2127,9 @@ dependencies = [
 
 [[package]]
 name = "serde"
-version = "1.0.216"
+version = "1.0.217"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0b9781016e935a97e8beecf0c933758c97a5520d32930e460142b4cd80c6338e"
+checksum = "02fc4265df13d6fa1d00ecff087228cc0a2b5f3c0e87e258d8b94a156e984c70"
 dependencies = [
  "serde_derive",
 ]
@@ -2169,9 +2147,9 @@ dependencies = [
 
 [[package]]
 name = "serde_derive"
-version = "1.0.216"
+version = "1.0.217"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "46f859dbbf73865c6627ed570e78961cd3ac92407a2d117204c49232485da55e"
+checksum = "5a9bf7cf98d04a2b28aead066b7496853d4779c9cc183c440dbac457641e19a0"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -2191,9 +2169,9 @@ dependencies = [
 
 [[package]]
 name = "serde_json"
-version = "1.0.133"
+version = "1.0.134"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c7fceb2473b9166b2294ef05efcb65a3db80803f0b03ef86a5fc88a2b85ee377"
+checksum = "d00f4175c42ee48b15416f6193a959ba3a0d67fc699a0db9ad12df9f83991c7d"
 dependencies = [
  "indexmap 2.7.0",
  "itoa",
@@ -2213,9 +2191,9 @@ dependencies = [
 
 [[package]]
 name = "serde_with"
-version = "3.11.0"
+version = "3.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8e28bdad6db2b8340e449f7108f020b3b092e8583a9e3fb82713e1d4e71fe817"
+checksum = "d6b6f7f2fcb69f747921f79f3926bd1e203fce4fef62c268dd3abfb6d86029aa"
 dependencies = [
  "base64",
  "chrono",
@@ -2231,9 +2209,9 @@ dependencies = [
 
 [[package]]
 name = "serde_with_macros"
-version = "3.11.0"
+version = "3.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9d846214a9854ef724f3da161b426242d8de7c1fc7de2f89bb1efcb154dca79d"
+checksum = "8d00caa5193a3c8362ac2b73be6b9e768aa5a4b2f721d8f4b339600c3cb51f8e"
 dependencies = [
  "darling",
  "proc-macro2",
@@ -2369,9 +2347,9 @@ checksum = "b7401a30af6cb5818bb64852270bb722533397edcfc7344954a38f420819ece2"
 
 [[package]]
 name = "syn"
-version = "2.0.90"
+version = "2.0.93"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "919d3b74a5dd0ccd15aeb8f93e7006bd9e14c295087c9896a110f490752bcf31"
+checksum = "9c786062daee0d6db1132800e623df74274a0a87322d8e183338e01b3d98d058"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -2424,9 +2402,9 @@ dependencies = [
 
 [[package]]
 name = "termtree"
-version = "0.4.1"
+version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3369f5ac52d5eb6ab48c6b4ffdc8efbcad6b89c765749064ba298f2c68a16a76"
+checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683"
 
 [[package]]
 name = "textwrap"
@@ -2449,11 +2427,11 @@ dependencies = [
 
 [[package]]
 name = "thiserror"
-version = "2.0.7"
+version = "2.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "93605438cbd668185516ab499d589afb7ee1859ea3d5fc8f6b0755e1c7443767"
+checksum = "f072643fd0190df67a8bab670c20ef5d8737177d6ac6b2e9a236cb096206b2cc"
 dependencies = [
- "thiserror-impl 2.0.7",
+ "thiserror-impl 2.0.9",
 ]
 
 [[package]]
@@ -2469,9 +2447,9 @@ dependencies = [
 
 [[package]]
 name = "thiserror-impl"
-version = "2.0.7"
+version = "2.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e1d8749b4531af2117677a5fcd12b1348a3fe2b81e36e61ffeac5c4aa3273e36"
+checksum = "7b50fa271071aae2e6ee85f842e2e28ba8cd2c5fb67f11fcb1fd70b276f9e7d4"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -2537,9 +2515,9 @@ dependencies = [
 
 [[package]]
 name = "tinyvec"
-version = "1.8.0"
+version = "1.8.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "445e881f4f6d382d5f27c034e25eb92edd7c784ceab92a0937db7f2e9471b938"
+checksum = "022db8904dfa342efe721985167e9fcd16c29b226db4397ed752a761cfce81e8"
 dependencies = [
  "tinyvec_macros",
 ]
diff --git a/Cargo.toml b/Cargo.toml
index 20d2569e..ad556ee7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -43,6 +43,7 @@ rust-2018-idioms = "deny"
 # means we'll see it in local runs.
 [workspace.lints.clippy]
 nursery = { level = "warn", priority = -1 }
+needless_pass_by_value = "warn"
 
 # These lints may be worth enforcing, but cause a lot of noise at the moment.
 use_self = "allow"
diff --git a/cedar-policy-cli/Cargo.toml b/cedar-policy-cli/Cargo.toml
index 6c7a2514..5f55c704 100644
--- a/cedar-policy-cli/Cargo.toml
+++ b/cedar-policy-cli/Cargo.toml
@@ -37,8 +37,8 @@ protobufs = ["dep:prost", "dep:prost-build", "cedar-policy/protobufs", "cedar-po
 [dev-dependencies]
 assert_cmd = "2.0"
 tempfile = "3"
-glob = "0.3.1"
-predicates = "3.1.0"
+glob = "0.3.2"
+predicates = "3.1.3"
 rstest = "0.23.0"
 
 # We override the name of the binary for src/main.rs, which otherwise would be
diff --git a/cedar-policy-cli/src/lib.rs b/cedar-policy-cli/src/lib.rs
index 9a001c6b..089026f2 100644
--- a/cedar-policy-cli/src/lib.rs
+++ b/cedar-policy-cli/src/lib.rs
@@ -947,7 +947,7 @@ fn translate_policy_inner(args: &TranslatePolicyArgs) -> Result<String> {
     let translate = match args.direction {
         PolicyTranslationDirection::CedarToJson => translate_policy_to_json,
     };
-    read_from_file_or_stdin(args.input_file.clone(), "policy").and_then(translate)
+    read_from_file_or_stdin(args.input_file.as_ref(), "policy").and_then(translate)
 }
 
 pub fn translate_policy(args: &TranslatePolicyArgs) -> CedarExitCode {
@@ -984,7 +984,7 @@ fn translate_schema_inner(args: &TranslateSchemaArgs) -> Result<String> {
         SchemaTranslationDirection::JsonToCedar => translate_schema_to_cedar,
         SchemaTranslationDirection::CedarToJson => translate_schema_to_json,
     };
-    read_from_file_or_stdin(args.input_file.clone(), "schema").and_then(translate)
+    read_from_file_or_stdin(args.input_file.as_ref(), "schema").and_then(translate)
 }
 
 pub fn translate_schema(args: &TranslateSchemaArgs) -> CedarExitCode {
@@ -1377,7 +1377,7 @@ fn load_entities(entities_filename: impl AsRef<Path>, schema: Option<&Schema>) -
 /// This will rename template-linked policies to the id of their template, which may
 /// cause id conflicts, so only call this function before instancing
 /// templates into the policy set.
-fn rename_from_id_annotation(ps: PolicySet) -> Result<PolicySet> {
+fn rename_from_id_annotation(ps: &PolicySet) -> Result<PolicySet> {
     let mut new_ps = PolicySet::new();
     let t_iter = ps.templates().map(|t| match t.annotation("id") {
         None => Ok(t.clone()),
@@ -1443,7 +1443,7 @@ fn read_cedar_policy_set(
             Report::new(err).with_source_code(NamedSource::new(name, ps_str))
         })
         .wrap_err_with(|| format!("failed to parse {context}"))?;
-    rename_from_id_annotation(ps)
+    rename_from_id_annotation(&ps)
 }
 
 /// Read a policy set, static policy or policy template, in Cedar JSON (EST) syntax, from the file given
diff --git a/cedar-policy-core/Cargo.toml b/cedar-policy-core/Cargo.toml
index 9c0fa7ba..31992c6d 100644
--- a/cedar-policy-core/Cargo.toml
+++ b/cedar-policy-core/Cargo.toml
@@ -14,7 +14,7 @@ repository.workspace = true
 
 [dependencies]
 serde = { version = "1.0", features = ["derive", "rc"] }
-serde_with = { version = "3.0", features = ["json"] }
+serde_with = { version = "3.12", features = ["json"] }
 serde_json = "1.0"
 lalrpop-util = { version = "0.22.0", features = ["lexer"] }
 lazy_static = "1.4"
diff --git a/cedar-policy-core/src/ast/extension.rs b/cedar-policy-core/src/ast/extension.rs
index db1555d0..559af3f2 100644
--- a/cedar-policy-core/src/ast/extension.rs
+++ b/cedar-policy-core/src/ast/extension.rs
@@ -124,9 +124,22 @@ pub enum CallStyle {
 
 // Note: we could use currying to make this a little nicer
 
-/// Trait object that implements the extension function call.
-pub type ExtensionFunctionObject =
-    Box<dyn Fn(&[Value]) -> evaluator::Result<ExtensionOutputValue> + Sync + Send + 'static>;
+macro_rules! extension_function_object {
+    ( $( $tys:ty ), * ) => {
+        Box<dyn Fn($($tys,)*) -> evaluator::Result<ExtensionOutputValue> + Sync + Send + 'static>
+    }
+}
+
+/// Trait object that implements the extension function call accepting any number of arguments.
+pub type ExtensionFunctionObject = extension_function_object!(&[Value]);
+/// Trait object that implements the extension function call accepting exactly 0 arguments
+pub type NullaryExtensionFunctionObject = extension_function_object!();
+/// Trait object that implements the extension function call accepting exactly 1 arguments
+pub type UnaryExtensionFunctionObject = extension_function_object!(&Value);
+/// Trait object that implements the extension function call accepting exactly 2 arguments
+pub type BinaryExtensionFunctionObject = extension_function_object!(&Value, &Value);
+/// Trait object that implements the extension function call accepting exactly 3 arguments
+pub type TernaryExtensionFunctionObject = extension_function_object!(&Value, &Value, &Value);
 
 /// Extension function. These can be called by the given `name` in Ceder
 /// expressions.
@@ -172,7 +185,7 @@ impl ExtensionFunction {
     pub fn nullary(
         name: Name,
         style: CallStyle,
-        func: Box<dyn Fn() -> evaluator::Result<ExtensionOutputValue> + Sync + Send + 'static>,
+        func: NullaryExtensionFunctionObject,
         return_type: SchemaType,
     ) -> Self {
         Self::new(
@@ -200,14 +213,14 @@ impl ExtensionFunction {
     pub fn partial_eval_unknown(
         name: Name,
         style: CallStyle,
-        func: Box<dyn Fn(Value) -> evaluator::Result<ExtensionOutputValue> + Sync + Send + 'static>,
+        func: UnaryExtensionFunctionObject,
         arg_type: SchemaType,
     ) -> Self {
         Self::new(
             name.clone(),
             style,
             Box::new(move |args: &[Value]| match args.first() {
-                Some(arg) => func(arg.clone()),
+                Some(arg) => func(arg),
                 None => Err(evaluator::EvaluationError::wrong_num_arguments(
                     name.clone(),
                     1,
@@ -221,10 +234,11 @@ impl ExtensionFunction {
     }
 
     /// Create a new `ExtensionFunction` taking one argument
+    #[allow(clippy::type_complexity)]
     pub fn unary(
         name: Name,
         style: CallStyle,
-        func: Box<dyn Fn(Value) -> evaluator::Result<ExtensionOutputValue> + Sync + Send + 'static>,
+        func: UnaryExtensionFunctionObject,
         return_type: SchemaType,
         arg_type: SchemaType,
     ) -> Self {
@@ -232,7 +246,7 @@ impl ExtensionFunction {
             name.clone(),
             style,
             Box::new(move |args: &[Value]| match &args {
-                &[arg] => func(arg.clone()),
+                &[arg] => func(arg),
                 _ => Err(evaluator::EvaluationError::wrong_num_arguments(
                     name.clone(),
                     1,
@@ -246,12 +260,11 @@ impl ExtensionFunction {
     }
 
     /// Create a new `ExtensionFunction` taking two arguments
+    #[allow(clippy::type_complexity)]
     pub fn binary(
         name: Name,
         style: CallStyle,
-        func: Box<
-            dyn Fn(Value, Value) -> evaluator::Result<ExtensionOutputValue> + Sync + Send + 'static,
-        >,
+        func: BinaryExtensionFunctionObject,
         return_type: SchemaType,
         arg_types: (SchemaType, SchemaType),
     ) -> Self {
@@ -259,7 +272,7 @@ impl ExtensionFunction {
             name.clone(),
             style,
             Box::new(move |args: &[Value]| match &args {
-                &[first, second] => func(first.clone(), second.clone()),
+                &[first, second] => func(first, second),
                 _ => Err(evaluator::EvaluationError::wrong_num_arguments(
                     name.clone(),
                     2,
@@ -273,15 +286,11 @@ impl ExtensionFunction {
     }
 
     /// Create a new `ExtensionFunction` taking three arguments
+    #[allow(clippy::type_complexity)]
     pub fn ternary(
         name: Name,
         style: CallStyle,
-        func: Box<
-            dyn Fn(Value, Value, Value) -> evaluator::Result<ExtensionOutputValue>
-                + Sync
-                + Send
-                + 'static,
-        >,
+        func: TernaryExtensionFunctionObject,
         return_type: SchemaType,
         arg_types: (SchemaType, SchemaType, SchemaType),
     ) -> Self {
@@ -289,7 +298,7 @@ impl ExtensionFunction {
             name.clone(),
             style,
             Box::new(move |args: &[Value]| match &args {
-                &[first, second, third] => func(first.clone(), second.clone(), third.clone()),
+                &[first, second, third] => func(first, second, third),
                 _ => Err(evaluator::EvaluationError::wrong_num_arguments(
                     name.clone(),
                     3,
diff --git a/cedar-policy-core/src/ast/policy_set.rs b/cedar-policy-core/src/ast/policy_set.rs
index af844f8a..c5267a9a 100644
--- a/cedar-policy-core/src/ast/policy_set.rs
+++ b/cedar-policy-core/src/ast/policy_set.rs
@@ -433,7 +433,7 @@ impl PolicySet {
         match self.templates.remove(policy_id) {
             Some(t) => {
                 self.template_to_links_map.remove(policy_id);
-                Ok((*t).clone())
+                Ok(Arc::unwrap_or_clone(t))
             }
             None => panic!("Found in template_to_links_map but not in templates"),
         }
@@ -630,11 +630,10 @@ mod test {
         .expect("Failed to parse");
         pset.add_template(template).expect("Add failed");
 
-        let env: HashMap<SlotId, EntityUID> = [(
+        let env: HashMap<SlotId, EntityUID> = std::iter::once((
             SlotId::principal(),
             r#"Test::"test""#.parse().expect("Failed to parse"),
-        )]
-        .into_iter()
+        ))
         .collect();
 
         let r = pset.link(PolicyID::from_string("t"), PolicyID::from_string("id"), env);
@@ -669,11 +668,10 @@ mod test {
             )
             .expect("Failed to parse"),
         );
-        let env1: HashMap<SlotId, EntityUID> = [(
+        let env1: HashMap<SlotId, EntityUID> = std::iter::once((
             SlotId::principal(),
             r#"Test::"test1""#.parse().expect("Failed to parse"),
-        )]
-        .into_iter()
+        ))
         .collect();
 
         let p1 = Template::link(Arc::clone(&template), PolicyID::from_string("link"), env1)
@@ -686,11 +684,10 @@ mod test {
             "Adding link should implicitly add the template"
         );
 
-        let env2: HashMap<SlotId, EntityUID> = [(
+        let env2: HashMap<SlotId, EntityUID> = std::iter::once((
             SlotId::principal(),
             r#"Test::"test2""#.parse().expect("Failed to parse"),
-        )]
-        .into_iter()
+        ))
         .collect();
 
         let p2 = Template::link(
@@ -717,11 +714,10 @@ mod test {
             )
             .expect("Failed to parse"),
         );
-        let env3: HashMap<SlotId, EntityUID> = [(
+        let env3: HashMap<SlotId, EntityUID> = std::iter::once((
             SlotId::resource(),
             r#"Test::"test3""#.parse().expect("Failed to parse"),
-        )]
-        .into_iter()
+        ))
         .collect();
 
         let p4 = Template::link(
@@ -781,9 +777,7 @@ mod test {
         set.link(
             PolicyID::from_string("template"),
             PolicyID::from_string("id"),
-            [(SlotId::principal(), EntityUID::with_eid("eid"))]
-                .into_iter()
-                .collect(),
+            std::iter::once((SlotId::principal(), EntityUID::with_eid("eid"))).collect(),
         )
         .expect("Linking failed!");
         assert_eq!(set.static_policies().count(), 1);
diff --git a/cedar-policy-core/src/entities.rs b/cedar-policy-core/src/entities.rs
index b5c039b7..6ea78cef 100644
--- a/cedar-policy-core/src/entities.rs
+++ b/cedar-policy-core/src/entities.rs
@@ -508,6 +508,7 @@ pub enum TCComputation {
 #[cfg(test)]
 // PANIC SAFETY unit tests
 #[allow(clippy::panic)]
+#[allow(clippy::cognitive_complexity)]
 mod json_parsing_tests {
 
     use super::*;
@@ -1731,13 +1732,13 @@ mod json_parsing_tests {
     }
 
     /// helper function
-    fn test_entities() -> (Entity, Entity, Entity, Entity) {
-        (
+    fn test_entities() -> [Entity; 4] {
+        [
             Entity::with_uid(EntityUID::with_eid("test_principal")),
             Entity::with_uid(EntityUID::with_eid("test_action")),
             Entity::with_uid(EntityUID::with_eid("test_resource")),
             Entity::with_uid(EntityUID::with_eid("test")),
-        )
+        ]
     }
 
     /// Test that we can take an Entities, write it to JSON, parse that JSON
@@ -1750,9 +1751,8 @@ mod json_parsing_tests {
             roundtrip(&empty_entities).expect("should roundtrip without errors")
         );
 
-        let (e0, e1, e2, e3) = test_entities();
         let entities = Entities::from_entities(
-            [e0, e1, e2, e3],
+            test_entities(),
             None::<&NoEntitiesSchema>,
             TCComputation::ComputeNow,
             Extensions::none(),
@@ -1960,17 +1960,18 @@ mod json_parsing_tests {
 
 // PANIC SAFETY: Unit Test Code
 #[allow(clippy::panic)]
+#[allow(clippy::cognitive_complexity)]
 #[cfg(test)]
-// PANIC SAFETY unit tests
-#[allow(clippy::panic)]
 mod entities_tests {
     use super::*;
 
     #[test]
     fn empty_entities() {
         let e = Entities::new();
-        let es = e.iter().collect::<Vec<_>>();
-        assert!(es.is_empty(), "This vec should be empty");
+        assert!(
+            e.iter().next().is_none(),
+            "The entity store should be empty"
+        );
     }
 
     /// helper function
@@ -2051,6 +2052,7 @@ mod entities_tests {
 
 // PANIC SAFETY: Unit Test Code
 #[allow(clippy::panic)]
+#[allow(clippy::cognitive_complexity)]
 #[cfg(test)]
 mod schema_based_parsing_tests {
     use super::json::NullEntityTypeDescription;
@@ -2079,9 +2081,7 @@ mod schema_based_parsing_tests {
                 r#"Action::"view""# => Some(Arc::new(Entity::new_with_attr_partial_value(
                     action.clone(),
                     [(SmolStr::from("foo"), PartialValue::from(34))],
-                    [r#"Action::"readOnly""#.parse().expect("valid uid")]
-                        .into_iter()
-                        .collect(),
+                    std::iter::once(r#"Action::"readOnly""#.parse().expect("valid uid")).collect(),
                 ))),
                 r#"Action::"readOnly""# => Some(Arc::new(Entity::with_uid(
                     r#"Action::"readOnly""#.parse().expect("valid uid"),
@@ -2175,11 +2175,10 @@ mod schema_based_parsing_tests {
                         (
                             "inner3".into(),
                             AttributeType::required(SchemaType::Record {
-                                attrs: [(
+                                attrs: std::iter::once((
                                     "innerinner".into(),
                                     AttributeType::required(employee_ty()),
-                                )]
-                                .into_iter()
+                                ))
                                 .collect(),
                                 open_attrs: false,
                             }),
diff --git a/cedar-policy-core/src/est/expr.rs b/cedar-policy-core/src/est/expr.rs
index 49edc729..0440da51 100644
--- a/cedar-policy-core/src/est/expr.rs
+++ b/cedar-policy-core/src/est/expr.rs
@@ -673,96 +673,96 @@ impl Expr {
                 ExprNoExt::Var(_) => Ok(self),
                 ExprNoExt::Slot(_) => Ok(self),
                 ExprNoExt::Not { arg } => Ok(Expr::ExprNoExt(ExprNoExt::Not {
-                    arg: Arc::new((*arg).clone().sub_entity_literals(mapping)?),
+                    arg: Arc::new(Arc::unwrap_or_clone(arg).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Neg { arg } => Ok(Expr::ExprNoExt(ExprNoExt::Neg {
-                    arg: Arc::new((*arg).clone().sub_entity_literals(mapping)?),
+                    arg: Arc::new(Arc::unwrap_or_clone(arg).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Eq { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Eq {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::NotEq { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::NotEq {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::In { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::In {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Less { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Less {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::LessEq { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::LessEq {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Greater { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Greater {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::GreaterEq { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::GreaterEq {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::And { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::And {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Or { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Or {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Add { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Add {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Sub { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Sub {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Mul { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Mul {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::Contains { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::Contains {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::ContainsAll { left, right } => {
                     Ok(Expr::ExprNoExt(ExprNoExt::ContainsAll {
-                        left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                        right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                        left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                        right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                     }))
                 }
                 ExprNoExt::ContainsAny { left, right } => {
                     Ok(Expr::ExprNoExt(ExprNoExt::ContainsAny {
-                        left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                        right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                        left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                        right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                     }))
                 }
                 ExprNoExt::IsEmpty { arg } => Ok(Expr::ExprNoExt(ExprNoExt::IsEmpty {
-                    arg: Arc::new((*arg).clone().sub_entity_literals(mapping)?),
+                    arg: Arc::new(Arc::unwrap_or_clone(arg).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::GetTag { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::GetTag {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::HasTag { left, right } => Ok(Expr::ExprNoExt(ExprNoExt::HasTag {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
-                    right: Arc::new((*right).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
+                    right: Arc::new(Arc::unwrap_or_clone(right).sub_entity_literals(mapping)?),
                 })),
                 ExprNoExt::GetAttr { left, attr } => Ok(Expr::ExprNoExt(ExprNoExt::GetAttr {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
                     attr,
                 })),
                 ExprNoExt::HasAttr { left, attr } => Ok(Expr::ExprNoExt(ExprNoExt::HasAttr {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
                     attr,
                 })),
                 ExprNoExt::Like { left, pattern } => Ok(Expr::ExprNoExt(ExprNoExt::Like {
-                    left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
+                    left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
                     pattern,
                 })),
                 ExprNoExt::Is {
@@ -771,12 +771,14 @@ impl Expr {
                     in_expr,
                 } => match in_expr {
                     Some(in_expr) => Ok(Expr::ExprNoExt(ExprNoExt::Is {
-                        left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
+                        left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
                         entity_type,
-                        in_expr: Some(Arc::new((*in_expr).clone().sub_entity_literals(mapping)?)),
+                        in_expr: Some(Arc::new(
+                            Arc::unwrap_or_clone(in_expr).sub_entity_literals(mapping)?,
+                        )),
                     })),
                     None => Ok(Expr::ExprNoExt(ExprNoExt::Is {
-                        left: Arc::new((*left).clone().sub_entity_literals(mapping)?),
+                        left: Arc::new(Arc::unwrap_or_clone(left).sub_entity_literals(mapping)?),
                         entity_type,
                         in_expr: None,
                     })),
@@ -786,9 +788,15 @@ impl Expr {
                     then_expr,
                     else_expr,
                 } => Ok(Expr::ExprNoExt(ExprNoExt::If {
-                    cond_expr: Arc::new((*cond_expr).clone().sub_entity_literals(mapping)?),
-                    then_expr: Arc::new((*then_expr).clone().sub_entity_literals(mapping)?),
-                    else_expr: Arc::new((*else_expr).clone().sub_entity_literals(mapping)?),
+                    cond_expr: Arc::new(
+                        Arc::unwrap_or_clone(cond_expr).sub_entity_literals(mapping)?,
+                    ),
+                    then_expr: Arc::new(
+                        Arc::unwrap_or_clone(then_expr).sub_entity_literals(mapping)?,
+                    ),
+                    else_expr: Arc::new(
+                        Arc::unwrap_or_clone(else_expr).sub_entity_literals(mapping)?,
+                    ),
                 })),
                 ExprNoExt::Set(v) => {
                     let mut new_v = vec![];
@@ -833,90 +841,92 @@ impl Expr {
             Expr::ExprNoExt(ExprNoExt::Var(var)) => Ok(ast::Expr::var(var)),
             Expr::ExprNoExt(ExprNoExt::Slot(slot)) => Ok(ast::Expr::slot(slot)),
             Expr::ExprNoExt(ExprNoExt::Not { arg }) => {
-                Ok(ast::Expr::not((*arg).clone().try_into_ast(id)?))
+                Ok(ast::Expr::not(Arc::unwrap_or_clone(arg).try_into_ast(id)?))
             }
             Expr::ExprNoExt(ExprNoExt::Neg { arg }) => {
-                Ok(ast::Expr::neg((*arg).clone().try_into_ast(id)?))
+                Ok(ast::Expr::neg(Arc::unwrap_or_clone(arg).try_into_ast(id)?))
             }
             Expr::ExprNoExt(ExprNoExt::Eq { left, right }) => Ok(ast::Expr::is_eq(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::NotEq { left, right }) => Ok(ast::Expr::noteq(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::In { left, right }) => Ok(ast::Expr::is_in(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Less { left, right }) => Ok(ast::Expr::less(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::LessEq { left, right }) => Ok(ast::Expr::lesseq(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Greater { left, right }) => Ok(ast::Expr::greater(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::GreaterEq { left, right }) => Ok(ast::Expr::greatereq(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::And { left, right }) => Ok(ast::Expr::and(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Or { left, right }) => Ok(ast::Expr::or(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Add { left, right }) => Ok(ast::Expr::add(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Sub { left, right }) => Ok(ast::Expr::sub(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Mul { left, right }) => Ok(ast::Expr::mul(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Contains { left, right }) => Ok(ast::Expr::contains(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::ContainsAll { left, right }) => Ok(ast::Expr::contains_all(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::ContainsAny { left, right }) => Ok(ast::Expr::contains_any(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
+            )),
+            Expr::ExprNoExt(ExprNoExt::IsEmpty { arg }) => Ok(ast::Expr::is_empty(
+                Arc::unwrap_or_clone(arg).try_into_ast(id)?,
             )),
-            Expr::ExprNoExt(ExprNoExt::IsEmpty { arg }) => {
-                Ok(ast::Expr::is_empty((*arg).clone().try_into_ast(id)?))
-            }
             Expr::ExprNoExt(ExprNoExt::GetTag { left, right }) => Ok(ast::Expr::get_tag(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::HasTag { left, right }) => Ok(ast::Expr::has_tag(
-                (*left).clone().try_into_ast(id.clone())?,
-                (*right).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(right).try_into_ast(id)?,
+            )),
+            Expr::ExprNoExt(ExprNoExt::GetAttr { left, attr }) => Ok(ast::Expr::get_attr(
+                Arc::unwrap_or_clone(left).try_into_ast(id)?,
+                attr,
+            )),
+            Expr::ExprNoExt(ExprNoExt::HasAttr { left, attr }) => Ok(ast::Expr::has_attr(
+                Arc::unwrap_or_clone(left).try_into_ast(id)?,
+                attr,
             )),
-            Expr::ExprNoExt(ExprNoExt::GetAttr { left, attr }) => {
-                Ok(ast::Expr::get_attr((*left).clone().try_into_ast(id)?, attr))
-            }
-            Expr::ExprNoExt(ExprNoExt::HasAttr { left, attr }) => {
-                Ok(ast::Expr::has_attr((*left).clone().try_into_ast(id)?, attr))
-            }
             Expr::ExprNoExt(ExprNoExt::Like { left, pattern }) => Ok(ast::Expr::like(
-                (*left).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(left).try_into_ast(id)?,
                 crate::ast::Pattern::from(pattern.as_slice()),
             )),
             Expr::ExprNoExt(ExprNoExt::Is {
@@ -926,14 +936,14 @@ impl Expr {
             }) => ast::EntityType::from_normalized_str(entity_type.as_str())
                 .map_err(FromJsonError::InvalidEntityType)
                 .and_then(|entity_type_name| {
-                    let left: ast::Expr = (*left).clone().try_into_ast(id.clone())?;
+                    let left: ast::Expr = Arc::unwrap_or_clone(left).try_into_ast(id.clone())?;
                     let is_expr = ast::Expr::is_entity_type(left.clone(), entity_type_name);
                     match in_expr {
                         // The AST doesn't have an `... is ... in ..` node, so
                         // we represent it as a conjunction of `is` and `in`.
                         Some(in_expr) => Ok(ast::Expr::and(
                             is_expr,
-                            ast::Expr::is_in(left, (*in_expr).clone().try_into_ast(id)?),
+                            ast::Expr::is_in(left, Arc::unwrap_or_clone(in_expr).try_into_ast(id)?),
                         )),
                         None => Ok(is_expr),
                     }
@@ -943,9 +953,9 @@ impl Expr {
                 then_expr,
                 else_expr,
             }) => Ok(ast::Expr::ite(
-                (*cond_expr).clone().try_into_ast(id.clone())?,
-                (*then_expr).clone().try_into_ast(id.clone())?,
-                (*else_expr).clone().try_into_ast(id)?,
+                Arc::unwrap_or_clone(cond_expr).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(then_expr).try_into_ast(id.clone())?,
+                Arc::unwrap_or_clone(else_expr).try_into_ast(id)?,
             )),
             Expr::ExprNoExt(ExprNoExt::Set(elements)) => Ok(ast::Expr::set(
                 elements
@@ -1511,7 +1521,7 @@ impl TryFrom<&Node<Option<cst::Member>>> for Expr {
                                     extract_single_argument(args, "containsAny()", &access.loc)?,
                                 )),
                                 "isEmpty" => {
-                                    require_zero_arguments(args, "isEmpty()", &access.loc)?;
+                                    require_zero_arguments(&args, "isEmpty()", &access.loc)?;
                                     Either::Right(Expr::is_empty(left))
                                 }
                                 "getTag" => Either::Right(Expr::get_tag(
@@ -1584,7 +1594,7 @@ pub fn extract_single_argument<T>(
 
 /// Return a wrong arity error if the iterator has any elements.
 pub fn require_zero_arguments<T>(
-    args: impl ExactSizeIterator<Item = T>,
+    args: &impl ExactSizeIterator<Item = T>,
     fn_name: &'static str,
     loc: &Loc,
 ) -> Result<(), ParseErrors> {
diff --git a/cedar-policy-core/src/evaluator.rs b/cedar-policy-core/src/evaluator.rs
index a8b30e52..5a434d72 100644
--- a/cedar-policy-core/src/evaluator.rs
+++ b/cedar-policy-core/src/evaluator.rs
@@ -720,7 +720,7 @@ impl<'e> Evaluator<'e> {
         // `rhs` is a list of all the UIDs for which we need to
         // check if `uid1` is a descendant of
         let rhs = match arg2.value {
-            ValueKind::Lit(Literal::EntityUID(uid)) => vec![(*uid).clone()],
+            ValueKind::Lit(Literal::EntityUID(uid)) => vec![Arc::unwrap_or_clone(uid)],
             // we assume that iterating the `authoritative` BTreeSet is
             // approximately the same cost as iterating the `fast` HashSet
             ValueKind::Set(Set { authoritative, .. }) => authoritative
@@ -988,6 +988,7 @@ fn stack_size_check() -> Result<()> {
 
 // PANIC SAFETY: Unit Test Code
 #[allow(clippy::panic)]
+#[allow(clippy::cognitive_complexity)]
 #[cfg(test)]
 pub(crate) mod test {
     use std::str::FromStr;
@@ -5033,9 +5034,8 @@ pub(crate) mod test {
             Either::Right(expr) => {
                 println!("{expr}");
                 assert!(expr.contains_unknown());
-                let m: HashMap<_, _> = [("principal".into(), Value::from(euid))]
-                    .into_iter()
-                    .collect();
+                let m: HashMap<_, _> =
+                    std::iter::once(("principal".into(), Value::from(euid))).collect();
                 let new_expr = expr.substitute_typed(&m).unwrap();
                 assert_eq!(
                     e.partial_interpret(&new_expr, &HashMap::new())
diff --git a/cedar-policy-core/src/extensions/datetime.rs b/cedar-policy-core/src/extensions/datetime.rs
index 31e43aca..b98c1185 100644
--- a/cedar-policy-core/src/extensions/datetime.rs
+++ b/cedar-policy-core/src/extensions/datetime.rs
@@ -71,7 +71,7 @@ mod constants {
 }
 
 // The `datetime` type, represented internally as an `i64`.
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
 struct DateTime {
     // The number of non-leap milliseconds from the Unix epoch
     epoch: i64,
@@ -91,7 +91,7 @@ fn extension_err(
 }
 
 fn construct_from_str<Ext>(
-    arg: Value,
+    arg: &Value,
     constructor_name: Name,
     constructor: impl Fn(&str) -> Result<Ext, EvaluationError>,
 ) -> evaluator::Result<ExtensionOutputValue>
@@ -101,8 +101,11 @@ where
     let s = arg.get_as_string()?;
     let ext_value: Ext = constructor(s)?;
     let arg_source_loc = arg.source_loc().cloned();
-    let e =
-        RepresentableExtensionValue::new(Arc::new(ext_value), constructor_name, vec![arg.into()]);
+    let e = RepresentableExtensionValue::new(
+        Arc::new(ext_value),
+        constructor_name,
+        vec![arg.clone().into()],
+    );
     Ok(Value {
         value: ValueKind::ExtensionValue(Arc::new(e)),
         loc: arg_source_loc, // follow the same convention as the `decimal` extension
@@ -112,7 +115,7 @@ where
 
 /// Cedar function that constructs a `datetime` Cedar type from a
 /// Cedar string
-fn datetime_from_str(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
+fn datetime_from_str(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
     construct_from_str(arg, DATETIME_CONSTRUCTOR_NAME.clone(), |s| {
         parse_datetime(s).map(DateTime::from).map_err(|err| {
             extension_err(
@@ -158,23 +161,23 @@ where
 }
 
 /// Check that `v` is a datetime type and, if it is, return the wrapped value
-fn as_datetime(v: &Value) -> Result<&DateTime, evaluator::EvaluationError> {
-    as_ext(v, &DATETIME_CONSTRUCTOR_NAME)
+fn as_datetime(v: &Value) -> Result<DateTime, evaluator::EvaluationError> {
+    as_ext(v, &DATETIME_CONSTRUCTOR_NAME).copied()
 }
 
 /// Check that `v` is a duration type and, if it is, return the wrapped value
-fn as_duration(v: &Value) -> Result<&Duration, evaluator::EvaluationError> {
-    as_ext(v, &DURATION_CONSTRUCTOR_NAME)
+fn as_duration(v: &Value) -> Result<Duration, evaluator::EvaluationError> {
+    as_ext(v, &DURATION_CONSTRUCTOR_NAME).copied()
 }
 
-fn offset(datetime: Value, duration: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let datetime = as_datetime(&datetime)?;
-    let duration = as_duration(&duration)?;
-    let ret = datetime.offset(duration.clone()).ok_or_else(|| {
+fn offset(datetime: &Value, duration: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let datetime = as_datetime(datetime)?;
+    let duration = as_duration(duration)?;
+    let ret = datetime.offset(duration).ok_or_else(|| {
         extension_err(
             format!(
                 "overflows when adding an offset: {}+({})",
-                RestrictedExpr::from(datetime.clone()),
+                RestrictedExpr::from(datetime),
                 duration
             ),
             &OFFSET_METHOD_NAME,
@@ -188,15 +191,15 @@ fn offset(datetime: Value, duration: Value) -> evaluator::Result<ExtensionOutput
     .into())
 }
 
-fn duration_since(lhs: Value, rhs: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let lhs = as_datetime(&lhs)?;
-    let rhs = as_datetime(&rhs)?;
-    let ret = lhs.duration_since(rhs.clone()).ok_or_else(|| {
+fn duration_since(lhs: &Value, rhs: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let lhs = as_datetime(lhs)?;
+    let rhs = as_datetime(rhs)?;
+    let ret = lhs.duration_since(rhs).ok_or_else(|| {
         extension_err(
             format!(
                 "overflows when computing the duration between {} and {}",
-                RestrictedExpr::from(lhs.clone()),
-                RestrictedExpr::from(rhs.clone())
+                RestrictedExpr::from(lhs),
+                RestrictedExpr::from(rhs)
             ),
             &DURATION_SINCE_NAME,
             None,
@@ -209,13 +212,13 @@ fn duration_since(lhs: Value, rhs: Value) -> evaluator::Result<ExtensionOutputVa
     .into())
 }
 
-fn to_date(value: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let d = as_datetime(&value)?;
+fn to_date(value: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let d = as_datetime(value)?;
     let ret = d.to_date().ok_or_else(|| {
         extension_err(
             format!(
                 "overflows when computing the date of {}",
-                RestrictedExpr::from(d.clone()),
+                RestrictedExpr::from(d),
             ),
             &TO_DATE_NAME,
             None,
@@ -228,8 +231,8 @@ fn to_date(value: Value) -> evaluator::Result<ExtensionOutputValue> {
     .into())
 }
 
-fn to_time(value: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let d = as_datetime(&value)?;
+fn to_time(value: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let d = as_datetime(value)?;
     let ret = d.to_time();
     Ok(Value {
         value: ValueKind::ExtensionValue(Arc::new(ret.into())),
@@ -251,13 +254,13 @@ impl DateTime {
     const DAY_IN_MILLISECONDS: i64 = 1000 * 3600 * 24;
     const UNIX_EPOCH_STR: &'static str = "1970-01-01";
 
-    fn offset(&self, duration: Duration) -> Option<Self> {
+    fn offset(self, duration: Duration) -> Option<Self> {
         self.epoch
             .checked_add(duration.ms)
             .map(|epoch| Self { epoch })
     }
 
-    fn duration_since(&self, other: DateTime) -> Option<Duration> {
+    fn duration_since(self, other: DateTime) -> Option<Duration> {
         self.epoch
             .checked_sub(other.epoch)
             .map(|ms| Duration { ms })
@@ -265,7 +268,7 @@ impl DateTime {
 
     // essentially `self.epoch.div_floor(Self::DAY_IN_MILLISECONDS) * Self::DAY_IN_MILLISECONDS`
     // but `div_floor` is only available on nightly
-    fn to_date(&self) -> Option<Self> {
+    fn to_date(self) -> Option<Self> {
         if self.epoch.is_negative() {
             if self.epoch % Self::DAY_IN_MILLISECONDS == 0 {
                 Some(self.epoch)
@@ -278,7 +281,7 @@ impl DateTime {
         .map(|epoch| Self { epoch })
     }
 
-    fn to_time(&self) -> Duration {
+    fn to_time(self) -> Duration {
         Duration {
             ms: if self.epoch.is_negative() {
                 let rem = self.epoch % Self::DAY_IN_MILLISECONDS;
@@ -293,7 +296,7 @@ impl DateTime {
         }
     }
 
-    fn as_ext_func_call(&self) -> (Name, Vec<RestrictedExpr>) {
+    fn as_ext_func_call(self) -> (Name, Vec<RestrictedExpr>) {
         (
             OFFSET_METHOD_NAME.clone(),
             vec![
@@ -335,7 +338,7 @@ impl From<NaiveDateTime> for DateTime {
 }
 
 // The `duration` type
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 struct Duration {
     // The number of milliseconds
     ms: i64,
@@ -376,7 +379,7 @@ impl From<Duration> for RepresentableExtensionValue {
 
 /// Cedar function that constructs a `duration` Cedar type from a
 /// Cedar string
-fn duration_from_str(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
+fn duration_from_str(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
     construct_from_str(arg, DURATION_CONSTRUCTOR_NAME.clone(), |s| {
         parse_duration(s).map_err(|err| {
             extension_err(
@@ -389,27 +392,27 @@ fn duration_from_str(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
 }
 
 impl Duration {
-    fn to_milliseconds(&self) -> i64 {
+    fn to_milliseconds(self) -> i64 {
         self.ms
     }
 
-    fn to_seconds(&self) -> i64 {
+    fn to_seconds(self) -> i64 {
         self.to_milliseconds() / 1000
     }
 
-    fn to_minutes(&self) -> i64 {
+    fn to_minutes(self) -> i64 {
         self.to_seconds() / 60
     }
 
-    fn to_hours(&self) -> i64 {
+    fn to_hours(self) -> i64 {
         self.to_minutes() / 60
     }
 
-    fn to_days(&self) -> i64 {
+    fn to_days(self) -> i64 {
         self.to_hours() / 24
     }
 
-    fn as_ext_func_call(&self) -> (Name, Vec<RestrictedExpr>) {
+    fn as_ext_func_call(self) -> (Name, Vec<RestrictedExpr>) {
         (
             DURATION_CONSTRUCTOR_NAME.clone(),
             vec![Value::from(self.to_string()).into()],
@@ -418,10 +421,10 @@ impl Duration {
 }
 
 fn duration_method(
-    value: Value,
-    internal_func: impl Fn(&Duration) -> i64,
+    value: &Value,
+    internal_func: impl Fn(Duration) -> i64,
 ) -> evaluator::Result<ExtensionOutputValue> {
-    let d = as_duration(&value)?;
+    let d = as_duration(value)?;
     Ok(Value::from(internal_func(d)).into())
 }
 
@@ -719,6 +722,7 @@ pub fn extension() -> Extension {
 }
 
 #[cfg(test)]
+#[allow(clippy::cognitive_complexity)]
 mod tests {
     use std::{str::FromStr, sync::Arc};
 
@@ -1023,12 +1027,12 @@ mod tests {
         let unix_epoch = DateTime { epoch: 0 };
         let today: DateTime = parse_datetime("2024-10-24").unwrap().into();
         assert_eq!(
-            today.duration_since(unix_epoch.clone()),
+            today.duration_since(unix_epoch),
             Some(parse_duration("20020d").unwrap())
         );
         let yesterday: DateTime = parse_datetime("2024-10-23").unwrap().into();
         assert_eq!(
-            yesterday.duration_since(today.clone()),
+            yesterday.duration_since(today),
             Some(parse_duration("-1d").unwrap())
         );
         assert_eq!(
@@ -1047,12 +1051,12 @@ mod tests {
         let unix_epoch = DateTime { epoch: 0 };
         let today: DateTime = parse_datetime("2024-10-24").unwrap().into();
         assert_eq!(
-            today.duration_since(unix_epoch.clone()),
+            today.duration_since(unix_epoch),
             Some(parse_duration("20020d").unwrap())
         );
         let yesterday: DateTime = parse_datetime("2024-10-23").unwrap().into();
         assert_eq!(
-            yesterday.duration_since(today.clone()),
+            yesterday.duration_since(today),
             Some(parse_duration("-1d").unwrap())
         );
         let some_day_before_unix_epoch: DateTime = parse_datetime("1900-01-01").unwrap().into();
@@ -1060,22 +1064,17 @@ mod tests {
         let max_day_offset = parse_duration("23h59m59s999ms").unwrap();
         let min_day_offset = parse_duration("-23h59m59s999ms").unwrap();
 
-        for d in [
-            today,
-            yesterday,
-            unix_epoch.clone(),
-            some_day_before_unix_epoch,
-        ] {
+        for d in [today, yesterday, unix_epoch, some_day_before_unix_epoch] {
             assert_eq!(d.to_date().expect("should not overflow"), d);
             assert_eq!(
-                d.offset(max_day_offset.clone())
+                d.offset(max_day_offset)
                     .unwrap()
                     .to_date()
                     .expect("should not overflow"),
                 d
             );
             assert_eq!(
-                d.offset(min_day_offset.clone())
+                d.offset(min_day_offset)
                     .unwrap()
                     .to_date()
                     .expect("should not overflow"),
@@ -1095,12 +1094,12 @@ mod tests {
         let unix_epoch = DateTime { epoch: 0 };
         let today: DateTime = parse_datetime("2024-10-24").unwrap().into();
         assert_eq!(
-            today.duration_since(unix_epoch.clone()),
+            today.duration_since(unix_epoch),
             Some(parse_duration("20020d").unwrap())
         );
         let yesterday: DateTime = parse_datetime("2024-10-23").unwrap().into();
         assert_eq!(
-            yesterday.duration_since(today.clone()),
+            yesterday.duration_since(today),
             Some(parse_duration("-1d").unwrap())
         );
         let some_day_before_unix_epoch: DateTime = parse_datetime("1900-01-01").unwrap().into();
@@ -1109,12 +1108,9 @@ mod tests {
         let min_day_offset = parse_duration("-23h59m59s999ms").unwrap();
 
         for d in [today, yesterday, unix_epoch, some_day_before_unix_epoch] {
+            assert_eq!(d.offset(max_day_offset).unwrap().to_time(), max_day_offset);
             assert_eq!(
-                d.offset(max_day_offset.clone()).unwrap().to_time(),
-                max_day_offset
-            );
-            assert_eq!(
-                d.offset(min_day_offset.clone()).unwrap().to_time(),
+                d.offset(min_day_offset).unwrap().to_time(),
                 parse_duration("1ms").unwrap(),
             );
         }
diff --git a/cedar-policy-core/src/extensions/decimal.rs b/cedar-policy-core/src/extensions/decimal.rs
index 7e3081de..bd601a95 100644
--- a/cedar-policy-core/src/extensions/decimal.rs
+++ b/cedar-policy-core/src/extensions/decimal.rs
@@ -184,7 +184,7 @@ fn extension_err(msg: impl Into<String>, advice: Option<String>) -> evaluator::E
 
 /// Cedar function that constructs a `decimal` Cedar type from a
 /// Cedar string
-fn decimal_from_str(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
+fn decimal_from_str(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
     let str = arg.get_as_string()?;
     let decimal =
         Decimal::from_str(str.as_str()).map_err(|e| extension_err(e.to_string(), None))?;
@@ -192,7 +192,7 @@ fn decimal_from_str(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
     let e = RepresentableExtensionValue::new(
         Arc::new(decimal),
         constants::DECIMAL_FROM_STR_NAME.clone(),
-        vec![arg.into()],
+        vec![arg.clone().into()],
     );
     Ok(Value {
         value: ValueKind::ExtensionValue(Arc::new(e)),
@@ -234,33 +234,33 @@ fn as_decimal(v: &Value) -> Result<&Decimal, evaluator::EvaluationError> {
 
 /// Cedar function that tests whether the first `decimal` Cedar type is
 /// less than the second `decimal` Cedar type, returning a Cedar bool
-fn decimal_lt(left: Value, right: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let left = as_decimal(&left)?;
-    let right = as_decimal(&right)?;
+fn decimal_lt(left: &Value, right: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let left = as_decimal(left)?;
+    let right = as_decimal(right)?;
     Ok(Value::from(left < right).into())
 }
 
 /// Cedar function that tests whether the first `decimal` Cedar type is
 /// less than or equal to the second `decimal` Cedar type, returning a Cedar bool
-fn decimal_le(left: Value, right: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let left = as_decimal(&left)?;
-    let right = as_decimal(&right)?;
+fn decimal_le(left: &Value, right: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let left = as_decimal(left)?;
+    let right = as_decimal(right)?;
     Ok(Value::from(left <= right).into())
 }
 
 /// Cedar function that tests whether the first `decimal` Cedar type is
 /// greater than the second `decimal` Cedar type, returning a Cedar bool
-fn decimal_gt(left: Value, right: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let left = as_decimal(&left)?;
-    let right = as_decimal(&right)?;
+fn decimal_gt(left: &Value, right: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let left = as_decimal(left)?;
+    let right = as_decimal(right)?;
     Ok(Value::from(left > right).into())
 }
 
 /// Cedar function that tests whether the first `decimal` Cedar type is
 /// greater than or equal to the second `decimal` Cedar type, returning a Cedar bool
-fn decimal_ge(left: Value, right: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let left = as_decimal(&left)?;
-    let right = as_decimal(&right)?;
+fn decimal_ge(left: &Value, right: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let left = as_decimal(left)?;
+    let right = as_decimal(right)?;
     Ok(Value::from(left >= right).into())
 }
 
diff --git a/cedar-policy-core/src/extensions/ipaddr.rs b/cedar-policy-core/src/extensions/ipaddr.rs
index a5c62304..ed88f790 100644
--- a/cedar-policy-core/src/extensions/ipaddr.rs
+++ b/cedar-policy-core/src/extensions/ipaddr.rs
@@ -305,13 +305,13 @@ fn str_contains_colons_and_dots(s: &str) -> Result<(), String> {
 
 /// Cedar function which constructs an `ipaddr` Cedar type from a
 /// Cedar string
-fn ip_from_str(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
+fn ip_from_str(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
     let str = arg.get_as_string()?;
     let arg_source_loc = arg.source_loc().cloned();
     let ipaddr = RepresentableExtensionValue::new(
         Arc::new(IPAddr::from_str(str.as_str()).map_err(extension_err)?),
         names::IP_FROM_STR_NAME.clone(),
-        vec![arg.into()],
+        vec![arg.clone().into()],
     );
     Ok(Value {
         value: ValueKind::ExtensionValue(Arc::new(ipaddr)),
@@ -352,38 +352,38 @@ fn as_ipaddr(v: &Value) -> Result<&IPAddr, evaluator::EvaluationError> {
 
 /// Cedar function which tests whether an `ipaddr` Cedar type is an IPv4
 /// address, returning a Cedar bool
-fn is_ipv4(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let ipaddr = as_ipaddr(&arg)?;
+fn is_ipv4(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let ipaddr = as_ipaddr(arg)?;
     Ok(ipaddr.is_ipv4().into())
 }
 
 /// Cedar function which tests whether an `ipaddr` Cedar type is an IPv6
 /// address, returning a Cedar bool
-fn is_ipv6(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let ipaddr = as_ipaddr(&arg)?;
+fn is_ipv6(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let ipaddr = as_ipaddr(arg)?;
     Ok(ipaddr.is_ipv6().into())
 }
 
 /// Cedar function which tests whether an `ipaddr` Cedar type is a
 /// loopback address, returning a Cedar bool
-fn is_loopback(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let ipaddr = as_ipaddr(&arg)?;
+fn is_loopback(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let ipaddr = as_ipaddr(arg)?;
     Ok(ipaddr.is_loopback().into())
 }
 
 /// Cedar function which tests whether an `ipaddr` Cedar type is a
 /// multicast address, returning a Cedar bool
-fn is_multicast(arg: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let ipaddr = as_ipaddr(&arg)?;
+fn is_multicast(arg: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let ipaddr = as_ipaddr(arg)?;
     Ok(ipaddr.is_multicast().into())
 }
 
 /// Cedar function which tests whether the first `ipaddr` Cedar type is
 /// in the IP range represented by the second `ipaddr` Cedar type, returning
 /// a Cedar bool
-fn is_in_range(child: Value, parent: Value) -> evaluator::Result<ExtensionOutputValue> {
-    let child_ip = as_ipaddr(&child)?;
-    let parent_ip = as_ipaddr(&parent)?;
+fn is_in_range(child: &Value, parent: &Value) -> evaluator::Result<ExtensionOutputValue> {
+    let child_ip = as_ipaddr(child)?;
+    let parent_ip = as_ipaddr(parent)?;
     Ok(child_ip.is_in_range(parent_ip).into())
 }
 
@@ -444,6 +444,7 @@ pub fn extension() -> Extension {
 // PANIC SAFETY: Unit Test Code
 #[allow(clippy::panic)]
 #[cfg(test)]
+#[allow(clippy::cognitive_complexity)]
 mod tests {
     use super::*;
     use crate::ast::{Expr, Type, Value};
diff --git a/cedar-policy-core/src/extensions/partial_evaluation.rs b/cedar-policy-core/src/extensions/partial_evaluation.rs
index 13f2e6a8..c271c55b 100644
--- a/cedar-policy-core/src/extensions/partial_evaluation.rs
+++ b/cedar-policy-core/src/extensions/partial_evaluation.rs
@@ -23,7 +23,7 @@ use crate::{
 };
 
 /// Create a new untyped `Unknown`
-fn create_new_unknown(v: Value) -> evaluator::Result<ExtensionOutputValue> {
+fn create_new_unknown(v: &Value) -> evaluator::Result<ExtensionOutputValue> {
     Ok(ExtensionOutputValue::Unknown(Unknown::new_untyped(
         v.get_as_string()?.clone(),
     )))
diff --git a/cedar-policy-core/src/parser.rs b/cedar-policy-core/src/parser.rs
index 428adb6b..b6d06dc0 100644
--- a/cedar-policy-core/src/parser.rs
+++ b/cedar-policy-core/src/parser.rs
@@ -327,6 +327,7 @@ pub(crate) mod test_utils {
 
 // PANIC SAFETY: Unit Test Code
 #[allow(clippy::panic, clippy::indexing_slicing)]
+#[allow(clippy::cognitive_complexity)]
 #[cfg(test)]
 /// Tests for the top-level parsing APIs
 mod tests {
diff --git a/cedar-policy-core/src/parser/cst_to_ast.rs b/cedar-policy-core/src/parser/cst_to_ast.rs
index bb329ea4..88ceecc2 100644
--- a/cedar-policy-core/src/parser/cst_to_ast.rs
+++ b/cedar-policy-core/src/parser/cst_to_ast.rs
@@ -490,7 +490,7 @@ impl ast::UnreservedId {
             "containsAny" => extract_single_argument(args.into_iter(), "containsAny", loc)
                 .map(|arg| construct_method_contains_any(e, arg, loc.clone())),
             "isEmpty" => {
-                require_zero_arguments(args.into_iter(), "isEmpty", loc)?;
+                require_zero_arguments(&args.into_iter(), "isEmpty", loc)?;
                 Ok(construct_method_is_empty(e, loc.clone()))
             }
             "getTag" => extract_single_argument(args.into_iter(), "getTag", loc)
@@ -984,7 +984,7 @@ impl Node<Option<cst::Relation>> {
                 });
                 let (target, field) = flatten_tuple_2(maybe_target, maybe_field)?;
                 Ok(ExprOrSpecial::Expr {
-                    expr: construct_exprs_extended_has(target, field, self.loc.clone()),
+                    expr: construct_exprs_extended_has(target, &field, &self.loc),
                     loc: self.loc.clone(),
                 })
             }
@@ -1941,7 +1941,7 @@ fn construct_expr_has_attr(t: ast::Expr, s: SmolStr, loc: Loc) -> ast::Expr {
 fn construct_expr_get_attr(t: ast::Expr, s: SmolStr, loc: Loc) -> ast::Expr {
     ast::ExprBuilder::new().with_source_loc(loc).get_attr(t, s)
 }
-fn construct_exprs_extended_has(t: ast::Expr, attrs: NonEmpty<SmolStr>, loc: Loc) -> ast::Expr {
+fn construct_exprs_extended_has(t: ast::Expr, attrs: &NonEmpty<SmolStr>, loc: &Loc) -> ast::Expr {
     let (first, rest) = attrs.split_first();
     let has_expr = construct_expr_has_attr(t.clone(), first.to_owned(), loc.clone());
     let get_expr = construct_expr_get_attr(t, first.to_owned(), loc.clone());
@@ -1973,7 +1973,7 @@ fn construct_exprs_extended_has(t: ast::Expr, attrs: NonEmpty<SmolStr>, loc: Loc
                     has_expr,
                     construct_expr_has_attr(get_expr.clone(), attr.to_owned(), loc.clone()),
                     std::iter::empty(),
-                    &loc,
+                    loc,
                 ),
                 construct_expr_get_attr(get_expr, attr.to_owned(), loc.clone()),
             )
@@ -2046,6 +2046,7 @@ fn construct_expr_record(kvs: Vec<(SmolStr, ast::Expr)>, loc: Loc) -> Result<ast
 #[allow(clippy::panic)]
 // PANIC SAFETY: Unit Test Code
 #[allow(clippy::indexing_slicing)]
+#[allow(clippy::cognitive_complexity)]
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/cedar-policy-core/src/parser/err.rs b/cedar-policy-core/src/parser/err.rs
index 49b75052..ab96cf86 100644
--- a/cedar-policy-core/src/parser/err.rs
+++ b/cedar-policy-core/src/parser/err.rs
@@ -834,11 +834,12 @@ impl ParseErrors {
 
     /// Flatten a `Vec<ParseErrors>` into a single `ParseErrors`, returning
     /// `None` if the input vector is empty.
-    pub(crate) fn flatten(v: Vec<ParseErrors>) -> Option<Self> {
-        let (first, rest) = v.split_first()?;
-        let mut first = first.clone();
-        rest.iter()
-            .for_each(|errs| first.extend(errs.iter().cloned()));
+    pub(crate) fn flatten(errs: impl IntoIterator<Item = ParseErrors>) -> Option<Self> {
+        let mut errs = errs.into_iter();
+        let mut first = errs.next()?;
+        for inner in errs {
+            first.extend(inner);
+        }
         Some(first)
     }
 
diff --git a/cedar-policy-core/src/parser/text_to_cst.rs b/cedar-policy-core/src/parser/text_to_cst.rs
index aaf7cce1..e3a953c1 100644
--- a/cedar-policy-core/src/parser/text_to_cst.rs
+++ b/cedar-policy-core/src/parser/text_to_cst.rs
@@ -1003,12 +1003,7 @@ mod tests {
             .expect("parser error")
             .node
             .expect("no data");
-        let success = policies
-            .0
-            .into_iter()
-            .filter_map(|p| p.node)
-            .collect::<Vec<_>>();
-        assert!(success.len() == 2);
+        assert_eq!(policies.0.into_iter().filter_map(|p| p.node).count(), 2);
     }
 
     #[test]
diff --git a/cedar-policy-formatter/src/pprint/doc.rs b/cedar-policy-formatter/src/pprint/doc.rs
index 22e92080..bb31f02a 100644
--- a/cedar-policy-formatter/src/pprint/doc.rs
+++ b/cedar-policy-formatter/src/pprint/doc.rs
@@ -415,7 +415,7 @@ impl Doc for Node<Option<Unary>> {
                                         } else {
                                             RcDoc::text("-")
                                         },
-                                        comment.get(i as usize)?.clone(),
+                                        comment.get(i as usize)?,
                                         RcDoc::nil(),
                                     ))
                                 })
diff --git a/cedar-policy-formatter/src/pprint/utils.rs b/cedar-policy-formatter/src/pprint/utils.rs
index 7ad9d448..ad8e3d50 100644
--- a/cedar-policy-formatter/src/pprint/utils.rs
+++ b/cedar-policy-formatter/src/pprint/utils.rs
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+use std::borrow::Borrow;
+
 use itertools::Itertools;
 use pretty::RcDoc;
 
@@ -132,11 +134,11 @@ pub fn get_comment_in_range<'src>(
 /// trailing comment, then it will introduce a newline at the end.
 pub fn add_comment<'src>(
     d: RcDoc<'src>,
-    comment: Comment<'src>,
+    comment: impl Borrow<Comment<'src>>,
     next_doc: RcDoc<'src>,
 ) -> RcDoc<'src> {
-    let leading_comment = comment.leading_comment();
-    let trailing_comment = comment.trailing_comment();
+    let leading_comment = comment.borrow().leading_comment();
+    let trailing_comment = comment.borrow().trailing_comment();
     let leading_comment_doc = get_leading_comment_doc_from_str(leading_comment);
     let trailing_comment_doc = get_trailing_comment_doc_from_str(trailing_comment, next_doc);
     leading_comment_doc.append(d).append(trailing_comment_doc)
diff --git a/cedar-policy-validator/Cargo.toml b/cedar-policy-validator/Cargo.toml
index 6a4b3548..bc42f34a 100644
--- a/cedar-policy-validator/Cargo.toml
+++ b/cedar-policy-validator/Cargo.toml
@@ -14,7 +14,7 @@ repository.workspace = true
 cedar-policy-core = { version = "=4.3.0", path = "../cedar-policy-core" }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0", features = ["preserve_order"] }
-serde_with = "3.0"
+serde_with = "3.12"
 miette = "7.4.0"
 thiserror = "2.0"
 itertools = "0.13"
diff --git a/cedar-policy-validator/src/cedar_schema/err.rs b/cedar-policy-validator/src/cedar_schema/err.rs
index 394f759f..d5b65b2b 100644
--- a/cedar-policy-validator/src/cedar_schema/err.rs
+++ b/cedar-policy-validator/src/cedar_schema/err.rs
@@ -437,7 +437,7 @@ pub enum ToJsonSchemaError {
 }
 
 impl ToJsonSchemaError {
-    pub(crate) fn duplicate_context(name: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_context(name: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicateContext(DuplicateContext {
             name: name.to_smolstr(),
             loc1,
@@ -445,7 +445,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn duplicate_decls(decl: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_decls(decl: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicateDeclarations(DuplicateDeclarations {
             decl: decl.to_smolstr(),
             loc1,
@@ -454,7 +454,7 @@ impl ToJsonSchemaError {
     }
 
     pub(crate) fn duplicate_namespace(
-        namespace_id: impl ToSmolStr,
+        namespace_id: &impl ToSmolStr,
         loc1: Option<Loc>,
         loc2: Option<Loc>,
     ) -> Self {
@@ -465,7 +465,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn duplicate_principal(name: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_principal(name: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicatePrincipalOrResource(DuplicatePrincipalOrResource {
             name: name.to_smolstr(),
             kind: PR::Principal,
@@ -474,7 +474,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn duplicate_resource(name: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_resource(name: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicatePrincipalOrResource(DuplicatePrincipalOrResource {
             name: name.to_smolstr(),
             kind: PR::Resource,
@@ -483,7 +483,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn no_principal(name: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn no_principal(name: &impl ToSmolStr, loc: Loc) -> Self {
         Self::NoPrincipalOrResource(NoPrincipalOrResource {
             kind: PR::Principal,
             name: name.to_smolstr(),
@@ -491,7 +491,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn no_resource(name: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn no_resource(name: &impl ToSmolStr, loc: Loc) -> Self {
         Self::NoPrincipalOrResource(NoPrincipalOrResource {
             kind: PR::Resource,
             name: name.to_smolstr(),
@@ -499,14 +499,14 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn reserved_name(name: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn reserved_name(name: &impl ToSmolStr, loc: Loc) -> Self {
         Self::ReservedName(ReservedName {
             name: name.to_smolstr(),
             loc,
         })
     }
 
-    pub(crate) fn reserved_keyword(keyword: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn reserved_keyword(keyword: &impl ToSmolStr, loc: Loc) -> Self {
         Self::ReservedSchemaKeyword(ReservedSchemaKeyword {
             keyword: keyword.to_smolstr(),
             loc,
diff --git a/cedar-policy-validator/src/cedar_schema/fmt.rs b/cedar-policy-validator/src/cedar_schema/fmt.rs
index 5f20f11e..f559f66a 100644
--- a/cedar-policy-validator/src/cedar_schema/fmt.rs
+++ b/cedar-policy-validator/src/cedar_schema/fmt.rs
@@ -95,21 +95,22 @@ impl<N: Display> Display for json_schema::RecordType<N> {
     }
 }
 
-/// Create a non-empty with borrowed contents from a slice
-fn non_empty_slice<T>(v: &[T]) -> Option<NonEmpty<&T>> {
-    NonEmpty::collect(v.iter())
-}
-
-fn fmt_vec<T: Display>(f: &mut std::fmt::Formatter<'_>, ets: NonEmpty<T>) -> std::fmt::Result {
-    let contents = ets.iter().map(T::to_string).join(", ");
-    write!(f, "[{contents}]")
+fn fmt_non_empty_slice<T: Display>(
+    f: &mut std::fmt::Formatter<'_>,
+    (head, tail): (&T, &[T]),
+) -> std::fmt::Result {
+    write!(f, "[{head}")?;
+    for e in tail {
+        write!(f, ", {e}")?;
+    }
+    write!(f, "]")
 }
 
 impl<N: Display> Display for json_schema::EntityType<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        if let Some(non_empty) = non_empty_slice(&self.member_of_types) {
+        if let Some(non_empty) = self.member_of_types.split_first() {
             write!(f, " in ")?;
-            fmt_vec(f, non_empty)?;
+            fmt_non_empty_slice(f, non_empty)?;
         }
 
         let ty = &self.shape;
@@ -128,18 +129,14 @@ impl<N: Display> Display for json_schema::EntityType<N> {
 
 impl<N: Display> Display for json_schema::ActionType<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        if let Some(parents) = self
-            .member_of
-            .as_ref()
-            .and_then(|refs| non_empty_slice(refs.as_slice()))
-        {
+        if let Some(parents) = self.member_of.as_ref().and_then(|refs| refs.split_first()) {
             write!(f, " in ")?;
-            fmt_vec(f, parents)?;
+            fmt_non_empty_slice(f, parents)?;
         }
         if let Some(spec) = &self.applies_to {
             match (
-                non_empty_slice(spec.principal_types.as_slice()),
-                non_empty_slice(spec.resource_types.as_slice()),
+                spec.principal_types.split_first(),
+                spec.resource_types.split_first(),
             ) {
                 // One of the lists is empty
                 // This can only be represented by the empty action
@@ -151,9 +148,9 @@ impl<N: Display> Display for json_schema::ActionType<N> {
                 (Some(ps), Some(rs)) => {
                     write!(f, " appliesTo {{")?;
                     write!(f, "\n  principal: ")?;
-                    fmt_vec(f, ps)?;
+                    fmt_non_empty_slice(f, ps)?;
                     write!(f, ",\n  resource: ")?;
-                    fmt_vec(f, rs)?;
+                    fmt_non_empty_slice(f, rs)?;
                     write!(f, ",\n  context: {}", &spec.context.0)?;
                     write!(f, "\n}}")?;
                 }
diff --git a/cedar-policy-validator/src/cedar_schema/grammar.lalrpop b/cedar-policy-validator/src/cedar_schema/grammar.lalrpop
index 3db5e48a..91052835 100644
--- a/cedar-policy-validator/src/cedar_schema/grammar.lalrpop
+++ b/cedar-policy-validator/src/cedar_schema/grammar.lalrpop
@@ -90,14 +90,7 @@ match {
 
 }
 
-#[inline]
-AnyIdent: Node<AnyId> = {
-    <l:@L> <id:IDENTIFIER> <r:@R>
-        // `IDENTIFIER` should produce a valid `AnyId`
-        => Node::with_source_loc(AnyId::from_str(id).unwrap(), Loc::new(l..r, Arc::clone(src))),
-}
-
-// Annotations := {'@' IDENTIFIER '(' String ')'}
+// Annotations := {'@' AnyIdent '(' String ')'}
 Annotation: Node<(Node<AnyId>, Option<Node<SmolStr>>)> = {
     <l:@L> "@" <key:AnyIdent> <value: ("(" <STR> ")")?> <r:@R> => Node::with_source_loc((key, value), Loc::new(l..r, Arc::clone(src)))
 }
@@ -225,8 +218,15 @@ Comma<E>: Vec<E> = {
     },
 }
 
-// IDENT := ['_''a'-'z''A'-'Z']['_''a'-'z''A'-'Z''0'-'9']*
+// IDENT := ['_''a'-'z''A'-'Z']['_''a'-'z''A'-'Z''0'-'9']* - RESERVED
 Ident: Node<Id> = {
+    <id: AnyIdent> =>? Id::from_str(id.node.as_ref()).map(|i| Node::with_source_loc(i, id.loc.clone())).map_err(|err : cedar_policy_core::parser::err::ParseErrors| ParseError::User {
+        error: UserError::ReservedIdentifierUsed(Node::with_source_loc(id.node.to_smolstr(), id.loc.clone()))
+    }),
+}
+
+// AnyIdent := ['_''a'-'z''A'-'Z']['_''a'-'z''A'-'Z''0'-'9']*
+AnyIdent: Node<AnyId> = {
     <l:@L> NAMESPACE <r:@R>
         => Node::with_source_loc("namespace".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> ENTITY <r:@R>
@@ -256,17 +256,9 @@ Ident: Node<Id> = {
     <l:@L> TYPE <r:@R>
         => Node::with_source_loc("type".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> IN <r:@R>
-        =>? Err(ParseError::User {
-            error: UserError::ReservedIdentifierUsed(Node::with_source_loc("in".into(), Loc::new(l..r, Arc::clone(src))))
-        }),
+        => Node::with_source_loc("in".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> <i:IDENTIFIER> <r:@R>
-        =>? Id::from_str(i)
-        .map(|id : Id| Node::with_source_loc(id, Loc::new(l..r, Arc::clone(src))))
-        .map_err(|err : cedar_policy_core::parser::err::ParseErrors|
-            ParseError::User {
-                error: UserError::ReservedIdentifierUsed(Node::with_source_loc(i.to_smolstr(), Loc::new(l..r, Arc::clone(src))))
-            }
-        )
+        => Node::with_source_loc(i.parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
 }
 
 STR: Node<SmolStr> = {
diff --git a/cedar-policy-validator/src/cedar_schema/to_json_schema.rs b/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
index f696c2f9..d84190c7 100644
--- a/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
+++ b/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
@@ -197,7 +197,7 @@ impl TryFrom<Annotated<Namespace>> for json_schema::NamespaceDefinition<RawName>
                 let id = UnreservedId::try_from(decl.data.name.node)
                     .map_err(|e| ToJsonSchemaError::reserved_name(e.name(), name_loc.clone()))?;
                 let ctid = json_schema::CommonTypeId::new(id)
-                    .map_err(|e| ToJsonSchemaError::reserved_keyword(e.id, name_loc))?;
+                    .map_err(|e| ToJsonSchemaError::reserved_keyword(&e.id, name_loc))?;
                 Ok((
                     ctid,
                     CommonType {
@@ -273,7 +273,7 @@ fn convert_app_decls(
             } => match context {
                 Some(existing_context) => {
                     return Err(ToJsonSchemaError::duplicate_context(
-                        name.clone(),
+                        name,
                         existing_context.loc,
                         loc,
                     )
@@ -300,7 +300,7 @@ fn convert_app_decls(
             } => match principal_types {
                 Some(existing_tys) => {
                     return Err(ToJsonSchemaError::duplicate_principal(
-                        name.clone(),
+                        name,
                         existing_tys.loc,
                         loc,
                     )
@@ -325,12 +325,9 @@ fn convert_app_decls(
                 loc,
             } => match resource_types {
                 Some(existing_tys) => {
-                    return Err(ToJsonSchemaError::duplicate_resource(
-                        name.clone(),
-                        existing_tys.loc,
-                        loc,
-                    )
-                    .into());
+                    return Err(
+                        ToJsonSchemaError::duplicate_resource(name, existing_tys.loc, loc).into(),
+                    );
                 }
                 None => {
                     resource_types = Some(Node::with_source_loc(
@@ -344,10 +341,10 @@ fn convert_app_decls(
     Ok(json_schema::ApplySpec {
         resource_types: resource_types
             .map(|node| node.node)
-            .ok_or_else(|| ToJsonSchemaError::no_resource(name.clone(), name_loc.clone()))?,
+            .ok_or_else(|| ToJsonSchemaError::no_resource(&name, name_loc.clone()))?,
         principal_types: principal_types
             .map(|node| node.node)
-            .ok_or_else(|| ToJsonSchemaError::no_principal(name.clone(), name_loc.clone()))?,
+            .ok_or_else(|| ToJsonSchemaError::no_principal(&name, name_loc.clone()))?,
         context: context.map(|c| c.node).unwrap_or_default(),
     })
 }
@@ -524,7 +521,7 @@ where
     for (key, node) in i {
         match map.entry(key.clone()) {
             Entry::Occupied(entry) => Err(ToJsonSchemaError::duplicate_decls(
-                key,
+                &key,
                 entry.get().loc.clone(),
                 node.loc,
             )),
@@ -614,7 +611,7 @@ fn update_namespace_record(
 ) -> Result<(), ToJsonSchemaErrors> {
     match map.entry(name.clone()) {
         Entry::Occupied(entry) => Err(ToJsonSchemaError::duplicate_namespace(
-            name.map_or("".into(), |n| n.to_smolstr()),
+            &name.map_or("".into(), |n| n.to_smolstr()),
             record.loc,
             entry.get().loc.clone(),
         )
diff --git a/cedar-policy-validator/src/entity_manifest/type_annotations.rs b/cedar-policy-validator/src/entity_manifest/type_annotations.rs
index 0c70e771..0bf4b071 100644
--- a/cedar-policy-validator/src/entity_manifest/type_annotations.rs
+++ b/cedar-policy-validator/src/entity_manifest/type_annotations.rs
@@ -69,7 +69,7 @@ impl RootAccessTrie {
                         match key {
                             EntityRoot::Literal(lit) => slice.to_typed(
                                 request_type,
-                                &Type::euid_literal(lit.clone(), schema).ok_or_else(|| {
+                                &Type::euid_literal(lit, schema).ok_or_else(|| {
                                     MismatchedMissingEntityError {
                                         entity: lit.clone(),
                                     }
@@ -77,7 +77,7 @@ impl RootAccessTrie {
                                 schema,
                             )?,
                             EntityRoot::Var(Var::Action) => {
-                                let ty = Type::euid_literal(request_type.action.clone(), schema)
+                                let ty = Type::euid_literal(&request_type.action, schema)
                                     .ok_or_else(|| MismatchedMissingEntityError {
                                         entity: request_type.action.clone(),
                                     })?;
diff --git a/cedar-policy-validator/src/json_schema.rs b/cedar-policy-validator/src/json_schema.rs
index f05a9cc1..d5825c69 100644
--- a/cedar-policy-validator/src/json_schema.rs
+++ b/cedar-policy-validator/src/json_schema.rs
@@ -2779,8 +2779,7 @@ mod test_json_roundtrip {
                 entity_types: BTreeMap::new(),
                 actions: BTreeMap::new(),
                 annotations: Annotations::new(),
-            }
-            .into(),
+            },
         )]));
         roundtrip(fragment);
     }
@@ -2794,8 +2793,7 @@ mod test_json_roundtrip {
                 entity_types: BTreeMap::new(),
                 actions: BTreeMap::new(),
                 annotations: Annotations::new(),
-            }
-            .into(),
+            },
         )]));
         roundtrip(fragment);
     }
@@ -2816,8 +2814,7 @@ mod test_json_roundtrip {
                         }))),
                         tags: None,
                         annotations: Annotations::new(),
-                    }
-                    .into(),
+                    },
                 )]),
                 actions: BTreeMap::from([(
                     "action".into(),
@@ -2835,12 +2832,10 @@ mod test_json_roundtrip {
                         }),
                         member_of: None,
                         annotations: Annotations::new(),
-                    }
-                    .into(),
+                    },
                 )]),
                 annotations: Annotations::new(),
-            }
-            .into(),
+            },
         )]));
         roundtrip(fragment);
     }
@@ -2864,13 +2859,11 @@ mod test_json_roundtrip {
                             ))),
                             tags: None,
                             annotations: Annotations::new(),
-                        }
-                        .into(),
+                        },
                     )]),
                     actions: BTreeMap::new(),
                     annotations: Annotations::new(),
-                }
-                .into(),
+                },
             ),
             (
                 None,
@@ -2893,12 +2886,10 @@ mod test_json_roundtrip {
                             }),
                             member_of: None,
                             annotations: Annotations::new(),
-                        }
-                        .into(),
+                        },
                     )]),
                     annotations: Annotations::new(),
-                }
-                .into(),
+                },
             ),
         ]));
         roundtrip(fragment);
diff --git a/cedar-policy-validator/src/schema.rs b/cedar-policy-validator/src/schema.rs
index c3241fc8..c668b04d 100644
--- a/cedar-policy-validator/src/schema.rs
+++ b/cedar-policy-validator/src/schema.rs
@@ -3588,6 +3588,7 @@ pub(crate) mod test {
 mod test_579; // located in separate file test_579.rs
 
 #[cfg(test)]
+#[allow(clippy::cognitive_complexity)]
 mod test_rfc70 {
     use super::test::utils::*;
     use super::ValidatorSchema;
@@ -4578,6 +4579,7 @@ mod test_rfc70 {
 
 /// Tests involving entity tags (RFC 82)
 #[cfg(test)]
+#[allow(clippy::cognitive_complexity)]
 mod entity_tags {
     use super::{test::utils::*, *};
     use cedar_policy_core::{
diff --git a/cedar-policy-validator/src/typecheck.rs b/cedar-policy-validator/src/typecheck.rs
index 36e87e63..a6d4e9c6 100644
--- a/cedar-policy-validator/src/typecheck.rs
+++ b/cedar-policy-validator/src/typecheck.rs
@@ -184,12 +184,11 @@ impl<'a> Typechecker<'a> {
         // explicit that `expect_type` will be called for every element of
         // request_env without short circuiting.
         let policy_condition = &t.condition();
-        for requeste in self
-            .unlinked_request_envs()
-            .flat_map(|env| self.link_request_env(env, t))
-        {
-            let check = typecheck_fn(&requeste, policy_condition);
-            result_checks.push((requeste, check))
+        for unlinked_e in self.unlinked_request_envs() {
+            for linked_e in self.link_request_env(&unlinked_e, t) {
+                let check = typecheck_fn(&linked_e, policy_condition);
+                result_checks.push((linked_e, check))
+            }
         }
         result_checks
     }
@@ -208,7 +207,7 @@ impl<'a> Typechecker<'a> {
             let mut policy_checks = Vec::new();
             for t in policy_templates.iter() {
                 let condition_expr = t.condition();
-                for linked_env in self.link_request_env(request.clone(), t) {
+                for linked_env in self.link_request_env(&request, t) {
                     let mut type_errors = Vec::new();
                     let empty_prior_capability = CapabilitySet::new();
                     let ty = self.expect_type(
@@ -276,11 +275,11 @@ impl<'a> Typechecker<'a> {
 
     /// Given a request environment and a template, return new environments
     /// formed by linking template slots with possible entity types.
-    fn link_request_env<'b>(
+    fn link_request_env<'b, 'c>(
         &'b self,
-        env: RequestEnv<'b>,
+        env: &'c RequestEnv<'b>,
         t: &'b Template,
-    ) -> Box<dyn Iterator<Item = RequestEnv<'b>> + 'b> {
+    ) -> Box<dyn Iterator<Item = RequestEnv<'b>> + 'c> {
         match env {
             RequestEnv::UndeclaredAction => Box::new(std::iter::once(RequestEnv::UndeclaredAction)),
             RequestEnv::DeclaredAction {
@@ -469,7 +468,7 @@ impl<'a> Typechecker<'a> {
                 // detected by a different part of the validator, so a ValidationError is
                 // not generated here. We still return `TypecheckFail` so that
                 // typechecking is not considered successful.
-                match Type::euid_literal((**euid).clone(), self.schema) {
+                match Type::euid_literal(euid.as_ref(), self.schema) {
                     // The entity type is undeclared, but that's OK for a
                     // partial schema. The attributes record will be empty if we
                     // try to access it later, so all attributes will have the
diff --git a/cedar-policy-validator/src/types.rs b/cedar-policy-validator/src/types.rs
index da4a162f..d8e22c4a 100644
--- a/cedar-policy-validator/src/types.rs
+++ b/cedar-policy-validator/src/types.rs
@@ -119,10 +119,10 @@ impl Type {
 
     /// Construct a type for a literal EUID. This type will be a named entity
     /// type for the type of the [`EntityUID`].
-    pub(crate) fn euid_literal(entity: EntityUID, schema: &ValidatorSchema) -> Option<Type> {
+    pub(crate) fn euid_literal(entity: &EntityUID, schema: &ValidatorSchema) -> Option<Type> {
         if entity.entity_type().is_action() {
             schema
-                .get_action_id(&entity)
+                .get_action_id(entity)
                 .map(Type::entity_reference_from_action_id)
         } else {
             schema
@@ -2534,7 +2534,7 @@ mod test {
     #[test]
     fn test_action_entity_lub() {
         let action_view_ty =
-            Type::euid_literal(r#"Action::"view""#.parse().unwrap(), &action_schema()).unwrap();
+            Type::euid_literal(&r#"Action::"view""#.parse().unwrap(), &action_schema()).unwrap();
 
         assert_least_upper_bound(
             action_schema(),
@@ -2551,7 +2551,7 @@ mod test {
             action_schema(),
             ValidationMode::Strict,
             action_view_ty.clone(),
-            Type::euid_literal(r#"Action::"edit""#.parse().unwrap(), &action_schema()).unwrap(),
+            Type::euid_literal(&r#"Action::"edit""#.parse().unwrap(), &action_schema()).unwrap(),
             Ok(action_view_ty.clone()),
         );
 
@@ -2561,14 +2561,16 @@ mod test {
             action_schema(),
             ValidationMode::Permissive,
             action_view_ty.clone(),
-            Type::euid_literal(r#"ns::Action::"move""#.parse().unwrap(), &action_schema()).unwrap(),
+            Type::euid_literal(&r#"ns::Action::"move""#.parse().unwrap(), &action_schema())
+                .unwrap(),
             Ok(Type::any_entity_reference()),
         );
         assert_least_upper_bound(
             action_schema(),
             ValidationMode::Strict,
             action_view_ty.clone(),
-            Type::euid_literal(r#"ns::Action::"move""#.parse().unwrap(), &action_schema()).unwrap(),
+            Type::euid_literal(&r#"ns::Action::"move""#.parse().unwrap(), &action_schema())
+                .unwrap(),
             Err(LubHelp::EntityType),
         );
 
diff --git a/cedar-policy-validator/src/types/request_env.rs b/cedar-policy-validator/src/types/request_env.rs
index 22f18a53..dffe00f4 100644
--- a/cedar-policy-validator/src/types/request_env.rs
+++ b/cedar-policy-validator/src/types/request_env.rs
@@ -98,7 +98,7 @@ impl<'a> RequestEnv<'a> {
     /// [`Type`] of the `action` for this request environment
     pub fn action_type(&self, schema: &ValidatorSchema) -> Option<Type> {
         match self.action_entity_uid() {
-            Some(action) => Type::euid_literal(action.clone(), schema),
+            Some(action) => Type::euid_literal(action, schema),
             None => Some(Type::any_entity_reference()),
         }
     }
diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md
index 9a0c76a9..91ab8a83 100644
--- a/cedar-policy/CHANGELOG.md
+++ b/cedar-policy/CHANGELOG.md
@@ -17,10 +17,12 @@ Cedar Language Version: TBD
 
 - Implemented [RFC 48 (schema annotations)](https://github.com/cedar-policy/rfcs/blob/main/text/0048-schema-annotations.md)  (#1316)
 - New `.isEmpty()` operator on sets (#1358, resolving #1356)
+- New `Entity::new_with_tags()` and `Entity::tag()` functions (#1402, resolving #1374)
 - Added protobuf schemas and (de)serialization code using on `prost` crate behind the experimental `protobufs` flag.
 - Added protobuf and JSON generation code to `cedar-policy-cli`.
 - Added a new get helper method to Context that allows easy extraction of generic values from the context by key. This method simplifies the common use case of retrieving values from Context objects.
 - Implemented [RFC 62 (extended `has` operator)](https://github.com/cedar-policy/rfcs/blob/main/text/0062-extended-has.md)  (#1327, resolving #1329)
+- Added a helper method to `PartialResponse` to accept substitutions from an iterator.
 
 ### Changed
 
diff --git a/cedar-policy/Cargo.toml b/cedar-policy/Cargo.toml
index 5558a8b6..6dfddb30 100644
--- a/cedar-policy/Cargo.toml
+++ b/cedar-policy/Cargo.toml
@@ -23,7 +23,7 @@ miette = "7.4.0"
 thiserror = "2.0"
 smol_str = { version = "0.3", features = ["serde"] }
 dhat = { version = "0.3.2", optional = true }
-serde_with = "3.3.0"
+serde_with = "3.12.0"
 nonempty = "0.10"
 prost = { version = "0.13", optional = true }
 
@@ -72,7 +72,7 @@ cedar-policy-core = { version = "=4.3.0", features = [
 # NON-CRYPTOGRAPHIC random number generators
 oorandom = "11.1"
 
-proptest = "1.0.0"
+proptest = "1.6.0"
 
 [[bench]]
 name = "cedar_benchmarks"
diff --git a/cedar-policy/src/api.rs b/cedar-policy/src/api.rs
index 91c859d9..8ef7bb8b 100644
--- a/cedar-policy/src/api.rs
+++ b/cedar-policy/src/api.rs
@@ -166,15 +166,7 @@ impl Entity {
         attrs: HashMap<String, RestrictedExpression>,
         parents: HashSet<EntityUid>,
     ) -> Result<Self, EntityAttrEvaluationError> {
-        // note that we take a "parents" parameter here; we will compute TC when
-        // the `Entities` object is created
-        Ok(Self(ast::Entity::new(
-            uid.into(),
-            attrs.into_iter().map(|(k, v)| (SmolStr::from(k), v.0)),
-            parents.into_iter().map(EntityUid::into).collect(),
-            [],
-            Extensions::all_available(),
-        )?))
+        Self::new_with_tags(uid, attrs, parents, [])
     }
 
     /// Create a new `Entity` with no attributes.
@@ -191,6 +183,27 @@ impl Entity {
         ))
     }
 
+    /// Create a new `Entity` with this Uid, attributes, parents, and tags.
+    ///
+    /// Attribute and tag values are specified here as "restricted expressions".
+    /// See docs on [`RestrictedExpression`].
+    pub fn new_with_tags(
+        uid: EntityUid,
+        attrs: impl IntoIterator<Item = (String, RestrictedExpression)>,
+        parents: impl IntoIterator<Item = EntityUid>,
+        tags: impl IntoIterator<Item = (String, RestrictedExpression)>,
+    ) -> Result<Self, EntityAttrEvaluationError> {
+        // note that we take a "parents" parameter here, not "ancestors"; we
+        // will compute TC when the `Entities` object is created
+        Ok(Self(ast::Entity::new(
+            uid.into(),
+            attrs.into_iter().map(|(k, v)| (k.into(), v.0)),
+            parents.into_iter().map(EntityUid::into).collect(),
+            tags.into_iter().map(|(k, v)| (k.into(), v.0)),
+            Extensions::all_available(),
+        )?))
+    }
+
     /// Create a new `Entity` with this Uid, no attributes, and no parents.
     /// ```
     /// # use cedar_policy::{Entity, EntityId, EntityTypeName, EntityUid};
@@ -240,11 +253,21 @@ impl Entity {
     /// assert!(entity.attr("foo").is_none());
     /// ```
     pub fn attr(&self, attr: &str) -> Option<Result<EvalResult, PartialValueToValueError>> {
-        let v = match ast::Value::try_from(self.0.get(attr)?.clone()) {
-            Ok(v) => v,
-            Err(e) => return Some(Err(e)),
-        };
-        Some(Ok(EvalResult::from(v)))
+        match ast::Value::try_from(self.0.get(attr)?.clone()) {
+            Ok(v) => Some(Ok(EvalResult::from(v))),
+            Err(e) => Some(Err(e)),
+        }
+    }
+
+    /// Get the value for the given tag, or `None` if not present.
+    ///
+    /// This can also return Some(Err) if the tag is not a value (i.e., is
+    /// unknown due to partial evaluation).
+    pub fn tag(&self, tag: &str) -> Option<Result<EvalResult, PartialValueToValueError>> {
+        match ast::Value::try_from(self.0.get_tag(tag)?.clone()) {
+            Ok(v) => Some(Ok(EvalResult::from(v))),
+            Err(e) => Some(Err(e)),
+        }
     }
 
     /// Consume the entity and return the entity's owned Uid, attributes and parents.
@@ -1056,12 +1079,25 @@ impl PartialResponse {
         self.0.get(id.as_ref()).map(Policy::from_ast)
     }
 
-    /// Attempt to re-authorize this response given a mapping from unknowns to values
+    /// Attempt to re-authorize this response given a mapping from unknowns to values.
+    #[allow(clippy::needless_pass_by_value)]
+    #[deprecated = "use reauthorize_with_bindings"]
     pub fn reauthorize(
         &self,
         mapping: HashMap<SmolStr, RestrictedExpression>,
         auth: &Authorizer,
         es: &Entities,
+    ) -> Result<Self, ReauthorizationError> {
+        self.reauthorize_with_bindings(mapping.iter().map(|(k, v)| (k.as_str(), v)), auth, es)
+    }
+
+    /// Attempt to re-authorize this response given a mapping from unknowns to values, provided as an iterator.
+    /// Exhausts the iterator, returning any evaluation errors in the restricted expressions, regardless whether there is a matching unknown.
+    pub fn reauthorize_with_bindings<'m>(
+        &self,
+        mapping: impl IntoIterator<Item = (&'m str, &'m RestrictedExpression)>,
+        auth: &Authorizer,
+        es: &Entities,
     ) -> Result<Self, ReauthorizationError> {
         let exts = Extensions::all_available();
         let evaluator = RestrictedEvaluator::new(exts);
@@ -1070,7 +1106,7 @@ impl PartialResponse {
             .map(|(name, expr)| {
                 evaluator
                     .interpret(BorrowedRestrictedExpr::new_unchecked(expr.0.as_ref()))
-                    .map(|v| (name, v))
+                    .map(|v| (name.into(), v))
             })
             .collect::<Result<HashMap<_, _>, EvaluationError>>()?;
         let r = self.0.reauthorize(&mapping, &auth.0, &es.0)?;
diff --git a/cedar-policy/src/ffi/check_parse.rs b/cedar-policy/src/ffi/check_parse.rs
index 28e45f2b..eb18f080 100644
--- a/cedar-policy/src/ffi/check_parse.rs
+++ b/cedar-policy/src/ffi/check_parse.rs
@@ -260,12 +260,12 @@ mod test {
     use serde_json::json;
 
     #[track_caller]
-    fn assert_check_parse_is_ok(parse_result: CheckParseAnswer) {
+    fn assert_check_parse_is_ok(parse_result: &CheckParseAnswer) {
         assert_matches!(parse_result, CheckParseAnswer::Success);
     }
 
     #[track_caller]
-    fn assert_check_parse_is_err(parse_result: CheckParseAnswer) -> Vec<DetailedError> {
+    fn assert_check_parse_is_err(parse_result: &CheckParseAnswer) -> &[DetailedError] {
         assert_matches!(
             parse_result,
             CheckParseAnswer::Failure { errors } => errors
@@ -278,7 +278,7 @@ mod test {
                 "staticPolicies": "permit(principal, action, resource);"
         });
         let answer = serde_json::from_value(check_parse_policy_set_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -287,7 +287,7 @@ mod test {
             "staticPolicies": "forbid(principal, action, resource); permit(principal == User::\"alice\", action == Action::\"view\", resource in Albums::\"alice_albums\");"
         });
         let answer = serde_json::from_value(check_parse_policy_set_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -296,9 +296,9 @@ mod test {
             "staticPolicies": "forbid(principal, action, resource);permit(2pac, action, resource)"
         });
         let answer = serde_json::from_value(check_parse_policy_set_json(call).unwrap()).unwrap();
-        let errs = assert_check_parse_is_err(answer);
+        let errs = assert_check_parse_is_err(&answer);
         assert_exactly_one_error(
-            &errs,
+            errs,
             "failed to parse policies from string: unexpected token `2`",
             None,
         );
@@ -312,14 +312,14 @@ mod test {
             }
         });
         let answer = serde_json::from_value(check_parse_policy_set_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
     fn check_parse_schema_succeeds_empty_schema() {
         let call = json!({});
         let answer = serde_json::from_value(check_parse_schema_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -331,7 +331,7 @@ mod test {
           }
         });
         let answer = serde_json::from_value(check_parse_schema_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -342,9 +342,9 @@ mod test {
           }
         });
         let answer = serde_json::from_value(check_parse_schema_json(call).unwrap()).unwrap();
-        let errs = assert_check_parse_is_err(answer);
+        let errs = assert_check_parse_is_err(&answer);
         assert_exactly_one_error(
-            &errs,
+            errs,
             "failed to parse schema from JSON: missing field `actions`",
             None,
         );
@@ -389,7 +389,7 @@ mod test {
             }
         });
         let answer = serde_json::from_value(check_parse_entities_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -410,7 +410,7 @@ mod test {
             ]
         });
         let answer = serde_json::from_value(check_parse_entities_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -445,9 +445,9 @@ mod test {
             }
         });
         let answer = serde_json::from_value(check_parse_entities_json(call).unwrap()).unwrap();
-        let errs = assert_check_parse_is_err(answer);
+        let errs = assert_check_parse_is_err(&answer);
         assert_exactly_one_error(
-            &errs,
+            errs,
             "error during entity deserialization: in uid field of <unknown entity>, expected a literal entity reference, but got `\"TheNamespace::User::\\\"alice\\\"\"`",
             Some("literal entity references can be made with `{ \"type\": \"SomeType\", \"id\": \"SomeId\" }`")
         );
@@ -491,7 +491,7 @@ mod test {
 
         });
         let answer = serde_json::from_value(check_parse_context_json(call).unwrap()).unwrap();
-        assert_check_parse_is_ok(answer);
+        assert_check_parse_is_ok(&answer);
     }
 
     #[test]
@@ -531,7 +531,7 @@ mod test {
             }
         });
         let answer = serde_json::from_value(check_parse_context_json(call).unwrap()).unwrap();
-        let errs = assert_check_parse_is_err(answer);
-        assert_exactly_one_error(&errs, "while parsing context, expected the record to have an attribute `referrer`, but it does not", None);
+        let errs = assert_check_parse_is_err(&answer);
+        assert_exactly_one_error(errs, "while parsing context, expected the record to have an attribute `referrer`, but it does not", None);
     }
 }


Test patch:
diff --git a/cedar-policy-cli/tests/sample.rs b/cedar-policy-cli/tests/sample.rs
index bf2c14d2..ae36bca5 100644
--- a/cedar-policy-cli/tests/sample.rs
+++ b/cedar-policy-cli/tests/sample.rs
@@ -106,7 +106,7 @@ fn run_link_test(
     links_file: impl Into<String>,
     template_id: impl Into<String>,
     linked_id: impl Into<String>,
-    env: HashMap<SlotId, String>,
+    env: impl IntoIterator<Item = (SlotId, String)>,
     expected: CedarExitCode,
 ) {
     let cmd = LinkArgs {
@@ -117,7 +117,9 @@ fn run_link_test(
         },
         template_id: template_id.into(),
         new_id: linked_id.into(),
-        arguments: Arguments { data: env },
+        arguments: Arguments {
+            data: HashMap::from_iter(env),
+        },
     };
     let output = link(&cmd);
     assert_eq!(output, expected);
@@ -792,9 +794,7 @@ fn test_link_samples() {
         &linked_file_name,
         "AccessVacation",
         "AliceAccess",
-        [(SlotId::principal(), "User::\"alice\"".to_string())]
-            .into_iter()
-            .collect(),
+        [(SlotId::principal(), "User::\"alice\"".to_string())],
         CedarExitCode::Failure,
     );
 
@@ -803,9 +803,7 @@ fn test_link_samples() {
         &linked_file_name,
         "AccessVacation",
         "AliceAccess",
-        [(SlotId::principal(), "invalid".to_string())]
-            .into_iter()
-            .collect(),
+        [(SlotId::principal(), "invalid".to_string())],
         CedarExitCode::Failure,
     );
 
@@ -814,9 +812,7 @@ fn test_link_samples() {
         &linked_file_name,
         "AccessVacation",
         "AliceAccess",
-        [(SlotId::principal(), "User::\"alice\"".to_string())]
-            .into_iter()
-            .collect(),
+        [(SlotId::principal(), "User::\"alice\"".to_string())],
         CedarExitCode::Success,
     );
 
@@ -845,9 +841,7 @@ fn test_link_samples() {
         &linked_file_name,
         "AccessVacation",
         "BobAccess",
-        [(SlotId::principal(), "User::\"bob\"".to_string())]
-            .into_iter()
-            .collect(),
+        [(SlotId::principal(), "User::\"bob\"".to_string())],
         CedarExitCode::Success,
     );
 
diff --git a/cedar-policy-validator/src/cedar_schema/test.rs b/cedar-policy-validator/src/cedar_schema/test.rs
index bf72a6ab..a5c48b2e 100644
--- a/cedar-policy-validator/src/cedar_schema/test.rs
+++ b/cedar-policy-validator/src/cedar_schema/test.rs
@@ -13,10 +13,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
+#![cfg(test)]
 // PANIC SAFETY: unit tests
-#[allow(clippy::panic)]
-#[cfg(test)]
+#![allow(
+    clippy::cognitive_complexity,
+    clippy::panic,
+    clippy::unwrap_used,
+    clippy::indexing_slicing,
+    clippy::unreachable
+)]
+
 mod demo_tests {
     use std::{
         collections::BTreeMap,
@@ -737,7 +743,7 @@ namespace Baz {action "Foo" appliesTo {
                 }),
             );
             assert_has_type(
-                &attributes.get("viewACL").unwrap(),
+                attributes.get("viewACL").unwrap(),
                 json_schema::Type::Type(json_schema::TypeVariant::EntityOrCommon {
                     type_name: "DocumentShare".parse().unwrap(),
                 }),
@@ -927,7 +933,6 @@ namespace Baz {action "Foo" appliesTo {
     }
 }
 
-#[cfg(test)]
 mod parser_tests {
     use crate::cedar_schema::parser::parse_schema;
     use cool_asserts::assert_matches;
@@ -1154,11 +1159,6 @@ mod parser_tests {
     }
 }
 
-// PANIC SAFETY: tests
-#[allow(clippy::unreachable)]
-// PANIC SAFETY: tests
-#[allow(clippy::panic)]
-#[cfg(test)]
 mod translator_tests {
     use cedar_policy_core::ast as cedar_ast;
     use cedar_policy_core::extensions::Extensions;
@@ -1177,6 +1177,8 @@ mod translator_tests {
         ValidatorSchema,
     };
 
+    use super::SPECIAL_IDS;
+
     // We allow translating schemas that violate RFC 52 to `json_schema::Fragment`.
     // The violations are reported during further translation to `ValidatorSchema`
     #[test]
@@ -1450,10 +1452,6 @@ mod translator_tests {
         });
     }
 
-    // PANIC SAFETY: testing
-    #[allow(clippy::unwrap_used)]
-    // PANIC SAFETY: testing
-    #[allow(clippy::indexing_slicing)]
     #[test]
     fn type_name_resolution_cross_namespace() {
         let (schema, _) = json_schema::Fragment::from_cedarschema_str(
@@ -1998,6 +1996,28 @@ mod translator_tests {
         .expect("should translate to JSON schema");
         assert_eq!(serde_json::to_value(schema).unwrap(), json_value);
     }
+
+    #[test]
+    fn any_id() {
+        for id in SPECIAL_IDS {
+            test_translation(
+                &format!("@{id} entity User {{}};"),
+                serde_json::json!({
+                    "": {
+                        "entityTypes": {
+                            "User": {
+                                "annotations": {
+                                    id: "",
+                                }
+                            }
+                        },
+                        "actions": {},
+                    }
+                }),
+            )
+        }
+    }
+
     #[test]
     fn annotations() {
         // namespace annotations
@@ -2250,7 +2270,6 @@ mod translator_tests {
     }
 }
 
-#[cfg(test)]
 mod common_type_references {
     use cool_asserts::assert_matches;
 
@@ -2563,7 +2582,6 @@ mod common_type_references {
 }
 
 /// Tests involving entity tags (RFC 82)
-#[cfg(test)]
 mod entity_tags {
     use crate::json_schema;
     use crate::schema::test::utils::collect_warnings;
@@ -2633,13 +2651,44 @@ mod entity_tags {
     }
 }
 
+pub(crate) const SPECIAL_IDS: [&str; 18] = [
+    "principal",
+    "action",
+    "resource",
+    "context",
+    "true",
+    "false",
+    "permit",
+    "forbid",
+    "when",
+    "unless",
+    "in",
+    "has",
+    "like",
+    "is",
+    "if",
+    "then",
+    "else",
+    "__cedar",
+];
+
 // RFC 48 test cases
-#[cfg(test)]
 mod annotations {
     use cool_asserts::assert_matches;
 
     use crate::cedar_schema::parser::parse_schema;
 
+    use super::SPECIAL_IDS;
+
+    // test if annotation keys can be any id
+    #[test]
+    fn any_id() {
+        for id in SPECIAL_IDS {
+            let schema_str = format!("@{id} entity User {{}};");
+            assert_matches!(parse_schema(&schema_str), Ok(_));
+        }
+    }
+
     #[test]
     fn no_keys() {
         assert_matches!(
diff --git a/cedar-policy-validator/src/typecheck/test.rs b/cedar-policy-validator/src/typecheck/test.rs
index 813a73e6..ce734326 100644
--- a/cedar-policy-validator/src/typecheck/test.rs
+++ b/cedar-policy-validator/src/typecheck/test.rs
@@ -19,6 +19,7 @@
 #![allow(clippy::panic)]
 // PANIC SAFETY unit tests
 #![allow(clippy::indexing_slicing)]
+#![allow(clippy::cognitive_complexity)]
 
 pub(crate) mod test_utils;
 
diff --git a/cedar-policy/src/prop_test_policy_set.rs b/cedar-policy/src/prop_test_policy_set.rs
index 6772311f..4d81a632 100644
--- a/cedar-policy/src/prop_test_policy_set.rs
+++ b/cedar-policy/src/prop_test_policy_set.rs
@@ -112,7 +112,7 @@ impl PolicySetModel {
                         panic!("template to link map should have Vec for existing template")
                     }
                 };
-                assert!(self.link_to_template_map.get(policy_name).is_none());
+                assert!(!self.link_to_template_map.contains_key(policy_name));
                 self.link_to_template_map
                     .insert(policy_name.to_owned(), template_name.clone());
             }
diff --git a/cedar-policy/src/tests.rs b/cedar-policy/src/tests.rs
index e8b0fb77..7ed580b0 100644
--- a/cedar-policy/src/tests.rs
+++ b/cedar-policy/src/tests.rs
@@ -17,6 +17,7 @@
 #![cfg(test)]
 // PANIC SAFETY unit tests
 #![allow(clippy::panic)]
+#![allow(clippy::cognitive_complexity, clippy::too_many_lines)]
 
 use super::*;
 
@@ -4858,7 +4859,7 @@ mod policy_set_est_tests {
     #[test]
     fn test_partition_fold_err() {
         let even_or_odd = |s: &str| {
-            i64::from_str_radix(s, 10).map(|i| {
+            s.parse::<i64>().map(|i| {
                 if i % 2 == 0 {
                     Either::Left(i)
                 } else {
@@ -5074,10 +5075,11 @@ mod policy_set_est_tests {
                 r#"User::"John""#.parse().unwrap()
             )]))
         );
-        if let Err(_) = policyset
+        if policyset
             .get_linked_policies(PolicyId::new("template"))
             .unwrap()
             .exactly_one()
+            .is_err()
         {
             panic!("Should have exactly one");
         };
@@ -6323,30 +6325,25 @@ mod reserved_keywords_in_policies {
     }
 
     #[track_caller]
-    fn assert_valid_expression(src: String) {
-        assert_matches!(Expression::from_str(&src), Ok(_));
+    fn assert_valid_expression(src: &str) {
+        assert_matches!(Expression::from_str(src), Ok(_));
     }
 
     #[track_caller]
-    fn assert_invalid_expression(src: String, error: String, underline: String) {
-        let expected_err = ExpectedErrorMessageBuilder::error(&error)
-            .exactly_one_underline(&underline)
+    fn assert_invalid_expression(src: &str, error: &str, underline: &str) {
+        let expected_err = ExpectedErrorMessageBuilder::error(error)
+            .exactly_one_underline(underline)
             .build();
-        assert_matches!(Expression::from_str(&src), Err(err) => expect_err(&*src, &Report::new(err), &expected_err));
+        assert_matches!(Expression::from_str(src), Err(err) => expect_err(src, &Report::new(err), &expected_err));
     }
 
     #[track_caller]
-    fn assert_invalid_expression_with_help(
-        src: String,
-        error: String,
-        underline: String,
-        help: String,
-    ) {
-        let expected_err = ExpectedErrorMessageBuilder::error(&error)
-            .exactly_one_underline(&underline)
-            .help(&help)
+    fn assert_invalid_expression_with_help(src: &str, error: &str, underline: &str, help: &str) {
+        let expected_err = ExpectedErrorMessageBuilder::error(error)
+            .exactly_one_underline(underline)
+            .help(help)
             .build();
-        assert_matches!(Expression::from_str(&src), Err(err) => expect_err(&*src, &Report::new(err), &expected_err));
+        assert_matches!(Expression::from_str(src), Err(err) => expect_err(src, &Report::new(err), &expected_err));
     }
 
     #[test]
@@ -6367,16 +6364,16 @@ mod reserved_keywords_in_policies {
             .chain(RESERVED_NAMESPACE.iter())
             .chain(OTHER_SPECIAL_IDENTS.iter())
             .for_each(|id| {
-                assert_valid_expression(format!("{{ \"{id}\": 1 }}"));
-                assert_valid_expression(format!("principal has \"{id}\""));
-                assert_valid_expression(format!("principal[\"{id}\"] == \"foo\""));
+                assert_valid_expression(&format!("{{ \"{id}\": 1 }}"));
+                assert_valid_expression(&format!("principal has \"{id}\""));
+                assert_valid_expression(&format!("principal[\"{id}\"] == \"foo\""));
             });
 
         // No restrictions on OTHER_SPECIAL_IDENTS
         for id in &OTHER_SPECIAL_IDENTS {
-            assert_valid_expression(format!("{{ {id}: 1 }}"));
-            assert_valid_expression(format!("principal has {id}"));
-            assert_valid_expression(format!("principal.{id} == \"foo\""));
+            assert_valid_expression(&format!("{{ {id}: 1 }}"));
+            assert_valid_expression(&format!("principal has {id}"));
+            assert_valid_expression(&format!("principal.{id} == \"foo\""));
         }
 
         // RESERVED_IDENTS cannot be used as keys without quotes
@@ -6385,66 +6382,62 @@ mod reserved_keywords_in_policies {
             match id {
                 "true" | "false" => {
                     assert_invalid_expression_with_help(
-                        format!("{{ {id}: 1 }}"),
-                        format!("invalid attribute name: {id}"),
-                        id.into(),
-                        "attribute names can either be identifiers or string literals".into(),
+                        &format!("{{ {id}: 1 }}"),
+                        &format!("invalid attribute name: {id}"),
+                        id,
+                        "attribute names can either be identifiers or string literals",
                     );
                     assert_invalid_expression(
-                        format!("principal has {id}"),
-                        RESERVED_IDENT_MSG(id),
-                        id.to_string(),
+                        &format!("principal has {id}"),
+                        &RESERVED_IDENT_MSG(id),
+                        id,
                     );
                 }
                 "if" => {
                     assert_invalid_expression(
-                        format!("{{ {id}: 1 }}"),
-                        RESERVED_IDENT_MSG(id),
-                        format!("{id}: 1"),
+                        &format!("{{ {id}: 1 }}"),
+                        &RESERVED_IDENT_MSG(id),
+                        &format!("{id}: 1"),
                     );
                     assert_invalid_expression(
-                        format!("principal has {id}"),
-                        RESERVED_IDENT_MSG(id),
-                        id.to_string(),
+                        &format!("principal has {id}"),
+                        &RESERVED_IDENT_MSG(id),
+                        id,
                     );
                 }
                 _ => {
                     assert_invalid_expression(
-                        format!("{{ {id}: 1 }}"),
-                        RESERVED_IDENT_MSG(id),
-                        id.into(),
+                        &format!("{{ {id}: 1 }}"),
+                        &RESERVED_IDENT_MSG(id),
+                        id,
                     );
                     assert_invalid_expression(
-                        format!("principal has {id}"),
-                        RESERVED_IDENT_MSG(id),
-                        id.into(),
+                        &format!("principal has {id}"),
+                        &RESERVED_IDENT_MSG(id),
+                        id,
                     );
                 }
             }
             // this case leads to a consistent error for all keywords
             assert_invalid_expression(
-                format!("principal.{id} == \"foo\""),
-                RESERVED_IDENT_MSG(id),
-                id.into(),
+                &format!("principal.{id} == \"foo\""),
+                &RESERVED_IDENT_MSG(id),
+                id,
             );
         }
 
         // RESERVED_NAMESPACE cannot be used as keys without quotes
         for id in RESERVED_NAMESPACE {
+            assert_invalid_expression(&format!("{{ {id}: 1 }}"), &RESERVED_NAMESPACE_MSG(id), id);
             assert_invalid_expression(
-                format!("{{ {id}: 1 }}"),
-                RESERVED_NAMESPACE_MSG(id),
-                id.into(),
+                &format!("principal has {id}"),
+                &RESERVED_NAMESPACE_MSG(id),
+                id,
             );
             assert_invalid_expression(
-                format!("principal has {id}"),
-                RESERVED_NAMESPACE_MSG(id),
-                id.into(),
-            );
-            assert_invalid_expression(
-                format!("principal.{id} == \"foo\""),
-                RESERVED_NAMESPACE_MSG(id),
-                id.into(),
+                &format!("principal.{id} == \"foo\""),
+                &RESERVED_NAMESPACE_MSG(id),
+                id,
             );
         }
     }
@@ -6453,35 +6446,31 @@ mod reserved_keywords_in_policies {
     fn test_reserved_namespace_elements() {
         // No restrictions on OTHER_SPECIAL_IDENTS
         for id in &OTHER_SPECIAL_IDENTS {
-            assert_valid_expression(format!("foo::{id}::\"bar\""));
-            assert_valid_expression(format!("principal is {id}::foo"));
+            assert_valid_expression(&format!("foo::{id}::\"bar\""));
+            assert_valid_expression(&format!("principal is {id}::foo"));
         }
 
         // RESERVED_IDENTS cannot be used in namespaces
         for id in RESERVED_IDENTS {
+            assert_invalid_expression(&format!("foo::{id}::\"bar\""), &RESERVED_IDENT_MSG(id), id);
             assert_invalid_expression(
-                format!("foo::{id}::\"bar\""),
-                RESERVED_IDENT_MSG(id),
-                id.into(),
-            );
-            assert_invalid_expression(
-                format!("principal is {id}::foo"),
-                RESERVED_IDENT_MSG(id),
-                id.into(),
+                &format!("principal is {id}::foo"),
+                &RESERVED_IDENT_MSG(id),
+                id,
             );
         }
 
         // RESERVED_NAMESPACE cannot be used in namespaces
         for id in RESERVED_NAMESPACE {
             assert_invalid_expression(
-                format!("foo::{id}::\"bar\""),
-                RESERVED_NAMESPACE_MSG(&format!("foo::{id}")),
-                format!("foo::{id}"),
+                &format!("foo::{id}::\"bar\""),
+                &RESERVED_NAMESPACE_MSG(&format!("foo::{id}")),
+                &format!("foo::{id}"),
             );
             assert_invalid_expression(
-                format!("principal is {id}::foo"),
-                RESERVED_NAMESPACE_MSG(&format!("{id}::foo")),
-                format!("{id}::foo"),
+                &format!("principal is {id}::foo"),
+                &RESERVED_NAMESPACE_MSG(&format!("{id}::foo")),
+                &format!("{id}::foo"),
             );
         }
     }
@@ -6493,40 +6482,32 @@ mod reserved_keywords_in_policies {
 
         for id in RESERVED_IDENTS {
             assert_invalid_expression(
-                format!("extension::function::{id}(\"foo\")"),
-                RESERVED_IDENT_MSG(id),
-                id.into(),
-            );
-            assert_invalid_expression(
-                format!("context.{id}(1)"),
-                RESERVED_IDENT_MSG(id),
-                id.into(),
+                &format!("extension::function::{id}(\"foo\")"),
+                &RESERVED_IDENT_MSG(id),
+                id,
             );
+            assert_invalid_expression(&format!("context.{id}(1)"), &RESERVED_IDENT_MSG(id), id);
         }
 
         for id in RESERVED_NAMESPACE {
             assert_invalid_expression(
-                format!("extension::function::{id}(\"foo\")"),
-                RESERVED_NAMESPACE_MSG(&format!("extension::function::{id}")),
-                format!("extension::function::{id}"),
-            );
-            assert_invalid_expression(
-                format!("context.{id}(1)"),
-                RESERVED_NAMESPACE_MSG(id),
-                id.into(),
+                &format!("extension::function::{id}(\"foo\")"),
+                &RESERVED_NAMESPACE_MSG(&format!("extension::function::{id}")),
+                &format!("extension::function::{id}"),
             );
+            assert_invalid_expression(&format!("context.{id}(1)"), &RESERVED_NAMESPACE_MSG(id), id);
         }
 
         for id in OTHER_SPECIAL_IDENTS {
             assert_invalid_expression(
-                format!("extension::function::{id}(\"foo\")"),
-                format!("`extension::function::{id}` is not a valid function"),
-                format!("extension::function::{id}(\"foo\")"),
+                &format!("extension::function::{id}(\"foo\")"),
+                &format!("`extension::function::{id}` is not a valid function"),
+                &format!("extension::function::{id}(\"foo\")"),
             );
             assert_invalid_expression(
-                format!("context.{id}(1)"),
-                format!("`{id}` is not a valid method"),
-                format!("context.{id}(1)"),
+                &format!("context.{id}(1)"),
+                &format!("`{id}` is not a valid method"),
+                &format!("context.{id}(1)"),
             );
         }
     }