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.

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

[FS-1140] add Boolean-returning and return-type-directed partial active patterns #16473

Merged
merged 34 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
4d1f7c4
make `remarks` and `returns` visible in quick info
ijklam Dec 9, 2023
a7f5d23
move `remarks` to the end of summarys
ijklam Dec 10, 2023
d6d7a39
format code
ijklam Dec 10, 2023
b940c49
fix tests
ijklam Dec 10, 2023
c5612f3
format code
ijklam Dec 10, 2023
ec0ae2d
add bool partial active pattern
ijklam Dec 28, 2023
ea01b34
Merge branch 'dotnet:main' into main
ijklam Dec 28, 2023
ad146ce
fix cannot pass arguments to bool AP
ijklam Dec 29, 2023
8d5842b
`bool` partial AP always be treated as `unit option`
ijklam Dec 30, 2023
3243208
rename code, add and fix tests
ijklam Dec 30, 2023
47a70d8
add tests and release note
ijklam Dec 30, 2023
f22d382
Merge branch 'main' into main
ijklam Dec 30, 2023
337f169
fix test
ijklam Dec 30, 2023
7241ec1
fix test
ijklam Dec 30, 2023
ab2cff0
rename and fix typo
ijklam Jan 1, 2024
1a264c4
Merge branch 'main' into main
ijklam Jan 2, 2024
5ceb20e
modify `ActivePatternReturnKind.IsStruct`
ijklam Jan 2, 2024
36a7244
Merge branch 'main' of https://github.com/Tangent-90/fsharp
ijklam Jan 2, 2024
53a0d6c
fix build error
ijklam Jan 2, 2024
2c15fc3
Merge branch 'main' into main
vzarytovskii Jan 16, 2024
781caaa
Merge branch 'main' into main
ijklam Jan 19, 2024
eff53af
rename the feature
ijklam Jan 19, 2024
3c58a1a
update comment
ijklam Jan 19, 2024
17fe616
move tests
ijklam Jan 19, 2024
1859fe0
format code
ijklam Jan 19, 2024
2d67527
fix test
ijklam Jan 19, 2024
5438fa2
fix tests
ijklam Jan 20, 2024
f443d87
fix tests
ijklam Jan 20, 2024
b86dd32
fix tests
ijklam Jan 20, 2024
89c9231
Merge branch 'main' into main
vzarytovskii Jan 23, 2024
5a882c9
Updated FCS release notes as well
vzarytovskii Jan 23, 2024
a8c296a
Merge branch 'main' into main
ijklam Jan 29, 2024
ecce3c2
Merge branch 'main' into main
ijklam Feb 2, 2024
721056c
update test
ijklam Feb 2, 2024
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/8.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* Parser recovers on complex primary constructor patterns, better tree representation for primary constructor patterns. ([PR #16425](https://github.com/dotnet/fsharp/pull/16425))
* Name resolution: keep type vars in subsequent checks ([PR #16456](https://github.com/dotnet/fsharp/pull/16456))
* Higher-order-function-based API for working with the untyped abstract syntax tree. ([PR #16462](https://github.com/dotnet/fsharp/pull/16462))
* Allow returning bool instead of unit option for partial active patterns. ([Language suggestion #1041](https://github.com/fsharp/fslang-suggestions/issues/1041), [PR #16473](https://github.com/dotnet/fsharp/pull/16473))

### Changed

Expand Down
1 change: 1 addition & 0 deletions docs/release-notes/.Language/preview.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Better generic unmanaged structs handling. ([Language suggestion #692](https://github.com/fsharp/fslang-suggestions/issues/692), [PR #12154](https://github.com/dotnet/fsharp/pull/12154))
* Bidirectional F#/C# interop for 'unmanaged' constraint. ([PR #12154](https://github.com/dotnet/fsharp/pull/12154))
* Make `.Is*` discriminated union properties visible. ([Language suggestion #222](https://github.com/fsharp/fslang-suggestions/issues/222), [PR #16341](https://github.com/dotnet/fsharp/pull/16341))
* Allow returning bool instead of unit option for partial active patterns. ([Language suggestion #1041](https://github.com/fsharp/fslang-suggestions/issues/1041), [PR #16473](https://github.com/dotnet/fsharp/pull/16473))

### Fixed

Expand Down
29 changes: 22 additions & 7 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5093,7 +5093,11 @@ and TcPatLongIdentActivePatternCase warnOnUpper (cenv: cenv) (env: TcEnv) vFlags

if dtys.Length = args.Length + 1 &&
((isOptionTy g retTy && isUnitTy g (destOptionTy g retTy)) ||
(isValueOptionTy g retTy && isUnitTy g (destValueOptionTy g retTy))) then
(isValueOptionTy g retTy && isUnitTy g (destValueOptionTy g retTy))) ||
// `bool` partial AP always be treated as `unit option`
ijklam marked this conversation as resolved.
Show resolved Hide resolved
// For `val (|P|_|) : _ -> bool`, only allow `match x with | P -> ...`
// For `val (|P|_|) : _ -> _ -> bool`, only allow `match x with | P parameter -> ...`
(not apinfo.IsTotal && isBoolTy g retTy) then
args, SynPat.Const(SynConst.Unit, m)
else
List.frontAndBack args
Expand Down Expand Up @@ -10752,14 +10756,25 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt
| Some (apinfo, apOverallTy, _) ->
let activePatResTys = NewInferenceTypes g apinfo.ActiveTags
let _, apReturnTy = stripFunTy g apOverallTy

if isStructRetTy && apinfo.IsTotal then
errorR(Error(FSComp.SR.tcInvalidStructReturn(), mBinding))

if isStructRetTy then
let apRetTy =
if apinfo.IsTotal then
if isStructRetTy then errorR(Error(FSComp.SR.tcInvalidStructReturn(), mBinding))
ActivePatternReturnKind.RefTypeWrapper
else
if isStructRetTy || isValueOptionTy cenv.g apReturnTy then ActivePatternReturnKind.StructTypeWrapper
elif isBoolTy cenv.g apReturnTy then ActivePatternReturnKind.Boolean
else ActivePatternReturnKind.RefTypeWrapper

match apRetTy with
| ActivePatternReturnKind.Boolean ->
checkLanguageFeatureError g.langVersion LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern mBinding
| ActivePatternReturnKind.StructTypeWrapper when not isStructRetTy ->
checkLanguageFeatureError g.langVersion LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern mBinding
| ActivePatternReturnKind.StructTypeWrapper ->
checkLanguageFeatureError g.langVersion LanguageFeature.StructActivePattern mBinding
| ActivePatternReturnKind.RefTypeWrapper -> ()

UnifyTypes cenv env mBinding (apinfo.ResultType g rhsExpr.Range activePatResTys isStructRetTy) apReturnTy
UnifyTypes cenv env mBinding (apinfo.ResultType g rhsExpr.Range activePatResTys apRetTy) apReturnTy

| None ->
if isStructRetTy then
Expand Down
10 changes: 6 additions & 4 deletions src/Compiler/Checking/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,20 @@ let ActivePatternElemsOfValRef g (vref: ValRef) =
match TryGetActivePatternInfo vref with
| Some apinfo ->

let isStructRetTy =
let retKind =
if apinfo.IsTotal then
false
ActivePatternReturnKind.RefTypeWrapper
else
let _, apReturnTy = stripFunTy g vref.TauType
let hasStructAttribute() =
vref.Attribs
|> List.exists (function
| Attrib(targetsOpt = Some(System.AttributeTargets.ReturnValue)) as a -> IsMatchingFSharpAttribute g g.attrib_StructAttribute a
| _ -> false)
isStructTy g apReturnTy || hasStructAttribute()
apinfo.ActiveTags |> List.mapi (fun i _ -> APElemRef(apinfo, vref, i, isStructRetTy))
if isValueOptionTy g apReturnTy || hasStructAttribute() then ActivePatternReturnKind.StructTypeWrapper
elif isBoolTy g apReturnTy then ActivePatternReturnKind.Boolean
else ActivePatternReturnKind.RefTypeWrapper
apinfo.ActiveTags |> List.mapi (fun i _ -> APElemRef(apinfo, vref, i, retKind))
| None -> []

/// Try to make a reference to a value in a module.
Expand Down
37 changes: 24 additions & 13 deletions src/Compiler/Checking/PatternMatchCompilation.fs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type Pattern =
| TPat_as of Pattern * PatternValBinding * range (* note: can be replaced by TPat_var, i.e. equals TPat_conjs([TPat_var; pat]) *)
| TPat_disjs of Pattern list * range
| TPat_conjs of Pattern list * range
| TPat_query of (Expr * TType list * bool * (ValRef * TypeInst) option * int * ActivePatternInfo) * Pattern * range
| TPat_query of (Expr * TType list * ActivePatternReturnKind * (ValRef * TypeInst) option * int * ActivePatternInfo) * Pattern * range
| TPat_unioncase of UnionCaseRef * TypeInst * Pattern list * range
| TPat_exnconstr of TyconRef * Pattern list * range
| TPat_tuple of TupInfo * Pattern list * TType list * range
Expand Down Expand Up @@ -618,8 +618,8 @@ let getDiscrimOfPattern (g: TcGlobals) tpinst t =
Some(DecisionTreeTest.UnionCase (c, instTypes tpinst tyargs'))
| TPat_array (args, ty, _m) ->
Some(DecisionTreeTest.ArrayLength (args.Length, ty))
| TPat_query ((activePatExpr, resTys, isStructRetTy, apatVrefOpt, idx, apinfo), _, _m) ->
Some (DecisionTreeTest.ActivePatternCase (activePatExpr, instTypes tpinst resTys, isStructRetTy, apatVrefOpt, idx, apinfo))
| TPat_query ((activePatExpr, resTys, retKind, apatVrefOpt, idx, apinfo), _, _m) ->
Some (DecisionTreeTest.ActivePatternCase (activePatExpr, instTypes tpinst resTys, retKind, apatVrefOpt, idx, apinfo))

| TPat_error range ->
Some (DecisionTreeTest.Error range)
Expand Down Expand Up @@ -941,8 +941,8 @@ let rec investigationPoints inpPat =

let rec erasePartialPatterns inpPat =
match inpPat with
| TPat_query ((expr, resTys, isStructRetTy, apatVrefOpt, idx, apinfo), p, m) ->
if apinfo.IsTotal then TPat_query ((expr, resTys, isStructRetTy, apatVrefOpt, idx, apinfo), erasePartialPatterns p, m)
| TPat_query ((expr, resTys, retKind, apatVrefOpt, idx, apinfo), p, m) ->
if apinfo.IsTotal then TPat_query ((expr, resTys, retKind, apatVrefOpt, idx, apinfo), erasePartialPatterns p, m)
else TPat_disjs ([], m) (* always fail *)
| TPat_as (p, x, m) -> TPat_as (erasePartialPatterns p, x, m)
| TPat_disjs (subPats, m) -> TPat_disjs(erasePartials subPats, m)
Expand Down Expand Up @@ -1293,15 +1293,20 @@ let CompilePatternBasic

// Active pattern matches: create a variable to hold the results of executing the active pattern.
// If a struct return we continue with an expression for taking the address of that location.
| EdgeDiscrim(_, DecisionTreeTest.ActivePatternCase(activePatExpr, resTys, isStructRetTy, _apatVrefOpt, _, apinfo), m) :: _ ->
| EdgeDiscrim(_, DecisionTreeTest.ActivePatternCase(activePatExpr, resTys, retKind, _apatVrefOpt, _, apinfo), m) :: _ ->

if not (isNil origInputValTypars) then error(InternalError("Unexpected generalized type variables when compiling an active pattern", m))

let resTy = apinfo.ResultType g m resTys isStructRetTy
let resTy = apinfo.ResultType g m resTys retKind
let argExpr = GetSubExprOfInput subexpr
let appExpr = mkApps g ((activePatExpr, tyOfExpr g activePatExpr), [], [argExpr], m)

let vOpt, addrExp, _readonly, _writeonly = mkExprAddrOfExprAux g isStructRetTy false NeverMutates appExpr None mMatch
let mustTakeAddress =
match retKind with
| ActivePatternReturnKind.StructTypeWrapper -> true
| ActivePatternReturnKind.RefTypeWrapper
| ActivePatternReturnKind.Boolean -> false
let vOpt, addrExp, _readonly, _writeonly = mkExprAddrOfExprAux g mustTakeAddress false NeverMutates appExpr None mMatch
match vOpt with
| None ->
let v, vExpr = mkCompGenLocal m ("activePatternResult" + string (newUnique())) resTy
Expand Down Expand Up @@ -1357,13 +1362,17 @@ let CompilePatternBasic
// Convert active pattern edges to tests on results data
let discrim' =
match discrim with
| DecisionTreeTest.ActivePatternCase(_pexp, resTys, isStructRetTy, _apatVrefOpt, idx, apinfo) ->
| DecisionTreeTest.ActivePatternCase(_pexp, resTys, retKind, _apatVrefOpt, idx, apinfo) ->
let aparity = apinfo.ActiveTags.Length
let total = apinfo.IsTotal
if not total && aparity > 1 then
error(Error(FSComp.SR.patcPartialActivePatternsGenerateOneResult(), m))

if not total then DecisionTreeTest.UnionCase(mkAnySomeCase g isStructRetTy, resTys)
if not total then
match retKind with
| ActivePatternReturnKind.Boolean -> DecisionTreeTest.Const(Const.Bool true)
| ActivePatternReturnKind.RefTypeWrapper -> DecisionTreeTest.UnionCase(mkAnySomeCase g false, resTys)
| ActivePatternReturnKind.StructTypeWrapper -> DecisionTreeTest.UnionCase(mkAnySomeCase g true, resTys)
elif aparity <= 1 then DecisionTreeTest.Const(Const.Unit)
else DecisionTreeTest.UnionCase(mkChoiceCaseRef g m aparity idx, resTys)
| _ -> discrim
Expand Down Expand Up @@ -1435,7 +1444,7 @@ let CompilePatternBasic
let newActives = removeActive path actives
match patAtActive with
| TPat_wild _ | TPat_as _ | TPat_tuple _ | TPat_disjs _ | TPat_conjs _ | TPat_recd _ -> failwith "Unexpected projection pattern"
| TPat_query ((_, resTys, isStructRetTy, apatVrefOpt, idx, apinfo), p, m) ->
| TPat_query ((_, resTys, retKind, apatVrefOpt, idx, apinfo), p, m) ->
if apinfo.IsTotal then
// Total active patterns always return choice values
let hasParam = (match apatVrefOpt with None -> true | Some (vref, _) -> doesActivePatternHaveFreeTypars g vref)
Expand Down Expand Up @@ -1463,10 +1472,12 @@ let CompilePatternBasic
if i = iInvestigated then
let subAccess _j tpinst _ =
let expr = Option.get inpExprOpt
if isStructRetTy then
match retKind with
| ActivePatternReturnKind.Boolean -> expr
| ActivePatternReturnKind.StructTypeWrapper ->
// In this case, the inpExprOpt is already an address-of expression
mkUnionCaseFieldGetProvenViaExprAddr (expr, mkValueSomeCase g, instTypes tpinst resTys, 0, mExpr)
else
| ActivePatternReturnKind.RefTypeWrapper ->
mkUnionCaseFieldGetUnprovenViaExprAddr (expr, mkSomeCase g, instTypes tpinst resTys, 0, mExpr)
mkSubFrontiers path subAccess newActives [p] (fun path j -> PathQuery(path, int64 j))
else
Expand Down
5 changes: 4 additions & 1 deletion src/Compiler/Checking/PatternMatchCompilation.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ type Pattern =
| TPat_as of Pattern * PatternValBinding * range
| TPat_disjs of Pattern list * range
| TPat_conjs of Pattern list * range
| TPat_query of (Expr * TType list * bool * (ValRef * TypeInst) option * int * ActivePatternInfo) * Pattern * range
| TPat_query of
(Expr * TType list * ActivePatternReturnKind * (ValRef * TypeInst) option * int * ActivePatternInfo) *
Pattern *
range
| TPat_unioncase of UnionCaseRef * TypeInst * Pattern list * range
| TPat_exnconstr of TyconRef * Pattern list * range
| TPat_tuple of TupInfo * Pattern list * TType list * range
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1593,6 +1593,7 @@ featurePreferExtensionMethodOverPlainProperty,"prefer extension method over plai
featureWarningIndexedPropertiesGetSetSameType,"Indexed properties getter and setter must have the same type"
featureChkTailCallAttrOnNonRec,"Raises warnings if the 'TailCall' attribute is used on non-recursive functions."
featureUnionIsPropertiesVisible,"Union case test properties"
featureBooleanReturningAndReturnTypeDirectedPartialActivePattern,"Boolean-returning and return-type-directed partial active patterns"
3354,tcNotAFunctionButIndexerNamedIndexingNotYetEnabled,"This value supports indexing, e.g. '%s.[index]'. The syntax '%s[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation."
3354,tcNotAFunctionButIndexerIndexingNotYetEnabled,"This expression supports indexing, e.g. 'expr.[index]'. The syntax 'expr[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation."
3355,tcNotAnIndexerNamedIndexingNotYetEnabled,"The value '%s' is not a function and does not support index notation."
Expand Down
4 changes: 4 additions & 0 deletions src/Compiler/Facilities/LanguageFeatures.fs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type LanguageFeature =
| PreferExtensionMethodOverPlainProperty
| WarningIndexedPropertiesGetSetSameType
| WarningWhenTailCallAttrOnNonRec
| BooleanReturningAndReturnTypeDirectedPartialActivePattern

/// LanguageVersion management
type LanguageVersion(versionText) =
Expand Down Expand Up @@ -195,6 +196,7 @@ type LanguageVersion(versionText) =
LanguageFeature.WarningIndexedPropertiesGetSetSameType, previewVersion
LanguageFeature.WarningWhenTailCallAttrOnNonRec, previewVersion
LanguageFeature.UnionIsPropertiesVisible, previewVersion
LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern, previewVersion
]

static let defaultLanguageVersion = LanguageVersion("default")
Expand Down Expand Up @@ -336,6 +338,8 @@ type LanguageVersion(versionText) =
| LanguageFeature.PreferExtensionMethodOverPlainProperty -> FSComp.SR.featurePreferExtensionMethodOverPlainProperty ()
| LanguageFeature.WarningIndexedPropertiesGetSetSameType -> FSComp.SR.featureWarningIndexedPropertiesGetSetSameType ()
| LanguageFeature.WarningWhenTailCallAttrOnNonRec -> FSComp.SR.featureChkTailCallAttrOnNonRec ()
| LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern ->
FSComp.SR.featureBooleanReturningAndReturnTypeDirectedPartialActivePattern ()

/// 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
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type LanguageFeature =
| PreferExtensionMethodOverPlainProperty
| WarningIndexedPropertiesGetSetSameType
| WarningWhenTailCallAttrOnNonRec
| BooleanReturningAndReturnTypeDirectedPartialActivePattern

/// LanguageVersion management
type LanguageVersion =
Expand Down
21 changes: 16 additions & 5 deletions src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4565,6 +4565,17 @@ type DecisionTreeCase =
member x.DebugText = x.ToString()

override x.ToString() = sprintf "DecisionTreeCase(...)"

[<Struct; NoComparison; NoEquality; RequireQualifiedAccess>]
type ActivePatternReturnKind =
| RefTypeWrapper
| StructTypeWrapper
| Boolean
member this.IsStruct with get () =
match this with
| RefTypeWrapper -> false
| StructTypeWrapper
| Boolean -> true

[<NoEquality; NoComparison; RequireQualifiedAccess (*; StructuredFormatDisplay("{DebugText}") *) >]
type DecisionTreeTest =
Expand All @@ -4585,20 +4596,20 @@ type DecisionTreeTest =
/// Test if the input to a decision tree is an instance of the given type
| IsInst of source: TType * target: TType

/// Test.ActivePatternCase(activePatExpr, activePatResTys, isStructRetTy, activePatIdentity, idx, activePatInfo)
/// Test.ActivePatternCase(activePatExpr, activePatResTys, activePatRetKind, activePatIdentity, idx, activePatInfo)
///
/// Run the active pattern and bind a successful result to a
/// variable in the remaining tree.
/// activePatExpr -- The active pattern function being called, perhaps applied to some active pattern parameters.
/// activePatResTys -- The result types (case types) of the active pattern.
/// isStructRetTy -- Is the active pattern a struct return
/// activePatRetKind -- Indicating what is returning from the active pattern
/// activePatIdentity -- The value and the types it is applied to. If there are any active pattern parameters then this is empty.
/// idx -- The case number of the active pattern which the test relates to.
/// activePatternInfo -- The extracted info for the active pattern.
| ActivePatternCase of
activePatExpr: Expr *
activePatResTys: TTypes *
isStructRetTy: bool *
activePatRetKind: ActivePatternReturnKind *
activePatIdentity: (ValRef * TypeInst) option *
idx: int *
activePatternInfo: ActivePatternInfo
Expand Down Expand Up @@ -4667,7 +4678,7 @@ type ActivePatternElemRef =
activePatternInfo: ActivePatternInfo *
activePatternVal: ValRef *
caseIndex: int *
isStructRetTy: bool
activePatRetKind: ActivePatternReturnKind

/// Get the full information about the active pattern being referred to
member x.ActivePatternInfo = (let (APElemRef(info, _, _, _)) = x in info)
Expand All @@ -4676,7 +4687,7 @@ type ActivePatternElemRef =
member x.ActivePatternVal = (let (APElemRef(_, vref, _, _)) = x in vref)

/// Get a reference to the value for the active pattern being referred to
member x.IsStructReturn = (let (APElemRef(_, _, _, isStructRetTy)) = x in isStructRetTy)
member x.ActivePatternRetKind = (let (APElemRef(_, _, _, activePatRetKind)) = x in activePatRetKind)

/// Get the index of the active pattern element within the overall active pattern
member x.CaseIndex = (let (APElemRef(_, _, n, _)) = x in n)
Expand Down
Loading
Loading