From 8dd81c6424bfafd02b31b2ddd92c20fc7cd0e817 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 20 Aug 2019 10:15:56 -0700 Subject: [PATCH] Added inref immutability assumption removal (#7407) * Added inref immutability assumption fix. Aware of IsReadOnly attribute, only on ILMethods. * Added isILStructTy * targeting assumption * Fixed tests * renaming * Better IsReadOnly check * Added tests * Splitting read-only assumption and read-only attribute check * Func removal * Better tests * Trying to fix test * More tests * Fixed extension member inref check * small cleanup * Added mkDereferencedByrefExpr * Minor cleanup * More tests. Changed how we deref addresses * Update comment * Being more specific on dereference addr work * Drastically simplified the solution * Added tests * Verifying current behavior --- src/fsharp/MethodCalls.fs | 13 ++ src/fsharp/PostInferenceChecks.fs | 3 +- src/fsharp/TastOps.fs | 65 ++++-- src/fsharp/TastOps.fsi | 3 + src/fsharp/TypeChecker.fs | 9 +- src/fsharp/infos.fs | 29 ++- src/fsharp/tast.fs | 203 +++++++++++-------- tests/fsharp/Compiler/CompilerAssert.fs | 4 +- tests/fsharp/Compiler/Language/ByrefTests.fs | 198 ++++++++++++++++++ tests/fsharp/FSharpSuite.Tests.fsproj | 1 + tests/fsharp/core/fsfromfsviacs/lib2.cs | 13 ++ tests/fsharp/core/fsfromfsviacs/test.fsx | 69 +++++++ 12 files changed, 498 insertions(+), 112 deletions(-) create mode 100644 tests/fsharp/Compiler/Language/ByrefTests.fs diff --git a/src/fsharp/MethodCalls.fs b/src/fsharp/MethodCalls.fs index 895f2d1f7..cf590b69e 100644 --- a/src/fsharp/MethodCalls.fs +++ b/src/fsharp/MethodCalls.fs @@ -938,6 +938,19 @@ let TakeObjAddrForMethodCall g amap (minfo: MethInfo) isMutable m objArgs f = let hasCallInfo = ccallInfo.IsSome let mustTakeAddress = hasCallInfo || minfo.ObjArgNeedsAddress(amap, m) let objArgTy = tyOfExpr g objArgExpr + + let isMutable = + match isMutable with + | DefinitelyMutates + | NeverMutates + | AddressOfOp -> isMutable + | PossiblyMutates -> + // Check to see if the method is read-only. Perf optimization. + // If there is an extension member whose first arg is an inref, we must return NeverMutates. + if mustTakeAddress && (minfo.IsReadOnly || minfo.IsReadOnlyExtensionMember (amap, m)) then + NeverMutates + else + isMutable let wrap, objArgExprAddr, isReadOnly, _isWriteOnly = mkExprAddrOfExpr g mustTakeAddress hasCallInfo isMutable objArgExpr None m diff --git a/src/fsharp/PostInferenceChecks.fs b/src/fsharp/PostInferenceChecks.fs index dd1ec5c2a..d97766ac1 100644 --- a/src/fsharp/PostInferenceChecks.fs +++ b/src/fsharp/PostInferenceChecks.fs @@ -263,7 +263,8 @@ let GetLimitValByRef cenv env m v = { scope = scope; flags = flags } let LimitVal cenv (v: Val) limit = - cenv.limitVals.[v.Stamp] <- limit + if not v.IgnoresByrefScope then + cenv.limitVals.[v.Stamp] <- limit let BindVal cenv env (v: Val) = //printfn "binding %s..." v.DisplayName diff --git a/src/fsharp/TastOps.fs b/src/fsharp/TastOps.fs index 188940447..55576c88d 100644 --- a/src/fsharp/TastOps.fs +++ b/src/fsharp/TastOps.fs @@ -3004,8 +3004,8 @@ let isByrefTyconRef (g: TcGlobals) (tcref: TyconRef) = let isByrefLikeTyconRef (g: TcGlobals) m (tcref: TyconRef) = tcref.CanDeref && match tcref.TryIsByRefLike with - | Some res -> res - | None -> + | ValueSome res -> res + | _ -> let res = isByrefTyconRef g tcref || (isStructTyconRef tcref && TyconRefHasAttribute g m g.attrib_IsByRefLikeAttribute tcref) @@ -5941,34 +5941,53 @@ let mkAndSimplifyMatch spBind exprm matchm ty tree targets = //------------------------------------------------------------------------- type Mutates = AddressOfOp | DefinitelyMutates | PossiblyMutates | NeverMutates -exception DefensiveCopyWarning of string * range +exception DefensiveCopyWarning of string * range let isRecdOrStructTyconRefAssumedImmutable (g: TcGlobals) (tcref: TyconRef) = tcref.CanDeref && not (isRecdOrUnionOrStructTyconRefDefinitelyMutable tcref) || - tyconRefEq g tcref g.decimal_tcr || + tyconRefEq g tcref g.decimal_tcr || tyconRefEq g tcref g.date_tcr -let isRecdOrStructTyconRefReadOnly (g: TcGlobals) m (tcref: TyconRef) = +let isTyconRefReadOnly g m (tcref: TyconRef) = tcref.CanDeref && match tcref.TryIsReadOnly with - | Some res -> res - | None -> - let isImmutable = isRecdOrStructTyconRefAssumedImmutable g tcref - let hasAttrib = TyconRefHasAttribute g m g.attrib_IsReadOnlyAttribute tcref - let res = isImmutable || hasAttrib + | ValueSome res -> res + | _ -> + let res = TyconRefHasAttribute g m g.attrib_IsReadOnlyAttribute tcref tcref.SetIsReadOnly res res -let isRecdOrStructTyReadOnly (g: TcGlobals) m ty = +let isTyconRefAssumedReadOnly g (tcref: TyconRef) = + tcref.CanDeref && + match tcref.TryIsAssumedReadOnly with + | ValueSome res -> res + | _ -> + let res = isRecdOrStructTyconRefAssumedImmutable g tcref + tcref.SetIsAssumedReadOnly res + res + +let isRecdOrStructTyconRefReadOnlyAux g m isInref (tcref: TyconRef) = + if isInref && tcref.IsILStructOrEnumTycon then + isTyconRefReadOnly g m tcref + else + isTyconRefReadOnly g m tcref || isTyconRefAssumedReadOnly g tcref + +let isRecdOrStructTyconRefReadOnly g m tcref = + isRecdOrStructTyconRefReadOnlyAux g m false tcref + +let isRecdOrStructTyReadOnlyAux (g: TcGlobals) m isInref ty = match tryDestAppTy g ty with | ValueNone -> false - | ValueSome tcref -> isRecdOrStructTyconRefReadOnly g m tcref + | ValueSome tcref -> isRecdOrStructTyconRefReadOnlyAux g m isInref tcref + +let isRecdOrStructTyReadOnly g m ty = + isRecdOrStructTyReadOnlyAux g m false ty -let CanTakeAddressOf g m ty mut = +let CanTakeAddressOf g m isInref ty mut = match mut with | NeverMutates -> true - | PossiblyMutates -> isRecdOrStructTyReadOnly g m ty + | PossiblyMutates -> isRecdOrStructTyReadOnlyAux g m isInref ty | DefinitelyMutates -> false | AddressOfOp -> true // you can take the address but you might get a (readonly) inref as a result @@ -5996,7 +6015,7 @@ let CanTakeAddressOfImmutableVal (g: TcGlobals) m (vref: ValRef) mut = // || valRefInThisAssembly g.compilingFslib vref // This is because we don't actually guarantee to generate static backing fields for all values like these, e.g. simple constants "let x = 1". // We always generate a static property but there is no field to take an address of - CanTakeAddressOf g m vref.Type mut + CanTakeAddressOf g m false vref.Type mut let MustTakeAddressOfVal (g: TcGlobals) (vref: ValRef) = vref.IsMutable && @@ -6008,7 +6027,7 @@ let MustTakeAddressOfByrefGet (g: TcGlobals) (vref: ValRef) = let CanTakeAddressOfByrefGet (g: TcGlobals) (vref: ValRef) mut = isInByrefTy g vref.Type && - CanTakeAddressOf g vref.Range (destByrefTy g vref.Type) mut + CanTakeAddressOf g vref.Range true (destByrefTy g vref.Type) mut let MustTakeAddressOfRecdField (rfref: RecdField) = // Static mutable fields must be private, hence we don't have to take their address @@ -6021,14 +6040,18 @@ let CanTakeAddressOfRecdFieldRef (g: TcGlobals) m (rfref: RecdFieldRef) tinst mu // We only do this if the field is defined in this assembly because we can't take addresses across assemblies for immutable fields entityRefInThisAssembly g.compilingFslib rfref.TyconRef && not rfref.RecdField.IsMutable && - CanTakeAddressOf g m (actualTyOfRecdFieldRef rfref tinst) mut + CanTakeAddressOf g m false (actualTyOfRecdFieldRef rfref tinst) mut let CanTakeAddressOfUnionFieldRef (g: TcGlobals) m (uref: UnionCaseRef) cidx tinst mut = // We only do this if the field is defined in this assembly because we can't take addresses across assemblies for immutable fields entityRefInThisAssembly g.compilingFslib uref.TyconRef && let rfref = uref.FieldByIndex cidx not rfref.IsMutable && - CanTakeAddressOf g m (actualTyOfUnionFieldRef uref cidx tinst) mut + CanTakeAddressOf g m false (actualTyOfUnionFieldRef uref cidx tinst) mut + +let mkDerefAddrExpr mAddrGet expr mExpr exprTy = + let v, _ = mkCompGenLocal mAddrGet "byrefReturn" exprTy + mkCompGenLet mExpr v expr (mkAddrGet mAddrGet (mkLocalValRef v)) /// Make the address-of expression and return a wrapper that adds any allocated locals at an appropriate scope. /// Also return a flag that indicates if the resulting pointer is a not a pointer where writing is allowed and will @@ -6166,8 +6189,12 @@ let rec mkExprAddrOfExprAux g mustTakeAddress useReadonlyForGenericArrayAddress // Take a defensive copy let tmp, _ = match mut with - | NeverMutates -> mkCompGenLocal m "copyOfStruct" ty + | NeverMutates -> mkCompGenLocal m "copyOfStruct" ty | _ -> mkMutableCompGenLocal m "copyOfStruct" ty + + // This local is special in that it ignore byref scoping rules. + tmp.SetIgnoresByrefScope() + let readonly = true let writeonly = false Some (tmp, expr), (mkValAddr m readonly (mkLocalValRef tmp)), readonly, writeonly diff --git a/src/fsharp/TastOps.fsi b/src/fsharp/TastOps.fsi index 1da227423..6119007d5 100755 --- a/src/fsharp/TastOps.fsi +++ b/src/fsharp/TastOps.fsi @@ -369,6 +369,9 @@ exception DefensiveCopyWarning of string * range type Mutates = AddressOfOp | DefinitelyMutates | PossiblyMutates | NeverMutates +/// Helper to create an expression that dereferences an address. +val mkDerefAddrExpr: mAddrGet: range -> expr: Expr -> mExpr: range -> exprTy: TType -> Expr + /// Helper to take the address of an expression val mkExprAddrOfExprAux : TcGlobals -> bool -> bool -> Mutates -> Expr -> ValRef option -> range -> (Val * Expr) option * Expr * bool * bool diff --git a/src/fsharp/TypeChecker.fs b/src/fsharp/TypeChecker.fs index 215ac3e54..58cc3af35 100644 --- a/src/fsharp/TypeChecker.fs +++ b/src/fsharp/TypeChecker.fs @@ -3394,8 +3394,7 @@ let AnalyzeArbitraryExprAsEnumerable cenv (env: TcEnv) localAlloc m exprty expr let currentExpr, enumElemTy = // Implicitly dereference byref for expr 'for x in ...' if isByrefTy cenv.g enumElemTy then - let v, _ = mkCompGenLocal m "byrefReturn" enumElemTy - let expr = mkCompGenLet currentExpr.Range v currentExpr (mkAddrGet m (mkLocalValRef v)) + let expr = mkDerefAddrExpr m currentExpr currentExpr.Range enumElemTy expr, destByrefTy cenv.g enumElemTy else currentExpr, enumElemTy @@ -4139,9 +4138,8 @@ let buildApp cenv expr resultTy arg m = | _ when isByrefTy g resultTy -> // Handle byref returns, byref-typed returns get implicitly dereferenced - let v, _ = mkCompGenLocal m "byrefReturn" resultTy let expr = expr.SupplyArgument (arg, m) - let expr = mkCompGenLet m v expr.Expr (mkAddrGet m (mkLocalValRef v)) + let expr = mkDerefAddrExpr m expr.Expr m resultTy let resultTy = destByrefTy g resultTy MakeApplicableExprNoFlex cenv expr, resultTy @@ -10214,8 +10212,7 @@ and TcMethodApplication // byref-typed returns get implicitly dereferenced let vty = tyOfExpr cenv.g callExpr0 if isByrefTy cenv.g vty then - let v, _ = mkCompGenLocal mMethExpr "byrefReturn" vty - mkCompGenLet mMethExpr v callExpr0 (mkAddrGet mMethExpr (mkLocalValRef v)) + mkDerefAddrExpr mMethExpr callExpr0 mMethExpr vty else callExpr0 diff --git a/src/fsharp/infos.fs b/src/fsharp/infos.fs index dd6f8b0e3..4ca16f77c 100755 --- a/src/fsharp/infos.fs +++ b/src/fsharp/infos.fs @@ -835,7 +835,15 @@ type ILMethInfo = member x.IsDllImport (g: TcGlobals) = match g.attrib_DllImportAttribute with | None -> false - | Some (AttribInfo(tref, _)) ->x.RawMetadata.CustomAttrs |> TryDecodeILAttribute g tref |> Option.isSome + | Some attr -> + x.RawMetadata.CustomAttrs + |> TryFindILAttribute attr + + /// Indicates if the method is marked with the [] attribute. This is done by looking at the IL custom attributes on + /// the method. + member x.IsReadOnly (g: TcGlobals) = + x.RawMetadata.CustomAttrs + |> TryFindILAttribute g.attrib_IsReadOnlyAttribute /// Get the (zero or one) 'self'/'this'/'object' arguments associated with an IL method. /// An instance extension method returns one object argument. @@ -1238,6 +1246,25 @@ type MethInfo = member x.IsStruct = isStructTy x.TcGlobals x.ApparentEnclosingType + /// Indicates if this method is read-only; usually by the [] attribute. + /// Must be an instance method. + /// Receiver must be a struct type. + member x.IsReadOnly = + // Perf Review: Is there a way we can cache this result? + x.IsInstance && + x.IsStruct && + match x with + | ILMeth (g, ilMethInfo, _) -> ilMethInfo.IsReadOnly g + | FSMeth _ -> false // F# defined methods not supported yet. Must be a language feature. + | _ -> false + + /// Indicates if this method is an extension member that is read-only. + /// An extension member is considered read-only if the first argument is a read-only byref (inref) type. + member x.IsReadOnlyExtensionMember (amap: Import.ImportMap, m) = + x.IsExtensionMember && + x.TryObjArgByrefType(amap, m, x.FormalMethodInst) + |> Option.exists (isInByrefTy amap.g) + /// Build IL method infos. static member CreateILMeth (amap: Import.ImportMap, m, ty: TType, md: ILMethodDef) = let tinfo = ILTypeInfo.FromType amap.g ty diff --git a/src/fsharp/tast.fs b/src/fsharp/tast.fs index d59c33f70..6e063cd83 100644 --- a/src/fsharp/tast.fs +++ b/src/fsharp/tast.fs @@ -116,133 +116,137 @@ type ValFlags(flags: int64) = new (recValInfo, baseOrThis, isCompGen, inlineInfo, isMutable, isModuleOrMemberBinding, isExtensionMember, isIncrClassSpecialMember, isTyFunc, allowTypeInst, isGeneratedEventVal) = let flags = (match baseOrThis with - | BaseVal -> 0b0000000000000000000L - | CtorThisVal -> 0b0000000000000000010L - | NormalVal -> 0b0000000000000000100L - | MemberThisVal -> 0b0000000000000000110L) ||| - (if isCompGen then 0b0000000000000001000L - else 0b00000000000000000000L) ||| + | BaseVal -> 0b00000000000000000000L + | CtorThisVal -> 0b00000000000000000010L + | NormalVal -> 0b00000000000000000100L + | MemberThisVal -> 0b00000000000000000110L) ||| + (if isCompGen then 0b00000000000000001000L + else 0b000000000000000000000L) ||| (match inlineInfo with - | ValInline.PseudoVal -> 0b0000000000000000000L - | ValInline.Always -> 0b0000000000000010000L - | ValInline.Optional -> 0b0000000000000100000L - | ValInline.Never -> 0b0000000000000110000L) ||| + | ValInline.PseudoVal -> 0b00000000000000000000L + | ValInline.Always -> 0b00000000000000010000L + | ValInline.Optional -> 0b00000000000000100000L + | ValInline.Never -> 0b00000000000000110000L) ||| (match isMutable with - | Immutable -> 0b0000000000000000000L - | Mutable -> 0b0000000000001000000L) ||| + | Immutable -> 0b00000000000000000000L + | Mutable -> 0b00000000000001000000L) ||| (match isModuleOrMemberBinding with - | false -> 0b0000000000000000000L - | true -> 0b0000000000010000000L) ||| + | false -> 0b00000000000000000000L + | true -> 0b00000000000010000000L) ||| (match isExtensionMember with - | false -> 0b0000000000000000000L - | true -> 0b0000000000100000000L) ||| + | false -> 0b00000000000000000000L + | true -> 0b00000000000100000000L) ||| (match isIncrClassSpecialMember with - | false -> 0b0000000000000000000L - | true -> 0b0000000001000000000L) ||| + | false -> 0b00000000000000000000L + | true -> 0b00000000001000000000L) ||| (match isTyFunc with - | false -> 0b0000000000000000000L - | true -> 0b0000000010000000000L) ||| + | false -> 0b00000000000000000000L + | true -> 0b00000000010000000000L) ||| (match recValInfo with - | ValNotInRecScope -> 0b0000000000000000000L - | ValInRecScope true -> 0b0000000100000000000L - | ValInRecScope false -> 0b0000001000000000000L) ||| + | ValNotInRecScope -> 0b00000000000000000000L + | ValInRecScope true -> 0b00000000100000000000L + | ValInRecScope false -> 0b00000001000000000000L) ||| (match allowTypeInst with - | false -> 0b0000000000000000000L - | true -> 0b0000100000000000000L) ||| + | false -> 0b00000000000000000000L + | true -> 0b00000100000000000000L) ||| (match isGeneratedEventVal with - | false -> 0b0000000000000000000L - | true -> 0b0100000000000000000L) + | false -> 0b00000000000000000000L + | true -> 0b00100000000000000000L) ValFlags flags member x.BaseOrThisInfo = - match (flags &&& 0b0000000000000000110L) with - | 0b0000000000000000000L -> BaseVal - | 0b0000000000000000010L -> CtorThisVal - | 0b0000000000000000100L -> NormalVal - | 0b0000000000000000110L -> MemberThisVal + match (flags &&& 0b00000000000000000110L) with + | 0b00000000000000000000L -> BaseVal + | 0b00000000000000000010L -> CtorThisVal + | 0b00000000000000000100L -> NormalVal + | 0b00000000000000000110L -> MemberThisVal | _ -> failwith "unreachable" - member x.IsCompilerGenerated = (flags &&& 0b0000000000000001000L) <> 0x0L + member x.IsCompilerGenerated = (flags &&& 0b00000000000000001000L) <> 0x0L member x.SetIsCompilerGenerated isCompGen = - let flags = (flags &&& ~~~0b0000000000000001000L) ||| + let flags = (flags &&& ~~~0b00000000000000001000L) ||| (match isCompGen with - | false -> 0b0000000000000000000L - | true -> 0b0000000000000001000L) + | false -> 0b00000000000000000000L + | true -> 0b00000000000000001000L) ValFlags flags member x.InlineInfo = - match (flags &&& 0b0000000000000110000L) with - | 0b0000000000000000000L -> ValInline.PseudoVal - | 0b0000000000000010000L -> ValInline.Always - | 0b0000000000000100000L -> ValInline.Optional - | 0b0000000000000110000L -> ValInline.Never + match (flags &&& 0b00000000000000110000L) with + | 0b00000000000000000000L -> ValInline.PseudoVal + | 0b00000000000000010000L -> ValInline.Always + | 0b00000000000000100000L -> ValInline.Optional + | 0b00000000000000110000L -> ValInline.Never | _ -> failwith "unreachable" member x.MutabilityInfo = - match (flags &&& 0b0000000000001000000L) with - | 0b0000000000000000000L -> Immutable - | 0b0000000000001000000L -> Mutable + match (flags &&& 0b00000000000001000000L) with + | 0b00000000000000000000L -> Immutable + | 0b00000000000001000000L -> Mutable | _ -> failwith "unreachable" member x.IsMemberOrModuleBinding = - match (flags &&& 0b0000000000010000000L) with - | 0b0000000000000000000L -> false - | 0b0000000000010000000L -> true + match (flags &&& 0b00000000000010000000L) with + | 0b00000000000000000000L -> false + | 0b00000000000010000000L -> true | _ -> failwith "unreachable" - member x.WithIsMemberOrModuleBinding = ValFlags(flags ||| 0b0000000000010000000L) + member x.WithIsMemberOrModuleBinding = ValFlags(flags ||| 0b00000000000010000000L) - member x.IsExtensionMember = (flags &&& 0b0000000000100000000L) <> 0L + member x.IsExtensionMember = (flags &&& 0b00000000000100000000L) <> 0L - member x.IsIncrClassSpecialMember = (flags &&& 0b0000000001000000000L) <> 0L + member x.IsIncrClassSpecialMember = (flags &&& 0b00000000001000000000L) <> 0L - member x.IsTypeFunction = (flags &&& 0b0000000010000000000L) <> 0L + member x.IsTypeFunction = (flags &&& 0b00000000010000000000L) <> 0L - member x.RecursiveValInfo = match (flags &&& 0b0000001100000000000L) with - | 0b0000000000000000000L -> ValNotInRecScope - | 0b0000000100000000000L -> ValInRecScope true - | 0b0000001000000000000L -> ValInRecScope false + member x.RecursiveValInfo = match (flags &&& 0b00000001100000000000L) with + | 0b00000000000000000000L -> ValNotInRecScope + | 0b00000000100000000000L -> ValInRecScope true + | 0b00000001000000000000L -> ValInRecScope false | _ -> failwith "unreachable" member x.WithRecursiveValInfo recValInfo = let flags = - (flags &&& ~~~0b0000001100000000000L) ||| + (flags &&& ~~~0b00000001100000000000L) ||| (match recValInfo with - | ValNotInRecScope -> 0b0000000000000000000L - | ValInRecScope true -> 0b0000000100000000000L - | ValInRecScope false -> 0b0000001000000000000L) + | ValNotInRecScope -> 0b00000000000000000000L + | ValInRecScope true -> 0b00000000100000000000L + | ValInRecScope false -> 0b00000001000000000000L) ValFlags flags - member x.MakesNoCriticalTailcalls = (flags &&& 0b0000010000000000000L) <> 0L + member x.MakesNoCriticalTailcalls = (flags &&& 0b00000010000000000000L) <> 0L - member x.WithMakesNoCriticalTailcalls = ValFlags(flags ||| 0b0000010000000000000L) + member x.WithMakesNoCriticalTailcalls = ValFlags(flags ||| 0b00000010000000000000L) - member x.PermitsExplicitTypeInstantiation = (flags &&& 0b0000100000000000000L) <> 0L + member x.PermitsExplicitTypeInstantiation = (flags &&& 0b00000100000000000000L) <> 0L - member x.HasBeenReferenced = (flags &&& 0b0001000000000000000L) <> 0L + member x.HasBeenReferenced = (flags &&& 0b00001000000000000000L) <> 0L - member x.WithHasBeenReferenced = ValFlags(flags ||| 0b0001000000000000000L) + member x.WithHasBeenReferenced = ValFlags(flags ||| 0b00001000000000000000L) - member x.IsCompiledAsStaticPropertyWithoutField = (flags &&& 0b0010000000000000000L) <> 0L + member x.IsCompiledAsStaticPropertyWithoutField = (flags &&& 0b00010000000000000000L) <> 0L - member x.WithIsCompiledAsStaticPropertyWithoutField = ValFlags(flags ||| 0b0010000000000000000L) + member x.WithIsCompiledAsStaticPropertyWithoutField = ValFlags(flags ||| 0b00010000000000000000L) - member x.IsGeneratedEventVal = (flags &&& 0b0100000000000000000L) <> 0L + member x.IsGeneratedEventVal = (flags &&& 0b00100000000000000000L) <> 0L - member x.IsFixed = (flags &&& 0b1000000000000000000L) <> 0L + member x.IsFixed = (flags &&& 0b01000000000000000000L) <> 0L - member x.WithIsFixed = ValFlags(flags ||| 0b1000000000000000000L) + member x.WithIsFixed = ValFlags(flags ||| 0b01000000000000000000L) + + member x.IgnoresByrefScope = (flags &&& 0b10000000000000000000L) <> 0L + + member x.WithIgnoresByrefScope = ValFlags(flags ||| 0b10000000000000000000L) /// Get the flags as included in the F# binary metadata member x.PickledBits = @@ -250,7 +254,7 @@ type ValFlags(flags: int64) = // Clear the IsCompiledAsStaticPropertyWithoutField, only used to determine whether to use a true field for a value, and to eliminate the optimization info for observable bindings // Clear the HasBeenReferenced, only used to report "unreferenced variable" warnings and to help collect 'it' values in FSI.EXE // Clear the IsGeneratedEventVal, since there's no use in propagating specialname information for generated add/remove event vals - (flags &&& ~~~0b0011001100000000000L) + (flags &&& ~~~0b10011001100000000000L) /// Represents the kind of a type parameter [] @@ -423,9 +427,9 @@ type EntityFlags(flags: int64) = /// These two bits represents the on-demand analysis about whether the entity has the IsByRefLike attribute member x.TryIsByRefLike = (flags &&& 0b000000011000000L) |> function - | 0b000000011000000L -> Some true - | 0b000000010000000L -> Some false - | _ -> None + | 0b000000011000000L -> ValueSome true + | 0b000000010000000L -> ValueSome false + | _ -> ValueNone /// Adjust the on-demand analysis about whether the entity has the IsByRefLike attribute member x.WithIsByRefLike flag = @@ -436,14 +440,14 @@ type EntityFlags(flags: int64) = | false -> 0b000000010000000L) EntityFlags flags - /// These two bits represents the on-demand analysis about whether the entity has the IsReadOnly attribute or is otherwise determined to be a readonly struct + /// These two bits represents the on-demand analysis about whether the entity has the IsReadOnly attribute member x.TryIsReadOnly = (flags &&& 0b000001100000000L) |> function - | 0b000001100000000L -> Some true - | 0b000001000000000L -> Some false - | _ -> None + | 0b000001100000000L -> ValueSome true + | 0b000001000000000L -> ValueSome false + | _ -> ValueNone - /// Adjust the on-demand analysis about whether the entity has the IsReadOnly attribute or is otherwise determined to be a readonly struct + /// Adjust the on-demand analysis about whether the entity has the IsReadOnly attribute member x.WithIsReadOnly flag = let flags = (flags &&& ~~~0b000001100000000L) ||| @@ -452,8 +456,24 @@ type EntityFlags(flags: int64) = | false -> 0b000001000000000L) EntityFlags flags + /// These two bits represents the on-demand analysis about whether the entity is assumed to be a readonly struct + member x.TryIsAssumedReadOnly = (flags &&& 0b000110000000000L) + |> function + | 0b000110000000000L -> ValueSome true + | 0b000100000000000L -> ValueSome false + | _ -> ValueNone + + /// Adjust the on-demand analysis about whether the entity is assumed to be a readonly struct + member x.WithIsAssumedReadOnly flag = + let flags = + (flags &&& ~~~0b000110000000000L) ||| + (match flag with + | true -> 0b000110000000000L + | false -> 0b000100000000000L) + EntityFlags flags + /// Get the flags as included in the F# binary metadata - member x.PickledBits = (flags &&& ~~~0b000001111000100L) + member x.PickledBits = (flags &&& ~~~0b000111111000100L) #if DEBUG @@ -1065,12 +1085,18 @@ and /// Represents a type definition, exception definition, module definition or /// Set the on-demand analysis about whether the entity has the IsByRefLike attribute member x.SetIsByRefLike b = x.entity_flags <- x.entity_flags.WithIsByRefLike b - /// These two bits represents the on-demand analysis about whether the entity has the IsReadOnly attribute or is otherwise determined to be a readonly struct + /// These two bits represents the on-demand analysis about whether the entity has the IsReadOnly attribute member x.TryIsReadOnly = x.entity_flags.TryIsReadOnly - /// Set the on-demand analysis about whether the entity has the IsReadOnly attribute or is otherwise determined to be a readonly struct + /// Set the on-demand analysis about whether the entity has the IsReadOnly attribute member x.SetIsReadOnly b = x.entity_flags <- x.entity_flags.WithIsReadOnly b + /// These two bits represents the on-demand analysis about whether the entity is assumed to be a readonly struct + member x.TryIsAssumedReadOnly = x.entity_flags.TryIsAssumedReadOnly + + /// Set the on-demand analysis about whether the entity is assumed to be a readonly struct + member x.SetIsAssumedReadOnly b = x.entity_flags <- x.entity_flags.WithIsAssumedReadOnly b + /// Indicates if this is an F# type definition whose r.h.s. is known to be some kind of F# object model definition member x.IsFSharpObjectModelTycon = match x.TypeReprInfo with | TFSharpObjectRepr _ -> true | _ -> false @@ -2725,6 +2751,9 @@ and [] /// Indicates if the value is pinned/fixed member x.IsFixed = x.val_flags.IsFixed + /// Indicates if the value will ignore byref scoping rules + member x.IgnoresByrefScope = x.val_flags.IgnoresByrefScope + /// Indicates if this value allows the use of an explicit type instantiation (i.e. does it itself have explicit type arguments, /// or does it have a signature?) member x.PermitsExplicitTypeInstantiation = x.val_flags.PermitsExplicitTypeInstantiation @@ -2948,6 +2977,8 @@ and [] member x.SetIsFixed() = x.val_flags <- x.val_flags.WithIsFixed + member x.SetIgnoresByrefScope() = x.val_flags <- x.val_flags.WithIgnoresByrefScope + member x.SetValReprInfo info = match x.val_opt_data with | Some optData -> optData.val_repr_info <- info @@ -3551,12 +3582,18 @@ and /// Set the on-demand analysis about whether the entity has the IsByRefLike attribute member x.SetIsByRefLike b = x.Deref.SetIsByRefLike b - /// The on-demand analysis about whether the entity has the IsByRefLike attribute + /// The on-demand analysis about whether the entity has the IsReadOnly attribute member x.TryIsReadOnly = x.Deref.TryIsReadOnly - /// Set the on-demand analysis about whether the entity has the IsReadOnly attribute or is otherwise determined to be a readonly struct + /// Set the on-demand analysis about whether the entity has the IsReadOnly attribute member x.SetIsReadOnly b = x.Deref.SetIsReadOnly b + /// The on-demand analysis about whether the entity is assumed to be a readonly struct + member x.TryIsAssumedReadOnly = x.Deref.TryIsAssumedReadOnly + + /// Set the on-demand analysis about whether the entity is assumed to be a readonly struct + member x.SetIsAssumedReadOnly b = x.Deref.SetIsAssumedReadOnly b + /// Indicates if this is an F# type definition whose r.h.s. definition is unknown (i.e. a traditional ML 'abstract' type in a signature, /// which in F# is called a 'unknown representation' type). member x.IsHiddenReprTycon = x.Deref.IsHiddenReprTycon diff --git a/tests/fsharp/Compiler/CompilerAssert.fs b/tests/fsharp/Compiler/CompilerAssert.fs index 89bc15f5f..61058df1e 100644 --- a/tests/fsharp/Compiler/CompilerAssert.fs +++ b/tests/fsharp/Compiler/CompilerAssert.fs @@ -108,11 +108,11 @@ let main argv = 0""" ProjectId = None SourceFiles = [|"test.fs"|] #if !NETCOREAPP - OtherOptions = [|"--preferreduilang:en-US";|] + OtherOptions = [|"--preferreduilang:en-US";"--warn:5"|] #else OtherOptions = let assemblies = getNetCoreAppReferences |> Array.map (fun x -> sprintf "-r:%s" x) - Array.append [|"--preferreduilang:en-US"; "--targetprofile:netcore"; "--noframework"|] assemblies + Array.append [|"--preferreduilang:en-US"; "--targetprofile:netcore"; "--noframework";"--warn:5"|] assemblies #endif ReferencedProjects = [||] IsIncompleteTypeCheckEnvironment = false diff --git a/tests/fsharp/Compiler/Language/ByrefTests.fs b/tests/fsharp/Compiler/Language/ByrefTests.fs new file mode 100644 index 000000000..2797125d7 --- /dev/null +++ b/tests/fsharp/Compiler/Language/ByrefTests.fs @@ -0,0 +1,198 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Compiler.UnitTests + +open NUnit.Framework +open FSharp.Compiler.SourceCodeServices + +[] +module ByrefTests = + + [] + let ``No defensive copy on .NET struct`` () = + CompilerAssert.Pass + """ +open System +open System.Runtime.CompilerServices + +let f (x: DateTime) = x.ToLocalTime() +let f2 () = + let x = DateTime.Now + x.ToLocalTime() + +[] +type Extensions = + + [] + static member Test(x: inref) = &x + + [] + static member Test2(x: byref) = &x + +let test (x: inref) = + x.Test() + +let test2 (x: byref) = + x.Test2() + +let test3 (x: byref) = + x.Test() + +let test4 () = + DateTime.Now.Test() + +let test5 (x: inref) = + &x.Test() + +let test6 () = + DateTime.Now.Test().Test().Test() + """ + + [] + let ``Extension method scope errors`` () = + CompilerAssert.TypeCheckWithErrors + """ +open System +open System.Runtime.CompilerServices + +[] +type Extensions = + + [] + static member Test(x: inref) = &x + +let f1 () = + &DateTime.Now.Test() + +let f2 () = + let result = + let dt = DateTime.Now + &dt.Test() + result + +let f3 () = + Extensions.Test(let dt = DateTime.Now in &dt) + +let f4 () = + let dt = DateTime.Now + &Extensions.Test(&dt) + +let f5 () = + &Extensions.Test(let dt = DateTime.Now in &dt) + """ + [| + ( + FSharpErrorSeverity.Error, + 3228, + (12, 6, 12, 25), + "The address of a value returned from the expression cannot be used at this point. This is to ensure the address of the local value does not escape its scope." + ) + ( + FSharpErrorSeverity.Error, + 3228, + (17, 10, 17, 19), + "The address of a value returned from the expression cannot be used at this point. This is to ensure the address of the local value does not escape its scope." + ) + ( + FSharpErrorSeverity.Error, + 3228, + (21, 5, 21, 50), + "The address of a value returned from the expression cannot be used at this point. This is to ensure the address of the local value does not escape its scope." + ) + ( + FSharpErrorSeverity.Error, + 3228, + (25, 6, 25, 26), + "The address of a value returned from the expression cannot be used at this point. This is to ensure the address of the local value does not escape its scope." + ) + ( + FSharpErrorSeverity.Error, + 3228, + (28, 6, 28, 51), + "The address of a value returned from the expression cannot be used at this point. This is to ensure the address of the local value does not escape its scope." + ) + |] + +// TODO: A better way to test the ones below are to use a custom struct in C# code that contains explicit use of their "readonly" keyword. +#if NETCOREAPP + // NETCORE makes DateTime a readonly struct; therefore, it should not error. + [] + let ``No defensive copy on .NET struct - netcore`` () = + CompilerAssert.Pass + """ +open System +let f (x: inref) = x.ToLocalTime() +let f2 () = + let x = DateTime.Now + let y = &x + y.ToLocalTime() +let f3 (x: inref) = &x +let f4 (x: inref) = + (f3 &x).ToLocalTime() + +open System.Runtime.CompilerServices +[] +type Extensions = + + [] + static member Test(x: inref) = &x + +let test1 () = + DateTime.Now.Test().Date + +let test2 () = + DateTime.Now.Test().Test().Date.Test().Test().Date.Test() + """ +#else + // Note: Currently this is assuming NET472. That may change which might break these tests. Consider using custom C# code. + [] + let ``Defensive copy on .NET struct for inref`` () = + CompilerAssert.TypeCheckWithErrors + """ +open System +let f (x: inref) = x.ToLocalTime() +let f2 () = + let x = DateTime.Now + let y = &x + y.ToLocalTime() +let f3 (x: inref) = &x +let f4 (x: inref) = + (f3 &x).ToLocalTime() + +open System.Runtime.CompilerServices +[] +type Extensions = + + [] + static member Test(x: inref) = &x + +let test1 () = + DateTime.Now.Test().Date + """ + [| + ( + FSharpErrorSeverity.Warning, + 52, + (3, 30, 3, 45), + "The value has been copied to ensure the original is not mutated by this operation or because the copy is implicit when returning a struct from a member and another member is then accessed" + ) + ( + FSharpErrorSeverity.Warning, + 52, + (7, 5, 7, 20), + "The value has been copied to ensure the original is not mutated by this operation or because the copy is implicit when returning a struct from a member and another member is then accessed" + ) + ( + FSharpErrorSeverity.Warning, + 52, + (10, 5, 10, 26), + "The value has been copied to ensure the original is not mutated by this operation or because the copy is implicit when returning a struct from a member and another member is then accessed" + ) + ( + FSharpErrorSeverity.Warning, + 52, + (20, 5, 20, 29), + "The value has been copied to ensure the original is not mutated by this operation or because the copy is implicit when returning a struct from a member and another member is then accessed" + ) + |] +#endif \ No newline at end of file diff --git a/tests/fsharp/FSharpSuite.Tests.fsproj b/tests/fsharp/FSharpSuite.Tests.fsproj index 1278cbe0b..79adf613e 100644 --- a/tests/fsharp/FSharpSuite.Tests.fsproj +++ b/tests/fsharp/FSharpSuite.Tests.fsproj @@ -51,6 +51,7 @@ + diff --git a/tests/fsharp/core/fsfromfsviacs/lib2.cs b/tests/fsharp/core/fsfromfsviacs/lib2.cs index eb6d6acc1..449f926d7 100644 --- a/tests/fsharp/core/fsfromfsviacs/lib2.cs +++ b/tests/fsharp/core/fsfromfsviacs/lib2.cs @@ -124,4 +124,17 @@ public class ApiWrapper public static Func f4 = new Func((int arg1, string arg2, byte arg3, sbyte arg4) => arg1 + arg2.Length + 1 + arg3 + arg4); public static Func f5 = new Func((int arg1, string arg2, byte arg3, sbyte arg4, Int16 arg5) => arg1 + arg2.Length + 1 + arg3 + arg4 + arg5); } +} + +namespace StructTests +{ + public struct NonReadOnlyStruct + { + public int X { get; set; } + + public void M(int x) + { + X = x; + } + } } \ No newline at end of file diff --git a/tests/fsharp/core/fsfromfsviacs/test.fsx b/tests/fsharp/core/fsfromfsviacs/test.fsx index 072a896c1..47e2e3597 100644 --- a/tests/fsharp/core/fsfromfsviacs/test.fsx +++ b/tests/fsharp/core/fsfromfsviacs/test.fsx @@ -210,6 +210,75 @@ let ToFSharpFunc() = test "vkejhwew904" (FuncConvert.FromFunc(FSharpFuncTests.ApiWrapper.f4)(3)("a")(6uy)(7y) = FSharpFuncTests.ApiWrapper.f4.Invoke(3, "a", 6uy, 7y)) test "vkejhwew905" (FuncConvert.FromFunc(FSharpFuncTests.ApiWrapper.f5)(3)("a")(6uy)(7y)(7s) = FSharpFuncTests.ApiWrapper.f5.Invoke(3, "a", 6uy, 7y, 7s)) +module TestStructs = + open StructTests + + let someFunc (s: NonReadOnlyStruct) = + s.M(456) + s.X + + let someByrefFunc (s: byref) = + s.M(456) + s.X + + let someInrefFunc (s: inref) = + s.M(456) + s.X + + let someFuncReturn (s: NonReadOnlyStruct) = + s.X + + let someInrefFuncReturn (s: inref) = + s.X + + let test1 () = + let s = NonReadOnlyStruct() + check "hdlcjiklhen1" s.X 0 + s.M(123) + check "hdlcjiklhen2" s.X 123 + check "hdlcjiklhen3" (someFunc s) 456 + check "hdlcjiklhen4" s.X 123 + + + let test2 () = + let mutable s = NonReadOnlyStruct() + check "hdlcjiklhen5" s.X 0 + s.M(123) + check "hdlcjiklhen6" s.X 123 + check "hdlcjiklhen7" (someByrefFunc &s) 456 + check "hdlcjiklhen8" s.X 456 + + + let test3 () = + let s = NonReadOnlyStruct() + check "hdlcjiklhen9" s.X 0 + s.M(123) + check "hdlcjiklhen10" s.X 123 + check "hdlcjiklhen11" (someInrefFunc &s) 123 + check "hdlcjiklhen12" s.X 123 + + let test4 () = + let s = NonReadOnlyStruct() + check "hdlcjiklhen13" s.X 0 + s.M(123) + check "hdlcjiklhen14" s.X 123 + check "hdlcjiklhen15" (someFuncReturn s) 0 // Technically a bug today, but test is to verify current behavior. + check "hdlcjiklhen16" s.X 123 + + let test5 () = + let s = NonReadOnlyStruct() + check "hdlcjiklhen17" s.X 0 + s.M(123) + check "hdlcjiklhen18" s.X 123 + check "hdlcjiklhen19" (someInrefFuncReturn &s) 123 + check "hdlcjiklhen20" s.X 123 + +TestStructs.test1 () +TestStructs.test2 () +TestStructs.test3 () +TestStructs.test4 () +TestStructs.test5 () + #endif #if TESTS_AS_APP -- GitLab