Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9eedfb7
WIP: Added init-only properties error + added some tests
vzarytovskii Jul 11, 2022
e2d9d68
WIP: Separate tests
vzarytovskii Jul 11, 2022
2a05ed2
Added check for explicit setters
vzarytovskii Jul 13, 2022
8a7d48c
Fix tests
vzarytovskii Jul 13, 2022
bdcab6b
Forbit calling setters on the methods
vzarytovskii Jul 14, 2022
63fc2fb
wip: support required attributes
vzarytovskii Jul 14, 2022
b1ffb44
Added required properties support + apply fantomas
vzarytovskii Jul 15, 2022
bb8b77a
Merge branch 'main' into required-and-init
vzarytovskii Jul 15, 2022
4c7029f
Merge remote-tracking branch 'upstream/main' into required-and-init
vzarytovskii Jul 22, 2022
2681745
Moved compilerfeaturerequired check to a method
vzarytovskii Jul 22, 2022
81da22c
Updated
vzarytovskii Jul 22, 2022
662186f
fantomas
vzarytovskii Jul 22, 2022
005af55
Internalize empty
vzarytovskii Jul 22, 2022
5784869
Add setter check + add more unsupported attributes
vzarytovskii Jul 25, 2022
bf2ae5e
Merge remote-tracking branch 'upstream/main' into required-and-init
vzarytovskii Jul 25, 2022
cc928ef
Fix prop pages
vzarytovskii Jul 25, 2022
0ef72fd
Update src/Compiler/Checking/CheckExpressions.fs
vzarytovskii Jul 28, 2022
07762ca
Addressed PR comments + added additional tests
vzarytovskii Jul 28, 2022
4051bdc
Merge remote-tracking branch 'upstream/main' into required-and-init
vzarytovskii Jul 28, 2022
3bc625b
Update src/Compiler/Checking/infos.fs
vzarytovskii Jul 28, 2022
66407b2
Moved condition outside of list expression
vzarytovskii Jul 28, 2022
764c158
PR fixes
vzarytovskii Jul 28, 2022
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
5 changes: 3 additions & 2 deletions src/Compiler/AbstractIL/il.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1203,10 +1203,11 @@ type ILAttribute =

[<NoEquality; NoComparison; Struct>]
type ILAttributes(array: ILAttribute[]) =
member _.AsArray() = array

member x.AsArray() = array
member _.AsList() = array |> Array.toList

member x.AsList() = array |> Array.toList
static member val internal Empty = ILAttributes([||])

[<NoEquality; NoComparison>]
type ILAttributesStored =
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/AbstractIL/il.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,8 @@ type ILAttributes =

member AsList: unit -> ILAttribute list

static member internal Empty: ILAttributes

/// Represents the efficiency-oriented storage of ILAttributes in another item.
[<NoEquality; NoComparison>]
type ILAttributesStored
Expand Down
36 changes: 26 additions & 10 deletions src/Compiler/Checking/AttributeChecking.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ open FSharp.Compiler.TypeHierarchy
#if !NO_TYPEPROVIDERS
open FSharp.Compiler.TypeProviders
open FSharp.Core.CompilerServices
open Features

#endif

exception ObsoleteWarning of string * range
Expand Down Expand Up @@ -229,22 +231,36 @@ let MethInfoHasAttribute g m attribSpec minfo =
|> Option.isSome


let private CheckCompilerFeatureRequiredAttribute (g: TcGlobals) cattrs msg m =
// In some cases C# will generate both ObsoleteAttribute and CompilerFeatureRequiredAttribute.
// Specifically, when default constructor is generated for class with any reqired members in them.
// ObsoleteAttribute should be ignored if CompilerFeatureRequiredAttribute is present, and its name is "RequiredMembers".
let (AttribInfo(tref,_)) = g.attrib_CompilerFeatureRequiredAttribute
match TryDecodeILAttribute tref cattrs with
| Some([ILAttribElem.String (Some featureName) ], _) when featureName = "RequiredMembers" ->
CompleteD
| _ ->
ErrorD (ObsoleteError(msg, m))

/// Check IL attributes for 'ObsoleteAttribute', returning errors and warnings as data
let private CheckILAttributes (g: TcGlobals) isByrefLikeTyconRef cattrs m =
let private CheckILAttributes (g: TcGlobals) isByrefLikeTyconRef cattrs m =
let (AttribInfo(tref,_)) = g.attrib_SystemObsolete
match TryDecodeILAttribute tref cattrs with
| Some ([ILAttribElem.String (Some msg) ], _) when not isByrefLikeTyconRef ->
match TryDecodeILAttribute tref cattrs with
| Some ([ILAttribElem.String (Some msg) ], _) when not isByrefLikeTyconRef ->
WarnD(ObsoleteWarning(msg, m))
| Some ([ILAttribElem.String (Some msg); ILAttribElem.Bool isError ], _) when not isByrefLikeTyconRef ->
if isError then
ErrorD (ObsoleteError(msg, m))
else
| Some ([ILAttribElem.String (Some msg); ILAttribElem.Bool isError ], _) when not isByrefLikeTyconRef ->
if isError then
if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) then
CheckCompilerFeatureRequiredAttribute g cattrs msg m
else
ErrorD (ObsoleteError(msg, m))
else
WarnD (ObsoleteWarning(msg, m))
| Some ([ILAttribElem.String None ], _) when not isByrefLikeTyconRef ->
| Some ([ILAttribElem.String None ], _) when not isByrefLikeTyconRef ->
WarnD(ObsoleteWarning("", m))
| Some _ when not isByrefLikeTyconRef ->
| Some _ when not isByrefLikeTyconRef ->
WarnD(ObsoleteWarning("", m))
| _ ->
| _ ->
CompleteD

let langVersionPrefix = "--langversion:preview"
Expand Down
56 changes: 54 additions & 2 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,44 @@ let CheckForAbnormalOperatorNames (cenv: cenv) (idRange: range) coreDisplayName
warning(StandardOperatorRedefinitionWarning(FSComp.SR.tcInvalidMemberNameFixedTypes opName, idRange))
| Other -> ()

let CheckInitProperties (g: TcGlobals) (minfo: MethInfo) methodName mItem =
if g.langVersion.SupportsFeature(LanguageFeature.InitPropertiesSupport) then
// Check, wheter this method has external init, emit an error diagnostic in this case.
if minfo.HasExternalInit then
errorR (Error (FSComp.SR.tcSetterForInitOnlyPropertyCannotBeCalled1 methodName, mItem))

let CheckRequiredProperties (g:TcGlobals) (env: TcEnv) (cenv: TcFileState) (minfo: MethInfo) finalAssignedItemSetters mMethExpr =
// Make sure, if apparent type has any required properties, they all are in the `finalAssignedItemSetters`.
// If it is a constructor, and it is not marked with `SetsRequiredMembersAttributeAttribute`, then:
// 1. Get all properties of the type.
// 2. Check if any of them has `IsRequired` set.
// 2.1. If there are none, proceed as usual
// 2.2. If there are any, make sure all of them (or their setters) are in `finalAssignedItemSetters`.
// 3. If some are missing, produce a diagnostic which missing ones.
if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport)
&& minfo.IsConstructor
&& not (TryFindILAttribute g.attrib_SetsRequiredMembersAttribute (minfo.GetCustomAttrs())) then

let requiredProps =
[
let props = GetImmediateIntrinsicPropInfosOfType (None, AccessibleFromSomeFSharpCode) g cenv.amap range0 minfo.ApparentEnclosingType
for prop in props do
if prop.IsRequired then
prop
]

if requiredProps.Length > 0 then
let setterPropNames =
finalAssignedItemSetters
|> List.choose (function | AssignedItemSetter(_, AssignedPropSetter (pinfo, _, _), _) -> Some pinfo.PropertyName | _ -> None)

let missingProps =
requiredProps
|> List.filter (fun pinfo -> not (List.contains pinfo.PropertyName setterPropNames))
if missingProps.Length > 0 then
let details = NicePrint.multiLineStringOfPropInfos g cenv.amap mMethExpr env.DisplayEnv missingProps
errorR(Error(FSComp.SR.tcMissingRequiredMembers details, mMethExpr))

let MakeAndPublishVal (cenv: cenv) env (altActualParent, inSig, declKind, valRecInfo, vscheme, attrs, xmlDoc, konst, isGeneratedEventVal) =

let g = cenv.g
Expand Down Expand Up @@ -8978,6 +9016,9 @@ and TcLookupThen cenv overallTy env tpenv mObjExpr objExpr objExprTy longId dela
// To get better warnings we special case some of the few known mutate-a-struct method names
let mutates = (if methodName = "MoveNext" || methodName = "GetNextArg" then DefinitelyMutates else PossiblyMutates)

// Check if we have properties with "init-only" setters, which we try to call after init is done.
CheckInitProperties g (List.head minfos) methodName mItem

#if !NO_TYPEPROVIDERS
match TryTcMethodAppToStaticConstantArgs cenv env tpenv (minfos, tyArgsOpt, mExprAndItem, mItem) with
| Some minfoAfterStaticArguments ->
Expand Down Expand Up @@ -9024,6 +9065,10 @@ and TcLookupThen cenv overallTy env tpenv mObjExpr objExpr objExprTy longId dela
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mExprAndItem mItem nm ad PossiblyMutates true meths afterResolution NormalValUse args atomicFlag delayed
else

if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) && pinfo.IsSetterInitOnly then
errorR (Error (FSComp.SR.tcInitOnlyPropertyCannotBeSet1 nm, mItem))

let args = if pinfo.IsIndexer then args else []
let mut = (if isStructTy g (tyOfExpr g objExpr) then DefinitelyMutates else PossiblyMutates)
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mStmt mItem nm ad mut true meths afterResolution NormalValUse (args @ [expr2]) atomicFlag []
Expand Down Expand Up @@ -9727,6 +9772,9 @@ and TcMethodApplication
// Handle post-hoc property assignments
let setterExprPrebinders, callExpr3 =
let expr = callExpr2b

CheckRequiredProperties g env cenv finalCalledMethInfo finalAssignedItemSetters mMethExpr

if isCheckingAttributeCall then
[], expr
elif isNil finalAssignedItemSetters then
Expand All @@ -9738,7 +9786,7 @@ and TcMethodApplication
// Build the expression that mutates the properties on the result of the call
let setterExprPrebinders, propSetExpr =
(mkUnit g mMethExpr, finalAssignedItemSetters) ||> List.mapFold (fun acc assignedItemSetter ->
let argExprPrebinder, action, m = TcSetterArgExpr cenv env denv objExpr ad assignedItemSetter
let argExprPrebinder, action, m = TcSetterArgExpr cenv env denv objExpr ad assignedItemSetter finalCalledMethInfo.IsConstructor
argExprPrebinder, mkCompGenSequential m acc action)

// now put them together
Expand Down Expand Up @@ -9784,7 +9832,7 @@ and TcMethodApplication
(callExpr6, finalAttributeAssignedNamedItems, delayed), tpenv

/// For Method(X = expr) 'X' can be a property, IL Field or F# record field
and TcSetterArgExpr cenv env denv objExpr ad (AssignedItemSetter(id, setter, CallerArg(callerArgTy, m, isOptCallerArg, argExpr))) =
and TcSetterArgExpr cenv env denv objExpr ad (AssignedItemSetter(id, setter, CallerArg(callerArgTy, m, isOptCallerArg, argExpr))) calledFromConstructor =
let g = cenv.g

if isOptCallerArg then
Expand All @@ -9793,6 +9841,10 @@ and TcSetterArgExpr cenv env denv objExpr ad (AssignedItemSetter(id, setter, Cal
let argExprPrebinder, action, defnItem =
match setter with
| AssignedPropSetter (pinfo, pminfo, pminst) ->

if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) && pinfo.IsSetterInitOnly && not calledFromConstructor then
errorR (Error (FSComp.SR.tcInitOnlyPropertyCannotBeSet1 pinfo.PropertyName, m))

MethInfoChecks g cenv.amap true None [objExpr] ad m pminfo
let calledArgTy = List.head (List.head (pminfo.GetParamTypes(cenv.amap, m, pminst)))
let tcVal = LightweightTcValForUsingInBuildMethodCall g
Expand Down
14 changes: 14 additions & 0 deletions src/Compiler/Checking/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1556,6 +1556,10 @@ module InfoMemberPrinting =
layoutType denv retTy ^^
getterSetter

let formatPropInfoToBufferFreeStyle g amap m denv os (pinfo: PropInfo) =
let resL = prettyLayoutOfPropInfoFreeStyle g amap m denv pinfo
resL |> bufferL os

let formatMethInfoToBufferFreeStyle amap m denv os (minfo: MethInfo) =
let _, resL = prettyLayoutOfMethInfoFreeStyle amap m denv emptyTyparInst minfo
resL |> bufferL os
Expand Down Expand Up @@ -2472,6 +2476,16 @@ let multiLineStringOfMethInfos infoReader m denv minfos =
|> List.map (sprintf "%s %s" Environment.NewLine)
|> String.concat ""

let stringOfPropInfo g amap m denv pinfo =
buildString (fun buf -> InfoMemberPrinting.formatPropInfoToBufferFreeStyle g amap m denv buf pinfo)

/// Convert PropInfos to lines separated by newline including a newline as the first character
let multiLineStringOfPropInfos g amap m denv pinfos =
pinfos
|> List.map (stringOfPropInfo g amap m denv)
|> List.map (sprintf "%s %s" Environment.NewLine)
|> String.concat ""

/// Convert a ParamData to a string
let stringOfParamData denv paramData = buildString (fun buf -> InfoMemberPrinting.formatParamDataToBuffer denv buf paramData)

Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/Checking/NicePrint.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ val stringOfMethInfo: infoReader: InfoReader -> m: range -> denv: DisplayEnv ->
val multiLineStringOfMethInfos:
infoReader: InfoReader -> m: range -> denv: DisplayEnv -> minfos: MethInfo list -> string

val stringOfPropInfo: g: TcGlobals -> amap: ImportMap -> m: range -> denv: DisplayEnv -> pinfo: PropInfo -> string

val multiLineStringOfPropInfos:
g: TcGlobals -> amap: ImportMap -> m: range -> denv: DisplayEnv -> pinfos: PropInfo list -> string

val stringOfParamData: denv: DisplayEnv -> paramData: ParamData -> string

val layoutOfParamData: denv: DisplayEnv -> paramData: ParamData -> Layout
Expand Down
44 changes: 43 additions & 1 deletion src/Compiler/Checking/infos.fs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ open FSharp.Compiler.Xml

#if !NO_TYPEPROVIDERS
open FSharp.Compiler.TypeProviders
open FSharp.Compiler.AbstractIL

#endif

//-------------------------------------------------------------------------
Expand Down Expand Up @@ -152,6 +154,11 @@ let private GetInstantiationForPropertyVal g (ty, vref) =
let memberParentTypars, memberMethodTypars, _retTy, parentTyArgs = AnalyzeTypeOfMemberVal false g (ty, vref)
CombineMethInsts memberParentTypars memberMethodTypars parentTyArgs (generalizeTypars memberMethodTypars)

let private HasExternalInit (mref: ILMethodRef) : bool =
match mref.ReturnType with
| ILType.Modified(_, cls, _) -> cls.FullName = "System.Runtime.CompilerServices.IsExternalInit"
| _ -> false

/// Describes the sequence order of the introduction of an extension method. Extension methods that are introduced
/// later through 'open' get priority in overload resolution.
type ExtensionMethodPriority = uint64
Expand Down Expand Up @@ -933,6 +940,12 @@ type MethInfo =
| FSMeth _ -> false // F# defined methods not supported yet. Must be a language feature.
| _ -> false

/// Indicates, wheter this method has `IsExternalInit` modreq.
member x.HasExternalInit =
match x with
| ILMeth (_, ilMethInfo, _) -> HasExternalInit ilMethInfo.ILMethodRef
| _ -> 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: ImportMap, m) =
Expand Down Expand Up @@ -1053,6 +1066,12 @@ type MethInfo =
else []
#endif

/// Get custom attributes for method (only applicable for IL methods)
member x.GetCustomAttrs() =
match x with
| ILMeth(_, ilMethInfo, _) -> ilMethInfo.RawMetadata.CustomAttrs
| _ -> ILAttributes.Empty

/// Get the parameter attributes of a method info, which get combined with the parameter names and types
member x.GetParamAttribs(amap, m) =
match x with
Expand Down Expand Up @@ -1562,6 +1581,10 @@ type ILPropInfo =
/// Indicates if the IL property has a 'set' method
member x.HasSetter = Option.isSome x.RawMetadata.SetMethod

/// Indidcates whether IL property has an init-only setter (i.e. has the `System.Runtime.CompilerServices.IsExternalInit` modifer)
member x.IsSetterInitOnly =
x.HasSetter && HasExternalInit x.SetterMethod.ILMethodRef

/// Indicates if the IL property is static
member x.IsStatic = (x.RawMetadata.CallingConv = ILThisConvention.Static)

Expand All @@ -1575,6 +1598,9 @@ type ILPropInfo =
(x.HasGetter && x.GetterMethod.IsNewSlot) ||
(x.HasSetter && x.SetterMethod.IsNewSlot)

/// Indicates if the property is required, i.e. has RequiredMemberAttribute applied.
member x.IsRequired = TryFindILAttribute x.TcGlobals.attrib_RequiredMemberAttribute x.RawMetadata.CustomAttrs

/// Get the names and types of the indexer arguments associated with the IL property.
///
/// Any type parameters of the enclosing type are instantiated in the type returned.
Expand Down Expand Up @@ -1688,6 +1714,22 @@ type PropInfo =
| ProvidedProp(_, pi, m) -> pi.PUntaint((fun pi -> pi.CanWrite), m)
#endif

member x.IsSetterInitOnly =
match x with
| ILProp ilpinfo -> ilpinfo.IsSetterInitOnly
| FSProp _ -> false
#if !NO_TYPEPROVIDERS
| ProvidedProp _ -> false
#endif

member x.IsRequired =
match x with
| ILProp ilpinfo -> ilpinfo.IsRequired
| FSProp _ -> false
#if !NO_TYPEPROVIDERS
| ProvidedProp _ -> false
#endif

/// Indicates if this is an extension member
member x.IsExtensionMember =
match x.ArbitraryValRef with
Expand Down Expand Up @@ -2263,4 +2305,4 @@ let PropInfosEquivByNameAndSig erasureFlag g amap m (pinfo: PropInfo) (pinfo2: P

let SettersOfPropInfos (pinfos: PropInfo list) = pinfos |> List.choose (fun pinfo -> if pinfo.HasSetter then Some(pinfo.SetterMethod, Some pinfo) else None)

let GettersOfPropInfos (pinfos: PropInfo list) = pinfos |> List.choose (fun pinfo -> if pinfo.HasGetter then Some(pinfo.GetterMethod, Some pinfo) else None)
let GettersOfPropInfos (pinfos: PropInfo list) = pinfos |> List.choose (fun pinfo -> if pinfo.HasGetter then Some(pinfo.GetterMethod, Some pinfo) else None)
15 changes: 15 additions & 0 deletions src/Compiler/Checking/infos.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ type MethInfo =
/// Receiver must be a struct type.
member IsReadOnly: bool

/// Indicates, wheter this method has `IsExternalInit` modreq.
member HasExternalInit: bool

/// Indicates if the enclosing type for the method is a value type.
///
/// For an extension method, this indicates if the method extends a struct type.
Expand Down Expand Up @@ -493,6 +496,9 @@ type MethInfo =
/// An instance method returns one object argument.
member GetObjArgTypes: amap: ImportMap * m: range * minst: TypeInst -> TType list

/// Get custom attributes for method (only applicable for IL methods)
member GetCustomAttrs: unit -> ILAttributes

/// Get the parameter attributes of a method info, which get combined with the parameter names and types
member GetParamAttribs:
amap: ImportMap * m: range -> (bool * bool * bool * OptionalArgInfo * CallerInfo * ReflectedArgInfo) list list
Expand Down Expand Up @@ -695,6 +701,9 @@ type ILPropInfo =
/// Get the declaring IL type of the IL property, including any generic instantiation
member ILTypeInfo: ILTypeInfo

/// Is the property requied (has the RequiredMemberAttribute).
member IsRequired: bool

/// Indicates if the IL property is logically a 'newslot', i.e. hides any previous slots of the same name.
member IsNewSlot: bool

Expand Down Expand Up @@ -787,6 +796,12 @@ type PropInfo =
/// Indicates if this property has an associated setter method.
member HasSetter: bool

/// Indidcates whether IL property has an init-only setter (i.e. has the `System.Runtime.CompilerServices.IsExternalInit` modifer)
member IsSetterInitOnly: bool

/// Is the property requied (has the RequiredMemberAttribute).
member IsRequired: bool

member ImplementedSlotSignatures: SlotSig list

/// Indicates if this property is marked 'override' and thus definitely overrides another property.
Expand Down
Loading