From ead4ea3a4e36289bd4424cb0f03f63ae5c1c8679 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Sat, 5 Jan 2019 19:56:23 -0800 Subject: [PATCH 1/4] Allow generic structs to possibly be unmanaged at construction --- src/fsharp/TastOps.fs | 45 +++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/fsharp/TastOps.fs b/src/fsharp/TastOps.fs index 19414aa5f5b..29409670cfd 100644 --- a/src/fsharp/TastOps.fs +++ b/src/fsharp/TastOps.fs @@ -1776,19 +1776,17 @@ let isRefTy g ty = isUnitTy g ty ) -// ECMA C# LANGUAGE SPECIFICATION, 27.2 -// An unmanaged-type is any type that isn't a reference-type, a type-parameter, or a generic struct-type and +// An unmanaged-type is any type that isn't a reference-type and // contains no fields whose type is not an unmanaged-type. In other words, an unmanaged-type is one of the // following: // - sbyte, byte, short, ushort, int, uint, long, ulong, char, float, double, decimal, or bool. // - Any enum-type. // - Any pointer-type. -// - Any non-generic user-defined struct-type that contains fields of unmanaged-types only. -// [Note: Constructed types and type-parameters are never unmanaged-types. end note] +// - Any generic user-defined struct-type that can be statically determined to be 'unmanaged' at construction. let rec isUnmanagedTy g ty = let ty = stripTyEqnsAndMeasureEqns g ty - match tryDestAppTy g ty with - | ValueSome tcref -> + match ty with + | TType_app(tcref, tinst) -> let isEq tcref2 = tyconRefEq g tcref tcref2 if isEq g.nativeptr_tcr || isEq g.nativeint_tcr || isEq g.sbyte_tcr || isEq g.byte_tcr || @@ -1807,11 +1805,38 @@ let rec isUnmanagedTy g ty = true elif tycon.IsStructOrEnumTycon then match tycon.TyparsNoRange with - | [] -> tycon.AllInstanceFieldsAsList |> List.forall (fun r -> isUnmanagedTy g r.rfield_type) - | _ -> false // generic structs are never + | [] -> tycon.AllInstanceFieldsAsList |> List.forall (fun r -> isUnmanagedTy g r.rfield_type) + | typars -> + // Handle generic structs + // REVIEW: This may not be the most optimal, but it's probably better than + // having to iterate over all type arguments for every field that is a 'TType_var'. + // A possible more optimal solution would be to have a flag (hopefully without having to pickle it) on a Typar that indicates it's used as a backing field. (see 'TyparFlags' type) + // That way we can ignore fields that are a 'TType_var' and just check the type arguments from 'TType_app' that have the flag. + // However, what we have currently is probably just fine as unmanaged constraints are used infrequently; even more so when combined with generic struct construction. + let lookup = Dictionary(typars.Length) + (typars, tinst) + ||> List.iter2 (fun typar ty -> lookup.Add(typar.Stamp, ty)) + + tycon.AllInstanceFieldsAsList + |> List.forall (fun r -> + match tryDestTyparTy g r.rfield_type with + | ValueSome(fieldTypar) -> + match lookup.TryGetValue(fieldTypar.Stamp) with + | true, ty -> + match tryDestTyparTy g ty with + | ValueSome typar -> + typar.Constraints |> List.exists (function | TyparConstraint.IsUnmanaged _ -> true | _ -> false) + | _ -> + isUnmanagedTy g ty + | _ -> false + | _ -> + isUnmanagedTy g r.rfield_type + ) else false - | ValueNone -> - false + // Handle struct tuples + | TType_tuple(TupInfo.Const true, tinst) -> + tinst |> List.forall (isUnmanagedTy g) + | _ -> false let isInterfaceTycon x = isILInterfaceTycon x || x.IsFSharpInterfaceTycon From 5712fd9045aa333dab3264aec5f143791ba34ea4 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Sat, 5 Jan 2019 20:53:59 -0800 Subject: [PATCH 2/4] Fixed nested generic structs for unmanaged --- src/fsharp/TastOps.fs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/fsharp/TastOps.fs b/src/fsharp/TastOps.fs index 29409670cfd..6fac994f8d3 100644 --- a/src/fsharp/TastOps.fs +++ b/src/fsharp/TastOps.fs @@ -1810,8 +1810,6 @@ let rec isUnmanagedTy g ty = // Handle generic structs // REVIEW: This may not be the most optimal, but it's probably better than // having to iterate over all type arguments for every field that is a 'TType_var'. - // A possible more optimal solution would be to have a flag (hopefully without having to pickle it) on a Typar that indicates it's used as a backing field. (see 'TyparFlags' type) - // That way we can ignore fields that are a 'TType_var' and just check the type arguments from 'TType_app' that have the flag. // However, what we have currently is probably just fine as unmanaged constraints are used infrequently; even more so when combined with generic struct construction. let lookup = Dictionary(typars.Length) (typars, tinst) @@ -1819,8 +1817,9 @@ let rec isUnmanagedTy g ty = tycon.AllInstanceFieldsAsList |> List.forall (fun r -> - match tryDestTyparTy g r.rfield_type with - | ValueSome(fieldTypar) -> + let fieldTy = stripTyEqnsAndMeasureEqns g r.rfield_type + match fieldTy with + | TType_var fieldTypar -> match lookup.TryGetValue(fieldTypar.Stamp) with | true, ty -> match tryDestTyparTy g ty with @@ -1829,8 +1828,20 @@ let rec isUnmanagedTy g ty = | _ -> isUnmanagedTy g ty | _ -> false - | _ -> - isUnmanagedTy g r.rfield_type + | TType_app(fieldTcref, fieldTinst) -> + // This will also handle nested generic structs + let fieldTinst = + fieldTinst + |> List.map (fun ty -> + match tryDestTyparTy g ty with + | ValueSome(typar) -> + match lookup.TryGetValue(typar.Stamp) with + | true, ty -> ty + | _ -> ty + | _ -> ty + ) + isUnmanagedTy g (mkAppTy fieldTcref fieldTinst) + | _ -> isUnmanagedTy g fieldTy ) else false // Handle struct tuples From a8935b4aff988a6465b9777c9c7af31bc0000cf4 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Sat, 5 Jan 2019 21:00:15 -0800 Subject: [PATCH 3/4] Updated comment --- src/fsharp/TastOps.fs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/fsharp/TastOps.fs b/src/fsharp/TastOps.fs index 6fac994f8d3..1b1200d77ad 100644 --- a/src/fsharp/TastOps.fs +++ b/src/fsharp/TastOps.fs @@ -1808,8 +1808,7 @@ let rec isUnmanagedTy g ty = | [] -> tycon.AllInstanceFieldsAsList |> List.forall (fun r -> isUnmanagedTy g r.rfield_type) | typars -> // Handle generic structs - // REVIEW: This may not be the most optimal, but it's probably better than - // having to iterate over all type arguments for every field that is a 'TType_var'. + // REVIEW: This may not be the most optimal, but it's probably better than having to iterate over all type arguments for every field. // However, what we have currently is probably just fine as unmanaged constraints are used infrequently; even more so when combined with generic struct construction. let lookup = Dictionary(typars.Length) (typars, tinst) @@ -1825,8 +1824,7 @@ let rec isUnmanagedTy g ty = match tryDestTyparTy g ty with | ValueSome typar -> typar.Constraints |> List.exists (function | TyparConstraint.IsUnmanaged _ -> true | _ -> false) - | _ -> - isUnmanagedTy g ty + | _ -> isUnmanagedTy g ty | _ -> false | TType_app(fieldTcref, fieldTinst) -> // This will also handle nested generic structs From e24bc7181d759fbb415e3ef1a22429b636931137 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Sat, 5 Jan 2019 21:03:12 -0800 Subject: [PATCH 4/4] Changed nested generic struct condition slightly --- src/fsharp/TastOps.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fsharp/TastOps.fs b/src/fsharp/TastOps.fs index 1b1200d77ad..e6dbff68a05 100644 --- a/src/fsharp/TastOps.fs +++ b/src/fsharp/TastOps.fs @@ -1826,8 +1826,8 @@ let rec isUnmanagedTy g ty = typar.Constraints |> List.exists (function | TyparConstraint.IsUnmanaged _ -> true | _ -> false) | _ -> isUnmanagedTy g ty | _ -> false - | TType_app(fieldTcref, fieldTinst) -> - // This will also handle nested generic structs + | TType_app(fieldTcref, fieldTinst) when fieldTinst.Length > 0 -> + // This will handle nested generic structs let fieldTinst = fieldTinst |> List.map (fun ty ->