Skip to content

Fix lambda normalization by renumbering parameters from innermost scope#568

Open
rishi-jat wants to merge 11 commits into
zio:developfrom
rishi-jat:fix-lambda-normal-form
Open

Fix lambda normalization by renumbering parameters from innermost scope#568
rishi-jat wants to merge 11 commits into
zio:developfrom
rishi-jat:fix-lambda-normal-form

Conversation

@rishi-jat

@rishi-jat rishi-jat commented Dec 20, 2025

Copy link
Copy Markdown

/claim #379

Normalize lambda parameter depths in LightTypeTagRef.Lambda
by recursively renumbering parameters from the innermost scope outward.

Normalization is applied locally during lambda construction and avoids
any changes to global parameter numbering or application logic.

Implements the approach suggested by @pshirshov.

Closes #379

Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Copilot AI review requested due to automatic review settings December 20, 2025 10:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors lambda normalization in LightTypeTagRef by removing the FAKE parameter substitution approach and implementing a depth-based renumbering scheme. The new approach assigns depth 0 to innermost lambda parameters and negative depths (-1, -2, etc.) to outer scopes, making the normalized form suitable as the primary representation.

Key changes:

  • Replaces makeFakeParams and lambdaFakeParamDepth with a recursive normalize function
  • Implements depth renumbering based on lambda nesting level
  • Removes normalizedParams as it's no longer needed with the new approach

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
@rishi-jat

Copy link
Copy Markdown
Author

@pshirshov @neko-kai , PR is ready for review please have a look when you get a chance. Thanks!

@neko-kai

Copy link
Copy Markdown
Member

You should at least add the test from the issue itself to check the new normalized forms

Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
@rishi-jat rishi-jat marked this pull request as draft December 20, 2025 16:50
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
@rishi-jat rishi-jat marked this pull request as ready for review December 20, 2025 17:15
@rishi-jat

Copy link
Copy Markdown
Author

@neko-kai please have a look when you get a chance. Thanks!

@rishi-jat

Copy link
Copy Markdown
Author

@pshirshov @neko-kai, can you please review the PR ? Thanks!

@rishi-jat

Copy link
Copy Markdown
Author

/cc @pshirshov
/cc @neko-kai

Signed-off-by: Rishi Jat <rishijat098@gmail.com>
@rishi-jat rishi-jat requested a review from neko-kai January 9, 2026 00:44
@pshirshov

Copy link
Copy Markdown
Member

Could you add some tests for string representations of the renormalized lambdas?

@rishi-jat

Copy link
Copy Markdown
Author

Could you add some tests for string representations of the renormalized lambdas?

will do asap. Thanks!

Signed-off-by: Rishi Jat <rishijat098@gmail.com>
@rishi-jat

rishi-jat commented Feb 1, 2026

Copy link
Copy Markdown
Author

hi @pshirshov @neko-kai ,
I’ve added a regression test that checks the string representation of normalized lambdas.

The test now covers a real nested lambda case, verifies that FAKE_* parameters are gone, and asserts that outer-scope references are rendered with the correct depth in the output. All tests pass on Scala 2.12, 2.13, and 3.

Please let me know if you’d like the assertion to be stricter or if there’s another case you want covered.

@rishi-jat

Copy link
Copy Markdown
Author

/cc @neko-kai
/cc @pshirshov

@rishi-jat

Copy link
Copy Markdown
Author

@neko-kai @pshirshov , did you get a chance to review the PR ? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better lambda normal form

4 participants