Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4cf7aae

Browse files
vasily-kirichenkoKevinRansom
authored andcommitted
Fix unused opens analyzer on rec modules and optimize it (#5005)
* filter out duplicates in GetOpenDeclarations* Add rec module to unused opens test* add unused opens tests for rec modules* fix unused opens for rec modules* filter out symbol uses that lays above the open statement* filter out union cases definitions* fix ItemsAreEffectivelyEqualHash for Item.ModuleOrNamespaces* use Dictionary to lookup symbol uses by declaring entity* formatting* cleanup* use Dictionary to filter out already processed modules* add BagAdd and BagExistsValueForKey Dictionary extensions
1 parent039c708 commit4cf7aae

File tree

5 files changed

+198
-46
lines changed

5 files changed

+198
-46
lines changed

‎src/absil/illib.fs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,12 @@ module public Microsoft.FSharp.Compiler.AbstractIL.Internal.Library
55

66

77
openSystem
8-
openSystem.Collections
98
openSystem.Collections.Generic
109
openSystem.Diagnostics
1110
openSystem.IO
1211
openSystem.Reflection
13-
openSystem.Text
1412
openSystem.Threading
15-
openInternal.Utilities
13+
openSystem.Runtime.CompilerServices
1614

1715
#if FX_RESHAPED_REFLECTION
1816
openMicrosoft.FSharp.Core.ReflectionAdapters
@@ -561,10 +559,23 @@ module String =
561559
// http://stackoverflow.com/questions/19365404/stringreader-omits-trailing-linebreak
562560
yield String.Empty
563561
|]
564-
moduleDictionary=
565562

563+
moduleDictionary=
566564
let inlinenewWithSize(size:int)= Dictionary<_,_>(size, HashIdentity.Structural)
567-
565+
566+
[<Extension>]
567+
typeDictionaryExtensions()=
568+
[<Extension>]
569+
static member inlineBagAdd(dic:Dictionary<'key,'valuelist>,key:'key,value:'value)=
570+
match dic.TryGetValue keywith
571+
|true, values-> dic.[key]<- value:: values
572+
|_-> dic.[key]<-[value]
573+
574+
[<Extension>]
575+
static member inlineBagExistsValueForKey(dic:Dictionary<'key,'valuelist>,key:'key,f:'value->bool)=
576+
match dic.TryGetValue keywith
577+
|true, values-> values|> List.exists f
578+
|_->false
568579

569580
moduleLazy=
570581
letforce(x:Lazy<'T>)= x.Force()

‎src/fsharp/NameResolution.fs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,7 +1437,7 @@ let ItemsAreEffectivelyEqualHash (g: TcGlobals) orig =
14371437
| UnionCaseUse ucase-> hash ucase.CaseName
14381438
| RecordFieldUse(name,_)-> hash name
14391439
| EventUse einfo-> einfo.ComputeHashCode()
1440-
| Item.ModuleOrNamespaces_->100013
1440+
| Item.ModuleOrNamespaces(mref::_)->hash mref.DefinitionRange
14411441
|_->389329
14421442

14431443
[<System.Diagnostics.DebuggerDisplay("{DebugToString()}")>]
@@ -1518,7 +1518,7 @@ type TcResultsSinkImpl(g, ?source: string) =
15181518
member__.Equals((m1, item1),(m2, item2))= m1= m2&& ItemsAreEffectivelyEqual g item1 item2})
15191519

15201520
letcapturedMethodGroupResolutions= ResizeArray<_>()
1521-
letcapturedOpenDeclarations= ResizeArray<_>()
1521+
letcapturedOpenDeclarations= ResizeArray<OpenDeclaration>()
15221522
letallowedRange(m:range)=not m.IsSynthetic
15231523

15241524
memberthis.GetResolutions()=
@@ -1527,7 +1527,8 @@ type TcResultsSinkImpl(g, ?source: string) =
15271527
memberthis.GetSymbolUses()=
15281528
TcSymbolUses(g, capturedNameResolutions, capturedFormatSpecifierLocations.ToArray())
15291529

1530-
memberthis.GetOpenDeclarations()= capturedOpenDeclarations.ToArray()
1530+
memberthis.GetOpenDeclarations()=
1531+
capturedOpenDeclarations|> Seq.distinctBy(fun x-> x.Range, x.AppliedScope, x.IsOwnNamespace)|> Seq.toArray
15311532

15321533
interface ITypecheckResultsSinkwith
15331534
membersink.NotifyEnvWithScope(m,nenv,ad)=

‎src/fsharp/service/ServiceAnalysis.fs‎

Lines changed: 45 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ open Microsoft.FSharp.Compiler.Range
88
openMicrosoft.FSharp.Compiler.PrettyNaming
99
openSystem.Collections.Generic
1010
openSystem.Runtime.CompilerServices
11+
openMicrosoft.FSharp.Compiler.AbstractIL.Internal.Library
1112

1213
moduleUnusedOpens=
1314

@@ -101,10 +102,15 @@ module UnusedOpens =
101102
|:? FSharpMemberOrFunctionOrValueas fvwhennot fv.IsModuleValueOrMember->
102103
// Local values can be ignored
103104
false
105+
|:? FSharpMemberOrFunctionOrValuewhen su.IsFromDefinition->
106+
// Value definitions should be ignored
107+
false
104108
|:? FSharpGenericParameter->
105109
// Generic parameters can be ignored, they never come into scope via 'open'
106110
false
107-
|_->
111+
|:? FSharpUnionCasewhen su.IsFromDefinition->
112+
false
113+
|_->
108114
// For the rest of symbols we pick only those which are the first part of a long ident, because it's they which are
109115
// contained in opened namespaces / modules. For example, we pick `IO` from long ident `IO.File.OpenWrite` because
110116
// it's `open System` which really brings it into scope.
@@ -122,27 +128,20 @@ module UnusedOpens =
122128
|_->false
123129
|_->false)
124130

125-
/// Represents intermediate tracking data used to track the modules which are known to have been used so far
126-
typeUsedModule=
127-
{ Module:FSharpEntity
128-
AppliedScope:range}
129-
130131
/// Given an 'open' statement, find fresh modules/namespaces referred to by that statement where there is some use of a revealed symbol
131132
/// in the scope of the 'open' is from that module.
132133
///
133134
/// Performance will be roughly NumberOfOpenStatements x NumberOfSymbolUses
134-
letgetUsedModules(symbolUses1:FSharpSymbolUse[],symbolUses2:FSharpSymbolUse[])(usedModules:UsedModule list)(openStatement:OpenStatement)=
135+
letisOpenStatementUsed(symbolUses2:FSharpSymbolUse[])(symbolUsesRangesByDeclaringEntity:Dictionary<FSharpEntity,rangelist>)
136+
(usedModules:Dictionary<FSharpEntity,rangelist>)(openStatement:OpenStatement)=
135137

136138
// Don't re-check modules whose symbols are already known to have been used
137139
letopenedGroupsToExamine=
138140
openStatement.OpenedGroups|> List.choose(fun openedGroup->
139141
letopenedEntitiesToExamine=
140142
openedGroup.OpenedModules
141143
|> List.filter(fun openedEntity->
142-
not(usedModules
143-
|> List.exists(fun used->
144-
rangeContainsRange used.AppliedScope openStatement.AppliedScope&&
145-
used.Module.IsEffectivelySameAs openedEntity.Entity)))
144+
not(usedModules.BagExistsValueForKey(openedEntity.Entity,fun scope-> rangeContainsRange scope openStatement.AppliedScope)))
146145

147146
match openedEntitiesToExaminewith
148147
|[]-> None
@@ -153,45 +152,59 @@ module UnusedOpens =
153152
letnewlyUsedOpenedGroups=
154153
openedGroupsToExamine|> List.filter(fun openedGroup->
155154
openedGroup.OpenedModules|> List.exists(fun openedEntity->
156-
157-
symbolUses1|> Array.exists(fun symbolUse->
158-
rangeContainsRange openStatement.AppliedScope symbolUse.RangeAlternate&&
159-
match symbolUse.Symbolwith
160-
|:? FSharpMemberOrFunctionOrValueas f->
161-
match f.DeclaringEntitywith
162-
| Some entwhen ent.IsNamespace|| ent.IsFSharpModule-> ent.IsEffectivelySameAs openedEntity.Entity
163-
|_->false
164-
|_->false)||
165-
155+
symbolUsesRangesByDeclaringEntity.BagExistsValueForKey(openedEntity.Entity,fun symbolUseRange->
156+
rangeContainsRange openStatement.AppliedScope symbolUseRange&&
157+
Range.posGt symbolUseRange.Start openStatement.Range.End)||
158+
166159
symbolUses2|> Array.exists(fun symbolUse->
167160
rangeContainsRange openStatement.AppliedScope symbolUse.RangeAlternate&&
161+
Range.posGt symbolUse.RangeAlternate.Start openStatement.Range.End&&
168162
openedEntity.RevealedSymbolsContains symbolUse.Symbol)))
169163

170164
// Return them as interim used entities
171-
newlyUsedOpenedGroups|> List.collect(fun openedGroup->
172-
openedGroup.OpenedModules|> List.map(fun x->{ Module= x.Entity; AppliedScope= openStatement.AppliedScope}))
165+
letnewlyOpenedModules= newlyUsedOpenedGroups|> List.collect(fun openedGroup-> openedGroup.OpenedModules)
166+
for openedModulein newlyOpenedModulesdo
167+
letscopes=
168+
match usedModules.TryGetValue openedModule.Entitywith
169+
|true, scopes-> openStatement.AppliedScope:: scopes
170+
|_->[openStatement.AppliedScope]
171+
usedModules.[openedModule.Entity]<- scopes
172+
newlyOpenedModules.Length>0
173173

174174
/// Incrementally filter out the open statements one by one. Filter those whose contents are referred to somewhere in the symbol uses.
175175
/// Async to allow cancellation.
176-
let recfilterOpenStatementsIncremental symbolUses(openStatements:OpenStatement list)(usedModules:UsedModule list)acc=
176+
let recfilterOpenStatementsIncremental symbolUses2(symbolUsesRangesByDeclaringEntity:Dictionary<FSharpEntity,rangelist>)(openStatements:OpenStatement list)
177+
(usedModules:Dictionary<FSharpEntity,rangelist>)acc=
177178
async{
178179
match openStatementswith
179180
| openStatement:: rest->
180-
match getUsedModules symbolUses usedModules openStatementwith
181-
|[]->
181+
if isOpenStatementUsed symbolUses2 symbolUsesRangesByDeclaringEntity usedModules openStatementthen
182+
return! filterOpenStatementsIncremental symbolUses2 symbolUsesRangesByDeclaringEntity rest usedModules acc
183+
else
182184
// The open statement has not been used, include it in the results
183-
return! filterOpenStatementsIncremental symbolUses rest usedModules(openStatement:: acc)
184-
| moreUsedModules->
185-
// The open statement has been used, add the modules which are already known to be used to the list of things we don't need to re-check
186-
return! filterOpenStatementsIncremental symbolUses rest(moreUsedModules@ usedModules) acc
185+
return! filterOpenStatementsIncremental symbolUses2 symbolUsesRangesByDeclaringEntity rest usedModules(openStatement:: acc)
187186
|[]->return List.rev acc
188187
}
189188

189+
letentityHash= HashIdentity.FromFunctions(fun(x: FSharpEntity)-> x.GetEffectivelySameAsHash())(fun x y-> x.IsEffectivelySameAs(y))
190+
190191
/// Filter out the open statements whose contents are referred to somewhere in the symbol uses.
191192
/// Async to allow cancellation.
192-
letfilterOpenStatementssymbolUses openStatements=
193+
letfilterOpenStatements(symbolUses1:FSharpSymbolUse[],symbolUses2:FSharpSymbolUse[])openStatements=
193194
async{
194-
let!results= filterOpenStatementsIncremental symbolUses(List.ofArray openStatements)[][]
195+
// the key is a namespace or module, the value is a list of FSharpSymbolUse range of symbols defined in the
196+
// namespace or module. So, it's just symbol uses ranges groupped by namespace or module where they are _defined_.
197+
letsymbolUsesRangesByDeclaringEntity= Dictionary<FSharpEntity, range list>(entityHash)
198+
for symbolUsein symbolUses1do
199+
match symbolUse.Symbolwith
200+
|:? FSharpMemberOrFunctionOrValue as f->
201+
match f.DeclaringEntitywith
202+
| Some entitywhen entity.IsNamespace|| entity.IsFSharpModule->
203+
symbolUsesRangesByDeclaringEntity.BagAdd(entity, symbolUse.RangeAlternate)
204+
|_->()
205+
|_->()
206+
207+
let!results= filterOpenStatementsIncremental symbolUses2 symbolUsesRangesByDeclaringEntity(List.ofArray openStatements)(Dictionary(entityHash))[]
195208
return results|> List.map(fun os-> os.Range)
196209
}
197210

‎tests/service/ProjectAnalysisTests.fs‎

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5281,13 +5281,15 @@ let ``#4030, Incremental builder creation warnings`` (args, errorSeverities) =
52815281
//------------------------------------------------------
52825282

52835283
[<Test>]
5284-
let``Unused opens smoke test 1``()=
5284+
let``Unused opensin rec modulesmoke test 1``()=
52855285

52865286
letfileName1= Path.ChangeExtension(Path.GetTempFileName(),".fs")
52875287
letbase2= Path.GetTempFileName()
52885288
letdllName= Path.ChangeExtension(base2,".dll")
52895289
letprojFileName= Path.ChangeExtension(base2,".fsproj")
52905290
letfileSource1="""
5291+
module rec Module
5292+
52915293
open System.Collections // unused
52925294
open System.Collections.Generic // used, should not appear
52935295
open FSharp.Control // unused
@@ -5343,11 +5345,83 @@ type UseTheThings(i:int) =
53435345
letunusedOpens= UnusedOpens.getUnusedOpens(fileCheckResults,(fun i-> lines.[i-1]))|> Async.RunSynchronously
53445346
letunusedOpensData=[for uoin unusedOpens-> tups uo, lines.[uo.StartLine-1]]
53455347
letexpected=
5346-
[(((2,5),(2,23)),"open System.Collections // unused");
5347-
(((4,5),(4,19)),"open FSharp.Control // unused");
5348-
(((5,5),(5,16)),"open FSharp.Data // unused");
5349-
(((6,5),(6,25)),"open System.Globalization // unused");
5350-
(((23,5),(23,21)),"open SomeUnusedModule")]
5348+
[(((4,5),(4,23)),"open System.Collections // unused");
5349+
(((6,5),(6,19)),"open FSharp.Control // unused");
5350+
(((7,5),(7,16)),"open FSharp.Data // unused");
5351+
(((8,5),(8,25)),"open System.Globalization // unused");
5352+
(((25,5),(25,21)),"open SomeUnusedModule")]
5353+
unusedOpensData|> shouldEqual expected
5354+
5355+
[<Test>]
5356+
let``Unused opens in non rec module smoke test 1``()=
5357+
5358+
letfileName1= Path.ChangeExtension(Path.GetTempFileName(),".fs")
5359+
letbase2= Path.GetTempFileName()
5360+
letdllName= Path.ChangeExtension(base2,".dll")
5361+
letprojFileName= Path.ChangeExtension(base2,".fsproj")
5362+
letfileSource1="""
5363+
module Module
5364+
5365+
open System.Collections // unused
5366+
open System.Collections.Generic // used, should not appear
5367+
open FSharp.Control // unused
5368+
open FSharp.Data // unused
5369+
open System.Globalization // unused
5370+
5371+
module SomeUnusedModule =
5372+
let f x = x
5373+
5374+
module SomeUsedModuleContainingFunction =
5375+
let g x = x
5376+
5377+
module SomeUsedModuleContainingActivePattern =
5378+
let (|ActivePattern|) x = x
5379+
5380+
module SomeUsedModuleContainingExtensionMember =
5381+
type System.Int32 with member x.Q = 1
5382+
5383+
module SomeUsedModuleContainingUnion =
5384+
type Q = A | B
5385+
5386+
open SomeUnusedModule
5387+
open SomeUsedModuleContainingFunction
5388+
open SomeUsedModuleContainingExtensionMember
5389+
open SomeUsedModuleContainingActivePattern
5390+
open SomeUsedModuleContainingUnion
5391+
5392+
type UseTheThings(i:int) =
5393+
member x.Value = Dictionary<int,int>() // use something from System.Collections.Generic, as a constructor
5394+
member x.UseSomeUsedModuleContainingFunction() = g 3
5395+
member x.UseSomeUsedModuleContainingActivePattern(ActivePattern g) = g
5396+
member x.UseSomeUsedModuleContainingExtensionMember() = (3).Q
5397+
member x.UseSomeUsedModuleContainingUnion() = A
5398+
"""
5399+
File.WriteAllText(fileName1, fileSource1)
5400+
5401+
letfileNames=[fileName1]
5402+
letargs= mkProjectCommandLineArgs(dllName, fileNames)
5403+
letkeepAssemblyContentsChecker= FSharpChecker.Create(keepAssemblyContents=true)
5404+
letoptions= keepAssemblyContentsChecker.GetProjectOptionsFromCommandLineArgs(projFileName, args)
5405+
5406+
letfileCheckResults=
5407+
keepAssemblyContentsChecker.ParseAndCheckFileInProject(fileName1,0, fileSource1, options)|> Async.RunSynchronously
5408+
|>function
5409+
|_, FSharpCheckFileAnswer.Succeeded(res)-> res
5410+
|_-> failwithf"Parsing aborted unexpectedly..."
5411+
//let symbolUses = fileCheckResults.GetAllUsesOfAllSymbolsInFile() |> Async.RunSynchronously |> Array.indexed
5412+
// Fragments used to check hash codes:
5413+
//(snd symbolUses.[42]).Symbol.IsEffectivelySameAs((snd symbolUses.[37]).Symbol)
5414+
//(snd symbolUses.[42]).Symbol.GetEffectivelySameAsHash()
5415+
//(snd symbolUses.[37]).Symbol.GetEffectivelySameAsHash()
5416+
letlines= File.ReadAllLines(fileName1)
5417+
letunusedOpens= UnusedOpens.getUnusedOpens(fileCheckResults,(fun i-> lines.[i-1]))|> Async.RunSynchronously
5418+
letunusedOpensData=[for uoin unusedOpens-> tups uo, lines.[uo.StartLine-1]]
5419+
letexpected=
5420+
[(((4,5),(4,23)),"open System.Collections // unused");
5421+
(((6,5),(6,19)),"open FSharp.Control // unused");
5422+
(((7,5),(7,16)),"open FSharp.Data // unused");
5423+
(((8,5),(8,25)),"open System.Globalization // unused");
5424+
(((25,5),(25,21)),"open SomeUnusedModule")]
53515425
unusedOpensData|> shouldEqual expected
53525426

53535427
[<Test>]

‎vsintegration/tests/UnitTests/UnusedOpensTests.fs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,3 +753,56 @@ module M2 =
753753
let _ = T()
754754
"""
755755
=>[]
756+
757+
[<Test>]
758+
let``unused open declaration in top level rec module``()=
759+
"""
760+
module rec TopModule
761+
open System
762+
open System.IO
763+
let _ = DateTime.Now
764+
"""
765+
=>[4,(5,14)]
766+
767+
[<Test>]
768+
let``unused open declaration in rec namespace``()=
769+
"""
770+
namespace rec TopNamespace
771+
open System
772+
open System.IO
773+
module Nested =
774+
let _ = DateTime.Now
775+
"""
776+
=>[4,(5,14)]
777+
778+
[<Test>]
779+
let``unused inner module open declaration in rec module``()=
780+
"""
781+
module rec TopModule
782+
783+
module Nested =
784+
let x = 1
785+
let f x = x
786+
type T() = class end
787+
type R = { F: int }
788+
789+
open Nested
790+
"""
791+
=>[10,(5,11)]
792+
793+
[<Test>]
794+
let``used inner module open declaration in rec module``()=
795+
"""
796+
module rec TopModule
797+
798+
module Nested =
799+
let x = 1
800+
let f x = x
801+
type T() = class end
802+
type R = { F: int }
803+
804+
open Nested
805+
806+
let _ = f 1
807+
"""
808+
=>[]

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp