Skip to content

Commit 8598f7a

Browse files
smoothdevelopersmoothdeveloper
committed
address code review nits / minimize diff / add comment on PrettifyDiscriminantAndTypePairs
1 parent 662e9fd commit 8598f7a

8 files changed

+58
-49
lines changed

‎TESTGUIDE.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,17 @@
44

55
To run tests, use variations such as the following, depending on which test suite and build configuration you want:
66

7-
./build -testAll -c Release
8-
./build -test -c Release
9-
./build -testCambridge -c Release
10-
./build -testCompiler -c Release
11-
./build -testDependencyManager -c Release
12-
./build -testDesktop -c Release
13-
./build -testCoreClr -c Release
14-
./build -testFSharpCore -c Release
15-
./build -testFSharpQA -c Release
16-
./build -testScripting -c Release
17-
./build -testVs -c Release
7+
.\build -testAll -c Release
8+
.\build -test -c Release
9+
.\build -testCambridge -c Release
10+
.\build -testCompiler -c Release
11+
.\build -testDependencyManager -c Release
12+
.\build -testDesktop -c Release
13+
.\build -testCoreClr -c Release
14+
.\build -testFSharpCore -c Release
15+
.\build -testFSharpQA -c Release
16+
.\build -testScripting -c Release
17+
.\build -testVs -c Release
1818

1919
You can also submit pull requests to https://github.com/dotnet/fsharp and run the tests via continuous integration. Most people do wholesale testing that way.
2020

‎src/fsharp/ConstraintSolver.fs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ type ContextInfo =
183183
| SequenceExpression of TType
184184

185185
/// Captures relevant information for a particular failed overload resolution.
186-
[<Struct>]
187186
type OverloadInformation =
188187
{
189188
methodSlot: CalledMeth<Expr>
@@ -206,7 +205,7 @@ exception ConstraintSolverInfiniteTypes of displayEnv: DisplayEnv * contextInfo:
206205

207206
exception ConstraintSolverTypesNotInEqualityRelation of displayEnv: DisplayEnv * TType * TType * range * range * ContextInfo
208207

209-
exception ConstraintSolverTypesNotInSubsumptionRelation of displayEnv: DisplayEnv * argT: TType * paramT: TType * callRange: range * parameterRange: range
208+
exception ConstraintSolverTypesNotInSubsumptionRelation of displayEnv: DisplayEnv * argTy: TType * paramTy: TType * callRange: range * parameterRange: range
210209

211210
exception ConstraintSolverMissingConstraint of displayEnv: DisplayEnv * Tast.Typar * Tast.TyparConstraint * range * range
212211

@@ -216,9 +215,9 @@ exception ConstraintSolverRelatedInformation of string option * range * exn
216215

217216
exception ErrorFromApplyingDefault of tcGlobals: TcGlobals * displayEnv: DisplayEnv * Tast.Typar * TType * exn * range
218217

219-
exception ErrorFromAddingTypeEquation of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actual: TType * expected: TType * exn * range
218+
exception ErrorFromAddingTypeEquation of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actualTy: TType * expectedTy: TType * exn * range
220219

221-
exception ErrorsFromAddingSubsumptionConstraint of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actual: TType * expected: TType * exn * ContextInfo * parameterRange: range
220+
exception ErrorsFromAddingSubsumptionConstraint of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actualTy: TType * expectedTy: TType * exn * ContextInfo * parameterRange: range
222221

223222
exception ErrorFromAddingConstraint of displayEnv: DisplayEnv * exn * range
224223

@@ -1507,8 +1506,8 @@ and SolveMemberConstraint (csenv: ConstraintSolverEnv) ignoreUnresolvedOverload
15071506
|> List.choose (fun minfo ->
15081507
if minfo.IsCurried then None else
15091508
let callerArgs =
1510-
{ Unnamed = List.singleton (argtys |> List.map (fun argty -> { Type = argty; Range = m; IsOptional = false; Expr = dummyExpr }))
1511-
Named = List.singleton List.empty }
1509+
{ Unnamed = [ (argtys |> List.map (fun argty -> { Type = argty; Range = m; IsOptional = false; Expr = dummyExpr })) ]
1510+
Named = [ [ ] ] }
15121511
let minst = FreshenMethInfo m minfo
15131512
let objtys = minfo.GetObjArgTypes(amap, m, minst)
15141513
Some(CalledMeth<Expr>(csenv.InfoReader, None, false, FreshenMethInfo, m, AccessibleFromEverywhere, minfo, minst, minst, None, objtys, callerArgs, false, false, None)))
@@ -2147,6 +2146,7 @@ and SolveTypeRequiresDefaultConstructor (csenv: ConstraintSolverEnv) ndeep m2 tr
21472146
CompleteD
21482147
| _ ->
21492148
ErrorD (ConstraintSolverError(FSComp.SR.csGenericConstructRequiresPublicDefaultConstructor(NicePrint.minimalStringOfType denv origTy), m, m2))
2149+
21502150
// Parameterized compatibility relation between member signatures. The real work
21512151
// is done by "equateTypes" and "subsumeTypes" and "subsumeArg"
21522152
and CanMemberSigsMatchUpToCheck
@@ -2467,7 +2467,7 @@ and ResolveOverloading
24672467
// Exact match rule.
24682468
//
24692469
// See what candidates we have based on current inferred type information
2470-
// and _exact_ matches of argument types.
2470+
// and _exact_ matches of argument types.
24712471
match candidates |> FilterEachThenUndo (fun newTrace calledMeth ->
24722472
let cxsln = Option.map (fun traitInfo -> (traitInfo, MemberConstraintSolutionOfMethInfo csenv.SolverState m calledMeth.Method calledMeth.CalledTyArgs)) cx
24732473
CanMemberSigsMatchUpToCheck

‎src/fsharp/ConstraintSolver.fsi

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ type ContextInfo =
9191
| SequenceExpression of TType
9292

9393
/// Captures relevant information for a particular failed overload resolution.
94-
[<Struct>]
9594
type OverloadInformation =
9695
{
9796
methodSlot: CalledMeth<Expr>
@@ -111,18 +110,19 @@ type OverloadResolutionFailure =
111110
exception ConstraintSolverTupleDiffLengths of displayEnv: DisplayEnv * TType list * TType list * range * range
112111
exception ConstraintSolverInfiniteTypes of displayEnv: DisplayEnv * contextInfo: ContextInfo * TType * TType * range * range
113112
exception ConstraintSolverTypesNotInEqualityRelation of displayEnv: DisplayEnv * TType * TType * range * range * ContextInfo
114-
exception ConstraintSolverTypesNotInSubsumptionRelation of displayEnv: DisplayEnv * argT: TType * paramT: TType * callRange: range * parameterRange: range
113+
exception ConstraintSolverTypesNotInSubsumptionRelation of displayEnv: DisplayEnv * argTy: TType * paramTy: TType * callRange: range * parameterRange: range
115114
exception ConstraintSolverMissingConstraint of displayEnv: DisplayEnv * Typar * TyparConstraint * range * range
116115
exception ConstraintSolverError of string * range * range
117116
exception ConstraintSolverRelatedInformation of string option * range * exn
118117

119118
exception ErrorFromApplyingDefault of tcGlobals: TcGlobals * displayEnv: DisplayEnv * Typar * TType * exn * range
120-
exception ErrorFromAddingTypeEquation of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actual: TType * expected: TType * exn * range
121-
exception ErrorsFromAddingSubsumptionConstraint of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actual: TType * expected: TType * exn * ContextInfo * parameterRange: range
119+
exception ErrorFromAddingTypeEquation of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actualTy: TType * expectedTy: TType * exn * range
120+
exception ErrorsFromAddingSubsumptionConstraint of tcGlobals: TcGlobals * displayEnv: DisplayEnv * actualTy: TType * expectedTy: TType * exn * ContextInfo * parameterRange: range
122121
exception ErrorFromAddingConstraint of displayEnv: DisplayEnv * exn * range
123122
exception UnresolvedConversionOperator of displayEnv: DisplayEnv * TType * TType * range
124123
exception UnresolvedOverloading of displayEnv: DisplayEnv * callerArgs: CallerArgs<Expr> * failure: OverloadResolutionFailure * range
125124
exception NonRigidTypar of displayEnv: DisplayEnv * string option * range * TType * TType * range
125+
126126
exception ArgDoesNotMatchError of error: ErrorsFromAddingSubsumptionConstraint * calledMeth: CalledMeth<Expr> * calledArg: CalledArg * callerArg: CallerArg<Expr>
127127
/// A function that denotes captured tcVal, Used in constraint solver and elsewhere to get appropriate expressions for a ValRef.
128128
type TcValF = (ValRef -> ValUseFlag -> TType list -> range -> Expr * TType)

‎src/fsharp/MethodCalls.fs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,15 @@ type CallerNamedArg<'T> =
101101
member x.CallerArg = (let (CallerNamedArg(_, a)) = x in a)
102102

103103
/// Represents the list of unnamed / named arguments at method call site
104-
// todo: figure out / document why we are using list²
104+
/// remark: The usage of list list is due to tupling and currying of arguments,
105+
/// stemming from SynValInfo in the AST.
105106
[<Struct>]
106107
type CallerArgs<'T> =
107108
{
108109
Unnamed: CallerArg<'T> list list
109110
Named: CallerNamedArg<'T> list list
110111
}
111-
static member Empty : CallerArgs<'T> = { Unnamed = List.empty; Named = List.empty }
112+
static member Empty : CallerArgs<'T> = { Unnamed = []; Named = [] }
112113
member x.CallerArgCounts = (List.length x.Unnamed, List.length x.Named)
113114
member x.CurriedCallerArgs = List.zip x.Unnamed x.Named
114115
member x.ArgumentNamesAndTypes =

‎src/fsharp/NicePrint.fs

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,32 +1147,36 @@ module private PrintTypes =
11471147
let retTy = instType typarInst retTy
11481148
let argInfos = prettyArgInfos denv typarInst argInfos
11491149
let argInfos,retTy,genParamTys, cxs =
1150-
let typesWithDiscrimants =
1151-
[
1152-
yield 0uy,retTy
1153-
for ty,_ in argInfos do
1154-
yield 1uy, ty
1155-
for ty in genParamTys do
1156-
yield 2uy, ty
1157-
]
1158-
let typesWithDiscrimants,typarsAndCxs = PrettyTypes.PrettifyDiscriminantAndTypePairs denv.g typesWithDiscrimants
1159-
let retTy = typesWithDiscrimants |> List.find (function (0uy, _) -> true | _ -> false) |> snd
1160-
let argInfos =
1161-
typesWithDiscrimants
1162-
|> List.choose (function (1uy,ty) -> Some ty | _ -> None)
1163-
|> List.zip argInfos
1164-
|> List.map (fun ((_,argInfo),tTy) -> tTy, argInfo)
1165-
let genParamTys =
1166-
typesWithDiscrimants
1167-
|> List.choose (function (2uy,ty) -> Some ty | _ -> None)
1168-
1169-
argInfos, retTy, genParamTys, typarsAndCxs
1150+
// using 0, 1, 2 as discriminant for return, arguments and generic parameters
1151+
// respectively, in order to easily retrieve each of the types with their
1152+
// expected quality below.
1153+
let typesWithDiscrimants =
1154+
[
1155+
yield 0,retTy
1156+
for ty,_ in argInfos do
1157+
yield 1, ty
1158+
for ty in genParamTys do
1159+
yield 2, ty
1160+
]
1161+
let typesWithDiscrimants,typarsAndCxs = PrettyTypes.PrettifyDiscriminantAndTypePairs denv.g typesWithDiscrimants
1162+
let retTy = typesWithDiscrimants |> List.find (function (0, _) -> true | _ -> false) |> snd
1163+
let argInfos =
1164+
typesWithDiscrimants
1165+
|> List.choose (function (1,ty) -> Some ty | _ -> None)
1166+
|> List.zip argInfos
1167+
|> List.map (fun ((_,argInfo),tTy) -> tTy, argInfo)
1168+
let genParamTys =
1169+
typesWithDiscrimants
1170+
|> List.choose (function (2,ty) -> Some ty | _ -> None)
1171+
1172+
argInfos, retTy, genParamTys, typarsAndCxs
11701173

11711174
let env = SimplifyTypes.CollectInfo true (List.collect (List.map fst) [argInfos]) cxs
11721175
let cxsL = layoutConstraintsWithInfo denv env env.postfixConstraints
1173-
List.foldBack (---) (layoutArgInfos denv env [argInfos]) cxsL
1174-
, layoutReturnType denv env retTy
1175-
, layoutGenericParameterTypes denv env genParamTys
1176+
1177+
(List.foldBack (---) (layoutArgInfos denv env [argInfos]) cxsL,
1178+
layoutReturnType denv env retTy,
1179+
layoutGenericParameterTypes denv env genParamTys)
11761180

11771181
let prettyLayoutOfType denv ty =
11781182
let ty, cxs = PrettyTypes.PrettifyType denv.g ty

‎src/fsharp/TastOps.fs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2507,6 +2507,7 @@ module PrettyTypes =
25072507
let PrettifyType g x = PrettifyThings g id id x
25082508
let PrettifyTypePair g x = PrettifyThings g (fun f -> foldPair (f, f)) (fun f -> mapPair (f, f)) x
25092509
let PrettifyTypes g x = PrettifyThings g List.fold List.map x
2510+
25102511
let PrettifyDiscriminantAndTypePairs g x =
25112512
let tys, cxs = (PrettifyThings g List.fold List.map (x |> List.map snd))
25122513
List.zip (List.map fst x) tys, cxs

‎src/fsharp/TastOps.fsi

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,9 @@ module PrettyTypes =
932932

933933
val PrettifyTypes : TcGlobals -> TTypes -> TTypes * TyparConstraintsWithTypars
934934
935+
/// same as PrettifyTypes, but allows passing the types along with a discriminant value
936+
/// useful to prettify many types that need to be sorted out after prettifying operation
937+
/// took place.
935938
val PrettifyDiscriminantAndTypePairs : TcGlobals -> ('Discriminant * TType) list -> ('Discriminant * TType) list * TyparConstraintsWithTypars
936939

937940
val PrettifyInst : TcGlobals -> TyparInst -> TyparInst * TyparConstraintsWithTypars

‎src/fsharp/TypeChecker.fs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10155,8 +10155,8 @@ and TcMethodApplication
1015510155

1015610156
// STEP 3. Resolve overloading
1015710157
/// Select the called method that's the result of overload resolution
10158-
let finalCalledMeth =
10159-
10158+
let finalCalledMeth =
10159+
1016010160
let callerArgs = { Unnamed = unnamedCurriedCallerArgs ; Named = namedCurriedCallerArgs }
1016110161

1016210162
let postArgumentTypeCheckingCalledMethGroup =

0 commit comments

Comments
 (0)