Skip to content

Fix de-duplication of module names #6086

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
dsyme merged 2 commits into from
Jan 14, 2019
Merged

Fix de-duplication of module names #6086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5e07b1
possible fix for signature issue
dsyme Jan 12, 2019
f478815
fix de-duplication
dsyme Jan 12, 2019
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
51 changes: 27 additions & 24 deletions src/fsharp/CompileOps.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3520,34 +3520,37 @@ let PostParseModuleSpecs (defaultNamespace, filename, isLastCompiland, ParsedSig

ParsedInput.SigFile(ParsedSigFileInput(filename, qualName, scopedPragmas, hashDirectives, specs))

type ModuleNamesDict = Map<string,Map<string,QualifiedNameOfFile>>

/// Checks if a module name is already given and deduplicates the name if needed.
let DeduplicateModuleName (moduleNamesDict:IDictionary<string, Set<string>>) (paths: Set<string>) path (qualifiedNameOfFile: QualifiedNameOfFile) =
let count = if paths.Contains path then paths.Count else paths.Count + 1
moduleNamesDict.[qualifiedNameOfFile.Text] <- Set.add path paths
let id = qualifiedNameOfFile.Id
if count = 1 then qualifiedNameOfFile else QualifiedNameOfFile(Ident(id.idText + "___" + count.ToString(), id.idRange))
let DeduplicateModuleName (moduleNamesDict:ModuleNamesDict) fileName (qualNameOfFile: QualifiedNameOfFile) =
let path = Path.GetDirectoryName fileName
let path = if FileSystem.IsPathRootedShim path then try FileSystem.GetFullPathShim path with _ -> path else path
Copy link
Contributor

@cartermp cartermp Jan 14, 2019

Choose a reason for hiding this comment

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

How often is DedeplicateModuleName called? I worry that this line could have a similar effect as #5932

Copy link
Contributor Author

@dsyme dsyme Jan 14, 2019

Choose a reason for hiding this comment

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

Once per check of a file - i.e. very rarely (In #5932 the GetFullPath was called like every identifier and every token in a file)

Copy link
Contributor

@cartermp cartermp Jan 14, 2019

Choose a reason for hiding this comment

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

Sounds good

match moduleNamesDict.TryGetValue qualNameOfFile.Text with
| true, paths ->
if paths.ContainsKey path then
paths.[path], moduleNamesDict
else
let count = paths.Count + 1
let id = qualNameOfFile.Id
let qualNameOfFileT = if count = 1 then qualNameOfFile else QualifiedNameOfFile(Ident(id.idText + "___" + count.ToString(), id.idRange))
let moduleNamesDictT = moduleNamesDict.Add(qualNameOfFile.Text, paths.Add(path, qualNameOfFileT))
qualNameOfFileT, moduleNamesDictT
| _ ->
let moduleNamesDictT = moduleNamesDict.Add(qualNameOfFile.Text, Map.empty.Add(path, qualNameOfFile))
qualNameOfFile, moduleNamesDictT

/// Checks if a ParsedInput is using a module name that was already given and deduplicates the name if needed.
let DeduplicateParsedInputModuleName (moduleNamesDict:IDictionary<string, Set<string>>) input =
let DeduplicateParsedInputModuleName (moduleNamesDict: ModuleNamesDict) input =
match input with
| ParsedInput.ImplFile (ParsedImplFileInput.ParsedImplFileInput(fileName, isScript, qualifiedNameOfFile, scopedPragmas, hashDirectives, modules, (isLastCompiland, isExe))) ->
let path = Path.GetDirectoryName fileName
match moduleNamesDict.TryGetValue qualifiedNameOfFile.Text with
| true, paths ->
let qualifiedNameOfFile = DeduplicateModuleName moduleNamesDict paths path qualifiedNameOfFile
ParsedInput.ImplFile(ParsedImplFileInput.ParsedImplFileInput(fileName, isScript, qualifiedNameOfFile, scopedPragmas, hashDirectives, modules, (isLastCompiland, isExe)))
| _ ->
moduleNamesDict.[qualifiedNameOfFile.Text] <- Set.singleton path
input
| ParsedInput.SigFile (ParsedSigFileInput.ParsedSigFileInput(fileName, qualifiedNameOfFile, scopedPragmas, hashDirectives, modules)) ->
let path = Path.GetDirectoryName fileName
match moduleNamesDict.TryGetValue qualifiedNameOfFile.Text with
| true, paths ->
let qualifiedNameOfFile = DeduplicateModuleName moduleNamesDict paths path qualifiedNameOfFile
ParsedInput.SigFile (ParsedSigFileInput.ParsedSigFileInput(fileName, qualifiedNameOfFile, scopedPragmas, hashDirectives, modules))
| _ ->
moduleNamesDict.[qualifiedNameOfFile.Text] <- Set.singleton path
input
| ParsedInput.ImplFile (ParsedImplFileInput.ParsedImplFileInput(fileName, isScript, qualNameOfFile, scopedPragmas, hashDirectives, modules, (isLastCompiland, isExe))) ->
let qualNameOfFileT, moduleNamesDictT = DeduplicateModuleName moduleNamesDict fileName qualNameOfFile
let inputT = ParsedInput.ImplFile(ParsedImplFileInput.ParsedImplFileInput(fileName, isScript, qualNameOfFileT, scopedPragmas, hashDirectives, modules, (isLastCompiland, isExe)))
inputT, moduleNamesDictT
| ParsedInput.SigFile (ParsedSigFileInput.ParsedSigFileInput(fileName, qualNameOfFile, scopedPragmas, hashDirectives, modules)) ->
let qualNameOfFileT, moduleNamesDictT = DeduplicateModuleName moduleNamesDict fileName qualNameOfFile
let inputT = ParsedInput.SigFile (ParsedSigFileInput.ParsedSigFileInput(fileName, qualNameOfFileT, scopedPragmas, hashDirectives, modules))
inputT, moduleNamesDictT

let ParseInput (lexer, errorLogger:ErrorLogger, lexbuf:UnicodeLexing.Lexbuf, defaultNamespace, filename, isLastCompiland) =
// The assert below is almost ok, but it fires in two cases:
Expand Down
7 changes: 4 additions & 3 deletions src/fsharp/CompileOps.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,13 @@ val ComputeQualifiedNameOfFileFromUniquePath: range * string list -> Ast.Qualifi

val PrependPathToInput: Ast.Ident list -> Ast.ParsedInput -> Ast.ParsedInput

/// Checks if a module name is already given and deduplicates the name if needed.
val DeduplicateModuleName: IDictionary<string,Set<string>> -> Set<string> -> string -> Ast.QualifiedNameOfFile -> Ast.QualifiedNameOfFile
/// State used to de-deuplicate module names along a list of file names
type ModuleNamesDict = Map<string,Map<string,QualifiedNameOfFile>>

/// Checks if a ParsedInput is using a module name that was already given and deduplicates the name if needed.
val DeduplicateParsedInputModuleName: IDictionary<string,Set<string>> -> Ast.ParsedInput -> Ast.ParsedInput
val DeduplicateParsedInputModuleName: ModuleNamesDict -> Ast.ParsedInput -> Ast.ParsedInput * ModuleNamesDict

/// Parse a single input (A signature file or implementation file)
val ParseInput: (UnicodeLexing.Lexbuf -> Parser.token) * ErrorLogger * UnicodeLexing.Lexbuf * string option * string * isLastCompiland:(bool * bool) -> Ast.ParsedInput

//----------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/TypeChecker.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10060,7 +10060,7 @@ and TcMethodApplication
| CallerLineNumber, _ when typeEquiv cenv.g currCalledArgTy cenv.g.int_ty ->
emptyPreBinder, Expr.Const(Const.Int32(mMethExpr.StartLine), mMethExpr, currCalledArgTy)
| CallerFilePath, _ when typeEquiv cenv.g currCalledArgTy cenv.g.string_ty ->
emptyPreBinder, Expr.Const(Const.String(System.IO.Path.GetFullPath(mMethExpr.FileName)), mMethExpr, currCalledArgTy)
emptyPreBinder, Expr.Const(Const.String(FileSystem.GetFullPathShim(mMethExpr.FileName)), mMethExpr, currCalledArgTy)
| CallerMemberName, Some(callerName) when (typeEquiv cenv.g currCalledArgTy cenv.g.string_ty) ->
emptyPreBinder, Expr.Const(Const.String(callerName), mMethExpr, currCalledArgTy)
| _ ->
Expand Down Expand Up @@ -10099,7 +10099,7 @@ and TcMethodApplication
let lineExpr = Expr.Const(Const.Int32(mMethExpr.StartLine), mMethExpr, calledNonOptTy)
emptyPreBinder, mkUnionCaseExpr(mkSomeCase cenv.g, [calledNonOptTy], [lineExpr], mMethExpr)
| CallerFilePath, _ when typeEquiv cenv.g calledNonOptTy cenv.g.string_ty ->
let filePathExpr = Expr.Const(Const.String(System.IO.Path.GetFullPath(mMethExpr.FileName)), mMethExpr, calledNonOptTy)
let filePathExpr = Expr.Const(Const.String(FileSystem.GetFullPathShim(mMethExpr.FileName)), mMethExpr, calledNonOptTy)
emptyPreBinder, mkUnionCaseExpr(mkSomeCase cenv.g, [calledNonOptTy], [filePathExpr], mMethExpr)
| CallerMemberName, Some(callerName) when typeEquiv cenv.g calledNonOptTy cenv.g.string_ty ->
let memberNameExpr = Expr.Const(Const.String(callerName), mMethExpr, calledNonOptTy)
Expand Down
10 changes: 4 additions & 6 deletions src/fsharp/fsc.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1773,11 +1773,9 @@ let main0(ctok, argv, legacyReferenceResolver, bannerAlreadyPrinted, reduceMemor
errorRecoveryNoRange e
exiter.Exit 1

let inputs =
// Deduplicate module names
let moduleNamesDict = ConcurrentDictionary<string,Set<string>>()
inputs
|> List.map (fun (input,x) -> DeduplicateParsedInputModuleName moduleNamesDict input,x)
let inputs, _ =
(Map.empty, inputs)
||> List.mapFold (fun state (input,x) -> let inputT, stateT = DeduplicateParsedInputModuleName state input in (inputT,x), stateT)

if tcConfig.parseOnly then exiter.Exit 0
if not tcConfig.continueAfterParseFailure then
Expand Down Expand Up @@ -2036,7 +2034,7 @@ let main4 dynamicAssemblyCreator (Args (ctok, tcConfig, tcImports: TcImports, t

DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent ctok

let pdbfile = pdbfile |> Option.map (tcConfig.MakePathAbsolute >> Path.GetFullPath)
let pdbfile = pdbfile |> Option.map (tcConfig.MakePathAbsolute >> FileSystem.GetFullPathShim)

let normalizeAssemblyRefs (aref:ILAssemblyRef) =
match tcImports.TryFindDllInfo (ctok, Range.rangeStartup, aref.Name, lookupOnly=false) with
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/range.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ type range(code1:int64, code2: int64) =

let mkRange f b e =
// remove relative parts from full path
let normalizedFilePath = if Path.IsPathRooted f then try Path.GetFullPath f with _ -> f else f
let normalizedFilePath = if FileSystem.IsPathRootedShim f then try FileSystem.GetFullPathShim f with _ -> f else f
range (fileIndexOfFile normalizedFilePath, b, e)

let mkFileIndexRange fi b e = range (fi, b, e)
Expand Down
Loading