Skip to content

Support ValueOption + Struct attribute as optional parameter - #18098

Merged
T-Gro merged 17 commits into
dotnet:mainfrom
vzarytovskii:optional-parameters-valueoption
Dec 20, 2024
Merged

Support ValueOption + Struct attribute as optional parameter#18098
T-Gro merged 17 commits into
dotnet:mainfrom
vzarytovskii:optional-parameters-valueoption

Conversation

@vzarytovskii

@vzarytovskiivzarytovskii commented Dec 2, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Description

Implements fsharp/fslang-suggestions#1136 - (fsharp/fslang-suggestions#1136 (comment))

Checklist

  • Test cases added

I'm not 100% happy with working with attributes as synattributes (though can live with it), though we do it in the graph checking already, but alternative would be abstract option in the compiler and then make a decision during codegen. This will complicate overall checking quite a bit.

@vzarytovskii
vzarytovskii requested a review from a team as a code ownerDecember 2, 2024 15:38
@github-actions

github-actionsBot commented Dec 2, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

❗ Release notes required

✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
src/FSharp.Coredocs/release-notes/.FSharp.Core/9.0.200.md
LanguageFeatures.fsidocs/release-notes/.Language/preview.md

T-Gro
T-Gro reviewed Dec 2, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
T-Gro
T-Gro reviewed Dec 2, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
T-Gro
T-Gro reviewed Dec 2, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
T-Gro
T-Gro reviewed Dec 2, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
T-Gro
T-Gro reviewed Dec 2, 2024
View reviewed changes
Comment threadsrc/FSharp.Core/prim-types.fs
@vzarytovskii

vzarytovskii commented Dec 4, 2024

Copy link
Copy Markdown
MemberAuthor

@T-Gro@KevinRansom@0101 this is ready

vzarytovskii
vzarytovskii commented Dec 4, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckDeclarations.fs
T-Gro
T-Gro reviewed Dec 4, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/MethodCalls.fs Outdated
T-Gro
T-Gro reviewed Dec 4, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
T-Gro
T-Gro reviewed Dec 4, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
T-Gro
T-Gro reviewed Dec 4, 2024
View reviewed changes
Comment threadsrc/Compiler/Checking/CheckPatterns.fs Outdated
CopilotAI reviewed Dec 6, 2024
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot reviewed 13 out of 30 changed files in this pull request and generated no suggestions.

Files not reviewed (17)
  • src/Compiler/Checking/CheckDeclarations.fs: Language not supported
  • src/Compiler/Checking/CheckPatterns.fs: Language not supported
  • src/Compiler/Checking/MethodCalls.fs: Language not supported
  • src/Compiler/FSComp.txt: Language not supported
  • src/Compiler/Facilities/LanguageFeatures.fs: Language not supported
  • src/Compiler/Facilities/LanguageFeatures.fsi: Language not supported
  • src/Compiler/TypedTree/TypedTreeOps.fs: Language not supported
  • src/Compiler/TypedTree/TypedTreeOps.fsi: Language not supported
  • src/Compiler/xlf/FSComp.txt.cs.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.de.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.es.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.fr.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.it.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.ja.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.ko.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.pl.xlf: Language not supported
  • src/Compiler/xlf/FSComp.txt.pt-BR.xlf: Language not supported

@vzarytovskii

vzarytovskii commented Dec 16, 2024

Copy link
Copy Markdown
MemberAuthor

@T-Gro I think I have addressed everything. If tests are passing, this is ready. Lemme know if I forgot anything.

@github-actions
Automated command ran: fantomas
0c7f1f3
 Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>
@dotnetdotnet deleted a comment from github-actionsBotDec 18, 2024
@T-Gro
T-Gro merged commit 3285c75 into dotnet:mainDec 20, 2024
This was referenced Jan 8, 2025
Closed
Closed
Closed
Closed
@edgarfgpedgarfgp mentioned this pull request Sep 8, 2025
Closed
5 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@T-GroT-GroT-Gro approved these changes
+2 more reviewers
@KevinRansomKevinRansomKevinRansom approved these changes
@edgarfgpedgarfgpedgarfgp approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vzarytovskii@KevinRansom@edgarfgp@T-Gro