fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Spread operator `{ ...expr }`, `{ ...ty }` for records

Open brianrourkeboll opened this issue 3 months ago • 7 comments

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
    • [x] Parsing and error recovery
    • [x] Set algebra
    • [x] Accessibility
    • [x] Mutability
    • [x] Generics
    • [x] Recursion
    • [x] Effects
    • [x] Allowed and disallowed sources
    • [ ] Conversions/coercions
    • [x] Emitted IL
      • [x] Attribute shadowing
      • [x] Most of the rest of the above
  • [ ] Add placeholder for without in parser? I had this originally but later removed it. I am unsure without is 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

brianrourkeboll avatar Sep 22 '25 01:09 brianrourkeboll

: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

github-actions[bot] avatar Sep 22 '25 01:09 github-actions[bot]

Amazing work! I was literally asking @edgarfgp a few days ago if we had something like type R3 = { ...R1; ...R2; E : int } in F#!

nojaf avatar Sep 22 '25 08:09 nojaf

@brianrourkeboll Great to see work starting in this direction!

dsyme avatar Sep 22 '25 12:09 dsyme

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):

image

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?

brianrourkeboll avatar Sep 30 '25 19:09 brianrourkeboll

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 avatar Oct 01 '25 13:10 T-Gro

@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.

brianrourkeboll avatar Oct 02 '25 16:10 brianrourkeboll

@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)

T-Gro avatar Nov 10 '25 14:11 T-Gro

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.

brianrourkeboll avatar Dec 15 '25 16:12 brianrourkeboll