From 5a000980839b35c65680d329d4c27d3ab3792377 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 5 Jun 2021 06:15:39 +0200 Subject: [PATCH] Add determinism to processing of files. Files can be processes in different orders on different architectures because of how their layout on disk. Add determinism by using sets instead of lists. --- analysis/src/NewCompletions.ml | 6 ++++-- analysis/src/Packages.ml | 8 ++++++-- analysis/src/ProcessCmt.ml | 12 ++++++------ analysis/src/References.ml | 6 +++--- analysis/src/SharedTypes.ml | 14 +++++++++++--- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/analysis/src/NewCompletions.ml b/analysis/src/NewCompletions.ml index bbec85cc9..97e81b5ed 100644 --- a/analysis/src/NewCompletions.ml +++ b/analysis/src/NewCompletions.ml @@ -773,7 +773,7 @@ let getItems ~full ~package ~rawOpens ~allFiles ~pos ~parts = in (* TODO complete the namespaced name too *) let localModuleNames = - allFiles + allFiles |> FileSet.elements |> Utils.filterMap (fun name -> if Utils.startsWith name suffix && not (String.contains name '-') then Some {(emptyDeclared name) with item = FileModule name} @@ -1148,7 +1148,9 @@ let computeCompletions ~uri ~textOpt ~pos = | Some full -> let rawOpens = PartialParser.findOpens text offset in let package = full.package in - let allFiles = package.projectFiles @ package.dependenciesFiles in + let allFiles = + FileSet.union package.projectFiles package.dependenciesFiles + in let findItems ~exact parts = let items = getItems ~full ~package ~rawOpens ~allFiles ~pos ~parts diff --git a/analysis/src/Packages.ml b/analysis/src/Packages.ml index d095503b7..7401d71f0 100644 --- a/analysis/src/Packages.ml +++ b/analysis/src/Packages.ml @@ -84,8 +84,12 @@ let newBsPackage rootPath = Log.log ("Opens from bsconfig: " ^ (opens |> String.concat " ")); { SharedTypes.rootPath; - projectFiles = projectFilesAndPaths |> List.map fst; - dependenciesFiles = dependenciesFilesAndPaths |> List.map fst; + projectFiles = + projectFilesAndPaths |> List.map fst + |> SharedTypes.FileSet.of_list; + dependenciesFiles = + dependenciesFilesAndPaths |> List.map fst + |> SharedTypes.FileSet.of_list; pathsForModule; opens; namespace; diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index e5b372827..e13e000bc 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -734,12 +734,12 @@ struct else [])) let addFileReference moduleName loc = - Hashtbl.replace extra.fileReferences moduleName - (loc - :: - (if Hashtbl.mem extra.fileReferences moduleName then - Hashtbl.find extra.fileReferences moduleName - else [])) + let newLocs = + match Hashtbl.find_opt extra.fileReferences moduleName with + | Some oldLocs -> LocationSet.add loc oldLocs + | None -> LocationSet.singleton loc + in + Hashtbl.replace extra.fileReferences moduleName newLocs let env = QueryEnv.fromFile Collector.file diff --git a/analysis/src/References.ml b/analysis/src/References.ml index 9dcc16aa7..f6e1e38fc 100644 --- a/analysis/src/References.ml +++ b/analysis/src/References.ml @@ -426,7 +426,7 @@ let forLocalStamp ~full:{file; extra; package} stamp tip = maybeLog ("Now checking path " ^ pathToString path); let thisModuleName = file.moduleName in let externals = - package.projectFiles + package.projectFiles |> FileSet.elements |> List.filter (fun name -> name <> file.moduleName) |> Utils.filterMap (fun name -> match ProcessCmt.fileForModule ~package name with @@ -460,7 +460,7 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem = match locItem.locType with | TopLevelModule moduleName -> let otherModulesReferences = - package.projectFiles + package.projectFiles |> FileSet.elements |> Utils.filterMap (fun name -> match ProcessCmt.fileForModule ~package name with | None -> None @@ -469,7 +469,7 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem = match Hashtbl.find_opt full.extra.fileReferences moduleName with | None -> [] | Some locs -> - locs + locs |> LocationSet.elements |> List.map (fun loc -> (Uri2.fromPath loc.Location.loc_start.pos_fname, [loc]))) |> List.flatten diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index 948996527..4f074194e 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -200,10 +200,16 @@ type openTracker = { mutable used : (path * tip * Location.t) list; } +module LocationSet = Set.Make (struct + include Location + + let compare loc1 loc2 = compare loc2 loc1 (* polymorphic compare should be OK *) +end) + type extra = { internalReferences : (int, Location.t list) Hashtbl.t; externalReferences : (string, (path * tip * Location.t) list) Hashtbl.t; - fileReferences : (string, Location.t list) Hashtbl.t; + fileReferences : (string, LocationSet.t) Hashtbl.t; mutable locItems : locItem list; (* This is the "open location", like the location... or maybe the >> location of the open ident maybe *) @@ -213,10 +219,12 @@ type extra = { type file = string +module FileSet = Set.Make (String) + type package = { rootPath : filePath; - projectFiles : file list; - dependenciesFiles : file list; + projectFiles : FileSet.t; + dependenciesFiles : FileSet.t; pathsForModule : (file, paths) Hashtbl.t; namespace : string option; opens : string list;