-
Notifications
You must be signed in to change notification settings - Fork 829
Spread operator { ...expr }
, { ...ty }
for records
#18927
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.
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
base: main
Are you sure you want to change the base?
Conversation
❗ Release notes requiredCaution 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:
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
|
Amazing work! I was literally asking @edgarfgp a few days ago if we had something like |
|
||
// F# preview (still preview in 10.0) | ||
LanguageFeature.FromEndSlicing, previewVersion // Unfinished features --- needs work | ||
LanguageFeature.RecordSpreads, previewVersion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see two options on how to integrate all forms of spreads:
-
1: Have a dedicate
feature/spreading
branch and have PRs merging incremental additions to it. This is how bigger features have been done in the past. -
2: Add the features to
main
(make us of the fact that NET10 development is close now, and within short timemain
will meannet11
already -> plenty of time) via dedicate feature switches. And prior to major release time, decide on the feature set to bring in.
Option 1 has advantages in terms of overall feature marketing and explain-ability to F# users.
Option 2 would give us options to dogfood selected pieces via preview SDK, and gather feedback sooner. (with the usual preview disclaimer of those bits being subject to potential change).
In this PR , Record spreads
IMO form a coherent addition to the language and could be integrated standalone (= I would vote for option 2 here, i.e. integrate to main once main means net11)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be a bit afraid that a feature branch would take a lot of effort to keep conflict-free. But yes, I understand the desire to avoid a situation like from-end slicing where the feature is never enabled but the extra code complexity sticks around (someday I will revive my slice branch that will address that particular problem, though 🙂).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am with you on merging it to main
as long as we can imagine final (e.g. at major .NET release time) user communication for that specific subset.
(sensible cuts I could imagine being separately release-able as of now basically follow the sections of the RFC Discussion: LanguageFeature.RecordSpread
, ObjectIntoRecordSpread
, InterfaceImplementationSpread
, SpreadingPattern
)
@brianrourkeboll Great to see work starting in this direction! |
|
||
/// Resolve a long identifier representing a record field | ||
let ResolveFieldPrim sink (ncenv: NameResolver) nenv ad ty (mp, id: Ident) allFields = | ||
let ResolveFieldPrim sink (ncenv: NameResolver) nenv ad ty (mp, id: Ident) isFromSpread allFields = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's early (and it's a draft), but I wonder if instead of boolean flag, it can be represented by the struct DU? Intention will be a bit more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize you probably meant something like type FromSpreadOrNah = FromSpread | Nah
, but since we technically already had that information in scope anyway...
It got a bit invasive, though.
e66b7a6
to
c61dbf4
Compare
6dd4f0b
to
d181c97
Compare
d181c97
to
b43fc1d
Compare
b43fc1d
to
46998d6
Compare
Description
Add support for the spread operator
...
in record types and expressions (nominal and anonymous).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 fsharp/fslang-suggestions#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
without
in parser?