Skip to content

Update GetCSharpStyleIndexedExtensionMembersForTyconRef to avoid code duplication #15687

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 27, 2023
Merged

Update GetCSharpStyleIndexedExtensionMembersForTyconRef to avoid code duplication #15687

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ffb4f61
Update GetCSharpStyleIndexedExtensionMembersForTyconRef to avoid code…
edgarfgp Jul 26, 2023
85e6d87
use named args
edgarfgp Jul 26, 2023
a0374cf
Merge branch 'main' into duplication-is-bad
edgarfgp Jul 27, 2023
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
97 changes: 33 additions & 64 deletions src/Compiler/Checking/NameResolution.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,25 +537,52 @@ let IsMethInfoPlainCSharpStyleExtensionMember g m isEnclExtTy (minfo: MethInfo)
not minfo.IsExtensionMember &&
(match minfo.NumArgs with [x] when x >= 1 -> true | _ -> false) &&
MethInfoHasAttribute g m g.attrib_ExtensionAttribute minfo

let GetTyconRefForExtensionMembers minfo (deref: Entity) amap m g =
try
let rs =
match metadataOfTycon deref, minfo with
| ILTypeMetadata (TILObjectReprData(scope=scoref)), ILMeth(ilMethInfo=ILMethInfo(ilMethodDef=ilMethod)) ->
match ilMethod.ParameterTypes with
| firstTy :: _ ->
match firstTy with
| ILType.Boxed tspec | ILType.Value tspec ->
let tref = (tspec |> rescopeILTypeSpec scoref).TypeRef
if Import.CanImportILTypeRef amap m tref then
let tcref = tref |> Import.ImportILTypeRef amap m
if isCompiledTupleTyconRef g tcref || tyconRefEq g tcref g.fastFunc_tcr then None
else Some tcref
else None
| _ -> None
| _ -> None
| _ ->
// The results are indexed by the TyconRef of the first 'this' argument, if any.
// So we need to go and crack the type of the 'this' argument.
let thisTy = minfo.GetParamTypes(amap, m, generalizeTypars minfo.FormalMethodTypars).Head.Head
match thisTy with
| AppTy g (tcrefOfTypeExtended, _) when not (isByrefTy g thisTy) -> Some tcrefOfTypeExtended
| _ -> None
Some rs
with e -> // Import of the ILType may fail, if so report the error and skip on
errorRecovery e m
None

/// Get the info for all the .NET-style extension members listed as static members in the type.
let private GetCSharpStyleIndexedExtensionMembersForTyconRef (amap: Import.ImportMap) m (tcrefOfStaticClass: TyconRef) =
let g = amap.g
let pri = NextExtensionMethodPriority()
Copy link
Member

@auduchinok auduchinok Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edgarfgp Was there any reason to raise the priority for each type, even if it doesn't contain any extension members? Not that it makes a lot of difference, but the numbers are quite big now when debugging 🙂

Copy link
Contributor Author

@edgarfgp edgarfgp Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was to reuse pri info in both places. But as you pointed it might not be the right thing to do.


if g.langVersion.SupportsFeature(LanguageFeature.CSharpExtensionAttributeNotRequired) then
let ty = generalizedTyconRef g tcrefOfStaticClass

let csharpStyleExtensionMembers =
if IsTyconRefUsedForCSharpStyleExtensionMembers g m tcrefOfStaticClass || tcrefOfStaticClass.IsLocalRef then
protectAssemblyExploration [] (fun () ->
let ty = generalizedTyconRef g tcrefOfStaticClass
GetImmediateIntrinsicMethInfosOfType (None, AccessorDomain.AccessibleFromSomeFSharpCode) g amap m ty
|> List.filter (IsMethInfoPlainCSharpStyleExtensionMember g m true))
else
[]

if not csharpStyleExtensionMembers.IsEmpty then
let pri = NextExtensionMethodPriority()

[ for minfo in csharpStyleExtensionMembers do
let ilExtMem = ILExtMem (tcrefOfStaticClass, minfo, pri)

Expand All @@ -568,37 +595,7 @@ let private GetCSharpStyleIndexedExtensionMembersForTyconRef (amap: Import.Impor
//
// We don't use the index for the IL extension method for tuple of F# function types (e.g. if extension
// methods for tuple occur in C# code)
let thisTyconRef =
try
let rs =
match metadataOfTycon tcrefOfStaticClass.Deref, minfo with
| ILTypeMetadata (TILObjectReprData(scoref, _, _)), ILMeth(_, ILMethInfo(_, _, _, ilMethod, _), _) ->
match ilMethod.ParameterTypes with
| firstTy :: _ ->
match firstTy with
| ILType.Boxed tspec | ILType.Value tspec ->
let tref = (tspec |> rescopeILTypeSpec scoref).TypeRef
if Import.CanImportILTypeRef amap m tref then
let tcref = tref |> Import.ImportILTypeRef amap m
if isCompiledTupleTyconRef g tcref || tyconRefEq g tcref g.fastFunc_tcr then None
else Some tcref
else None
| _ -> None
| _ -> None
| _ ->
// The results are indexed by the TyconRef of the first 'this' argument, if any.
// So we need to go and crack the type of the 'this' argument.
let thisTy = minfo.GetParamTypes(amap, m, generalizeTypars minfo.FormalMethodTypars).Head.Head
match thisTy with
| AppTy g (tcrefOfTypeExtended, _) when not (isByrefTy g thisTy) -> Some tcrefOfTypeExtended
| _ -> None

Some rs

with e -> // Import of the ILType may fail, if so report the error and skip on
errorRecovery e m
None

let thisTyconRef = GetTyconRefForExtensionMembers minfo tcrefOfStaticClass.Deref amap m g
match thisTyconRef with
| None -> ()
| Some (Some tcref) -> yield Choice1Of2(tcref, ilExtMem)
Expand All @@ -607,7 +604,6 @@ let private GetCSharpStyleIndexedExtensionMembersForTyconRef (amap: Import.Impor
[]
else
if IsTyconRefUsedForCSharpStyleExtensionMembers g m tcrefOfStaticClass then
let pri = NextExtensionMethodPriority()
let ty = generalizedTyconRef g tcrefOfStaticClass
let minfos = GetImmediateIntrinsicMethInfosOfType (None, AccessorDomain.AccessibleFromSomeFSharpCode) g amap m ty

Expand All @@ -623,34 +619,7 @@ let private GetCSharpStyleIndexedExtensionMembersForTyconRef (amap: Import.Impor
//
// We don't use the index for the IL extension method for tuple of F# function types (e.g. if extension
// methods for tuple occur in C# code)
let thisTyconRef =
try
let rs =
match metadataOfTycon tcrefOfStaticClass.Deref, minfo with
| ILTypeMetadata (TILObjectReprData(scoref, _, _)), ILMeth(_, ILMethInfo(_, _, _, ilMethod, _), _) ->
match ilMethod.ParameterTypes with
| firstTy :: _ ->
match firstTy with
| ILType.Boxed tspec | ILType.Value tspec ->
let tref = (tspec |> rescopeILTypeSpec scoref).TypeRef
if Import.CanImportILTypeRef amap m tref then
let tcref = tref |> Import.ImportILTypeRef amap m
if isCompiledTupleTyconRef g tcref || tyconRefEq g tcref g.fastFunc_tcr then None
else Some tcref
else None
| _ -> None
| _ -> None
| _ ->
// The results are indexed by the TyconRef of the first 'this' argument, if any.
// So we need to go and crack the type of the 'this' argument.
let thisTy = minfo.GetParamTypes(amap, m, generalizeTypars minfo.FormalMethodTypars).Head.Head
match thisTy with
| AppTy g (tcrefOfTypeExtended, _) when not (isByrefTy g thisTy) -> Some tcrefOfTypeExtended
| _ -> None
Some rs
with e -> // Import of the ILType may fail, if so report the error and skip on
errorRecovery e m
None
let thisTyconRef = GetTyconRefForExtensionMembers minfo tcrefOfStaticClass.Deref amap m g
match thisTyconRef with
| None -> ()
| Some (Some tcref) -> yield Choice1Of2(tcref, ilExtMem)
Expand Down