Skip to content

Commit 185ddc8

Browse files
dsymeDon Syme
andauthored
fix #12025, fix a case where wrong warning was reported for a method arg, update baselines (#12028)
Co-authored-by: Don Syme <[email protected]>
1 parent 664dd97 commit 185ddc8

File tree

5 files changed

+78
-46
lines changed

5 files changed

+78
-46
lines changed

src/fsharp/CheckExpressions.fs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5553,12 +5553,13 @@ and TcPropagatingExprLeafThenConvert cenv overallTy actualTy (env: TcEnv) (* can
55535553
match overallTy with
55545554
| MustConvertTo _ when cenv.g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions ->
55555555
assert (cenv.g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions)
5556-
//if not canAdhoc then
5557-
// AddCxTypeMustSubsumeType ContextInfo.NoContext env.DisplayEnv cenv.css m NoTrace reqdTy actualTy
5556+
55585557
// Compute the conversion _before_ processing the construct. We know enough to process this conversion eagerly.
55595558
UnifyOverallType cenv env m overallTy actualTy
5559+
55605560
// Process the construct
55615561
let expr, tpenv = f ()
5562+
55625563
// Build the conversion
55635564
let expr2 = TcAdjustExprForTypeDirectedConversions cenv overallTy actualTy env (* canAdhoc *) m expr
55645565
expr2, tpenv
@@ -5575,15 +5576,16 @@ and TcPropagatingExprLeafThenConvert cenv overallTy actualTy (env: TcEnv) (* can
55755576
/// 'processExpr' does the actual processing of the construct.
55765577
///
55775578
/// Used for
5578-
/// - tuple (exception is if overallTy is a tuple type, used to propagate structness from known type)
5579-
/// - anon record (exception is if overallTy is an anon record type, similarly)
5580-
/// - record (exception is (fun ty -> requiresCtor || haveCtor || isRecdTy cenv.g ty), similarly)
5579+
/// - tuple (except if overallTy is a tuple type or a variable type that can become one)
5580+
/// - anon record (except if overallTy is an anon record type or a variable type that can become one)
5581+
/// - record (except if overallTy is requiresCtor || haveCtor or a record type or a variable type that can become one))
55815582
and TcPossiblyPropogatingExprLeafThenConvert isPropagating cenv (overallTy: OverallTy) (env: TcEnv) m processExpr =
55825583
match overallTy with
55835584
| MustConvertTo(_, reqdTy) when cenv.g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions && not (isPropagating reqdTy) ->
55845585
TcNonPropagatingExprLeafThenConvert cenv overallTy env m (fun () ->
55855586
let exprTy = NewInferenceType()
5586-
// Here 'processExpr' will do the unification with exprTy.
5587+
5588+
// Here 'processExpr' will eventually do the unification with exprTy.
55875589
let expr, tpenv = processExpr exprTy
55885590
expr, exprTy, tpenv)
55895591
| _ ->
@@ -5601,8 +5603,10 @@ and TcPossiblyPropogatingExprLeafThenConvert isPropagating cenv (overallTy: Over
56015603
and TcNonPropagatingExprLeafThenConvert cenv (overallTy: OverallTy) (env: TcEnv) m processExpr =
56025604
// Process the construct
56035605
let expr, exprTy, tpenv = processExpr ()
5606+
56045607
// Now compute the conversion, based on the post-processing type
56055608
UnifyOverallType cenv env m overallTy exprTy
5609+
56065610
let expr2 = TcAdjustExprForTypeDirectedConversions cenv overallTy exprTy env (* true *) m expr
56075611
expr2, tpenv
56085612

@@ -5750,7 +5754,7 @@ and TcExprUndelayed cenv (overallTy: OverallTy) env tpenv (synExpr: SynExpr) =
57505754
expr, tpenv
57515755

57525756
| SynExpr.Tuple (isExplicitStruct, args, _, m) ->
5753-
TcPossiblyPropogatingExprLeafThenConvert (isAnyTupleTy cenv.g) cenv overallTy env m (fun overallTy ->
5757+
TcPossiblyPropogatingExprLeafThenConvert (fun ty -> isAnyTupleTy cenv.g ty || isTyparTy cenv.g ty) cenv overallTy env m (fun overallTy ->
57545758
let tupInfo, argTys = UnifyTupleTypeAndInferCharacteristics env.eContextInfo cenv env.DisplayEnv m overallTy isExplicitStruct args
57555759

57565760
let flexes = argTys |> List.map (fun _ -> false)
@@ -5760,7 +5764,7 @@ and TcExprUndelayed cenv (overallTy: OverallTy) env tpenv (synExpr: SynExpr) =
57605764
)
57615765

57625766
| SynExpr.AnonRecd (isStruct, optOrigExpr, unsortedFieldExprs, mWholeExpr) ->
5763-
TcPossiblyPropogatingExprLeafThenConvert (isAnonRecdTy cenv.g) cenv overallTy env mWholeExpr (fun overallTy ->
5767+
TcPossiblyPropogatingExprLeafThenConvert (fun ty -> isAnonRecdTy cenv.g ty || isTyparTy cenv.g ty) cenv overallTy env mWholeExpr (fun overallTy ->
57645768
TcAnonRecdExpr cenv overallTy env tpenv (isStruct, optOrigExpr, unsortedFieldExprs, mWholeExpr)
57655769
)
57665770

@@ -5837,7 +5841,7 @@ and TcExprUndelayed cenv (overallTy: OverallTy) env tpenv (synExpr: SynExpr) =
58375841
CallExprHasTypeSink cenv.tcSink (mWholeExpr, env.NameEnv, overallTy.Commit, env.AccessRights)
58385842
let requiresCtor = (GetCtorShapeCounter env = 1) // Get special expression forms for constructors
58395843
let haveCtor = Option.isSome inherits
5840-
TcPossiblyPropogatingExprLeafThenConvert (fun ty -> requiresCtor || haveCtor || isRecdTy cenv.g ty) cenv overallTy env mWholeExpr (fun overallTy ->
5844+
TcPossiblyPropogatingExprLeafThenConvert (fun ty -> requiresCtor || haveCtor || isRecdTy cenv.g ty || isTyparTy cenv.g ty) cenv overallTy env mWholeExpr (fun overallTy ->
58415845
TcRecdExpr cenv overallTy env tpenv (inherits, optOrigExpr, flds, mWholeExpr)
58425846
)
58435847

src/fsharp/ConstraintSolver.fs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2253,7 +2253,7 @@ and CanMemberSigsMatchUpToCheck
22532253
alwaysCheckReturn // always check the return type?
22542254
(unifyTypes: TType -> TType -> OperationResult<TypeDirectedConversionUsed>) // used to equate the formal method instantiation with the actual method instantiation for a generic method, and the return types
22552255
(subsumeTypes: TType -> TType -> OperationResult<TypeDirectedConversionUsed>) // used to compare the "obj" type
2256-
(subsumeOrConvertTypes: TType -> TType -> OperationResult<TypeDirectedConversionUsed>) // used to convert the "return" for MustConvertTo
2256+
(subsumeOrConvertTypes: bool -> TType -> TType -> OperationResult<TypeDirectedConversionUsed>) // used to convert the "return" for MustConvertTo
22572257
(subsumeOrConvertArg: CalledArg -> CallerArg<_> -> OperationResult<TypeDirectedConversionUsed>) // used to convert the arguments
22582258
(reqdRetTyOpt: OverallTy option)
22592259
(calledMeth: CalledMeth<_>): OperationResult<TypeDirectedConversionUsed> =
@@ -2354,9 +2354,9 @@ and CanMemberSigsMatchUpToCheck
23542354
match reqdRetTyOpt with
23552355
| Some _ when ( (* minfo.IsConstructor || *) not alwaysCheckReturn && isNil unnamedCalledOutArgs) ->
23562356
ResultD TypeDirectedConversionUsed.No
2357-
| Some (MustConvertTo(_, reqdTy)) when g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions ->
2357+
| Some (MustConvertTo(isMethodArg, reqdTy)) when g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions ->
23582358
let methodRetTy = calledMeth.CalledReturnTypeAfterOutArgTupling
2359-
subsumeOrConvertTypes reqdTy methodRetTy
2359+
subsumeOrConvertTypes isMethodArg reqdTy methodRetTy
23602360
| Some reqdRetTy ->
23612361
let methodRetTy = calledMeth.CalledReturnTypeAfterOutArgTupling
23622362
unifyTypes reqdRetTy.Commit methodRetTy
@@ -2464,9 +2464,9 @@ and TypesMustSubsume (csenv: ConstraintSolverEnv) ndeep trace cxsln m calledArgT
24642464
return TypeDirectedConversionUsed.No
24652465
}
24662466

2467-
and ReturnTypesMustSubsumeOrConvert (csenv: ConstraintSolverEnv) ad ndeep trace cxsln isConstraint m reqdTy actualTy =
2467+
and ReturnTypesMustSubsumeOrConvert (csenv: ConstraintSolverEnv) ad ndeep trace cxsln isConstraint m isMethodArg reqdTy actualTy =
24682468
trackErrors {
2469-
let reqdTy, usesTDC, eqn = AdjustRequiredTypeForTypeDirectedConversions csenv.InfoReader ad false isConstraint reqdTy actualTy m
2469+
let reqdTy, usesTDC, eqn = AdjustRequiredTypeForTypeDirectedConversions csenv.InfoReader ad isMethodArg isConstraint reqdTy actualTy m
24702470
match eqn with
24712471
| Some (ty1, ty2, msg) ->
24722472
do! SolveTypeEqualsType csenv ndeep m trace cxsln ty1 ty2
@@ -2981,7 +2981,7 @@ and ResolveOverloading
29812981
else
29822982
match reqdRetTy with
29832983
| MustConvertTo(isMethodArg, reqdRetTy) when g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions ->
2984-
let! _usesTDC = ReturnTypesMustSubsumeOrConvert csenv ad ndeep trace cxsln isMethodArg m reqdRetTy actualRetTy
2984+
let! _usesTDC = ReturnTypesMustSubsumeOrConvert csenv ad ndeep trace cxsln isMethodArg m isMethodArg reqdRetTy actualRetTy
29852985
return ()
29862986
| _ ->
29872987
let! _usesTDC = TypesEquiv csenv ndeep trace cxsln reqdRetTy.Commit actualRetTy

tests/fsharp/core/auto-widen/preview-default-warns/test.bsl

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@ test.fsx(147,18,147,19): typecheck error FS3391: This expression uses the implic
33

44
test.fsx(149,39,149,41): typecheck error FS3391: This expression uses the implicit conversion 'Xml.Linq.XNamespace.op_Implicit(namespaceName: string) : Xml.Linq.XNamespace' to convert type 'string' to type 'Xml.Linq.XNamespace'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
55

6-
test.fsx(172,18,172,21): typecheck error FS3391: This expression uses the implicit conversion 'static member Y.op_Implicit : y:Y -> X' to convert type 'Y' to type 'X'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
7-
8-
test.fsx(172,18,172,21): typecheck error FS3391: This expression uses the implicit conversion 'static member Y.op_Implicit : y:Y -> X' to convert type 'Y' to type 'X'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
9-
106
test.fsx(178,20,178,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:'T -> C<'T>' to convert type 'int' to type 'C<int>'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
117

128
test.fsx(180,15,180,16): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:'T -> C<'T>' to convert type 'int' to type 'C<int>'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
@@ -26,26 +22,10 @@ test.fsx(482,18,482,21): typecheck error FS3387: This expression has type 'B' an
2622
static member B.op_Implicit : x:B -> C
2723
static member C.op_Implicit : x:B -> C
2824

29-
test.fsx(482,18,482,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
30-
3125
test.fsx(482,18,482,21): typecheck error FS3387: This expression has type 'B' and is only made compatible with type 'C' through an ambiguous implicit conversion. Consider using an explicit call to 'op_Implicit'. The applicable implicit conversions are:
3226
static member B.op_Implicit : x:B -> C
3327
static member C.op_Implicit : x:B -> C
3428

35-
test.fsx(482,18,482,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
36-
37-
test.fsx(507,18,507,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
38-
39-
test.fsx(507,18,507,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
40-
41-
test.fsx(519,18,519,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
42-
43-
test.fsx(519,18,519,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
44-
45-
test.fsx(538,18,538,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
46-
47-
test.fsx(538,18,538,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
48-
4929
test.fsx(546,30,546,31): typecheck error FS0001: This expression was expected to have type
5030
'float32'
5131
but here has type

tests/fsharp/core/auto-widen/preview/test.bsl

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,9 @@ test.fsx(165,18,165,19): typecheck error FS3390: This expression uses the implic
165165

166166
test.fsx(165,18,165,19): typecheck error FS3388: This expression implicitly converts type 'int' to type 'C'. See https://aka.ms/fsharp-implicit-convs.
167167

168-
test.fsx(172,18,172,21): typecheck error FS3391: This expression uses the implicit conversion 'static member Y.op_Implicit : y:Y -> X' to convert type 'Y' to type 'X'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
168+
test.fsx(172,18,172,21): typecheck error FS3390: This expression uses the implicit conversion 'static member Y.op_Implicit : y:Y -> X' to convert type 'Y' to type 'X'.
169169

170-
test.fsx(172,18,172,21): typecheck error FS3391: This expression uses the implicit conversion 'static member Y.op_Implicit : y:Y -> X' to convert type 'Y' to type 'X'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
170+
test.fsx(172,18,172,21): typecheck error FS3390: This expression uses the implicit conversion 'static member Y.op_Implicit : y:Y -> X' to convert type 'Y' to type 'X'.
171171

172172
test.fsx(178,20,178,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:'T -> C<'T>' to convert type 'int' to type 'C<int>'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
173173

@@ -602,25 +602,25 @@ test.fsx(482,18,482,21): typecheck error FS3387: This expression has type 'B' an
602602
static member B.op_Implicit : x:B -> C
603603
static member C.op_Implicit : x:B -> C
604604

605-
test.fsx(482,18,482,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
605+
test.fsx(482,18,482,21): typecheck error FS3390: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
606606

607607
test.fsx(482,18,482,21): typecheck error FS3387: This expression has type 'B' and is only made compatible with type 'C' through an ambiguous implicit conversion. Consider using an explicit call to 'op_Implicit'. The applicable implicit conversions are:
608608
static member B.op_Implicit : x:B -> C
609609
static member C.op_Implicit : x:B -> C
610610

611-
test.fsx(482,18,482,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
611+
test.fsx(482,18,482,21): typecheck error FS3390: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
612612

613-
test.fsx(507,18,507,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
613+
test.fsx(507,18,507,21): typecheck error FS3390: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
614614

615-
test.fsx(507,18,507,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
615+
test.fsx(507,18,507,21): typecheck error FS3390: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
616616

617-
test.fsx(519,18,519,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
617+
test.fsx(519,18,519,21): typecheck error FS3390: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
618618

619-
test.fsx(519,18,519,21): typecheck error FS3391: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
619+
test.fsx(519,18,519,21): typecheck error FS3390: This expression uses the implicit conversion 'static member B.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
620620

621-
test.fsx(538,18,538,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
621+
test.fsx(538,18,538,21): typecheck error FS3390: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
622622

623-
test.fsx(538,18,538,21): typecheck error FS3391: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'. See https://aka.ms/fsharp-implicit-convs. This warning may be disabled using '#nowarn "3391".
623+
test.fsx(538,18,538,21): typecheck error FS3390: This expression uses the implicit conversion 'static member C.op_Implicit : x:B -> C' to convert type 'B' to type 'C'.
624624

625625
test.fsx(543,30,543,31): typecheck error FS0001: This expression was expected to have type
626626
'float32'

tests/fsharp/core/auto-widen/preview/test.fsx

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ module ConvertViaOpImplicit5 =
169169
static member M1(x:X) = 1
170170
type Y() =
171171
static member op_Implicit(y:Y) = X()
172-
let x = X.M1(Y())
172+
let x = X.M1(Y()) // Note this counts as a use in a method arg position, when processing the return type of 'Y'
173173

174174
module ConvertViaOpImplicitGeneric =
175175
type C<'T>() =
@@ -543,6 +543,54 @@ module TestNoWidening =
543543
let x4 : float32 list = [1;2;3;4]
544544
let x5 : float64 list = [1.0f;2.0f;3.0f;4.0f]
545545
#endif
546+
547+
548+
module TestRecursiveInferenceForPropagatingCases =
549+
550+
module Bug1 =
551+
type RecordTypeA =
552+
{ Name: string
553+
FieldA: string }
554+
555+
type RecordTypeB =
556+
{ Name: string
557+
FieldB: int}
558+
559+
// When the record expression is encountered, it must commit to "RecordTypeA"
560+
// because the return type of "f" is, at that point, a variable type
561+
// and must be correctly inferred by the point where we process the subsequence
562+
// dot-notation "f().Name"
563+
let rec f() =
564+
{ Name=""
565+
FieldA = f().Name
566+
}
567+
568+
module Bug2 =
569+
570+
type RecordTypeB =
571+
{ Name: string
572+
FieldB: int}
573+
574+
// When the anonymous record expression is encountered, it must commit to "RecordTypeA".
575+
// The return type of "f" is, at that point, a variable type
576+
// and must be correctly inferred by the point where we process the subsequence
577+
// dot-notation "f().Name"
578+
let rec f() =
579+
{| Name=""
580+
FieldA = f().Name
581+
|}
582+
583+
module Test3 =
584+
585+
// Check the return type of 'f' is known by the time we process "f().Length"
586+
let rec f() = [ f().Length ]
587+
588+
module Test4 =
589+
590+
// Check the return type of 'f' is known by the time we process "f().CompareTo(y)"
591+
let rec f() = { new System.IComparable with member x.CompareTo(y) = f().CompareTo(y) }
592+
593+
546594
printfn "test done"
547595

548596
let aa =

0 commit comments

Comments
 (0)