Skip to content

TPE: Always shadow the original variable#2299

Merged
john-h-kastner-aws merged 2 commits intocedar-policy:mainfrom
luxas:shadow-partially-evaluated-exprs
Apr 8, 2026
Merged

TPE: Always shadow the original variable#2299
john-h-kastner-aws merged 2 commits intocedar-policy:mainfrom
luxas:shadow-partially-evaluated-exprs

Conversation

@luxas
Copy link
Copy Markdown
Member

@luxas luxas commented Apr 8, 2026

Description of changes

Broken out from #2162.
Follow the pattern of shadowing the un-evaluated variable when interpreting it partially, so that the later code can never accidentally use/returned the initial/un-evaluated data.

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

  • A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)

I confirm that this PR (choose one, and delete the other options):

  • Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that cedar-spec (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

I confirm that docs.cedarpolicy.com (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar language specification.

value: false.into(),
ty: ty.clone(),
}),
Residual::Partial { .. } => {
Copy link
Copy Markdown
Member Author

@luxas luxas Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, this diff doesn't render well in Github (in VS Code it's better). What I did was just to add a new {} block so I could redefine the right variable, and removed the unnecessary clone.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting "hide whitespace" fixes the diff

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today I learned, nice! 💯

_ => Residual::Partial {
kind: ResidualKind::And {
left: Arc::new(left),
right: Arc::new(right),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I removed the unnecessary clone here now

_ => Residual::Partial {
kind: ResidualKind::Or {
left: Arc::new(left),
right: Arc::new(right),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I removed the unnecessary clone here now

@luxas luxas force-pushed the shadow-partially-evaluated-exprs branch from f304aad to ac0831d Compare April 8, 2026 12:50
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Coverage Report

Head Commit: f304aad20891a2b065bd6ce42af8610c3eb66f17

Base Commit: 03d1ca290f0c27afbb138c6c69990ec801944c4b

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 100.00%

Status: PASSED ✅

Details
File Status Covered Coverage Missed Lines
cedar-policy-core/src/tpe/evaluator.rs 🟢 69/69 100.00%

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.65%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-language-server 🟢 4722/5102 92.55% 92.55%
cedar-policy 🟡 4227/5353 78.97% 78.97%
cedar-policy-cli 🟡 1210/1614 74.97% 74.97%
cedar-policy-core 🟢 24010/27281 88.01% 88.01%
cedar-policy-formatter 🟢 914/1088 84.01% 84.01%
cedar-policy-symcc 🟢 6752/7262 92.98% 92.98%
cedar-wasm 🔴 0/28 0.00% 0.00%

@luxas luxas force-pushed the shadow-partially-evaluated-exprs branch from ac0831d to c83b0d9 Compare April 8, 2026 13:06
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Coverage Report

Head Commit: ac0831d0e3dc8b3576e90c299205515f69a65e94

Base Commit: 03d1ca290f0c27afbb138c6c69990ec801944c4b

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 100.00%

Status: PASSED ✅

Details
File Status Covered Coverage Missed Lines
cedar-policy-core/src/tpe/evaluator.rs 🟢 69/69 100.00%

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.65%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-language-server 🟢 4722/5102 92.55% 92.55%
cedar-policy 🟡 4227/5353 78.97% 78.97%
cedar-policy-cli 🟡 1210/1614 74.97% 74.97%
cedar-policy-core 🟢 24010/27281 88.01% 88.01%
cedar-policy-formatter 🟢 914/1088 84.01% 84.01%
cedar-policy-symcc 🟢 6752/7262 92.98% 92.98%
cedar-wasm 🔴 0/28 0.00% 0.00%

luxas added 2 commits April 8, 2026 16:15
Signed-off-by: Lucas Käldström <lucas.kaldstrom@upbound.io>
Signed-off-by: Lucas Käldström <lucas.kaldstrom@upbound.io>
@luxas luxas force-pushed the shadow-partially-evaluated-exprs branch from c83b0d9 to b5c08a3 Compare April 8, 2026 13:17
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Coverage Report

Head Commit: c83b0d9bbfec579880970f3b8cf677ce221abfff

Base Commit: 03d1ca290f0c27afbb138c6c69990ec801944c4b

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 100.00%

Status: PASSED ✅

Details
File Status Covered Coverage Missed Lines
cedar-policy-core/src/tpe/evaluator.rs 🟢 71/71 100.00%

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.65%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-language-server 🟢 4722/5102 92.55% 92.55%
cedar-policy 🟡 4227/5353 78.97% 78.97%
cedar-policy-cli 🟡 1210/1614 74.97% 74.97%
cedar-policy-core 🟢 24010/27281 88.01% 88.01%
cedar-policy-formatter 🟢 914/1088 84.01% 84.01%
cedar-policy-symcc 🟢 6752/7262 92.98% 92.98%
cedar-wasm 🔴 0/28 0.00% 0.00%

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Coverage Report

Head Commit: b5c08a359e696f441c2ce3c7987de86c533c6a22

Base Commit: 03d1ca290f0c27afbb138c6c69990ec801944c4b

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 100.00%

Status: PASSED ✅

Details
File Status Covered Coverage Missed Lines
cedar-policy-core/src/tpe/evaluator.rs 🟢 71/71 100.00%

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.65%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-language-server 🟢 4722/5102 92.55% 92.55%
cedar-policy 🟡 4227/5353 78.97% 78.97%
cedar-policy-cli 🟡 1210/1614 74.97% 74.97%
cedar-policy-core 🟢 24010/27281 88.01% 88.01%
cedar-policy-formatter 🟢 914/1088 84.01% 84.01%
cedar-policy-symcc 🟢 6752/7262 92.98% 92.98%
cedar-wasm 🔴 0/28 0.00% 0.00%

Copy link
Copy Markdown
Contributor

@john-h-kastner-aws john-h-kastner-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@john-h-kastner-aws john-h-kastner-aws merged commit 92723aa into cedar-policy:main Apr 8, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants