Spread operator `{ ...expr }`, `{ ...ty }` for records
Description
Add support for the spread operator ... in record types and expressions (nominal and anonymous).
type R1 = { A : int; B : int }
type R2 = { C : int; D : int }
type R3 = { ...R1; ...R2; E : int } // { A : int; B : int; C : int; D : int; E : int }
let r1 = { A = 1; B = 2 }
let r2 = { C = 3; D = 4 }
let r3 = { ...r1; ...r2; E = 5 } // { A = 1; B = 2; C = 3; D = 4; E = 5 }
let r1' : R1 = { ...r3; B = 99 } // { A = 1; B = 99 }
let r2' = {| ...r2 |} // {| C = 3; D = 4 |}
let r3' = { ...r1'; ...r2' } // { A = 1; B = 99; C = 3; D = 4 }
let r3'' = {| ...r1; ...r2 |} // {| A = 1; B = 2; C = 3; D = 4 |}
let r3''' = {| ...r1'; ...r2' |} // {| A = 1; B = 99; C = 3; D = 4 |}
- [x] Language suggestion: https://github.com/fsharp/fslang-suggestions/issues/1253
- [x] RFC discussion: https://github.com/fsharp/fslang-design/discussions/806
This PR is meant to begin probing the "spread operator for objects" space — especially the set algebra and associated mechanics — while leaving room for implementing more of the scenarios outlined in https://github.com/fsharp/fslang-suggestions/issues/1253 later. For example, should this prove viable, I would expect one of the next additions to be support for spreading non-records into records, i.e., mapping regular class/struct/interface properties/fields to record fields; this PR explicitly disallows that to ensure that we are free to add it later.
Checklist
- [ ] Release notes entry updated
- [x] Fix any regressions
- [ ] Enough™ tests
- [ ] Add placeholder for
withoutin parser? I had this originally but later removed it. I am unsurewithoutis really needed or worth the complexity — but if we want to keep our options open, I will add it back in. - [ ] RFC: https://github.com/fsharp/fslang-design/pull/805
- [x] Address remaining TODOs (mostly AST traversal branches)
- [ ] Clean up/refactor
Feature overview
I hope that the tests can serve as a reasonable overview of the feature and its behavior. If you see any glaring omissions, or if the tests are unclear or incomplete, please let me know. (I see that I still need to add tests for coercions/conversions…)
Parsing & error recovery
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L46-L69
Record type spreads: set algebra
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L72-L225
Record type spreads: accessibility
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L227-L244
Record type spreads: mutability
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L246-L261
Record type spreads: generic type parameters
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L263-L435
Record type spreads: non-record source (not allowed)
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L437-L534
Record type spreads: recursion (cycles not allowed)
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L536-L646
Anonymous record expression spreads: set algebra
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L649-L831
Anonymous record expression spreads: accessibility
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L833-L850
Anonymous record expression spreads: mutability
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L852-L874
Anonymous record expression spreads: generic type parameters
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L876-L935
Anonymous record expression spreads: non-record-source (not currently allowed)
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L937-L1101
Anonymous record expression spreads: non-field members on record sources are ignored
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1103-L1148
Anonymous record expression spreads: effects
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1150-L1172
Nominal record expression spreads: set algebra
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1231-L1391
Nominal record expression spreads: accessibility
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1393-L1411
Nominal record expression spreads: non-recourd source (not currently allowed)
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1413-L1547
Nominal record expression spreads: non-field members on record sources are ignored
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1549-L1600
Nominal record expression spreads: effects
https://github.com/dotnet/fsharp/blob/574e6b1b6ac19fa517f785d316cfd2a06dba2c5f/tests/FSharp.Compiler.ComponentTests/Language/SpreadTests.fs#L1602-L1626
:heavy_exclamation_mark: Release notes required
@brianrourkeboll,
[!CAUTION] No release notes found for the changed paths (see table below).
Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.
The following format is recommended for this repository:
* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.
You can open this PR in browser to add release notes: open in github.dev
| Change path | Release notes path | Description |
|---|---|---|
src/Compiler |
docs/release-notes/.FSharp.Compiler.Service/10.0.200.md | No release notes found or release notes format is not correct |
LanguageFeatures.fsi |
docs/release-notes/.Language/preview.md | No release notes found or release notes format is not correct |
Amazing work! I was literally asking @edgarfgp a few days ago if we had something like type R3 = { ...R1; ...R2; E : int } in F#!
@brianrourkeboll Great to see work starting in this direction!
Hmm. It looks like there are some inconsistencies in the IL emitted for some of the tests I added between the net472 (left) and net10.0 (right) targets (unrelated to this feature itself):
It seems like it would be somewhat tough to update the IL normalization code to ignore it safely and without false positives/negatives. Do I need to make separate baselines for the two target frameworks instead? (I hope not: that would be a massive amount of duplication for basically no reason...) Any other way to handle that?
I believe the state of the art so far has indeed been to duplicate the .bsl files due to it. This is a duck-typed attribute which is added only if the compilation unit cannot find it - maybe this fact can be hijacked somehow in order to unify netcore and net472 .bsl files?
Brainstorming:
- Add the attribute definition to FSharp.Core built for testing purposes (maybe too complicated?)
- Or add it there for real?
- Add a test-helper that will add the attribute definition to executed tests
Right it is IlxGen which adds it conditionally, leading to differences in IL. If it is added unconditionally, the baselines should not differ.
Alternative approach which I see I have used at: https://github.com/dotnet/fsharp/blob/694c51269c26ec52042449614428b8a92f0cbd91/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs#L915
Is to assert only a subset of the .bsl, not the full file (IMO this could be encoded into the framework if we want to - like a takeWhile line<>... rule for both sides of the IL comparison )
@T-Gro This was my solution for the time being: https://github.com/dotnet/fsharp/pull/18927/commits/ff2a3e63866b62ae141dcb599570ad492bd87c0c. Let me know if you think the risk is too high that some kind of pertinent difference could later be introduced in the behavior of spreads between .NET Framework and .NET (Core) — the risk seems pretty low to me, though.
@T-Gro This was my solution for the time being: ff2a3e6. Let me know if you think the risk is too high that some kind of pertinent difference could later be introduced in the behavior of spreads between .NET Framework and .NET (Core) — the risk seems pretty low to me, though.
This is fine - we would only need a split if there is anything hinting a different decision path, such as ilimport, dependency on attributes, or ilxgen. Right now there is neither of that.
We might later want an overall end to end scenario just as a smoke test for the desktop compiler, which is what is running in Visual Studio - more as a proof of overall stability. (right now the combination of compiler tfm and target tfm is coupled when it comes to ComponentTests)
This is really great to see. Thanks! But please, please update the RFC before merging this one. I don't think it needs much text, perhaps even just the test cases from the PR post. And if possible the related spec changes.
@Martin521 I agree.