Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added inref immutability assumption removal #7407

Merged
merged 22 commits into from
Aug 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/fsharp/MethodCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/fsharp/PostInferenceChecks.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 46 additions & 19 deletions src/fsharp/TastOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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<T> as a result

Expand Down Expand Up @@ -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 &&
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/TastOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 3 additions & 6 deletions src/fsharp/TypeChecker.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
29 changes: 28 additions & 1 deletion src/fsharp/infos.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 [<IsReadOnly>] 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.
Expand Down Expand Up @@ -1238,6 +1246,25 @@ type MethInfo =
member x.IsStruct =
isStructTy x.TcGlobals x.ApparentEnclosingType

/// Indicates if this method is read-only; usually by the [<IsReadOnly>] 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
Expand Down
Loading