Skip to content

Allow _ in async use! _ pattern (lift FS1228 restriction) #18189

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.

Sign up for GitHub

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

Closed
wants to merge 17 commits into from
Closed

Allow _ in async use! _ pattern (lift FS1228 restriction) #18189

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
fbbf038
Allow `_` in async use! `_` pattern (lift FS1228 restriction)
edgarfgp Dec 30, 2024
9673183
error when using use! binding with wrong form.
edgarfgp Dec 30, 2024
0d9320b
release notes
edgarfgp Dec 30, 2024
c0b7ec4
Merge branch 'main' into fix-16343
edgarfgp Dec 31, 2024
c3e22ee
Merge branch 'main' into fix-16343
edgarfgp Jan 2, 2025
718506f
deduplicate LetOrUseBang isUse = true
edgarfgp Jan 2, 2025
fc2e47b
Add LanguageFeature.UseBangBindingValueDiscard
edgarfgp Jan 2, 2025
9f54de8
Use UseBangBindingValueDiscard
edgarfgp Jan 2, 2025
cfb67a1
Update tests
edgarfgp Jan 2, 2025
26c62f4
release notes
edgarfgp Jan 2, 2025
61958b2
Merge branch 'main' into fix-16343
edgarfgp Jan 2, 2025
3be84b6
deduplicate more logic
edgarfgp Jan 6, 2025
aa73e1f
Merge branch 'main' into fix-16343
edgarfgp Jan 7, 2025
08eb286
Merge branch 'main' into fix-16343
edgarfgp Jan 7, 2025
0b664a9
Merge branch 'main' into fix-16343
edgarfgp Jan 19, 2025
cddbb7a
Merge branch 'main' into fix-16343
edgarfgp Jan 24, 2025
ed914e3
more tests
edgarfgp Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Fix a race condition in file book keeping in the compiler service ([#18008](https://github.com/dotnet/fsharp/pull/18008))
* Fix trimming '%' characters when lowering interpolated string to a concat call [PR #18123](https://github.com/dotnet/fsharp/pull/18123)
* Completion: fix qualified completion in sequence expressions [PR #18111](https://github.com/dotnet/fsharp/pull/18111)
* Allow `_` in `use! _` pattern (lift FS1228 restriction) ([PR #18189](https://github.com/dotnet/fsharp/pull/18189))
* Symbols: try to use ValReprInfoForDisplay in Mfv.CurriedParameterGroups ([PR #18124](https://github.com/dotnet/fsharp/pull/18124))
* Shim/file system: fix leaks of the shim [PR #18144](https://github.com/dotnet/fsharp/pull/18144)

Expand Down
225 changes: 141 additions & 84 deletions src/Compiler/Checking/Expressions/CheckComputationExpressions.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1900,105 +1900,162 @@ let rec TryTranslateComputationExpression
bindDebugPoint = spBind
isUse = true
isFromSource = isFromSource
pat = SynPat.Named(ident = SynIdent(id, _); isThisVal = false) as pat
pat = pat
rhs = rhsExpr
andBangs = []
body = innerComp
trivia = { LetOrUseBangKeyword = mBind })
| SynExpr.LetOrUseBang(
bindDebugPoint = spBind
isUse = true
isFromSource = isFromSource
pat = SynPat.LongIdent(longDotId = SynLongIdent(id = [ id ])) as pat
rhs = rhsExpr
andBangs = []
andBangs = andBangs
body = innerComp
trivia = { LetOrUseBangKeyword = mBind }) ->
match pat with
| SynPat.Named(ident = SynIdent(id, _); isThisVal = false)
| SynPat.LongIdent(longDotId = SynLongIdent(id = [ id ])) when andBangs.IsEmpty ->
if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), mBind))

if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), mBind))
if
isNil (
TryFindIntrinsicOrExtensionMethInfo
ResultCollectionSettings.AtMostOneResult
cenv
ceenv.env
mBind
ceenv.ad
"Using"
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod ("Using"), mBind))

if
isNil (
TryFindIntrinsicOrExtensionMethInfo
ResultCollectionSettings.AtMostOneResult
cenv
ceenv.env
mBind
ceenv.ad
"Using"
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod ("Using"), mBind))
if
isNil (
TryFindIntrinsicOrExtensionMethInfo
ResultCollectionSettings.AtMostOneResult
cenv
ceenv.env
mBind
ceenv.ad
"Bind"
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod ("Bind"), mBind))

if
isNil (
TryFindIntrinsicOrExtensionMethInfo
ResultCollectionSettings.AtMostOneResult
cenv
ceenv.env
mBind
ceenv.ad
"Bind"
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod ("Bind"), mBind))
let bindExpr =
let consumeExpr =
SynExpr.MatchLambda(
false,
mBind,
[
SynMatchClause(
pat,
None,
TranslateComputationExpressionNoQueryOps ceenv innerComp,
innerComp.Range,
DebugPointAtTarget.Yes,
SynMatchClauseTrivia.Zero
)
],
DebugPointAtBinding.NoneAtInvisible,
mBind
)

let bindExpr =
let consumeExpr =
SynExpr.MatchLambda(
false,
mBind,
[
SynMatchClause(
pat,
None,
TranslateComputationExpressionNoQueryOps ceenv innerComp,
innerComp.Range,
DebugPointAtTarget.Yes,
SynMatchClauseTrivia.Zero
)
],
DebugPointAtBinding.NoneAtInvisible,
mBind
)
let consumeExpr =
mkSynCall "Using" mBind [ SynExpr.Ident id; consumeExpr ] ceenv.builderValName

let consumeExpr =
mkSynCall "Using" mBind [ SynExpr.Ident id; consumeExpr ] ceenv.builderValName
let consumeExpr =
SynExpr.MatchLambda(
false,
mBind,
[
SynMatchClause(pat, None, consumeExpr, id.idRange, DebugPointAtTarget.No, SynMatchClauseTrivia.Zero)
],
DebugPointAtBinding.NoneAtInvisible,
mBind
)

let consumeExpr =
SynExpr.MatchLambda(
false,
mBind,
[
SynMatchClause(pat, None, consumeExpr, id.idRange, DebugPointAtTarget.No, SynMatchClauseTrivia.Zero)
],
DebugPointAtBinding.NoneAtInvisible,
mBind
)
let rhsExpr =
mkSourceExprConditional isFromSource rhsExpr ceenv.sourceMethInfo ceenv.builderValName

let rhsExpr =
mkSourceExprConditional isFromSource rhsExpr ceenv.sourceMethInfo ceenv.builderValName
mkSynCall "Bind" mBind [ rhsExpr; consumeExpr ] ceenv.builderValName
|> addBindDebugPoint spBind

mkSynCall "Bind" mBind [ rhsExpr; consumeExpr ] ceenv.builderValName
|> addBindDebugPoint spBind
Some(translatedCtxt bindExpr)

Some(translatedCtxt bindExpr)
| SynPat.Wild _ ->
if not (isNil andBangs) then
let m =
match andBangs with
| [] -> comp.Range
| h :: _ -> h.Trivia.AndBangKeyword

// 'use! pat = e1 ... in e2' where 'pat' is not a simple name -> error
| SynExpr.LetOrUseBang(isUse = true; andBangs = andBangs; trivia = { LetOrUseBangKeyword = mBind }) ->
if isNil andBangs then
error (Error(FSComp.SR.tcInvalidUseBangBinding (), mBind))
else
let m =
match andBangs with
| [] -> comp.Range
| h :: _ -> h.Trivia.AndBangKeyword
error (Error(FSComp.SR.tcInvalidUseBangBindingNoAndBangs (), m))
else
if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), mBind))

if
isNil (
TryFindIntrinsicOrExtensionMethInfo
ResultCollectionSettings.AtMostOneResult
cenv
ceenv.env
mBind
ceenv.ad
"Using"
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod "Using", mBind))

if
isNil (
TryFindIntrinsicOrExtensionMethInfo
ResultCollectionSettings.AtMostOneResult
cenv
ceenv.env
mBind
ceenv.ad
"Bind"
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod "Bind", mBind))

let bindExpr =
let consumeExpr =
SynExpr.MatchLambda(
false,
mBind,
[
SynMatchClause(
pat,
None,
TranslateComputationExpressionNoQueryOps ceenv innerComp,
innerComp.Range,
DebugPointAtTarget.Yes,
SynMatchClauseTrivia.Zero
)
],
DebugPointAtBinding.NoneAtInvisible,
mBind
)

let rhsExpr =
mkSourceExprConditional isFromSource rhsExpr ceenv.sourceMethInfo ceenv.builderValName

error (Error(FSComp.SR.tcInvalidUseBangBindingNoAndBangs (), m))
mkSynCall "Bind" mBind [ rhsExpr; consumeExpr ] ceenv.builderValName
Copy link
Member

Choose a reason for hiding this comment

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

I was expecting a call to "Using" somewhere, but cannot find it.
Is it implicit by recursive calling into a different handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way I understood the logic was that _ (wild) will need to be Bind in the same way that ident(any ident) __ (double underscore). But Im not very familiar with the CE Translation logic.

Tested this to "Using" and it does not work.

Copy link
Member

Choose a reason for hiding this comment

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

If it only does "Bind" and not an "Using", it will lack the CE's implementation of disposal (which can be anything the CE wants to do to cleanup).

|> addBindDebugPoint spBind

Some(translatedCtxt bindExpr)
| _ ->
if isNil andBangs then
error (Error(FSComp.SR.tcInvalidUseBangBinding (), mBind))
else
let m =
match andBangs with
| [] -> comp.Range
| h :: _ -> h.Trivia.AndBangKeyword

error (Error(FSComp.SR.tcInvalidUseBangBindingNoAndBangs (), m))
// 'let! pat1 = expr1 and! pat2 = expr2 in ...' -->
// build.BindN(expr1, expr2, ...)
// or
Expand Down
63 changes: 63 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,37 @@ let run r2 r3 =
(Error 3345, Line 18, Col 9, Line 18, Col 13, "use! may not be combined with and!")
]

Copy link
Member

Choose a reason for hiding this comment

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

Please do add positive tests which include a run operation and demonstrate that a resource which is being use! _ ='d get's cleaned up with a custom .Using method of the builder.

[<Fact>]
let ``multiple use! _ may not be combined with and!`` () =
Fsx """
module Result =
let zip x1 x2 =
match x1,x2 with
| Ok x1res, Ok x2res -> Ok (x1res, x2res)
| Error e, _ -> Error e
| _, Error e -> Error e

type ResultBuilder() =
member _.MergeSources(t1: Result<'T,'U>, t2: Result<'T1,'U>) = Result.zip t1 t2
member _.BindReturn(x: Result<'T,'U>, f) = Result.map f x

let result = ResultBuilder()

let run r2 r3 =
result {
use! _ = r2
and! c = r3
use! _ = r2
return b - c
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3345, Line 18, Col 9, Line 18, Col 13, "use! may not be combined with and!")
]

[<Fact>]
let ``multiple use! may not be combined with multiple and!`` () =
Fsx """
Expand Down Expand Up @@ -725,4 +756,36 @@ let x18mutable =
(Error 3147, Line 5, Col 17, Line 5, Col 20, "This 'let' definition may not be used in a query. Only simple value definitions may be used in queries.")
(Error 3147, Line 13, Col 20, Line 13, Col 23, "This 'let' definition may not be used in a query. Only simple value definitions may be used in queries.")
(Error 3147, Line 20, Col 21, Line 20, Col 22, "This 'let' definition may not be used in a query. Only simple value definitions may be used in queries.")
]

[<Fact>]
let ``Allow _ in async use! _ pattern (lift FS1228 restriction)`` () =
Fsx """
open System
let doSomething () =
async {
use _ = { new IDisposable with member _.Dispose() = printfn "disposed" }
use! _ = Async.OnCancel (fun () -> printfn "disposed")
use! res = Async.OnCancel (fun () -> printfn "disposed")
return ()
}
"""
|> typecheck
|> shouldSucceed

[<Fact>]
let ``error when using use! binding with wrong form.`` () =
Fsx """
open System
let doSomething () =
async {
use! [| r1; r2 |] = Async.Parallel [| async.Return(1); async.Return(2) |]
let y = 4
return r1,r2
}
"""
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 1228, Line 5, Col 9, Line 5, Col 13, "'use!' bindings must be of the form 'use! <var> = <expr>'")
]
Loading