Skip to content

Improve implied lambda and delegate argument names#15277

Merged
psfinaki merged 9 commits intodotnet:mainfrom
kerams:arg
Jun 5, 2023
Merged

Improve implied lambda and delegate argument names#15277
psfinaki merged 9 commits intodotnet:mainfrom
kerams:arg

Conversation

@kerams
Copy link
Contributor

@kerams kerams commented May 30, 2023 •
edited
Loading

Alleviates the problem in #11898 (without direct delegates) and improves fsharp/fslang-suggestions#1083 (comment).

> System.IO.File.OpenText;;
val it: path: string -> System.IO.StreamReader

> let handler (x: string) (y: int) =
-     System.Console.WriteLine(x, y)
-
- let invokeHandler (foo: System.Action<string, int>) =
-     for parameter in foo.Method.GetParameters() do
-         printfn "%s " parameter.Name
-
- invokeHandler (System.Action<_,_>(handler));;
x
y
val handler: x: string -> y: int -> unit
val invokeHandler: foo: System.Action<string,int> -> unit
val it: unit = ()

> let z = Ok;;
val z: ResultValue: 'a -> Result<'a,'b>

instead of

> System.IO.File.OpenText;;
val it: arg00: string -> System.IO.StreamReader

> let handler (x: string) (y: int) =
-     System.Console.WriteLine(x, y)
-
- let invokeHandler (foo: System.Action<string, int>) =
-     for parameter in foo.Method.GetParameters() do
-         printfn "%s " parameter.Name
-
- invokeHandler (System.Action<_,_>(handler));;
delegateArg0
delegateArg1
val handler: x: string -> y: int -> unit
val invokeHandler: foo: System.Action<string,int> -> unit
val it: unit = ()

> let z = Ok;;
val z: arg0: 'a -> Result<'a,'b>

kerams
kerams commented May 30, 2023
View reviewed changes
kerams
kerams commented May 30, 2023
View reviewed changes
@kerams kerams marked this pull request as ready for review May 31, 2023 13:37
@kerams kerams requested a review from a team as a code owner May 31, 2023 13:37
@kerams
Copy link
Contributor Author

kerams commented May 31, 2023

Is Test Unoptimized Declarations Project1 using different lang versions between WindowsCompressedMetadata desktop_release and WindowsCompressedMetadata coreclr_release or am I going crazy?

@vzarytovskii
Copy link
Member

vzarytovskii commented May 31, 2023

Is Test Unoptimized Declarations Project1 using different lang versions between WindowsCompressedMetadata desktop_release and WindowsCompressedMetadata coreclr_release or am I going crazy?

Uh, it should run differently built fcs, I can look into it tomorrow.

@kerams
Copy link
Contributor Author

kerams commented May 31, 2023 •
edited
Loading

Got it, desktop test options do not specify preview for the checking of scripts, so I forced it for the 2 tests in question.

@T-Gro
Copy link
Member

T-Gro commented Jun 5, 2023

Got it, desktop test options do not specify preview for the checking of scripts, so I forced it for the 2 tests in question.

Yep, the current passing of langversion:preview looks good to me.

psfinaki
psfinaki approved these changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

@kerams awesome job, thanks a lot for the testing and examples!

If you want to continue working on this (refactoring, perf) - go ahead, otherwise feel free to resolve the comments and merge as-is.

@psfinaki psfinaki enabled auto-merge (squash) June 5, 2023 11:42
@psfinaki psfinaki merged commit fd25719 into dotnet:main Jun 5, 2023
@kerams kerams deleted the arg branch June 5, 2023 11:44
psfinaki pushed a commit that referenced this pull request Jun 6, 2023
@dotnet-bot @oleksandr-didyk
@sudqijawabreh @nojaf @T-Gro @kerams
Merge main to release/dev17.7 (#15311)
ba66afe 
* add review comment to sb files (#15288)

* add review comment to sb files

* add CODEOWNERS entry for source-build

* Don't show inline hint for arguments with same names as the parameters in DU (#15305)

* Signature of nested type with generic type parameter (#15259)

* Proof of concept

* Add generic parameter names to ModuleOrType.

* Revert ModuleOrType change

* Process ticks in demangledPath of TType_app.

* Only apply new logic when includeStaticParametersInTypeNames is active.

* Use FactForNETCOREAPP

* Fix build

---------

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* Improve implied lambda and delegate argument names (#15277)

* Improve implied lambda and delegate argument names

* Fix

* Add tests

* Revert non-preview tests

* Sigh

* Re-revert

* Fix test

* Add testx

---------

Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Sudqi <sudqiomar@gmail.com>
Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: kerams <kerams@users.noreply.github.com>
T-Gro added a commit that referenced this pull request Jun 6, 2023
@dotnet-bot @oleksandr-didyk
@sudqijawabreh @nojaf @T-Gro @kerams
Merge main to release/net8 (#15312)
9b2f911 
* add review comment to sb files (#15288)

* add review comment to sb files

* add CODEOWNERS entry for source-build

* Don't show inline hint for arguments with same names as the parameters in DU (#15305)

* Signature of nested type with generic type parameter (#15259)

* Proof of concept

* Add generic parameter names to ModuleOrType.

* Revert ModuleOrType change

* Process ticks in demangledPath of TType_app.

* Only apply new logic when includeStaticParametersInTypeNames is active.

* Use FactForNETCOREAPP

* Fix build

---------

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* Improve implied lambda and delegate argument names (#15277)

* Improve implied lambda and delegate argument names

* Fix

* Add tests

* Revert non-preview tests

* Sigh

* Re-revert

* Fix test

* Add testx

---------

Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Sudqi <sudqiomar@gmail.com>
Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: kerams <kerams@users.noreply.github.com>
@allisonchou allisonchou mentioned this pull request Jun 11, 2023
Closed
@0-gravity
Copy link

0-gravity commented Jul 6, 2023

Will it be released any time soon?

@vzarytovskii
Copy link
Member

vzarytovskii commented Jul 6, 2023 •
edited
Loading

Will it be released any time soon?

Probability should be in the next vs/dotnet release

@0-gravity 0-gravity mentioned this pull request Jul 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@T-Gro T-Gro T-Gro approved these changes

+1 more reviewer

@psfinaki psfinaki psfinaki approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release/17.7

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kerams @vzarytovskii @T-Gro @0-gravity @psfinaki