Skip to content
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

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

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from 11 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
1 change: 1 addition & 0 deletions docs/release-notes/.Language/preview.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
### Fixed

* Warn on uppercase identifiers in patterns. ([PR #15816](https://github.com/dotnet/fsharp/pull/15816))
* Allow `_` in `use! _` pattern (lift FS1228 restriction) ([PR #18189](https://github.com/dotnet/fsharp/pull/18189))

### Changed
227 changes: 142 additions & 85 deletions src/Compiler/Checking/Expressions/CheckComputationExpressions.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1843,7 +1843,7 @@ let rec TryTranslateComputationExpression
ceenv.builderTy
)
then
error (Error(FSComp.SR.tcRequireBuilderMethod ("Using"), mBind))
error (Error(FSComp.SR.tcRequireBuilderMethod "Using", mBind))

Some(
translatedCtxt (mkSynCall "Using" mBind [ rhsExpr; consumeExpr ] ceenv.builderValName)
Expand Down 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
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
pat = 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 _ when cenv.g.langVersion.SupportsFeature(LanguageFeature.UseBangBindingValueDiscard) ->
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))

edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
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
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved

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
1 change: 1 addition & 0 deletions src/Compiler/FSComp.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1791,3 +1791,4 @@ featureDontWarnOnUppercaseIdentifiersInBindingPatterns,"Don't warn on uppercase
3873,chkDeprecatePlacesWhereSeqCanBeOmitted,"This construct is deprecated. Sequence expressions should be of the form 'seq {{ ... }}'"
featureDeprecatePlacesWhereSeqCanBeOmitted,"Deprecate places where 'seq' can be omitted"
featureSupportValueOptionsAsOptionalParameters,"Support ValueOption as valid type for optional member parameters"
featureUseBangBindingValueDiscard,"Use 'use!' binding for value discard"
3 changes: 3 additions & 0 deletions src/Compiler/Facilities/LanguageFeatures.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ type LanguageFeature =
| UseTypeSubsumptionCache
| DeprecatePlacesWhereSeqCanBeOmitted
| SupportValueOptionsAsOptionalParameters
| UseBangBindingValueDiscard

/// LanguageVersion management
type LanguageVersion(versionText) =
Expand Down Expand Up @@ -227,6 +228,7 @@ type LanguageVersion(versionText) =
LanguageFeature.DontWarnOnUppercaseIdentifiersInBindingPatterns, previewVersion
LanguageFeature.DeprecatePlacesWhereSeqCanBeOmitted, previewVersion
LanguageFeature.SupportValueOptionsAsOptionalParameters, previewVersion
LanguageFeature.UseBangBindingValueDiscard, previewVersion
]

static let defaultLanguageVersion = LanguageVersion("default")
Expand Down Expand Up @@ -388,6 +390,7 @@ type LanguageVersion(versionText) =
| LanguageFeature.UseTypeSubsumptionCache -> FSComp.SR.featureUseTypeSubsumptionCache ()
| LanguageFeature.DeprecatePlacesWhereSeqCanBeOmitted -> FSComp.SR.featureDeprecatePlacesWhereSeqCanBeOmitted ()
| LanguageFeature.SupportValueOptionsAsOptionalParameters -> FSComp.SR.featureSupportValueOptionsAsOptionalParameters ()
| LanguageFeature.UseBangBindingValueDiscard -> FSComp.SR.featureUseBangBindingValueDiscard ()

/// Get a version string associated with the given feature.
static member GetFeatureVersionString feature =
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/Facilities/LanguageFeatures.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type LanguageFeature =
| UseTypeSubsumptionCache
| DeprecatePlacesWhereSeqCanBeOmitted
| SupportValueOptionsAsOptionalParameters
| UseBangBindingValueDiscard

/// LanguageVersion management
type LanguageVersion =
Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.de.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.es.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.fr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.it.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ja.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ko.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading