Skip to content

Commit 4841e2d

Browse files
dsymeKevinRansom
authored andcommitted
fix anon recd creation bug (#6434) (#6619)
* fix anon recd creation bug * fix generation, make it more efficient * fix feasible equality for anonymous records
1 parent 3c77bae commit 4841e2d

File tree

4 files changed

+116
-79
lines changed

4 files changed

+116
-79
lines changed

src/fsharp/IlxGen.fs

Lines changed: 72 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,57 +1356,65 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu
13561356

13571357
let generateAnonType genToStringMethod (isStruct, ilTypeRef, nms) =
13581358

1359-
let flds = [ for (i, nm) in Array.indexed nms -> (nm, nm + "@", ILType.TypeVar (uint16 i)) ]
1359+
let propTys = [ for (i, nm) in Array.indexed nms -> nm, ILType.TypeVar (uint16 i) ]
1360+
13601361
// Note that this alternative below would give the same names as C#, but the generated
13611362
// comparison/equality doesn't know about these names.
13621363
//let flds = [ for (i, nm) in Array.indexed nms -> (nm, "<" + nm + ">" + "i__Field", ILType.TypeVar (uint16 i)) ]
1364+
let ilCtorRef = mkILMethRef(ilTypeRef, ILCallingConv.Instance, ".ctor", 0, List.map snd propTys, ILType.Void)
13631365

1364-
let ilGenericParams =
1365-
[ for nm in nms ->
1366-
{ Name = sprintf "<%s>j__TPar" nm
1367-
Constraints = []
1368-
Variance=NonVariant
1369-
CustomAttrsStored = storeILCustomAttrs emptyILCustomAttrs
1370-
HasReferenceTypeConstraint=false
1371-
HasNotNullableValueTypeConstraint=false
1372-
HasDefaultConstructorConstraint= false
1373-
MetadataIndex = NoMetadataIdx } ]
1366+
let ilMethodRefs =
1367+
[| for (propName, propTy) in propTys ->
1368+
mkILMethRef (ilTypeRef, ILCallingConv.Instance, "get_" + propName, 0, [], propTy) |]
13741369

1375-
let ilTy = mkILFormalNamedTy (if isStruct then ILBoxity.AsValue else ILBoxity.AsObject) ilTypeRef ilGenericParams
1370+
let ilTy = mkILNamedTy (if isStruct then ILBoxity.AsValue else ILBoxity.AsObject) ilTypeRef (List.map snd propTys)
13761371

1377-
// Generate the IL fields
1378-
let ilFieldDefs =
1379-
mkILFields
1380-
[ for (_, fldName, fldTy) in flds ->
1381-
let fdef = mkILInstanceField (fldName, fldTy, None, ILMemberAccess.Private)
1382-
fdef.With(customAttrs = mkILCustomAttrs [ g.DebuggerBrowsableNeverAttribute ]) ]
1372+
if ilTypeRef.Scope.IsLocalRef then
1373+
1374+
let flds = [ for (i, nm) in Array.indexed nms -> (nm, nm + "@", ILType.TypeVar (uint16 i)) ]
1375+
1376+
let ilGenericParams =
1377+
[ for nm in nms ->
1378+
{ Name = sprintf "<%s>j__TPar" nm
1379+
Constraints = []
1380+
Variance=NonVariant
1381+
CustomAttrsStored = storeILCustomAttrs emptyILCustomAttrs
1382+
HasReferenceTypeConstraint=false
1383+
HasNotNullableValueTypeConstraint=false
1384+
HasDefaultConstructorConstraint= false
1385+
MetadataIndex = NoMetadataIdx } ]
1386+
1387+
let ilTy = mkILFormalNamedTy (if isStruct then ILBoxity.AsValue else ILBoxity.AsObject) ilTypeRef ilGenericParams
1388+
1389+
// Generate the IL fields
1390+
let ilFieldDefs =
1391+
mkILFields
1392+
[ for (_, fldName, fldTy) in flds ->
1393+
let fdef = mkILInstanceField (fldName, fldTy, None, ILMemberAccess.Private)
1394+
fdef.With(customAttrs = mkILCustomAttrs [ g.DebuggerBrowsableNeverAttribute ]) ]
13831395

1384-
// Generate property definitions for the fields compiled as properties
1385-
let ilProperties =
1386-
mkILProperties
1387-
[ for (i, (propName, _fldName, fldTy)) in List.indexed flds ->
1388-
ILPropertyDef(name=propName,
1389-
attributes=PropertyAttributes.None,
1390-
setMethod=None,
1391-
getMethod=Some(mkILMethRef(ilTypeRef, ILCallingConv.Instance, "get_" + propName, 0, [], fldTy )),
1392-
callingConv=ILCallingConv.Instance.ThisConv,
1393-
propertyType=fldTy,
1394-
init= None,
1395-
args=[],
1396-
customAttrs=mkILCustomAttrs [ mkCompilationMappingAttrWithSeqNum g (int SourceConstructFlags.Field) i ]) ]
1396+
// Generate property definitions for the fields compiled as properties
1397+
let ilProperties =
1398+
mkILProperties
1399+
[ for (i, (propName, _fldName, fldTy)) in List.indexed flds ->
1400+
ILPropertyDef(name=propName,
1401+
attributes=PropertyAttributes.None,
1402+
setMethod=None,
1403+
getMethod=Some(mkILMethRef(ilTypeRef, ILCallingConv.Instance, "get_" + propName, 0, [], fldTy )),
1404+
callingConv=ILCallingConv.Instance.ThisConv,
1405+
propertyType=fldTy,
1406+
init= None,
1407+
args=[],
1408+
customAttrs=mkILCustomAttrs [ mkCompilationMappingAttrWithSeqNum g (int SourceConstructFlags.Field) i ]) ]
13971409

1398-
let ilMethods =
1399-
[ for (propName, fldName, fldTy) in flds ->
1400-
mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy)
1401-
yield! genToStringMethod ilTy ]
1410+
let ilMethods =
1411+
[ for (propName, fldName, fldTy) in flds ->
1412+
mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy)
1413+
yield! genToStringMethod ilTy ]
14021414

1403-
let ilBaseTy = (if isStruct then g.iltyp_ValueType else g.ilg.typ_Object)
1415+
let ilBaseTy = (if isStruct then g.iltyp_ValueType else g.ilg.typ_Object)
14041416

1405-
let ilCtorDef = mkILSimpleStorageCtorWithParamNames(None, (if isStruct then None else Some ilBaseTy.TypeSpec), ilTy, [], flds, ILMemberAccess.Public)
1406-
let ilCtorRef = mkRefToILMethod(ilTypeRef, ilCtorDef)
1407-
let ilMethodRefs = [| for mdef in ilMethods -> mkRefToILMethod(ilTypeRef, mdef) |]
1408-
1409-
if ilTypeRef.Scope.IsLocalRef then
1417+
let ilCtorDef = mkILSimpleStorageCtorWithParamNames(None, (if isStruct then None else Some ilBaseTy.TypeSpec), ilTy, [], flds, ILMemberAccess.Public)
14101418

14111419
// Create a tycon that looks exactly like a record definition, to help drive the generation of equality/comparison code
14121420
let m = range0
@@ -1482,12 +1490,12 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu
14821490
/// static init fields on script modules.
14831491
let mutable scriptInitFspecs: (ILFieldSpec * range) list = []
14841492

1485-
member mgbuf.AddScriptInitFieldSpec(fieldSpec, range) =
1493+
member __.AddScriptInitFieldSpec (fieldSpec, range) =
14861494
scriptInitFspecs <- (fieldSpec, range) :: scriptInitFspecs
14871495

14881496
/// This initializes the script in #load and fsc command-line order causing their
14891497
/// sideeffects to be executed.
1490-
member mgbuf.AddInitializeScriptsInOrderToEntryPoint() =
1498+
member mgbuf.AddInitializeScriptsInOrderToEntryPoint () =
14911499
// Get the entry point and initialized any scripts in order.
14921500
match explicitEntryPointInfo with
14931501
| Some tref ->
@@ -1496,57 +1504,54 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu
14961504
scriptInitFspecs |> List.iter IntializeCompiledScript
14971505
| None -> ()
14981506

1499-
member mgbuf.GenerateRawDataValueType(cloc, size) =
1507+
member __.GenerateRawDataValueType (cloc, size) =
15001508
// Byte array literals require a ValueType of size the required number of bytes.
15011509
// With fsi.exe, S.R.Emit TypeBuilder CreateType has restrictions when a ValueType VT is nested inside a type T, and T has a field of type VT.
15021510
// To avoid this situation, these ValueTypes are generated under the private implementation rather than in the current cloc. [was bug 1532].
15031511
let cloc = CompLocForPrivateImplementationDetails cloc
15041512
rawDataValueTypeGenerator.Apply((cloc, size))
15051513

1506-
member mgbuf.GenerateAnonType(genToStringMethod, anonInfo: AnonRecdTypeInfo) =
1514+
member __.GenerateAnonType (genToStringMethod, anonInfo: AnonRecdTypeInfo) =
15071515
let isStruct = evalAnonInfoIsStruct anonInfo
15081516
let key = anonInfo.Stamp
1509-
match anonTypeTable.Table.TryGetValue key with
1510-
| true, res -> res
1511-
| _ ->
1517+
if not (anonTypeTable.Table.ContainsKey key) then
15121518
let info = generateAnonType genToStringMethod (isStruct, anonInfo.ILTypeRef, anonInfo.SortedNames)
15131519
anonTypeTable.Table.[key] <- info
1514-
info
15151520

1516-
member mgbuf.LookupAnonType(anonInfo: AnonRecdTypeInfo) =
1521+
member __.LookupAnonType (anonInfo: AnonRecdTypeInfo) =
15171522
match anonTypeTable.Table.TryGetValue anonInfo.Stamp with
15181523
| true, res -> res
15191524
| _ -> failwithf "the anonymous record %A has not been generated in the pre-phase of generating this module" anonInfo.ILTypeRef
15201525

1521-
member mgbuf.GrabExtraBindingsToGenerate() =
1526+
member __.GrabExtraBindingsToGenerate () =
15221527
let result = extraBindingsToGenerate
15231528
extraBindingsToGenerate <- []
15241529
result
15251530

1526-
member mgbuf.AddTypeDef(tref: ILTypeRef, tdef, eliminateIfEmpty, addAtEnd, tdefDiscards) =
1531+
member __.AddTypeDef (tref: ILTypeRef, tdef, eliminateIfEmpty, addAtEnd, tdefDiscards) =
15271532
gtdefs.FindNestedTypeDefsBuilder(tref.Enclosing).AddTypeDef(tdef, eliminateIfEmpty, addAtEnd, tdefDiscards)
15281533

1529-
member mgbuf.GetCurrentFields(tref: ILTypeRef) =
1534+
member __.GetCurrentFields (tref: ILTypeRef) =
15301535
gtdefs.FindNestedTypeDefBuilder(tref).GetCurrentFields()
15311536

1532-
member mgbuf.AddReflectedDefinition(vspec: Tast.Val, expr) =
1537+
member __.AddReflectedDefinition (vspec: Tast.Val, expr) =
15331538
// preserve order by storing index of item
15341539
let n = reflectedDefinitions.Count
15351540
reflectedDefinitions.Add(vspec, (vspec.CompiledName, n, expr))
15361541

1537-
member mgbuf.ReplaceNameOfReflectedDefinition(vspec, newName) =
1542+
member __.ReplaceNameOfReflectedDefinition (vspec, newName) =
15381543
match reflectedDefinitions.TryGetValue vspec with
15391544
| true, (name, n, expr) when name <> newName -> reflectedDefinitions.[vspec] <- (newName, n, expr)
15401545
| _ -> ()
15411546

1542-
member mgbuf.AddMethodDef(tref: ILTypeRef, ilMethodDef) =
1547+
member __.AddMethodDef (tref: ILTypeRef, ilMethodDef) =
15431548
gtdefs.FindNestedTypeDefBuilder(tref).AddMethodDef(ilMethodDef)
15441549
if ilMethodDef.IsEntryPoint then
15451550
explicitEntryPointInfo <- Some tref
15461551

1547-
member mgbuf.AddExplicitInitToSpecificMethodDef(cond, tref, fspec, sourceOpt, feefee, seqpt) =
1548-
// Authoring a .cctor with effects forces the cctor for the 'initialization' module by doing a dummy store & load of a field
1549-
// Doing both a store and load keeps FxCop happier because it thinks the field is useful
1552+
member __.AddExplicitInitToSpecificMethodDef (cond, tref, fspec, sourceOpt, feefee, seqpt) =
1553+
// Authoring a .cctor with effects forces the cctor for the 'initialization' module by doing a dummy store & load of a field
1554+
// Doing both a store and load keeps FxCop happier because it thinks the field is useful
15501555
let instrs =
15511556
[ yield! (if condition "NO_ADD_FEEFEE_TO_CCTORS" then [] elif condition "ADD_SEQPT_TO_CCTORS" then seqpt else feefee) // mark start of hidden code
15521557
yield mkLdcInt32 0
@@ -1555,25 +1560,26 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu
15551560
yield AI_pop]
15561561
gtdefs.FindNestedTypeDefBuilder(tref).PrependInstructionsToSpecificMethodDef(cond, instrs, sourceOpt)
15571562

1558-
member mgbuf.AddEventDef(tref, edef) =
1563+
member __.AddEventDef (tref, edef) =
15591564
gtdefs.FindNestedTypeDefBuilder(tref).AddEventDef(edef)
15601565

1561-
member mgbuf.AddFieldDef(tref, ilFieldDef) =
1566+
member __.AddFieldDef (tref, ilFieldDef) =
15621567
gtdefs.FindNestedTypeDefBuilder(tref).AddFieldDef(ilFieldDef)
15631568

1564-
member mgbuf.AddOrMergePropertyDef(tref, pdef, m) =
1569+
member __.AddOrMergePropertyDef (tref, pdef, m) =
15651570
gtdefs.FindNestedTypeDefBuilder(tref).AddOrMergePropertyDef(pdef, m)
15661571

1567-
member mgbuf.Close() =
1572+
member __.Close() =
15681573
// old implementation adds new element to the head of list so result was accumulated in reversed order
15691574
let orderedReflectedDefinitions =
15701575
[for (KeyValue(vspec, (name, n, expr))) in reflectedDefinitions -> n, ((name, vspec), expr)]
15711576
|> List.sortBy (fst >> (~-)) // invert the result to get 'order-by-descending' behavior (items in list are 0..* so we don't need to worry about int.MinValue)
15721577
|> List.map snd
15731578
gtdefs.Close(), orderedReflectedDefinitions
1574-
member mgbuf.cenv = cenv
1575-
member mgbuf.GetExplicitEntryPointInfo() = explicitEntryPointInfo
15761579

1580+
member __.cenv = cenv
1581+
1582+
member __.GetExplicitEntryPointInfo() = explicitEntryPointInfo
15771583

15781584
/// Record the types of the things on the evaluation stack.
15791585
/// Used for the few times we have to flush the IL evaluation stack and to compute maxStack.
@@ -6408,7 +6414,7 @@ and GenTopImpl cenv (mgbuf: AssemblyBuilder) mainInfoOpt eenv (TImplFile (qname,
64086414

64096415
// Generate all the anonymous record types mentioned anywhere in this module
64106416
for anonInfo in anonRecdTypes.Values do
6411-
mgbuf.GenerateAnonType((fun ilThisTy -> GenToStringMethod cenv eenv ilThisTy m), anonInfo) |> ignore
6417+
mgbuf.GenerateAnonType((fun ilThisTy -> GenToStringMethod cenv eenv ilThisTy m), anonInfo)
64126418

64136419
let eenv = {eenv with cloc = { eenv.cloc with TopImplQualifiedName = qname.Text } }
64146420

@@ -7615,7 +7621,6 @@ let ClearGeneratedValue (ctxt: ExecutionContext) (_g: TcGlobals) eenv (v: Val) =
76157621
#endif
76167622
()
76177623

7618-
76197624
/// The published API from the ILX code generator
76207625
type IlxAssemblyGenerator(amap: ImportMap, tcGlobals: TcGlobals, tcVal: ConstraintSolver.TcValF, ccu: Tast.CcuThunk) =
76217626

src/fsharp/PostInferenceChecks.fs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ let BindVal cenv env (v: Val) =
285285

286286
let BindVals cenv env vs = List.iter (BindVal cenv env) vs
287287

288+
let RecordAnonRecdInfo cenv (anonInfo: AnonRecdTypeInfo) =
289+
if not (cenv.anonRecdTypes.ContainsKey anonInfo.Stamp) then
290+
cenv.anonRecdTypes <- cenv.anonRecdTypes.Add(anonInfo.Stamp, anonInfo)
291+
288292
//--------------------------------------------------------------------------
289293
// approx walk of type
290294
//--------------------------------------------------------------------------
@@ -334,8 +338,7 @@ let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, v
334338
| Some visitAppTy -> visitAppTy (tcref, tinst)
335339
| None -> ()
336340
| TType_anon (anonInfo, tys) ->
337-
if not (cenv.anonRecdTypes.ContainsKey anonInfo.Stamp) then
338-
cenv.anonRecdTypes <- cenv.anonRecdTypes.Add(anonInfo.Stamp, anonInfo)
341+
RecordAnonRecdInfo cenv anonInfo
339342
CheckTypesDeep cenv f g env tys
340343

341344
| TType_ucase (_, tinst) -> CheckTypesDeep cenv f g env tinst
@@ -1011,8 +1014,8 @@ and CheckExpr (cenv: cenv) (env: env) origExpr (context: PermitByRefExpr) : Limi
10111014
CheckValRef cenv env baseVal m PermitByRefExpr.No
10121015
CheckExprsPermitByRefLike cenv env rest
10131016

1014-
| Expr.Op (c, tyargs, args, m) ->
1015-
CheckExprOp cenv env (c, tyargs, args, m) context expr
1017+
| Expr.Op (op, tyargs, args, m) ->
1018+
CheckExprOp cenv env (op, tyargs, args, m) context expr
10161019

10171020
// Allow 'typeof<System.Void>' calls as a special case, the only accepted use of System.Void!
10181021
| TypeOfExpr g ty when isVoidTy g ty ->
@@ -1115,7 +1118,14 @@ and CheckExprOp cenv env (op, tyargs, args, m) context expr =
11151118
let ctorLimitedZoneCheck() =
11161119
if env.ctorLimitedZone then errorR(Error(FSComp.SR.chkObjCtorsCantUseExceptionHandling(), m))
11171120

1118-
(* Special cases *)
1121+
// Ensure anonynous record type requirements are recorded
1122+
match op with
1123+
| TOp.AnonRecdGet (anonInfo, _)
1124+
| TOp.AnonRecd anonInfo ->
1125+
RecordAnonRecdInfo cenv anonInfo
1126+
| _ -> ()
1127+
1128+
// Special cases
11191129
match op, tyargs, args with
11201130
// Handle these as special cases since mutables are allowed inside their bodies
11211131
| TOp.While _, _, [Expr.Lambda (_, _, _, [_], e1, _, _);Expr.Lambda (_, _, _, [_], e2, _, _)] ->

src/fsharp/TypeRelations.fs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,28 @@ let rec TypesFeasiblyEquiv ndeep g amap m ty1 ty2 =
6666
let ty1 = stripTyEqns g ty1
6767
let ty2 = stripTyEqns g ty2
6868
match ty1, ty2 with
69-
// QUERY: should these be false for non-equal rigid typars? warn-if-not-rigid typars?
7069
| TType_var _, _
7170
| _, TType_var _ -> true
71+
7272
| TType_app (tc1, l1), TType_app (tc2, l2) when tyconRefEq g tc1 tc2 ->
7373
List.lengthsEqAndForall2 (TypesFeasiblyEquiv ndeep g amap m) l1 l2
74+
75+
| TType_anon (anonInfo1, l1),TType_anon (anonInfo2, l2) ->
76+
(evalTupInfoIsStruct anonInfo1.TupInfo = evalTupInfoIsStruct anonInfo2.TupInfo) &&
77+
(match anonInfo1.Assembly, anonInfo2.Assembly with ccu1, ccu2 -> ccuEq ccu1 ccu2) &&
78+
(anonInfo1.SortedNames = anonInfo2.SortedNames) &&
79+
List.lengthsEqAndForall2 (TypesFeasiblyEquiv ndeep g amap m) l1 l2
80+
7481
| TType_tuple (tupInfo1, l1), TType_tuple (tupInfo2, l2) ->
7582
evalTupInfoIsStruct tupInfo1 = evalTupInfoIsStruct tupInfo2 &&
7683
List.lengthsEqAndForall2 (TypesFeasiblyEquiv ndeep g amap m) l1 l2
84+
7785
| TType_fun (d1, r1), TType_fun (d2, r2) ->
7886
(TypesFeasiblyEquiv ndeep g amap m) d1 d2 && (TypesFeasiblyEquiv ndeep g amap m) r1 r2
87+
7988
| TType_measure _, TType_measure _ ->
8089
true
90+
8191
| _ ->
8292
false
8393

@@ -88,18 +98,18 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 =
8898
let ty1 = stripTyEqns g ty1
8999
let ty2 = stripTyEqns g ty2
90100
match ty1, ty2 with
91-
// QUERY: should these be false for non-equal rigid typars? warn-if-not-rigid typars?
92101
| TType_var _, _ | _, TType_var _ -> true
93102

94103
| TType_app (tc1, l1), TType_app (tc2, l2) when tyconRefEq g tc1 tc2 ->
95104
List.lengthsEqAndForall2 (TypesFeasiblyEquiv ndeep g amap m) l1 l2
96-
| TType_tuple (tupInfo1, l1), TType_tuple (tupInfo2, l2) ->
97-
evalTupInfoIsStruct tupInfo1 = evalTupInfoIsStruct tupInfo2 &&
98-
List.lengthsEqAndForall2 (TypesFeasiblyEquiv ndeep g amap m) l1 l2
99-
| TType_fun (d1, r1), TType_fun (d2, r2) ->
100-
(TypesFeasiblyEquiv ndeep g amap m) d1 d2 && (TypesFeasiblyEquiv ndeep g amap m) r1 r2
105+
106+
| TType_tuple _, TType_tuple _
107+
| TType_anon _, TType_anon _
108+
| TType_fun _, TType_fun _ -> TypesFeasiblyEquiv ndeep g amap m ty1 ty2
109+
101110
| TType_measure _, TType_measure _ ->
102111
true
112+
103113
| _ ->
104114
// F# reference types are subtypes of type 'obj'
105115
(isObjTy g ty1 && (canCoerce = CanCoerce || isRefTy g ty2))

tests/fsharp/core/anon/test.fsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,18 @@ module CrossAssemblyTestTupleStruct =
6767
check "svrknvio4" (let res = SampleAPITupleStruct.SampleFunctionReturningStructTuple() in match res with (x,y) -> x + y.Length) 4
6868
tests()
6969

70+
module TypeNotGeneratedBug =
71+
72+
let foo (_: obj) = ()
73+
74+
let bar() = foo {| ThisIsUniqueToThisTest6353 = 1 |}
75+
76+
module FeasibleEqualityNotImplemented =
77+
type R = {| number: int |}
78+
let e = Event< R>()
79+
e.Trigger {|number = 3|}
80+
e.Publish.Add (printfn "%A") // error
81+
7082
#if TESTS_AS_APP
7183
let RUN() = !failures
7284
#else

0 commit comments

Comments
 (0)