diff --git a/src/Compiler/Driver/GraphChecking/DependencyResolution.fs b/src/Compiler/Driver/GraphChecking/DependencyResolution.fs index c3206efbdfb..243d5fb1772 100644 --- a/src/Compiler/Driver/GraphChecking/DependencyResolution.fs +++ b/src/Compiler/Driver/GraphChecking/DependencyResolution.fs @@ -17,33 +17,29 @@ let queryTriePartial (trie: TrieNode) (path: LongIdentifier) : TrieNode option = visit trie path -let mapNodeToQueryResult (currentFileIndex: FileIndex) (node: TrieNode option) : QueryTrieNodeResult = +let mapNodeToQueryResult (node: TrieNode option) : QueryTrieNodeResult = match node with | Some finalNode -> - if - Set.isEmpty finalNode.Files - // If this node exposes files which the current index cannot see, we consider it not to have data at all. - || Set.forall (fun idx -> idx >= currentFileIndex) finalNode.Files - then + if Set.isEmpty finalNode.Files then QueryTrieNodeResult.NodeDoesNotExposeData else QueryTrieNodeResult.NodeExposesData(finalNode.Files) | None -> QueryTrieNodeResult.NodeDoesNotExist /// Find a path in the Trie. -let queryTrie (currentFileIndex: FileIndex) (trie: TrieNode) (path: LongIdentifier) : QueryTrieNodeResult = - queryTriePartial trie path |> mapNodeToQueryResult currentFileIndex +let queryTrie (trie: TrieNode) (path: LongIdentifier) : QueryTrieNodeResult = + queryTriePartial trie path |> mapNodeToQueryResult /// Same as 'queryTrie' but allows passing in a path combined from two parts, avoiding list allocation. -let queryTrieDual (currentFileIndex: FileIndex) (trie: TrieNode) (path1: LongIdentifier) (path2: LongIdentifier) : QueryTrieNodeResult = +let queryTrieDual (trie: TrieNode) (path1: LongIdentifier) (path2: LongIdentifier) : QueryTrieNodeResult = match queryTriePartial trie path1 with | Some intermediateNode -> queryTriePartial intermediateNode path2 | None -> None - |> mapNodeToQueryResult currentFileIndex + |> mapNodeToQueryResult /// Process namespace declaration. let processNamespaceDeclaration (trie: TrieNode) (path: LongIdentifier) (state: FileContentQueryState) : FileContentQueryState = - let queryResult = queryTrie state.CurrentFile trie path + let queryResult = queryTrie trie path match queryResult with | QueryTrieNodeResult.NodeDoesNotExist -> state @@ -53,7 +49,7 @@ let processNamespaceDeclaration (trie: TrieNode) (path: LongIdentifier) (state: /// Process an "open" statement. /// The statement could link to files and/or should be tracked as an open namespace. let processOpenPath (trie: TrieNode) (path: LongIdentifier) (state: FileContentQueryState) : FileContentQueryState = - let queryResult = queryTrie state.CurrentFile trie path + let queryResult = queryTrie trie path match queryResult with | QueryTrieNodeResult.NodeDoesNotExist -> state @@ -103,13 +99,12 @@ let rec processStateEntry (trie: TrieNode) (state: FileContentQueryState) (entry ||> Array.fold (fun state takeParts -> let path = List.take takeParts path // process the name was if it were a FQN - let stateAfterFullIdentifier = - processIdentifier (queryTrieDual state.CurrentFile trie [] path) state + let stateAfterFullIdentifier = processIdentifier (queryTrieDual trie [] path) state // Process the name in combination with the existing open namespaces (stateAfterFullIdentifier, state.OpenNamespaces) ||> Set.fold (fun acc openNS -> - let queryResult = queryTrieDual state.CurrentFile trie openNS path + let queryResult = queryTrieDual trie openNS path processIdentifier queryResult acc)) | FileContentEntry.NestedModule (nestedContent = nestedContent) -> @@ -142,7 +137,7 @@ let collectGhostDependencies (fileIndex: FileIndex) (trie: TrieNode) (result: Fi // For each opened namespace, if none of already resolved dependencies define it, return the top-most file that defines it. Set.toArray result.OpenedNamespaces |> Array.choose (fun path -> - match queryTrie fileIndex trie path with + match queryTrie trie path with | QueryTrieNodeResult.NodeExposesData _ | QueryTrieNodeResult.NodeDoesNotExist -> None | QueryTrieNodeResult.NodeDoesNotExposeData -> @@ -201,20 +196,25 @@ let mkGraph (filePairs: FilePairMap) (files: FileInProject array) : Graph set + // The Trie we want to use is the one that contains only files before our current index. + // As we skip implementation files (backed by a signature), we cannot just use the current file index to find the right Trie. + let trieForFile = + trie + |> Array.fold (fun acc (idx, t) -> if idx < file.Idx then t else acc) TrieNode.Empty + // File depends on all files above it that define accessible symbols at the root level (global namespace). - let filesFromRoot = trie.Files |> Set.filter (fun rootIdx -> rootIdx < file.Idx) + let filesFromRoot = + trieForFile.Files |> Set.filter (fun rootIdx -> rootIdx < file.Idx) // Start by listing root-level dependencies. - let initialDepsResult = - (FileContentQueryState.Create file.Idx knownFiles filesFromRoot), fileContent + let initialDepsResult = (FileContentQueryState.Create filesFromRoot), fileContent // Sequentially process all relevant entries of the file and keep updating the state and set of dependencies. let depsResult = initialDepsResult // Seq is faster than List in this case. - ||> Seq.fold (processStateEntry trie) + ||> Seq.fold (processStateEntry trieForFile) // Add missing links for cases where an unused open namespace did not create a link. - let ghostDependencies = collectGhostDependencies file.Idx trie depsResult + let ghostDependencies = collectGhostDependencies file.Idx trieForFile depsResult // Add a link from implementation files to their signature files. let signatureDependency = @@ -237,4 +237,6 @@ let mkGraph (filePairs: FilePairMap) (files: FileInProject array) : Graph Array.Parallel.map (fun file -> file.Idx, findDependencies file) |> readOnlyDict + let trie = trie |> Array.last |> snd + graph, trie diff --git a/src/Compiler/Driver/GraphChecking/DependencyResolution.fsi b/src/Compiler/Driver/GraphChecking/DependencyResolution.fsi index a2c52adcea9..8804999117e 100644 --- a/src/Compiler/Driver/GraphChecking/DependencyResolution.fsi +++ b/src/Compiler/Driver/GraphChecking/DependencyResolution.fsi @@ -3,10 +3,9 @@ module internal FSharp.Compiler.GraphChecking.DependencyResolution /// /// Query a TrieNode to find a certain path. -/// The result will take the current file index into account to determine if the result node contains data. /// /// This code is only used directly in unit tests. -val queryTrie: currentFileIndex: FileIndex -> trie: TrieNode -> path: LongIdentifier -> QueryTrieNodeResult +val queryTrie: trie: TrieNode -> path: LongIdentifier -> QueryTrieNodeResult /// Process an open path (found in the ParsedInput) with a given FileContentQueryState. /// This code is only used directly in unit tests. diff --git a/src/Compiler/Driver/GraphChecking/TrieMapping.fs b/src/Compiler/Driver/GraphChecking/TrieMapping.fs index e34939abac1..8e836332071 100644 --- a/src/Compiler/Driver/GraphChecking/TrieMapping.fs +++ b/src/Compiler/Driver/GraphChecking/TrieMapping.fs @@ -1,16 +1,19 @@ module internal FSharp.Compiler.GraphChecking.TrieMapping open System.Collections.Generic +open System.Collections.Immutable open System.Text open FSharp.Compiler.IO open FSharp.Compiler.Syntax [] -module private HashSet = +module private ImmutableHashSet = /// Create a new HashSet<'T> with a single element. - let singleton value = HashSet(Seq.singleton value) + let singleton (value: 'T) = + ImmutableHashSet.Create<'T>(Array.singleton value) + /// Create new new HashSet<'T> with zero elements. - let empty () = HashSet(Seq.empty) + let empty () = ImmutableHashSet.Empty let autoOpenShapes = set @@ -57,72 +60,46 @@ let doesFileExposeContentToTheRoot (ast: ParsedInput) : bool = (isAnyAttributeAutoOpen attribs && longId.Length < 2) || kind = SynModuleOrNamespaceKind.GlobalNamespace) -let mergeTrieNodes (defaultChildSize: int) (tries: TrieNode array) = - /// Add the current node as child node to the root node. - /// If the node already exists and is a namespace node, the existing node will be updated with new information via mutation. - let rec mergeTrieNodesAux (root: TrieNode) (KeyValue (k, v)) = - if root.Children.ContainsKey k then - let node = root.Children[k] - - match node.Current, v.Current with - | TrieNodeInfo.Namespace (filesThatExposeTypes = currentFilesThatExposeTypes - filesDefiningNamespaceWithoutTypes = currentFilesWithoutTypes), - TrieNodeInfo.Namespace (filesThatExposeTypes = otherFiles; filesDefiningNamespaceWithoutTypes = otherFilesWithoutTypes) -> - currentFilesThatExposeTypes.UnionWith otherFiles - currentFilesWithoutTypes.UnionWith otherFilesWithoutTypes - // Edge case scenario detected in https://github.com/dotnet/fsharp/issues/15985 - | TrieNodeInfo.Namespace (filesThatExposeTypes = currentFilesThatExposeTypes), TrieNodeInfo.Module (_name, file) -> - // Keep the namespace (as it can still have nested children). - currentFilesThatExposeTypes.Add file |> ignore - | TrieNodeInfo.Module (_name, file), TrieNodeInfo.Namespace (filesThatExposeTypes = currentFilesThatExposeTypes) -> - currentFilesThatExposeTypes.Add file |> ignore - // Replace the module in favour of the namespace (which can hold nested children). - root.Children[ k ] <- v - | _ -> () - - for kv in v.Children do - mergeTrieNodesAux node kv - - else - root.Children.Add(k, v) - - match Array.tryExactlyOne tries with - | Some ({ Current = TrieNodeInfo.Root _ } as singleTrie) -> singleTrie - | _ -> - let rootFiles = HashSet.empty () - - let root = - { - Current = TrieNodeInfo.Root rootFiles - Children = Dictionary<_, _>(defaultChildSize) - } - - for trie in tries do - for rootIndex in trie.Files do - rootFiles.Add rootIndex |> ignore - - match trie.Current with - | TrieNodeInfo.Root _ -> () - | current -> System.Diagnostics.Debug.Assert(false, $"The top level node info of a trie should be Root, got {current}") - - for kv in trie.Children do - mergeTrieNodesAux root kv - - root - -let mkDictFromKeyValuePairs (items: KeyValuePair<'TKey, 'TValue> list) = - let dict = Dictionary(Seq.length items) +/// Merge all the accumulator Trie nodes into the current Trie node. +let rec mergeTrieNodes (accumulatorTrie: TrieNode) (currentTrie: TrieNode) : TrieNode = + let nextNodeInfo: TrieNodeInfo = + match accumulatorTrie.Current, currentTrie.Current with + | TrieNodeInfo.Root accFiles, TrieNodeInfo.Root currentFiles -> TrieNodeInfo.Root(accFiles.Union currentFiles) + | TrieNodeInfo.Namespace (name = name + filesThatExposeTypes = currentFilesThatExposeTypes + filesDefiningNamespaceWithoutTypes = currentFilesWithoutTypes), + TrieNodeInfo.Namespace (filesThatExposeTypes = otherFiles; filesDefiningNamespaceWithoutTypes = otherFilesWithoutTypes) -> + TrieNodeInfo.Namespace( + name, + currentFilesThatExposeTypes.Union otherFiles, + currentFilesWithoutTypes.Union otherFilesWithoutTypes + ) + // Edge case scenario detected in https://github.com/dotnet/fsharp/issues/15985 + // Keep the namespace (as it can still have nested children). + | TrieNodeInfo.Namespace (name, currentFilesThatExposeTypes, filesDefiningNamespaceWithoutTypes), TrieNodeInfo.Module (_name, file) + // Replace the module in favour of the namespace (which can hold nested children). + | TrieNodeInfo.Module (_name, file), TrieNodeInfo.Namespace (name, currentFilesThatExposeTypes, filesDefiningNamespaceWithoutTypes) -> + TrieNodeInfo.Namespace(name, currentFilesThatExposeTypes.Add file, filesDefiningNamespaceWithoutTypes) + | _ -> accumulatorTrie.Current + + let nextChildren = + (accumulatorTrie.Children, currentTrie.Children) + ||> Seq.fold (fun accChildren (KeyValue (k, v)) -> + if not (accChildren.ContainsKey k) then + accChildren.Add(k, v) + else + let accNode = accChildren[k] + accChildren.SetItem(k, mergeTrieNodes accNode v)) - for KeyValue (k, v) in items do - if not (dict.ContainsKey(k)) then - dict.Add(k, v) + { + Current = nextNodeInfo + Children = nextChildren + } - dict +let mkImmutableDictFromKeyValuePairs (items: KeyValuePair<'TKey, 'TValue> list) = ImmutableDictionary.CreateRange(items) let mkSingletonDict key value = - let dict = Dictionary(1) - dict.Add(key, value) - dict + ImmutableDictionary.Empty.Add(key, value) /// Process a top level SynModuleOrNamespace(Sig) let processSynModuleOrNamespace<'Decl> @@ -158,16 +135,16 @@ let processSynModuleOrNamespace<'Decl> if isNamespace then let filesThatExposeTypes, filesDefiningNamespaceWithoutTypes = if hasTypesOrAutoOpenNestedModules then - HashSet.singleton idx, HashSet.empty () + ImmutableHashSet.singleton idx, ImmutableHashSet.empty () else - HashSet.empty (), HashSet.singleton idx + ImmutableHashSet.empty (), ImmutableHashSet.singleton idx TrieNodeInfo.Namespace(name, filesThatExposeTypes, filesDefiningNamespaceWithoutTypes) else TrieNodeInfo.Module(name, idx) let children = - List.choose (mkTrieForDeclaration idx) decls |> mkDictFromKeyValuePairs + List.choose (mkTrieForDeclaration idx) decls |> mkImmutableDictFromKeyValuePairs mkSingletonDict name @@ -193,10 +170,10 @@ let processSynModuleOrNamespace<'Decl> let topLevelModuleOrNamespaceHasAutoOpen = isAnyAttributeAutoOpen attributes if topLevelModuleOrNamespaceHasAutoOpen && not isNamespace then - HashSet.singleton idx, HashSet.empty () + ImmutableHashSet.singleton idx, ImmutableHashSet.empty () else - HashSet.empty (), HashSet.singleton idx - | _ -> HashSet.empty (), HashSet.singleton idx + ImmutableHashSet.empty (), ImmutableHashSet.singleton idx + | _ -> ImmutableHashSet.empty (), ImmutableHashSet.singleton idx let current = TrieNodeInfo.Namespace(name, filesThatExposeTypes, filesDefiningNamespaceWithoutTypes) @@ -206,61 +183,69 @@ let processSynModuleOrNamespace<'Decl> if kind = SynModuleOrNamespaceKind.AnonModule then // We collect the child nodes from the decls - decls |> List.choose (mkTrieForDeclaration idx) |> mkDictFromKeyValuePairs + decls + |> List.choose (mkTrieForDeclaration idx) + |> mkImmutableDictFromKeyValuePairs else visit id name { - Current = Root(HashSet.empty ()) + Current = Root(ImmutableHashSet.empty ()) Children = children } -let rec mkTrieNodeFor (file: FileInProject) : TrieNode = +let rec mkTrieNodeFor (file: FileInProject) : FileIndex * TrieNode = let idx = file.Idx if doesFileExposeContentToTheRoot file.ParsedInput then // If a file exposes content which does not need an open statement to access, we consider the file to be part of the root. + idx, { - Current = Root(HashSet.singleton idx) - Children = Dictionary(0) + Current = Root(ImmutableHashSet.singleton idx) + Children = ImmutableDictionary.Empty } else - match file.ParsedInput with - | ParsedInput.SigFile (ParsedSigFileInput (contents = contents)) -> - contents - |> List.map - (fun (SynModuleOrNamespaceSig (longId = longId - kind = kind - attribs = attribs - decls = decls - accessibility = _accessibility)) -> - let hasTypesOrAutoOpenNestedModules = - decls - |> List.exists (function - | SynModuleSigDecl.Types _ -> true - | SynModuleSigDecl.NestedModule(moduleInfo = SynComponentInfo (attributes = attributes)) -> - isAnyAttributeAutoOpen attributes - | _ -> false) - - processSynModuleOrNamespace mkTrieForSynModuleSigDecl idx longId attribs kind hasTypesOrAutoOpenNestedModules decls) - |> List.toArray - |> mergeTrieNodes contents.Length - | ParsedInput.ImplFile (ParsedImplFileInput (contents = contents)) -> - contents - |> List.map - (fun (SynModuleOrNamespace (longId = longId; attribs = attribs; kind = kind; decls = decls; accessibility = _accessibility)) -> - let hasTypesOrAutoOpenNestedModules = - List.exists - (function - | SynModuleDecl.Types _ -> true - | SynModuleDecl.NestedModule(moduleInfo = SynComponentInfo (attributes = attributes)) -> - isAnyAttributeAutoOpen attributes - | _ -> false) + let trie = + match file.ParsedInput with + | ParsedInput.SigFile (ParsedSigFileInput (contents = contents)) -> + contents + |> List.map + (fun (SynModuleOrNamespaceSig (longId = longId + kind = kind + attribs = attribs + decls = decls + accessibility = _accessibility)) -> + let hasTypesOrAutoOpenNestedModules = decls - - processSynModuleOrNamespace mkTrieForSynModuleDecl idx longId attribs kind hasTypesOrAutoOpenNestedModules decls) - |> List.toArray - |> mergeTrieNodes contents.Length + |> List.exists (function + | SynModuleSigDecl.Types _ -> true + | SynModuleSigDecl.NestedModule(moduleInfo = SynComponentInfo (attributes = attributes)) -> + isAnyAttributeAutoOpen attributes + | _ -> false) + + processSynModuleOrNamespace mkTrieForSynModuleSigDecl idx longId attribs kind hasTypesOrAutoOpenNestedModules decls) + |> List.reduce mergeTrieNodes + | ParsedInput.ImplFile (ParsedImplFileInput (contents = contents)) -> + contents + |> List.map + (fun (SynModuleOrNamespace (longId = longId + attribs = attribs + kind = kind + decls = decls + accessibility = _accessibility)) -> + let hasTypesOrAutoOpenNestedModules = + List.exists + (function + | SynModuleDecl.Types _ -> true + | SynModuleDecl.NestedModule(moduleInfo = SynComponentInfo (attributes = attributes)) -> + isAnyAttributeAutoOpen attributes + | _ -> false) + decls + + processSynModuleOrNamespace mkTrieForSynModuleDecl idx longId attribs kind hasTypesOrAutoOpenNestedModules decls) + |> List.reduce mergeTrieNodes + + idx, trie and mkTrieForSynModuleDecl (fileIndex: FileIndex) (decl: SynModuleDecl) : KeyValuePair option = match decl with @@ -270,7 +255,7 @@ and mkTrieForSynModuleDecl (fileIndex: FileIndex) (decl: SynModuleDecl) : KeyVal let children = decls |> List.choose (mkTrieForSynModuleDecl fileIndex) - |> mkDictFromKeyValuePairs + |> mkImmutableDictFromKeyValuePairs Some( KeyValuePair( @@ -291,7 +276,7 @@ and mkTrieForSynModuleSigDecl (fileIndex: FileIndex) (decl: SynModuleSigDecl) : let children = decls |> List.choose (mkTrieForSynModuleSigDecl fileIndex) - |> mkDictFromKeyValuePairs + |> mkImmutableDictFromKeyValuePairs Some( KeyValuePair( @@ -302,11 +287,22 @@ and mkTrieForSynModuleSigDecl (fileIndex: FileIndex) (decl: SynModuleSigDecl) : } ) ) - | _ -> None -let mkTrie (files: FileInProject array) : TrieNode = - mergeTrieNodes 0 (files |> Array.Parallel.map mkTrieNodeFor) +let mkTrie (files: FileInProject array) : (FileIndex * TrieNode) array = + if files.Length = 1 then + Array.singleton (mkTrieNodeFor files[0]) + else + files + |> Array.take (files.Length - 1) // Do not process the last file, it will never be looked up by anything anyway. + |> Array.Parallel.map mkTrieNodeFor + |> Array.scan + (fun (_, acc) (idx, current) -> + let next = mergeTrieNodes acc current + idx, next) + (System.Int32.MinValue, TrieNode.Empty) + // We can ignore the initial state that was used in the scan + |> Array.skip 1 type MermaidBoxPos = | First @@ -323,7 +319,7 @@ let serializeToMermaid (path: string) (filesInProject: FileInProject array) (tri | Module (name, _) -> $"mod_{name}" | Namespace (name, _, _) -> $"ns_{name}" - let toBoxList (boxPos: MermaidBoxPos) (files: HashSet) = + let toBoxList (boxPos: MermaidBoxPos) (files: ImmutableHashSet) = let sb = StringBuilder() let orderedIndexes = Seq.sort files diff --git a/src/Compiler/Driver/GraphChecking/TrieMapping.fsi b/src/Compiler/Driver/GraphChecking/TrieMapping.fsi index 00784a5da33..649a2a25e0e 100644 --- a/src/Compiler/Driver/GraphChecking/TrieMapping.fsi +++ b/src/Compiler/Driver/GraphChecking/TrieMapping.fsi @@ -2,6 +2,6 @@ module internal FSharp.Compiler.GraphChecking.TrieMapping /// Process all the files (in parallel) in a project to construct a Root TrieNode. /// When the project has signature files, the implementation counterparts will not be processed. -val mkTrie: files: FileInProject array -> TrieNode +val mkTrie: files: FileInProject array -> (FileIndex * TrieNode) array val serializeToMermaid: path: string -> filesInProject: FileInProject array -> trie: TrieNode -> unit diff --git a/src/Compiler/Driver/GraphChecking/Types.fs b/src/Compiler/Driver/GraphChecking/Types.fs index 04359519b95..72370ed41d0 100644 --- a/src/Compiler/Driver/GraphChecking/Types.fs +++ b/src/Compiler/Driver/GraphChecking/Types.fs @@ -1,6 +1,6 @@ namespace FSharp.Compiler.GraphChecking -open System.Collections.Generic +open System.Collections.Immutable open FSharp.Compiler.Syntax /// The index of a file inside a project. @@ -30,9 +30,12 @@ type internal FileInProject = /// Only when the namespace exposes types that could later be inferred. /// Children of a namespace don't automatically depend on each other for that reason type internal TrieNodeInfo = - | Root of files: HashSet + | Root of files: ImmutableHashSet | Module of name: Identifier * file: FileIndex - | Namespace of name: Identifier * filesThatExposeTypes: HashSet * filesDefiningNamespaceWithoutTypes: HashSet + | Namespace of + name: Identifier * + filesThatExposeTypes: ImmutableHashSet * + filesDefiningNamespaceWithoutTypes: ImmutableHashSet member x.Files: Set = match x with @@ -43,11 +46,19 @@ type internal TrieNodeInfo = type internal TrieNode = { Current: TrieNodeInfo - Children: Dictionary + Children: ImmutableDictionary } member x.Files = x.Current.Files + static member Empty = + let rootFiles = ImmutableHashSet.Empty + + { + Current = TrieNodeInfo.Root rootFiles + Children = ImmutableDictionary.Empty + } + /// A significant construct found in the syntax tree of a file. /// This construct needs to be processed in order to deduce potential links to other files in the project. type internal FileContentEntry = @@ -75,25 +86,20 @@ type internal FileContentQueryState = OwnNamespace: LongIdentifier option OpenedNamespaces: Set FoundDependencies: Set - CurrentFile: FileIndex - KnownFiles: Set } - static member Create (fileIndex: FileIndex) (knownFiles: Set) (filesAtRoot: Set) = + static member Create(filesAtRoot: Set) = { OwnNamespace = None OpenedNamespaces = Set.empty FoundDependencies = filesAtRoot - CurrentFile = fileIndex - KnownFiles = knownFiles } member x.AddOwnNamespace(ns: LongIdentifier, ?files: Set) = match files with | None -> { x with OwnNamespace = Some ns } | Some files -> - let foundDependencies = - Set.filter x.KnownFiles.Contains files |> Set.union x.FoundDependencies + let foundDependencies = files |> Set.union x.FoundDependencies { x with OwnNamespace = Some ns @@ -101,7 +107,7 @@ type internal FileContentQueryState = } member x.AddDependencies(files: Set) : FileContentQueryState = - let files = Set.filter x.KnownFiles.Contains files |> Set.union x.FoundDependencies + let files = files |> Set.union x.FoundDependencies { x with FoundDependencies = files } member x.AddOpenNamespace(path: LongIdentifier, ?files: Set) = @@ -111,8 +117,7 @@ type internal FileContentQueryState = OpenedNamespaces = Set.add path x.OpenedNamespaces } | Some files -> - let foundDependencies = - Set.filter x.KnownFiles.Contains files |> Set.union x.FoundDependencies + let foundDependencies = files |> Set.union x.FoundDependencies { x with FoundDependencies = foundDependencies diff --git a/src/Compiler/Driver/GraphChecking/Types.fsi b/src/Compiler/Driver/GraphChecking/Types.fsi index 67403d51d98..468ef65889c 100644 --- a/src/Compiler/Driver/GraphChecking/Types.fsi +++ b/src/Compiler/Driver/GraphChecking/Types.fsi @@ -1,6 +1,6 @@ namespace FSharp.Compiler.GraphChecking -open System.Collections.Generic +open System.Collections.Immutable open FSharp.Compiler.Syntax /// The index of a file inside a project. @@ -28,14 +28,14 @@ type internal FileInProject = /// Only when the namespace exposes types that could later be inferred. /// Children of a namespace don't automatically depend on each other for that reason type internal TrieNodeInfo = - | Root of files: HashSet + | Root of files: ImmutableHashSet | Module of name: Identifier * file: FileIndex | Namespace of name: Identifier * /// Files that expose types that are part of this namespace. - filesThatExposeTypes: HashSet * + filesThatExposeTypes: ImmutableHashSet * /// Files that use this namespace but don't contain any types. - filesDefiningNamespaceWithoutTypes: HashSet + filesDefiningNamespaceWithoutTypes: ImmutableHashSet member Files: Set @@ -45,12 +45,14 @@ type internal TrieNode = /// Information about this node. Current: TrieNodeInfo /// Child nodes - Children: Dictionary + Children: ImmutableDictionary } /// Zero or more files that define the LongIdentifier represented by this node. member Files: Set + static member Empty: TrieNode + /// A significant construct found in the syntax tree of a file. /// This construct needs to be processed in order to deduce potential links to other files in the project. type internal FileContentEntry = @@ -75,12 +77,9 @@ type internal FileContent = type internal FileContentQueryState = { OwnNamespace: LongIdentifier option OpenedNamespaces: Set - FoundDependencies: Set - CurrentFile: FileIndex - KnownFiles: Set } + FoundDependencies: Set } - static member Create: - fileIndex: FileIndex -> knownFiles: Set -> filesAtRoot: Set -> FileContentQueryState + static member Create: filesAtRoot: Set -> FileContentQueryState member AddOwnNamespace: ns: LongIdentifier * ?files: Set -> FileContentQueryState member AddDependencies: files: Set -> FileContentQueryState member AddOpenNamespace: path: LongIdentifier * ?files: Set -> FileContentQueryState diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/QueryTrieTests.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/QueryTrieTests.fs index b82014b178a..e6fd44a3505 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/QueryTrieTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/QueryTrieTests.fs @@ -1,6 +1,7 @@ module TypeChecks.QueryTrieTests open System.Collections.Generic +open System.Collections.Immutable open NUnit.Framework open FSharp.Compiler.GraphChecking open FSharp.Compiler.GraphChecking.DependencyResolution @@ -614,15 +615,12 @@ let private files = |] let dictionary<'key, 'value when 'key: equality> (entries: ('key * 'value) seq) = - let dict = Dictionary(Seq.length entries) + entries + |> Seq.map KeyValuePair + |> ImmutableDictionary.CreateRange - for k, v in entries do - dict.Add(k, v) - - dict - -let private noChildren = Dictionary(0) -let emptyHS () = HashSet(0) +let private noChildren = ImmutableDictionary.Empty +let emptyHS () = ImmutableHashSet.Empty let indexOf name = Array.find (fun (fc: FileContent) -> fc.FileName = name) files |> fun fc -> fc.Idx @@ -653,14 +651,14 @@ let private fantomasCoreTrie: TrieNode = TrieNodeInfo.Namespace( "Fantomas", emptyHS (), - HashSet([| - indexOf "ISourceTextExtensions.fs" - indexOf "RangeHelpers.fs" - indexOf "AstExtensions.fs" - indexOf "TriviaTypes.fs" - indexOf "Utils.fs" - indexOf "SourceParser.fs" - |])) + ImmutableHashSet.CreateRange [| + indexOf "ISourceTextExtensions.fs" + indexOf "RangeHelpers.fs" + indexOf "AstExtensions.fs" + indexOf "TriviaTypes.fs" + indexOf "Utils.fs" + indexOf "SourceParser.fs" + |]) Children = dictionary [| @@ -670,14 +668,14 @@ let private fantomasCoreTrie: TrieNode = TrieNodeInfo.Namespace( "Core", emptyHS (), - HashSet([| - indexOf "ISourceTextExtensions.fs" - indexOf "RangeHelpers.fs" - indexOf "AstExtensions.fs" - indexOf "TriviaTypes.fs" - indexOf "Utils.fs" - indexOf "SourceParser.fs" - |])) + ImmutableHashSet.CreateRange [| + indexOf "ISourceTextExtensions.fs" + indexOf "RangeHelpers.fs" + indexOf "AstExtensions.fs" + indexOf "TriviaTypes.fs" + indexOf "Utils.fs" + indexOf "SourceParser.fs" + |]) Children = dictionary [| @@ -762,7 +760,7 @@ let private fantomasCoreTrie: TrieNode = [] let ``Query non existing node in trie`` () = let result = - queryTrie 7 fantomasCoreTrie [ "System"; "System"; "Runtime"; "CompilerServices" ] + queryTrie fantomasCoreTrie [ "System"; "System"; "Runtime"; "CompilerServices" ] match result with | QueryTrieNodeResult.NodeDoesNotExist -> Assert.Pass() @@ -770,7 +768,7 @@ let ``Query non existing node in trie`` () = [] let ``Query node that does not expose data in trie`` () = - let result = queryTrie 7 fantomasCoreTrie [ "Fantomas"; "Core" ] + let result = queryTrie fantomasCoreTrie [ "Fantomas"; "Core" ] match result with | QueryTrieNodeResult.NodeDoesNotExposeData -> Assert.Pass() @@ -779,7 +777,7 @@ let ``Query node that does not expose data in trie`` () = [] let ``Query module node that exposes one file`` () = let result = - queryTrie 7 fantomasCoreTrie [ "Fantomas"; "Core"; "ISourceTextExtensions" ] + queryTrie fantomasCoreTrie [ "Fantomas"; "Core"; "ISourceTextExtensions" ] match result with | QueryTrieNodeResult.NodeExposesData file -> @@ -789,22 +787,8 @@ let ``Query module node that exposes one file`` () = [] let ``ProcessOpenStatement full path match`` () = - let sourceParser = - Array.find (fun (f: FileContent) -> f.FileName = "SourceParser.fs") files - let state = - FileContentQueryState.Create - sourceParser.Idx - (set - [| - indexOf "AssemblyInfo.fs" - indexOf "ISourceTextExtensions.fs" - indexOf "RangeHelpers.fs" - indexOf "AstExtensions.fsi" - indexOf "TriviaTypes.fs" - indexOf "Utils.fs" - |]) - Set.empty + FileContentQueryState.Create Set.empty let result = processOpenPath fantomasCoreTrie [ "Fantomas"; "Core"; "AstExtensions" ] state diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs index 1211e6b19f3..acf321211d0 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -772,4 +772,4 @@ type DiGraph = obj """ (set [| 0 |]) ] - ] + ] \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs index d8a2e9da6a6..e5bc3ef0299 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs @@ -6,6 +6,8 @@ open TestUtils let private noDependencies = Set.empty +let private getLastTrie files = TrieMapping.mkTrie files |> Array.last |> snd + [] let ``Basic trie`` () = let sampleFiles = @@ -28,6 +30,7 @@ namespace X.Y type C = { CX: int; CY: int } """ + "D.fs", "module D" |] let files = @@ -39,7 +42,7 @@ type C = { CX: int; CY: int } ParsedInput = parseSourceCode (fileName, code) } : FileInProject) - let trie = TrieMapping.mkTrie files + let trie = getLastTrie files match trie.Current with | TrieNodeInfo.Root _ -> () @@ -64,7 +67,7 @@ type C = { CX: int; CY: int } [] let ``Toplevel AutoOpen module with prefixed namespace`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -91,7 +94,7 @@ let a = 0 [] let ``Toplevel AutoOpen module with multi prefixed namespace`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -120,7 +123,7 @@ let a = 0 [] let ``Global namespace should link files to the root node`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -148,6 +151,12 @@ type B = { Y : int } """ ) } + { + Idx = 2 + FileName = "B.fs" + // The last file shouldn't be processed + ParsedInput = Unchecked.defaultof + } |] Assert.AreEqual(set [| 0; 1 |], trie.Files) @@ -155,7 +164,7 @@ type B = { Y : int } [] let ``Module with a single ident and AutoOpen attribute should link files to root`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -185,6 +194,12 @@ type B = { Y : int } """ ) } + { + Idx = 2 + FileName = "B.fs" + // The last file shouldn't be processed + ParsedInput = Unchecked.defaultof + } |] Assert.AreEqual(set [| 0; 1 |], trie.Files) @@ -193,7 +208,7 @@ type B = { Y : int } [] let ``Module with AutoOpen attribute and two ident should expose file at two levels`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -220,7 +235,7 @@ type A = { A : int } [] let ``Module with AutoOpen attribute and three ident should expose file at last two levels`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -249,7 +264,7 @@ type A = { A : int } [] let ``Nested AutoOpen module in namespace will expose the file to the namespace node`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -280,7 +295,7 @@ module Z = [] let ``Two modules with the same name, only the first file exposes the index`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -317,7 +332,7 @@ let _ = () [] let ``Two nested modules with the same name, in named namespace`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -344,7 +359,7 @@ module ``module`` = [] let ``Two nested modules with the same name, in namespace global`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -370,7 +385,7 @@ module ``module`` = [] let ``Two nested modules with the same name, in anonymous module`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 1 @@ -394,7 +409,7 @@ module ``module`` = [] let ``Two nested modules with the same name, in nested module`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -423,7 +438,7 @@ module B = [] let ``Two nested modules with the same name, in nested module in signature file`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -450,7 +465,7 @@ module B = [] let ``Two namespaces with the same name in the same implementation file`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -476,7 +491,7 @@ module C = begin end [] let ``Two namespaces with the same name in the same signature file`` () = let trie = - TrieMapping.mkTrie + getLastTrie [| { Idx = 0 @@ -498,3 +513,33 @@ module C = begin end let aNode = trie.Children["A"] Assert.AreEqual(2, aNode.Children.Count) + +[] +let ``Tries are built up incrementally`` () = + let trie = + TrieMapping.mkTrie + [| + { + Idx = 0 + FileName = "A.fs" + ParsedInput = parseSourceCode ("A.fs", "module A") + } + { + Idx = 1 + FileName = "B.fs" + ParsedInput = parseSourceCode ("B.fs", "module B") + } + { + Idx = 2 + FileName = "C.fs" + ParsedInput = parseSourceCode ("C.fs", "module C") + } + { + Idx = 3 + FileName = "D.fs" + ParsedInput = parseSourceCode ("D.fs", "module D") + } + |] + + for idx, t in trie do + Assert.AreEqual(idx + 1, t.Children.Count)