Conversation
❗ Release notes required
|
|
@edgarfgp This is really good, thank you! What could also be done next is removing the |
…fgp/fsharp into reuse-synbinding-to-model-and-bang
There was a problem hiding this comment.
Pull Request Overview
This PR unifies the representation of and! bindings with regular bindings by replacing the specialized SynExprAndBang type with the general SynBinding type. This change simplifies the AST structure by eliminating duplicate functionality and leveraging the existing binding infrastructure.
Key changes:
- Replaced
SynExprAndBangtype withSynBindingfor modelingand!clauses - Updated all references throughout the codebase to use the unified binding representation
- Modified test baselines to reflect the new AST structure
Reviewed Changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Compiler/SyntaxTree/SyntaxTree.fsi |
Updated interface to use SynBinding list instead of SynExprAndBang list |
src/Compiler/SyntaxTree/SyntaxTree.fs |
Removed SynExprAndBang type definition and updated LetOrUseBang to use SynBinding |
src/Compiler/SyntaxTree/SyntaxTrivia.* |
Removed SynExprAndBangTrivia type as it's no longer needed |
src/Compiler/SyntaxTree/ParseHelpers.* |
Updated mkAndBang to return SynBinding instead of SynExprAndBang |
| Multiple service files | Updated field accessors from SynExprAndBang pattern to SynBinding pattern |
| Test baseline files | Updated expected AST output to reflect new SynBinding structure |
| Surface area test files | Removed SynExprAndBang API surface and added SynBinding.Trivia property |
| Release notes | Added entry documenting the change |
|
This is ready :) |
|
/run test-baseline SurfaceAreaTest |
🔧 CLI Command Report
✅ Command succeeded, no changes needed. |
# Conflicts: # tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl # tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl
Head branch was pushed to by a user without write access
|
/run test-baseline SurfaceAreaTest |
🔧 CLI Command Report
✅ Patch applied: |
Description
Aiming to unify
LetOrUseBangandLetOrUse. I propose we useSynbindingto modeland!which removes the need forSynExprAndBang.Checklist