[FS-1047] - match! (match-bang) by jwosty · Pull Request #4427 · dotnet/fsharp

jwosty · 2018-03-04T06:13:06Z

I've implemented a version of match! that works on my fork. However, since I've been working on Mono & OSX, and I do not have access to a Windows machine for the next few days, I haven't been able to write any kind of tests for this (though I'm not sure where they would go, anyway). Any help or pointers on this matter would be much appreciated (e.g. where are the computational expression tests)

msftclas · 2018-03-04T06:13:17Z

All CLA requirements met.

jwosty · 2018-03-04T06:16:58Z

Here's a quick little sample program using the new syntax for anyone to reference: https://gist.github.com/jwosty/bc7e1723477ea4c300e1fe6a2581d687

cartermp · 2018-03-04T16:02:18Z

@jwosty Thanks so much for the contribution! I'm sure @dsyme will offer a thorough review here, and likely have additional ideas about tests.

For now, you can add some tests here: https://github.com/Microsoft/visualfsharp/tree/master/tests/fsharp/core/patterns

forki · 2018-03-05T08:08:36Z

+        Async.Sleep 1
+        return Hearts }
+
+    Async.RunSynchronously <| async {


can you please reverse it so that it does not use <|

jwosty · 2018-03-05T08:08:47Z

Alright, trying out adding some tests.

forki · 2018-03-05T08:10:37Z

cool stuff. would love to use it

/cc @tpetricek

jwosty · 2018-03-05T08:11:32Z

Me too :) I have a lot of use cases for this in my own projects

forki · 2018-03-05T08:15:42Z

same here. lots and lots of let! followed by match - but inside task { }. since you did not touch FSharp.Core (so no change to async builder) I assume it would automatically work for tasks and promise builder as well?

realvictorprm · 2018-03-05T08:16:47Z

@jwosty ping me for a review, I'll also test it.

jwosty · 2018-03-05T08:16:57Z

Yep, it works with any computation expression. It's purely a compiler trick that desugars it into a let! and a normal match

EDIT: technically, it's actually a function

realvictorprm · 2018-03-05T08:17:59Z

@jwosty that's usually the trick with compilers 😄
Reminds me of the string interpolation. It's just a desugaring to string.concat with calls to "ToString" 😉

forki · 2018-03-05T08:18:02Z

@jwosty that's pretty nice!

jwosty · 2018-03-05T08:18:03Z

@realvictorprm I'd love that; I don't actually have easy access to a Windows machine (so its a little trickier to pull off the testing)

@forki sweet and simple does the trick! In fact I originally suggested having it optionally emit a new computation builder method (BindPattern or something), but @dsyme wanted to go the simpler route. Makes sense in the end; doesn't require any core library changes

jwosty · 2018-03-05T08:37:43Z

Also something to keep track of: What tooling changes should take place that would correspond to this? Which repos?

forki · 2018-03-05T08:42:19Z

@jwosty what kind of tooling changes to you mean?

jwosty · 2018-03-05T08:43:41Z

@forki As in, IDE stuff. E.g. syntax highlighting doesn't recognize it (at least in the way it appears in VS for Mac). Actually, syntax highlighting might be it -- I can't think of how it would effect intellisense

forki · 2018-03-05T08:53:10Z

@jwosty I think VS for Mac can't show it as keyword until @nosami updates the FCS that will bring your changes. So that will take a while.

@vasily-kirichenko do you know where FCS taks keywords from?

jwosty · 2018-03-05T08:56:55Z

@forki I see

auduchinok · 2018-03-05T09:35:34Z

@forki It's available in public API via Microsoft.FSharp.Compiler.SourceCodeServices.Keywords.
Internally it is defined in Microsoft.FSharp.Compiler.Lexhelp module.
https://github.com/Microsoft/visualfsharp/blob/f214dcc38f6d34db8902eb40bd5b037ade59a67c/src/fsharp/service/ServiceLexing.fsi#L237-L238
https://github.com/Microsoft/visualfsharp/blob/f214dcc38f6d34db8902eb40bd5b037ade59a67c/src/fsharp/lexhelp.fsi#L75

forki · 2018-03-05T09:40:09Z

@auduchinok thx

@jwosty so you would need to add it here: https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/lexhelp.fs#L396

nosami · 2018-03-05T09:56:35Z

@jwosty @forki keyword intellisense and syntax highlighting doesn't come from FCS in VS for Mac. It needs to be done separately from any work that is done here. I can add it though once this PR lands.

forki · 2018-03-05T10:07:58Z

@nosami ideally you should change it to FCS :P

jwosty · 2018-04-02T23:49:45Z

@dotnet-bot test this please

Thorium · 2018-04-17T11:53:46Z

Nice!
Just one question: Why this is match! and not with!?

abelbraaksma · 2018-04-17T14:15:42Z

Just one question: Why this is match! and not with!?

@Thorium, that seems just logical to me. I mean, the keyword with has multiple meanings (type extensions, copy-and-update record expressions, object expressions, match expressions), the keyword match doesn't. Using match! instead of with! keeps things simple and clear.

Might be interesting to consider function! though, as it is often used as shorthand for match. Though I'm not sure about the particulars.

But apparently I repeat myself and there's already an answer to that from @jwosty:

As for function!, I feel that would have to be another feature suggestion at this point. [...] Also, the semantics of that would warrant a discussion [...]

jwosty · 2018-04-17T14:39:17Z

@Thorium Because match! looks similar to the other CE constructs; the "bang" always comes before the element that you're binding, never after. To me, this:

let! x = expr1
match! expr2 with
| ...

looks more consistent, and thus more visually pleasing, than this:

let! x = expr1
match expr2 with!
| ...

@abelbraaksma I agree, it would be interesting to discuss that -- you should consider starting an fslang-suggestions thread

Thorium · 2018-04-17T15:00:22Z

@jwosty I was thinking of

match expr2 with!
| ...

similarity of what could be

try expr2 with!
| ...

when there is also a finally!

forki · 2018-04-17T15:03:48Z

That would be try! Tuomas Hietanen <notifications@github.com> schrieb am Di., 17. Apr. 2018, 17:00:

…

@jwosty <https://github.com/jwosty> I was thinking of match expr2 with!| ... similarity of what could be try expr2 with!| ... when there is also a finally! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4427 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNKRWFDQjycnTD67BvjycqeOrLjsBks5tpgOHgaJpZM4SbN02> .

PatrickMcDonald · 2018-04-17T15:59:41Z

As Yoda might say:

do or !do, there is no try

dsyme · 2018-05-21T11:17:15Z

@jwosty Could you merge with master? thanks

cartermp · 2018-05-21T17:07:32Z

Something is off here, there are far too many changes getting merged in

jwosty · 2018-05-21T17:30:52Z

@cartermp Ah, that's because I did a squash merge. I should probably roll it back and redo it without squashing. My bad.

For cleaner history, I'm gonna force push to delete the merge commit

jwosty · 2018-05-21T18:39:29Z

@dsyme Merged.

forki · 2018-05-22T04:52:10Z

Really looking forward to getting this in my app. I think I have at least 100 places where it would help. Small addition, but really useful. Thanks Phillip Carter <notifications@github.com> schrieb am Mo., 21. Mai 2018, 20:48:

…

***@***.**** approved this pull request. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4427 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNFMDSuPeuk2rQ9C6Nw71z8IvsHPhks5t0wwIgaJpZM4SbN02> .

jwosty · 2018-05-22T05:00:04Z

@forki I mean, I've literally written multiple instances today that need it! I cannot wait to be spoiled by this addition.

KevinRansom · 2018-05-25T00:23:40Z

Thank you for this.

jwosty · 2018-05-25T03:16:33Z

@KevinRansom of course! Glad to help F# move forward.

forki · 2018-05-25T04:44:36Z

Yay! John Wostenberg <notifications@github.com> schrieb am Fr., 25. Mai 2018, 05:16:

…

@KevinRansom <https://github.com/KevinRansom> of course! Glad to help F# move forward. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4427 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNHbg0Nh3dqlLLRAt-QePHxAia41xks5t13eSgaJpZM4SbN02> .

jwosty · 2018-07-30T17:33:53Z

+    type CardSuit = | Hearts | Diamonds | Clubs | Spades
+    let fetchSuit () = async {
+        // do something in order to not allow optimizing things away
+        Async.Sleep 1


Just noticed -- it should be do! Async.Sleep 1.

On second thought I'm not sure this is needed at all -- the compiler can't doesn't really optimize let foo () = async { return 42 }, does it?

* Attempt add match! syntax to parser (as normal match) (does not work) * Add name for MATCH_BANG token ("keyword 'match!') * Make match! valid in (and only in) computational expressions * match! works * Add match! to xlf localization files * Add two tests for match! * Don't use left-pipe * Give match! keyword a description * Fix syntax error, and change match! description * xlf updated * Add match! keyword description to resx * Update FSComp.fs * Write quotation test for match! * First crack at compile error tests * Fix baselines * Fix baseline one more time * Add vs baseline * Fix merge mistake * Fix merge mistake (#2) * Fix merge mistake in tests

jwosty added 5 commits March 2, 2018 17:26

Attempt add match! syntax to parser (as normal match) (does not work)

aafdb02

Add name for MATCH_BANG token ("keyword 'match!')

958a208

Make match! valid in (and only in) computational expressions

53a67cb

match! works

96f297b

Merge remote-tracking branch 'upstream/master' into match-bang

a3bcd93

Add match! to xlf localization files

8d9ee1b

Add two tests for match!

a3e40e0

forki reviewed Mar 5, 2018

View reviewed changes

Don't use left-pipe

582efb0

Fix merge mistake in tests

a94123e

jwosty mentioned this pull request Apr 3, 2018

Make UpdateXlf task work on Mono #4667

Closed

jwosty force-pushed the match-bang branch from aff0ec1 to a94123e Compare April 10, 2018 04:12

Merge branch 'master' into match-bang

0ef4996

Thorium mentioned this pull request Apr 17, 2018

finally! fsharp/fslang-suggestions#651

Open

cartermp added this to the 15.8 milestone May 20, 2018

jwosty force-pushed the match-bang branch from 77c9f64 to 0ef4996 Compare May 21, 2018 17:33

Merge branch 'master' into match-bang

fc7916f

cartermp approved these changes May 21, 2018

View reviewed changes

KevinRansom merged commit e33831a into dotnet:master May 25, 2018

jwosty mentioned this pull request Jul 16, 2018

Keyword descriptions of let-bang and friends should be consistent and not specifically refer to asyncs #5343

Closed

jwosty commented Jul 30, 2018

View reviewed changes

jwosty deleted the match-bang branch May 22, 2020 23:07

Uh oh!

Conversation

jwosty commented Mar 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msftclas commented Mar 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwosty commented Mar 4, 2018

Uh oh!

cartermp commented Mar 4, 2018

Uh oh!

forki Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwosty commented Mar 5, 2018

Uh oh!

forki commented Mar 5, 2018

Uh oh!

jwosty commented Mar 5, 2018

Uh oh!

forki commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

realvictorprm commented Mar 5, 2018

Uh oh!

jwosty commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

realvictorprm commented Mar 5, 2018

Uh oh!

forki commented Mar 5, 2018

Uh oh!

jwosty commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwosty commented Mar 5, 2018

Uh oh!

forki commented Mar 5, 2018

Uh oh!

jwosty commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

forki commented Mar 5, 2018

Uh oh!

jwosty commented Mar 5, 2018

Uh oh!

auduchinok commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

forki commented Mar 5, 2018

Uh oh!

nosami commented Mar 5, 2018

Uh oh!

forki commented Mar 5, 2018

Uh oh!

jwosty commented Apr 2, 2018

Uh oh!

Thorium commented Apr 17, 2018

Uh oh!

abelbraaksma commented Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwosty commented Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thorium commented Apr 17, 2018

Uh oh!

forki commented Apr 17, 2018 via email

Uh oh!

PatrickMcDonald commented Apr 17, 2018

Uh oh!

dsyme commented May 21, 2018

Uh oh!

cartermp commented May 21, 2018

Uh oh!

jwosty commented May 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwosty commented Mar 4, 2018 •

edited

Loading

msftclas commented Mar 4, 2018 •

edited

Loading

forki Mar 5, 2018 •

edited

Loading

forki commented Mar 5, 2018 •

edited

Loading

jwosty commented Mar 5, 2018 •

edited

Loading

jwosty commented Mar 5, 2018 •

edited

Loading

jwosty commented Mar 5, 2018 •

edited

Loading

auduchinok commented Mar 5, 2018 •

edited

Loading

abelbraaksma commented Apr 17, 2018 •

edited

Loading

jwosty commented Apr 17, 2018 •

edited

Loading

jwosty commented May 21, 2018 •

edited

Loading

jwosty Jul 30, 2018 •

edited

Loading