Skip to content
This repository was archived by the owner on Jan 3, 2023. It is now read-only.

Commit ac59b22

Browse files
TIHanTIHan
authored
Fixed extension members that extend a type using a byref to be called with an immutable value (#5447)
* Fixed extension members that extend a type using a byref to be called with an immutable value * Added more tests, but no updated baseline yet * Updated baselines * Minor cleanup * Fixed tests. Fixed output of tests * Updated buildfromsource * Minor cleanup
1 parent ce72df2 commit ac59b22

27 files changed

Lines changed: 285 additions & 55 deletions

‎src/buildfromsource/FSharp.Compiler.Private/FSComp.fs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4327,33 +4327,33 @@ type internal SR private() =
43274327
/// 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.
43284328
/// (Originally from ..\FSComp.txt:1433)
43294329
static member chkNoByrefAddressOfValueFromExpression() = (3228, GetStringFunc("chkNoByrefAddressOfValueFromExpression",",,,") )
4330-
/// The Span or IsByRefLike expression cannot be returned from this function or method, because it is composed using elements that may escape their scope.
4331-
/// (Originally from ..\FSComp.txt:1434)
4332-
static member chkNoReturnOfLimitedSpan() = (3229, GetStringFunc("chkNoReturnOfLimitedSpan",",,,") )
43334330
/// This value can't be assigned because the target '%s' may refer to non-stack-local memory, while the expression being assigned is assessed to potentially refer to stack-local memory. This is to help prevent pointers to stack-bound memory escaping their scope.
4334-
/// (Originally from ..\FSComp.txt:1435)
4335-
static member chkNoWriteToLimitedSpan(a0 : System.String) = (3230, GetStringFunc("chkNoWriteToLimitedSpan",",,,%s,,,") a0)
4331+
/// (Originally from ..\FSComp.txt:1434)
4332+
static member chkNoWriteToLimitedSpan(a0 : System.String) = (3229, GetStringFunc("chkNoWriteToLimitedSpan",",,,%s,,,") a0)
43364333
/// A value defined in a module must be mutable in order to take its address, e.g. 'let mutable x = ...'
4337-
/// (Originally from ..\FSComp.txt:1436)
4338-
static member tastValueMustBeLocal() = (3231, GetStringFunc("tastValueMustBeLocal",",,,") )
4334+
/// (Originally from ..\FSComp.txt:1435)
4335+
static member tastValueMustBeLocal() = (3230, GetStringFunc("tastValueMustBeLocal",",,,") )
43394336
/// A type annotated with IsReadOnly must also be a struct. Consider adding the [<Struct>] attribute to the type.
4337+
/// (Originally from ..\FSComp.txt:1436)
4338+
static member tcIsReadOnlyNotStruct() = (3231, GetStringFunc("tcIsReadOnlyNotStruct",",,,") )
4339+
/// Struct members cannot return the address of fields of the struct by reference
43404340
/// (Originally from ..\FSComp.txt:1437)
4341-
static member tcIsReadOnlyNotStruct() = (3232, GetStringFunc("tcIsReadOnlyNotStruct",",,,") )
4342-
/// Struct members cannot return 'this' or fields by reference
4343-
/// (Originally from ..\FSComp.txt:1438)
4344-
static member chkStructsMayNotReturnAddressesOfContents() = (3234, GetStringFunc("chkStructsMayNotReturnAddressesOfContents",",,,") )
4341+
static member chkStructsMayNotReturnAddressesOfContents() = (3232, GetStringFunc("chkStructsMayNotReturnAddressesOfContents",",,,") )
43454342
/// The function or method call cannot be used at this point, because one argument that is a byref of a non-stack-local Span or IsByRefLike type is used with another argument that is a stack-local Span or IsByRefLike type. This is to ensure the address of the local value does not escape its scope.
4346-
/// (Originally from ..\FSComp.txt:1439)
4347-
static member chkNoByrefLikeFunctionCall() = (3235, GetStringFunc("chkNoByrefLikeFunctionCall",",,,") )
4343+
/// (Originally from ..\FSComp.txt:1438)
4344+
static member chkNoByrefLikeFunctionCall() = (3233, GetStringFunc("chkNoByrefLikeFunctionCall",",,,") )
43484345
/// The Span or IsByRefLike variable '%s' cannot be used at this point. This is to ensure the address of the local value does not escape its scope.
4349-
/// (Originally from ..\FSComp.txt:1440)
4350-
static member chkNoSpanLikeVariable(a0 : System.String) = (3236, GetStringFunc("chkNoSpanLikeVariable",",,,%s,,,") a0)
4346+
/// (Originally from ..\FSComp.txt:1439)
4347+
static member chkNoSpanLikeVariable(a0 : System.String) = (3234, GetStringFunc("chkNoSpanLikeVariable",",,,%s,,,") a0)
43514348
/// A Span or IsByRefLike value returned from the expression cannot be used at ths point. This is to ensure the address of the local value does not escape its scope.
4352-
/// (Originally from ..\FSComp.txt:1441)
4353-
static member chkNoSpanLikeValueFromExpression() = (3237, GetStringFunc("chkNoSpanLikeValueFromExpression",",,,") )
4349+
/// (Originally from ..\FSComp.txt:1440)
4350+
static member chkNoSpanLikeValueFromExpression() = (3235, GetStringFunc("chkNoSpanLikeValueFromExpression",",,,") )
43544351
/// Cannot take the address of the value returned from the expression. Assign the returned value to a let-bound value before taking the address.
4352+
/// (Originally from ..\FSComp.txt:1441)
4353+
static member tastCantTakeAddressOfExpression() = (3236, GetStringFunc("tastCantTakeAddressOfExpression",",,,") )
4354+
/// Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref.
43554355
/// (Originally from ..\FSComp.txt:1442)
4356-
static member tastCantTakeAddressOfExpression() = (3238, GetStringFunc("tastCantTakeAddressOfExpression",",,,") )
4356+
static member tcCannotCallExtensionMemberInrefToByref() = (3237, GetStringFunc("tcCannotCallExtensionMemberInrefToByref",",,,") )
43574357

43584358
/// Call this method once to validate that all known resources are valid; throws if not
43594359
static member RunStartupValidation() =
@@ -5761,7 +5761,6 @@ type internal SR private() =
57615761
ignore(GetString("tcByrefReturnImplicitlyDereferenced"))
57625762
ignore(GetString("tcByRefLikeNotStruct"))
57635763
ignore(GetString("chkNoByrefAddressOfValueFromExpression"))
5764-
ignore(GetString("chkNoReturnOfLimitedSpan"))
57655764
ignore(GetString("chkNoWriteToLimitedSpan"))
57665765
ignore(GetString("tastValueMustBeLocal"))
57675766
ignore(GetString("tcIsReadOnlyNotStruct"))
@@ -5770,4 +5769,5 @@ type internal SR private() =
57705769
ignore(GetString("chkNoSpanLikeVariable"))
57715770
ignore(GetString("chkNoSpanLikeValueFromExpression"))
57725771
ignore(GetString("tastCantTakeAddressOfExpression"))
5772+
ignore(GetString("tcCannotCallExtensionMemberInrefToByref"))
57735773
()

‎src/buildfromsource/FSharp.Compiler.Private/FSComp.resx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4330,9 +4330,6 @@
43304330
<data name="chkNoByrefAddressOfValueFromExpression" xml:space="preserve">
43314331
<value>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.</value>
43324332
</data>
4333-
<data name="chkNoReturnOfLimitedSpan" xml:space="preserve">
4334-
<value>The Span or IsByRefLike expression cannot be returned from this function or method, because it is composed using elements that may escape their scope.</value>
4335-
</data>
43364333
<data name="chkNoWriteToLimitedSpan" xml:space="preserve">
43374334
<value>This value can't be assigned because the target '{0}' may refer to non-stack-local memory, while the expression being assigned is assessed to potentially refer to stack-local memory. This is to help prevent pointers to stack-bound memory escaping their scope.</value>
43384335
</data>
@@ -4343,7 +4340,7 @@
43434340
<value>A type annotated with IsReadOnly must also be a struct. Consider adding the [&lt;Struct&gt;] attribute to the type.</value>
43444341
</data>
43454342
<data name="chkStructsMayNotReturnAddressesOfContents" xml:space="preserve">
4346-
<value>Struct members cannot return 'this' or fields by reference</value>
4343+
<value>Struct members cannot return the address of fields of the struct by reference</value>
43474344
</data>
43484345
<data name="chkNoByrefLikeFunctionCall" xml:space="preserve">
43494346
<value>The function or method call cannot be used at this point, because one argument that is a byref of a non-stack-local Span or IsByRefLike type is used with another argument that is a stack-local Span or IsByRefLike type. This is to ensure the address of the local value does not escape its scope.</value>
@@ -4357,4 +4354,7 @@
43574354
<data name="tastCantTakeAddressOfExpression" xml:space="preserve">
43584355
<value>Cannot take the address of the value returned from the expression. Assign the returned value to a let-bound value before taking the address.</value>
43594356
</data>
4357+
<data name="tcCannotCallExtensionMemberInrefToByref" xml:space="preserve">
4358+
<value>Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref.</value>
4359+
</data>
43604360
</root>

‎src/fsharp/FSComp.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1438,4 +1438,5 @@ notAFunctionButMaybeDeclaration,"This value is not a function and cannot be appl
14381438
3233,chkNoByrefLikeFunctionCall,"The function or method call cannot be used at this point, because one argument that is a byref of a non-stack-local Span or IsByRefLike type is used with another argument that is a stack-local Span or IsByRefLike type. This is to ensure the address of the local value does not escape its scope."
14391439
3234,chkNoSpanLikeVariable,"The Span or IsByRefLike variable '%s' cannot be used at this point. This is to ensure the address of the local value does not escape its scope."
14401440
3235,chkNoSpanLikeValueFromExpression,"A Span or IsByRefLike value returned from the expression cannot be used at ths point. This is to ensure the address of the local value does not escape its scope."
1441-
3236,tastCantTakeAddressOfExpression,"Cannot take the address of the value returned from the expression. Assign the returned value to a let-bound value before taking the address."
1441+
3236,tastCantTakeAddressOfExpression,"Cannot take the address of the value returned from the expression. Assign the returned value to a let-bound value before taking the address."
1442+
3237,tcCannotCallExtensionMemberInrefToByref,"Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref."

‎src/fsharp/MethodCalls.fs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ let TakeObjAddrForMethodCall g amap (minfo:MethInfo) isMutable m objArgs f =
585585
let hasCallInfo = ccallInfo.IsSome
586586
let mustTakeAddress = hasCallInfo || minfo.ObjArgNeedsAddress(amap, m)
587587
let objArgTy = tyOfExpr g objArgExpr
588-
let wrap, objArgExpr', _readonly, _writeonly = mkExprAddrOfExpr g mustTakeAddress hasCallInfo isMutable objArgExpr None m
588+
let wrap, objArgExpr', isReadOnly, _isWriteOnly = mkExprAddrOfExpr g mustTakeAddress hasCallInfo isMutable objArgExpr None m
589589

590590
// Extension members and calls to class constraints may need a coercion for their object argument
591591
let objArgExpr' =
@@ -595,6 +595,32 @@ let TakeObjAddrForMethodCall g amap (minfo:MethInfo) isMutable m objArgs f =
595595
else
596596
objArgExpr'
597597

598+
// Check to see if the extension member uses the extending type as a byref.
599+
// If so, make sure we don't allow readonly/immutable values to be passed byref from an extension member.
600+
// An inref will work though.
601+
if mustTakeAddress && isReadOnly && minfo.IsExtensionMember then
602+
let tyOpt =
603+
match minfo with
604+
// For F# defined methods.
605+
| FSMeth(_, _, vref, _) ->
606+
let ty, _ = destFunTy g vref.Type
607+
Some(ty)
608+
609+
// For IL methods, defined outside of F#.
610+
| ILMeth(_, info, _) ->
611+
let paramTypes = info.GetRawArgTypes(amap, m, minfo.FormalMethodInst)
612+
match paramTypes with
613+
| [] -> failwith "impossible"
614+
| ty :: _ -> Some(ty)
615+
616+
| _ -> None
617+
618+
match tyOpt with
619+
| Some(ty) ->
620+
if isByrefTy g ty && not (isInByrefTy g ty) then
621+
errorR(Error(FSComp.SR.tcCannotCallExtensionMemberInrefToByref(), m))
622+
| _ -> ()
623+
598624
wrap, [objArgExpr']
599625

600626
| _ ->

‎src/fsharp/import.fs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ let rec ImportILType (env:ImportMap) m tinst ty =
168168
let inst = tspec.GenericArgs |> List.map (ImportILType env m tinst)
169169
ImportTyconRefApp env tcref inst
170170

171-
| ILType.Modified(_,tref,ILType.Byref ty) when tref.Name = "System.Runtime.InteropServices.InAttribute" -> mkInByrefTy env.g (ImportILType env m tinst ty)
172171
| ILType.Byref ty -> mkByrefTy env.g (ImportILType env m tinst ty)
173172
| ILType.Ptr ILType.Void when env.g.voidptr_tcr.CanDeref -> mkVoidPtrTy env.g
174173
| ILType.Ptr ty -> mkNativePtrTy env.g (ImportILType env m tinst ty)

‎src/fsharp/infos.fs‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,26 @@ let ExistsHeadTypeInEntireHierarchy g amap m typeToSearchFrom tcrefToLookFor =
284284
let ImportILTypeFromMetadata amap m scoref tinst minst ilty =
285285
ImportILType scoref amap m (tinst@minst) ilty
286286

287-
288-
/// Get the return type of an IL method, taking into account instantiations for type and method generic parameters, and
287+
/// Read an Abstract IL type from metadata, including any attributes that may affect the type itself, and convert to an F# type.
288+
let ImportILTypeFromMetadataWithAttributes amap m scoref tinst minst ilty cattrs =
289+
let ty = ImportILType scoref amap m (tinst@minst) ilty
290+
// If the type is a byref and one of attributes from a return or parameter has IsReadOnly, then it's a inref.
291+
if isByrefTy amap.g ty && TryFindILAttribute amap.g.attrib_IsReadOnlyAttribute cattrs then
292+
mkInByrefTy amap.g (destByrefTy amap.g ty)
293+
else
294+
ty
295+
296+
/// Get the parameter type of an IL method.
297+
let ImportParameterTypeFromMetadata amap m ilty cattrs scoref tinst mist =
298+
ImportILTypeFromMetadataWithAttributes amap m scoref tinst mist ilty cattrs
299+
300+
/// Get the return type of an IL method, taking into account instantiations for type, return attributes and method generic parameters, and
289301
/// translating 'void' to 'None'.
290-
let ImportReturnTypeFromMetaData amap m ty scoref tinst minst =
291-
match ty with
302+
let ImportReturnTypeFromMetadata amap m ilty cattrs scoref tinst minst =
303+
match ilty with
292304
| ILType.Void -> None
293-
| retTy -> Some (ImportILTypeFromMetadata amap m scoref tinst minst retTy)
305+
| retTy -> Some(ImportILTypeFromMetadataWithAttributes amap m scoref tinst minst retTy cattrs)
306+
294307

295308
/// Copy constraints. If the constraint comes from a type parameter associated
296309
/// with a type constructor then we are simply renaming type variables. If it comes
@@ -806,19 +819,19 @@ type ILMethInfo =
806819
/// Get the argument types of the the IL method. If this is an C#-style extension method
807820
/// then drop the object argument.
808821
member x.GetParamTypes(amap,m,minst) =
809-
x.ParamMetadata |> List.map (fun p -> ImportILTypeFromMetadata amap m x.MetadataScope x.DeclaringTypeInst minst p.Type)
822+
x.ParamMetadata |> List.map (fun p -> ImportParameterTypeFromMetadata amap m p.Type p.CustomAttrs x.MetadataScope x.DeclaringTypeInst minst)
810823

811824
/// Get all the argument types of the IL method. Include the object argument even if this is
812825
/// an C#-style extension method.
813826
member x.GetRawArgTypes(amap,m,minst) =
814-
x.RawMetadata.Parameters |> List.map (fun p -> ImportILTypeFromMetadata amap m x.MetadataScope x.DeclaringTypeInst minst p.Type)
827+
x.RawMetadata.Parameters |> List.map (fun p -> ImportParameterTypeFromMetadata amap m p.Type p.CustomAttrs x.MetadataScope x.DeclaringTypeInst minst)
815828

816829
/// Get info about the arguments of the IL method. If this is an C#-style extension method then
817830
/// drop the object argument.
818831
///
819832
/// Any type parameters of the enclosing type are instantiated in the type returned.
820833
member x.GetParamNamesAndTypes(amap,m,minst) =
821-
x.ParamMetadata |> List.map (fun p -> ParamNameAndType(Option.map (mkSynId m) p.Name, ImportILTypeFromMetadata amap m x.MetadataScope x.DeclaringTypeInst minst p.Type) )
834+
x.ParamMetadata |> List.map (fun p -> ParamNameAndType(Option.map (mkSynId m) p.Name, ImportParameterTypeFromMetadata amap m p.Type p.CustomAttrs x.MetadataScope x.DeclaringTypeInst minst) )
822835

823836
/// Get a reference to the method (dropping all generic instantiations), as an Abstract IL ILMethodRef.
824837
member x.ILMethodRef =
@@ -838,15 +851,16 @@ type ILMethInfo =
838851
// All C#-style extension methods are instance. We have to re-read the 'obj' type w.r.t. the
839852
// method instantiation.
840853
if x.IsILExtensionMethod then
841-
[ImportILTypeFromMetadata amap m x.MetadataScope x.DeclaringTypeInst minst x.RawMetadata.Parameters.Head.Type]
854+
let p = x.RawMetadata.Parameters.Head
855+
[ ImportParameterTypeFromMetadata amap m p.Type p.CustomAttrs x.MetadataScope x.DeclaringTypeInst minst ]
842856
else if x.IsInstance then
843857
[ x.ApparentEnclosingType ]
844858
else
845859
[]
846860

847861
/// Get the compiled return type of the method, where 'void' is None.
848862
member x.GetCompiledReturnTy (amap, m, minst) =
849-
ImportReturnTypeFromMetaData amap m x.RawMetadata.Return.Type x.MetadataScope x.DeclaringTypeInst minst
863+
ImportReturnTypeFromMetadata amap m x.RawMetadata.Return.Type x.RawMetadata.Return.CustomAttrs x.MetadataScope x.DeclaringTypeInst minst
850864

851865
/// Get the F# view of the return type of the method, where 'void' is 'unit'.
852866
member x.GetFSharpReturnTy (amap, m, minst) =
@@ -1503,7 +1517,7 @@ type MethInfo =
15031517
match x with
15041518
| ILMeth(_,ilminfo,_) ->
15051519
let ftinfo = ILTypeInfo.FromType g (TType_app(tcref,formalEnclosingTyparTys))
1506-
let formalRetTy = ImportReturnTypeFromMetaData amap m ilminfo.RawMetadata.Return.Type ftinfo.ILScopeRef ftinfo.TypeInstOfRawMetadata formalMethTyparTys
1520+
let formalRetTy = ImportReturnTypeFromMetadata amap m ilminfo.RawMetadata.Return.Type ilminfo.RawMetadata.Return.CustomAttrs ftinfo.ILScopeRef ftinfo.TypeInstOfRawMetadata formalMethTyparTys
15071521
let formalParams =
15081522
[ [ for p in ilminfo.RawMetadata.Parameters do
15091523
let paramType = ImportILTypeFromMetadata amap m ftinfo.ILScopeRef ftinfo.TypeInstOfRawMetadata formalMethTyparTys p.Type

‎src/fsharp/symbols/Exprs.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1033,7 +1033,7 @@ module FSharpExprConvert =
10331033
// is not sufficient to resolve to a symbol unambiguously in these cases.
10341034
let argtys = [ ilMethRef.ArgTypes |> List.map (ImportILTypeFromMetadata cenv.amap m scoref tinst1 tinst2) ]
10351035
let rty =
1036-
match ImportReturnTypeFromMetaData cenv.amap m ilMethRef.ReturnType scoref tinst1 tinst2 with
1036+
match ImportReturnTypeFromMetadata cenv.amap m ilMethRef.ReturnType emptyILCustomAttrs scoref tinst1 tinst2 with
10371037
| None -> if isCtor then enclosingType else cenv.g.unit_ty
10381038
| Some ty -> ty
10391039

‎src/fsharp/xlf/FSComp.txt.cs.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7057,6 +7057,11 @@
70577057
<target state="translated">Adresa hodnoty vrácená výrazem nejde převzít. Před převzetím adresy přiřaďte vrácenou hodnotu hodnotě s vazbou na klauzuli Let.</target>
70587058
<note />
70597059
</trans-unit>
7060+
<trans-unit id="tcCannotCallExtensionMemberInrefToByref">
7061+
<source>Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref.</source>
7062+
<target state="new">Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref.</target>
7063+
<note />
7064+
</trans-unit>
70607065
</body>
70617066
</file>
70627067
</xliff>

‎src/fsharp/xlf/FSComp.txt.de.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7057,6 +7057,11 @@
70577057
<target state="translated">Die Adresse des über den Ausdruck zurückgegebenen Werts kann nicht abgerufen werden. Weisen Sie den zurückgegebenen Wert einem let-bound-Wert zu, bevor Sie die Adresse abrufen.</target>
70587058
<note />
70597059
</trans-unit>
7060+
<trans-unit id="tcCannotCallExtensionMemberInrefToByref">
7061+
<source>Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref.</source>
7062+
<target state="new">Cannot call the extension member as it requires the value to be mutable or a byref type due to the extending type being used as a byref.</target>
7063+
<note />
7064+
</trans-unit>
70607065
</body>
70617066
</file>
70627067
</xliff>

0 commit comments

Comments
 (0)