Skip to content

Consolidate SynExpr.LetOrUseBang(isUse=false) #18482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Apr 25, 2025
Merged

Consolidate SynExpr.LetOrUseBang(isUse=false) #18482

merged 7 commits into from
Apr 25, 2025
+183 −188

Conversation

edgarfgp
Copy link
Contributor

Description

Continuation of #18472

  • Unifies the SynExpr.LetOrUseBang(isUse =false) logic into a single case.

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated:

@edgarfgp edgarfgp added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Apr 16, 2025
@edgarfgp edgarfgp changed the title Consolidate SynExpr.LetOrUseBang(isUse=false) Consolidate SynExpr.LetOrUseBang(isUse=false) Apr 16, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 16, 2025 •
edited
Loading

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@edgarfgp edgarfgp marked this pull request as ready for review April 16, 2025 18:16
@edgarfgp edgarfgp requested a review from a team as a code owner April 16, 2025 18:16
@edgarfgp edgarfgp mentioned this pull request Apr 23, 2025
Open
2 tasks
T-Gro
T-Gro reviewed Apr 24, 2025
View reviewed changes
Copy link
Member

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

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

Why is there suddenly code added?
I expected more code deletions to match it up here, what am I missing?

@edgarfgp edgarfgp force-pushed the more-ce-logic-deduplication branch from 9f5c4a3 to 41e7872 Compare April 24, 2025 18:38
@edgarfgp
Copy link
Contributor Author

Why is there suddenly code added? I expected more code deletions to match it up here, what am I missing?

@T-Gro No you are right. I consolidated the two cases into one(active pattern). But I forgot to remove the duplicated code :).

@T-Gro T-Gro enabled auto-merge (squash) April 25, 2025 08:24
@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Apr 25, 2025
@T-Gro T-Gro merged commit 9b4a7ce into dotnet:main Apr 25, 2025
33 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abonie abonie abonie approved these changes

@T-Gro T-Gro T-Gro approved these changes

Assignees
No one assigned
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
F# Compiler and Tooling
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edgarfgp @abonie @T-Gro