Skip to content

Commit 7487bd1

Browse files
evgTSVevgTSV
authored
Fix accessibility and type-matching for generic extension method lookups (#19536)
1 parent 7355966 commit 7487bd1

8 files changed

Lines changed: 251 additions & 8 deletions

File tree

‎docs/release-notes/.FSharp.Compiler.Service/11.0.100.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* Fix `YieldFromFinal`/`ReturnFromFinal` being incorrectly called in non-tail positions (`for`, `use`, `use!`, `try/with` handler). ([Issue #19402](https://github.com/dotnet/fsharp/issues/19402), [PR #19403](https://github.com/dotnet/fsharp/pull/19403))
2020
* Fixed how the source ranges of warn directives are reported (as trivia) in the parser output (by not reporting leading spaces). ([Issue #19405](https://github.com/dotnet/fsharp/issues/19405), [PR #19408]((https://github.com/dotnet/fsharp/pull/19408)))
2121
* Fix UoM value type `ToString()` returning garbage values when `--checknulls+` is enabled, caused by double address-taking in codegen. ([Issue #19435](https://github.com/dotnet/fsharp/issues/19435), [PR #19440](https://github.com/dotnet/fsharp/pull/19440))
22+
* Fix accessibility and type-matching for extension method lookups. ([Issue #19349](https://github.com/dotnet/fsharp/issues/19349), [PR #19536](https://github.com/dotnet/fsharp/pull/19536))
2223
* Fix completion inconsistently showing some obsolete members (fields and events) while hiding others (methods and properties). All obsolete members are now consistently hidden by default. ([Issue #13512](https://github.com/dotnet/fsharp/issues/13512), [PR #19506](https://github.com/dotnet/fsharp/pull/19506))
2324
* Fix O(n) `TypeStructure.GetHashCode` performance regression causing sustained high CPU in IDE mode with generative type providers. ([Issue #18925](https://github.com/dotnet/fsharp/issues/18925), [PR #19369](https://github.com/dotnet/fsharp/pull/19369))
2425
* Fix TypeLoadException when creating delegate with voidptr parameter. (Issue [#11132](https://github.com/dotnet/fsharp/issues/11132), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))

‎src/Compiler/Checking/Expressions/CheckComputationExpressions.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ let inline noTailCall ceenv = { ceenv with tailCall = false }
6767

6868
let inline TryFindIntrinsicOrExtensionMethInfo collectionSettings (cenv: cenv) (env: TcEnv) m ad nm ty =
6969
AllMethInfosOfTypeInScope collectionSettings cenv.infoReader env.NameEnv (Some nm) ad IgnoreOverrides m ty
70+
|> List.filter (IsExtensionMethCompatibleWithTy cenv.infoReader m ty)
7071

7172
/// Ignores an attribute
7273
let inline IgnoreAttribute _ = None

‎src/Compiler/Checking/Expressions/CheckExpressions.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3126,6 +3126,7 @@ let BuildPossiblyConditionalMethodCall (cenv: cenv) env isMutable m isProp minfo
31263126

31273127
let TryFindIntrinsicOrExtensionMethInfo collectionSettings (cenv: cenv) (env: TcEnv) m ad nm ty =
31283128
AllMethInfosOfTypeInScope collectionSettings cenv.infoReader env.NameEnv (Some nm) ad IgnoreOverrides m ty
3129+
|> List.filter (IsExtensionMethCompatibleWithTy cenv.infoReader m ty)
31293130

31303131
let TryFindFSharpSignatureInstanceGetterProperty (cenv: cenv) (env: TcEnv) m nm ty (sigTys: TType list) =
31313132
let g = cenv.g

‎src/Compiler/Checking/NameResolution.fs‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -727,8 +727,11 @@ let SelectMethInfosFromExtMembers (infoReader: InfoReader) optFilter apparentTy
727727
]
728728

729729
/// Query the available extension methods of a type (including extension methods for inherited types)
730-
let ExtensionMethInfosOfTypeInScope (collectionSettings: ResultCollectionSettings) (infoReader: InfoReader) (nenv: NameResolutionEnv) optFilter isInstanceFilter m ty =
731-
let extMemsDangling = SelectMethInfosFromExtMembers infoReader optFilter ty m nenv.eUnindexedExtensionMembers
730+
let ExtensionMethInfosOfTypeInScope (collectionSettings: ResultCollectionSettings) (infoReader: InfoReader) (nenv: NameResolutionEnv) ad optFilter isInstanceFilter m ty =
731+
let amap = infoReader.amap
732+
733+
let extMemsDangling = SelectMethInfosFromExtMembers infoReader optFilter ty m nenv.eUnindexedExtensionMembers
734+
732735
if collectionSettings = ResultCollectionSettings.AtMostOneResult && not (isNil extMemsDangling) then
733736
extMemsDangling
734737
else
@@ -743,6 +746,9 @@ let ExtensionMethInfosOfTypeInScope (collectionSettings: ResultCollectionSetting
743746
| _ -> [])
744747
extMemsDangling @ extMemsFromHierarchy
745748
|> List.filter (fun minfo ->
749+
let isAccesible = IsMethInfoAccessible amap m ad minfo
750+
751+
isAccesible &&
746752
match isInstanceFilter with
747753
| LookupIsInstance.Ambivalent -> true
748754
| LookupIsInstance.Yes -> minfo.IsInstance
@@ -754,7 +760,35 @@ let AllMethInfosOfTypeInScope collectionSettings infoReader nenv optFilter ad fi
754760
if collectionSettings = ResultCollectionSettings.AtMostOneResult && not (isNil intrinsic) then
755761
intrinsic
756762
else
757-
intrinsic @ ExtensionMethInfosOfTypeInScope collectionSettings infoReader nenv optFilter LookupIsInstance.Ambivalent m ty
763+
intrinsic @ ExtensionMethInfosOfTypeInScope collectionSettings infoReader nenv ad optFilter LookupIsInstance.Ambivalent m ty
764+
765+
let IsExtensionMethCompatibleWithTy (infoReader: InfoReader) m (ty: TType) (minfo: MethInfo) =
766+
let g = infoReader.g
767+
let amap = infoReader.amap
768+
769+
not minfo.IsExtensionMember ||
770+
match minfo.GetObjArgTypes(amap, m, []) with
771+
| thisTy :: _ ->
772+
let ty1 = thisTy |> stripTyEqns g
773+
let ty2 = ty |> stripTyEqns g
774+
775+
match ty1, ty2 with
776+
| TType_var (tp1, _), _ ->
777+
let coercesToConstraints =
778+
tp1.Constraints |> List.choose (function
779+
| TyparConstraint.CoercesTo(targetCTy, _) -> Some targetCTy
780+
| _ -> None)
781+
match coercesToConstraints with
782+
| [] -> true // No CoercesTo constraint means it could match anything
783+
| constraints ->
784+
constraints |> List.exists (fun targetCTy ->
785+
let cTy = targetCTy |> stripTyEqns g
786+
TypeRelations.TypeFeasiblySubsumesType 0 g amap m cTy TypeRelations.CanCoerce ty2)
787+
| _, TType_var _ -> true
788+
| _ ->
789+
TypeRelations.TypeFeasiblySubsumesType 0 g amap m ty1 TypeRelations.CanCoerce ty2
790+
| _ ->
791+
true
758792

759793
//-------------------------------------------------------------------------
760794
// Helpers to do with building environments
@@ -1184,7 +1218,7 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a
11841218
[|
11851219
// Extension methods
11861220
yield!
1187-
ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader nenv None LookupIsInstance.No m ty
1221+
ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader nenv ad None LookupIsInstance.No m ty
11881222
|> ChooseMethInfosForNameEnv g m ty
11891223

11901224
// Extension properties
@@ -2827,7 +2861,7 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf
28272861
| _ ->
28282862
// lookup in-scope extension methods
28292863
// to keep in sync with the same expression in `| Some(MethodItem msets) when isLookupExpr` below
2830-
match ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv optFilter isInstanceFilter m ty with
2864+
match ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv ad optFilter isInstanceFilter m ty with
28312865
| [] -> success [resInfo, x, rest]
28322866
| methods ->
28332867
let extensionMethods = Item.MakeMethGroup(nm, methods)
@@ -2841,7 +2875,7 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf
28412875
let minfos = msets |> ExcludeHiddenOfMethInfos g ncenv.amap m
28422876

28432877
// fold the available extension members into the overload resolution
2844-
let extensionMethInfos = ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv optFilter isInstanceFilter m ty
2878+
let extensionMethInfos = ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv ad optFilter isInstanceFilter m ty
28452879

28462880
success [resInfo, Item.MakeMethGroup (nm, minfos@extensionMethInfos), rest]
28472881

@@ -2860,7 +2894,7 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf
28602894

28612895
if not (isNil pinfos) && isLookUpExpr then OneResult(success (resInfo, Item.Property (nm, pinfos, None), rest)) else
28622896

2863-
let minfos = ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv optFilter isInstanceFilter m ty
2897+
let minfos = ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv ad optFilter isInstanceFilter m ty
28642898

28652899
if not (isNil minfos) && isLookUpExpr then
28662900
success [resInfo, Item.MakeMethGroup (nm, minfos), rest]
@@ -2898,7 +2932,7 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf
28982932
for p in ExtensionPropInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv None LookupIsInstance.Ambivalent ad m ty do
28992933
addToBuffer p.PropertyName
29002934

2901-
for m in ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv None LookupIsInstance.Ambivalent m ty do
2935+
for m in ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv ad None LookupIsInstance.Ambivalent m ty do
29022936
addToBuffer m.DisplayName
29032937

29042938
for p in GetIntrinsicPropInfosOfType ncenv.InfoReader None ad AllowMultiIntfInstantiations.No findFlag m ty do

‎src/Compiler/Checking/NameResolution.fsi‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,9 @@ val internal AllMethInfosOfTypeInScope:
689689
ty: TType ->
690690
MethInfo list
691691

692+
/// Check whether the 'this' argument of an extension method is compatible with the target type
693+
val internal IsExtensionMethCompatibleWithTy: infoReader: InfoReader -> m: range -> ty: TType -> minfo: MethInfo -> bool
694+
692695
/// Used to report an error condition where name resolution failed due to an indeterminate type
693696
exception internal IndeterminateType of range
694697

‎tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/UseBindings/UseBindingsAndExtensionMembers.fs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
module Conformance.BasicGrammarElements.UseBindExtensionMethodCapture
2+
3+
open Xunit
4+
open FSharp.Test.Compiler
5+
6+
[<Fact>]
7+
let ``Use binding doesn't capture an extension method with generic type``() =
8+
FSharp """
9+
open System
10+
open System.Runtime.CompilerServices
11+
12+
type FooClass() = class end
13+
14+
type Disposable() =
15+
interface IDisposable with
16+
member _.Dispose() = ()
17+
18+
[<Extension>]
19+
type PublicExtensions =
20+
[<Extension>]
21+
static member inline Dispose(this: #FooClass) =
22+
this
23+
24+
let foo() =
25+
use a = new Disposable()
26+
()
27+
28+
foo()
29+
"""
30+
|> asExe
31+
|> compile
32+
|> shouldSucceed

‎tests/FSharp.Compiler.ComponentTests/Conformance/Expressions/ComputationExpressions/CEExtensionMethodCapture.fs‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
module Conformance.Expressions.CEExtensionMethodCapture
2+
3+
open Xunit
4+
open FSharp.Test.Compiler
5+
6+
[<Fact>]
7+
let ``CE doesn't capture an extension method beyond the access domain``() =
8+
FSharp """
9+
open System.Runtime.CompilerServices
10+
11+
type AsyncSeq<'T>(i: 'T) =
12+
class
13+
let l = [i]
14+
member this.Data = l
15+
end
16+
17+
type AsyncSeqBuilder() =
18+
member _.Yield(x: 'T) : AsyncSeq<'T> =
19+
AsyncSeq(x)
20+
21+
[<Extension>]
22+
type PrivateExtensions =
23+
[<Extension>]
24+
static member inline private Run(this: AsyncSeqBuilder) =
25+
this
26+
27+
let asyncSeq = AsyncSeqBuilder()
28+
29+
let xs : AsyncSeq<int> =
30+
asyncSeq {
31+
yield 1
32+
}
33+
"""
34+
|> asExe
35+
|> compile
36+
|> shouldSucceed
37+
38+
[<Fact>]
39+
let ``CE doesn't capture an extension method with generic type``() =
40+
FSharp """
41+
open System.Runtime.CompilerServices
42+
43+
type FooClass = class end
44+
45+
type AsyncSeq<'T>(i: 'T) =
46+
class
47+
let l = [i]
48+
member this.Data = l
49+
end
50+
51+
type AsyncSeqBuilder() =
52+
member _.Yield(x: 'T) : AsyncSeq<'T> =
53+
AsyncSeq(x)
54+
55+
[<Extension>]
56+
type PublicExtensions =
57+
[<Extension>]
58+
static member inline Run(this: #FooClass) =
59+
this
60+
61+
let asyncSeq = AsyncSeqBuilder()
62+
63+
let xs : AsyncSeq<int> =
64+
asyncSeq {
65+
yield 1
66+
}
67+
"""
68+
|> asExe
69+
|> compile
70+
|> shouldSucceed
71+
72+
// Deliberately trigger an error to ensure that a method is captured
73+
[<Fact>]
74+
let ``CE captures a public extension method and procudes an error due to invalid args``() =
75+
FSharp """
76+
open System.Runtime.CompilerServices
77+
78+
type AsyncSeq<'T>(i: 'T) =
79+
class
80+
let l = [i]
81+
member this.Data = l
82+
end
83+
84+
type AsyncSeqBuilder() =
85+
member _.Yield(x: 'T) : AsyncSeq<'T> =
86+
AsyncSeq(x)
87+
88+
[<Extension>]
89+
type PublicExtensions =
90+
[<Extension>]
91+
static member inline Run(this: AsyncSeqBuilder, invalidArg: string) =
92+
this
93+
94+
let asyncSeq = AsyncSeqBuilder()
95+
96+
let xs : AsyncSeq<int> =
97+
asyncSeq {
98+
yield 1
99+
}
100+
"""
101+
|> asExe
102+
|> compile
103+
|> shouldFail
104+
105+
// Deliberately trigger an error to ensure that a method is captured
106+
[<Fact>]
107+
let ``CE captures a public extension method with valid generic constrainted type and procudes an error due to invalid args``() =
108+
FSharp """
109+
open System.Runtime.CompilerServices
110+
111+
type AsyncSeq<'T>(i: 'T) =
112+
class
113+
let l = [i]
114+
member this.Data = l
115+
end
116+
117+
type AsyncSeqBuilder() =
118+
member _.Yield(x: 'T) : AsyncSeq<'T> =
119+
AsyncSeq(x)
120+
121+
[<Extension>]
122+
type PublicExtensions =
123+
[<Extension>]
124+
static member inline Run(this: #AsyncSeqBuilder, invalidArg: string) =
125+
this
126+
127+
let asyncSeq = AsyncSeqBuilder()
128+
129+
let xs : AsyncSeq<int> =
130+
asyncSeq {
131+
yield 1
132+
}
133+
"""
134+
|> asExe
135+
|> compile
136+
|> shouldFail
137+
138+
// Deliberately trigger an error to ensure that a method is captured
139+
[<Fact>]
140+
let ``CE captures a public extension method with generic type and procudes an error due to invalid args``() =
141+
FSharp """
142+
open System.Runtime.CompilerServices
143+
144+
type AsyncSeq<'T>(i: 'T) =
145+
class
146+
let l = [i]
147+
member this.Data = l
148+
end
149+
150+
type AsyncSeqBuilder() =
151+
member _.Yield(x: 'T) : AsyncSeq<'T> =
152+
AsyncSeq(x)
153+
154+
[<Extension>]
155+
type PublicExtensions =
156+
[<Extension>]
157+
static member Run(this: 'T, invalidArg: string) =
158+
this
159+
160+
let asyncSeq = AsyncSeqBuilder()
161+
162+
let xs : AsyncSeq<int> =
163+
asyncSeq {
164+
yield 1
165+
}
166+
"""
167+
|> asExe
168+
|> compile
169+
|> shouldFail

‎tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
<Compile Include="Conformance\BasicGrammarElements\ValueRestriction\ValueRestriction.fs" />
7777
<Compile Include="Conformance\BasicGrammarElements\UseBindings\UseBindings.fs" />
7878
<Compile Include="Conformance\BasicGrammarElements\UseBindings\UseBangBindings.fs" />
79+
<Compile Include="Conformance\BasicGrammarElements\UseBindings\UseBindingsAndExtensionMembers.fs" />
7980
<Compile Include="Conformance\Constraints\Unmanaged.fs" />
8081
<Compile Include="Conformance\GeneratedEqualityHashingComparison\Attributes\Diags\Attributes_Diags.fs" />
8182
<Compile Include="Conformance\GeneratedEqualityHashingComparison\Attributes\Legacy\Attributes_Legacy.fs" />
@@ -85,6 +86,7 @@
8586
<Compile Include="Conformance\Expressions\ApplicationExpressions\Ctor.fs" />
8687
<Compile Include="Conformance\Expressions\BindingExpressions\BindingExpressions.fs" />
8788
<Compile Include="Conformance\Expressions\ComputationExpressions\ComputationExpressions.fs" />
89+
<Compile Include="Conformance\Expressions\ComputationExpressions\CEExtensionMethodCapture.fs" />
8890
<Compile Include="Conformance\Expressions\ObjectExpressions\ObjectExpressions.fs" />
8991
<Compile Include="Conformance\Expressions\ObjectExpressions\StructObjectExpression.fs" />
9092
<Compile Include="Conformance\Expressions\ControlFlowExpressions\PatternMatching\PatternMatching.fs" />

0 commit comments

Comments
 (0)