diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md index 4cd1275d7d6..b2529248f16 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md @@ -5,6 +5,8 @@ ### Fixed * Capture diagnostics emitted when transparent project checking finishes a project, including FS0247 for namespace-module name collisions. ([Issue #6036](https://github.com/dotnet/fsharp/issues/6036), [PR #20706](https://github.com/dotnet/fsharp/pull/20706)) +* A project reusing cached framework imports is checked with its own `--pathmap` instead of the map of the project that filled the cache, so the ranges of its in-memory reference data name its real files. ([Issue #20474](https://github.com/dotnet/fsharp/issues/20474), [PR #20476](https://github.com/dotnet/fsharp/pull/20476)) +* `GetDeclarationLocation` no longer names the compile-time directory twice for a declaration in an assembly built with a relative `--pathmap`. Such a build maps the directory it compiled in along with its file names, so a file from under that directory already starts with it, and joining the two left a path that matches no file. Other relative names, such as the `#line` names that compilers before F# 10 wrote, are still resolved against that directory. ([PR #20518](https://github.com/dotnet/fsharp/pull/20518)) * Split Linux CoreCLR CI into two parallel test batches to reduce test turnaround time. ([PR #20587](https://github.com/dotnet/fsharp/pull/20587)) * Fix `NativePtr.stackalloc` used inside the `with` handler, filter or `finally` block of a `try` expression producing an assembly that throws `InvalidProgramException` at method load. Direct or mandatory-inline allocations are now reported at compile time as error FS3924; calls to non-inline allocation helpers, including handler-local helpers, remain legal and retain their method boundary during inlining and let-binding elimination. ([Issue #20295](https://github.com/dotnet/fsharp/issues/20295), [PR #20491](https://github.com/dotnet/fsharp/pull/20491)) * `FsiEvaluationSession.EvalInteraction` and `EvalInteractionNonThrowing` evaluate every `;;`-separated interaction in the text they are given, as their documentation has always said, instead of silently discarding everything after the first. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) diff --git a/src/Compiler/Service/IncrementalBuild.fs b/src/Compiler/Service/IncrementalBuild.fs index f68f3495547..d6121baa6b3 100644 --- a/src/Compiler/Service/IncrementalBuild.fs +++ b/src/Compiler/Service/IncrementalBuild.fs @@ -560,12 +560,13 @@ type FrameworkImportsCache(size) = let node = this.GetNode(tcConfig, frameworkDLLs, nonFrameworkResolutions) let! tcGlobals, frameworkTcImports = node.GetOrComputeValue() - // If the tcGlobals was loaded from a different project, langVersion and realsig may be different - // for each cached project. So here we create a new tcGlobals, with the existing framework values - // and updated realsig and langversion + // If the tcGlobals was loaded from a different project, langVersion, realsig and pathMap may be + // different for each cached project. So here we create a new tcGlobals, with the existing framework + // values and the updated realsig, langversion and pathMap let tcGlobals = if tcGlobals.langVersion <> tcConfig.langVersion - || tcGlobals.realsig <> tcConfig.realsig then + || tcGlobals.realsig <> tcConfig.realsig + || tcGlobals.pathMap <> tcConfig.pathMap then TcGlobals( tcGlobals.compilingFSharpCore, tcGlobals.ilg, @@ -577,7 +578,7 @@ type FrameworkImportsCache(size) = tcGlobals.tryFindSysTypeCcuHelper, tcGlobals.emitDebugInfoInQuotations, tcGlobals.noDebugAttributes, - tcGlobals.pathMap, + tcConfig.pathMap, tcConfig.langVersion, tcConfig.realsig, tcConfig.compilationMode diff --git a/src/Compiler/Service/TransparentCompiler.fs b/src/Compiler/Service/TransparentCompiler.fs index 8821c39aaca..ca6e1a65abe 100644 --- a/src/Compiler/Service/TransparentCompiler.fs +++ b/src/Compiler/Service/TransparentCompiler.fs @@ -964,13 +964,14 @@ type internal TransparentCompiler // Prepare the frameworkTcImportsCache let! tcGlobals, frameworkTcImports = ComputeFrameworkImports tcConfig frameworkDLLs nonFrameworkResolutions - // If the tcGlobals was loaded from a different project, langVersion and realsig may be different - // for each cached project. So here we create a new tcGlobals, with the existing framework values - // and updated realsig and langversion + // If the tcGlobals was loaded from a different project, langVersion, realsig and pathMap may be + // different for each cached project. So here we create a new tcGlobals, with the existing + // framework values and the updated realsig, langversion and pathMap let tcGlobals = if tcGlobals.langVersion <> tcConfig.langVersion || tcGlobals.realsig <> tcConfig.realsig + || tcGlobals.pathMap <> tcConfig.pathMap then TcGlobals( tcGlobals.compilingFSharpCore, @@ -983,7 +984,7 @@ type internal TransparentCompiler tcGlobals.tryFindSysTypeCcuHelper, tcGlobals.emitDebugInfoInQuotations, tcGlobals.noDebugAttributes, - tcGlobals.pathMap, + tcConfig.pathMap, tcConfig.langVersion, tcConfig.realsig, tcConfig.compilationMode diff --git a/src/Compiler/Symbols/SymbolHelpers.fs b/src/Compiler/Symbols/SymbolHelpers.fs index 222edab7c13..97d5a08ebd4 100644 --- a/src/Compiler/Symbols/SymbolHelpers.fs +++ b/src/Compiler/Symbols/SymbolHelpers.fs @@ -226,18 +226,96 @@ module internal SymbolHelpers = // This is not expected: NewDef only occurs within checking | Item.NewDef _ -> None + /// The module or type of another assembly that an item is declared in, or the item itself when it is one. + let rec declaringEntityOfItem d = + let ofValRef (vref: ValRef) = + if vref.IsLocalRef then ValueNone else ValueSome vref.nlr.EnclosingEntity + + let ofArbitraryValRef (vref: ValRef option) = + match vref with + | Some vref -> ofValRef vref + | None -> ValueNone + + match d with + | Item.Value vref + | Item.CustomBuilder (_, vref) -> ofValRef vref + | Item.ActivePatternCase apref -> ofValRef apref.ActivePatternVal + | Item.UnionCase(ucinfo, _) + | Item.UnionCaseField (ucinfo, _) -> ValueSome ucinfo.TyconRef + | Item.RecdField rfinfo -> ValueSome rfinfo.TyconRef + | Item.ExnCase tcref -> ValueSome tcref + | Item.Event einfo -> ofArbitraryValRef einfo.ArbitraryValRef + | Item.Property(info = pinfo :: _) -> ofArbitraryValRef pinfo.ArbitraryValRef + | Item.MethodGroup(_, minfo :: _, _) + | Item.CtorGroup(_, minfo :: _) -> + match minfo.ArbitraryValRef with + | Some vref -> ofValRef vref + | None -> ValueSome minfo.DeclaringTyconRef + | Item.Types(_, ty :: _) + | Item.DelegateCtor ty -> + match tryNiceEntityRefOfTyOption ty with + | Some tcref -> ValueSome tcref + | None -> ValueNone + | Item.ModuleOrNamespaces (eref :: _) + | Item.UnqualifiedType (eref :: _) -> ValueSome eref + | Item.SetterArg (_, item) -> declaringEntityOfItem item + | _ -> ValueNone + + /// The module or type directly under a namespace that an entity of another assembly lies in, or is. + let outermostDeclaringEntity (eref: EntityRef) = + if eref.IsLocalRef then + ValueNone + else + let (NonLocalEntityRef(ccu, path)) = eref.nlr + + let rec search length = + if length > path.Length then + ValueNone + else + match NonLocalEntityRef(ccu, path[.. length - 1]).TryDeref(canError = false) with + | ValueSome entity when not entity.IsNamespace -> ValueSome entity + | ValueSome _ -> search (length + 1) + | ValueNone -> ValueNone + + search 1 + /// Work out the source file for an item and fix it up relative to the CCU if it is relative. let fileNameOfItem (g: TcGlobals) qualProjectDir (m: range) h = let file = m.FileName if verbose then dprintf "file stored in metadata is '%s'\n" file + + // Compilers before F# 10 named whatever follows a #line directive by the directive, and such a name can + // start with the directory too. The directive leaves the item in another file than the module or type + // around it, though, where a compiled file holds both. An entity read from a signature says nothing + // of the file its implementation is in. + let namedByLineDirective () = + match declaringEntityOfItem h |> ValueOption.bind outermostDeclaringEntity with + | ValueSome entity -> + let declaredIn = entity.Range + + m.FileIndex <> declaredIn.FileIndex + && not (CompilerConfig.FSharpSigFileSuffixes |> List.exists (FileSystemUtils.checkSuffix declaredIn.FileName)) + | ValueNone -> false + + // A relative name is relative to the directory the assembly was compiled in, as the #line names that + // compilers before F# 10 wrote are. A path map rewrites that directory along with the files compiled + // from under it, though, so such a file already starts with the directory: joining them repeats it. + let underDirectory (dir: string) = + let startsWithDirectory = + file.Length > dir.Length + && file.StartsWithOrdinal dir + && (file[dir.Length] = '\\' || file[dir.Length] = '/') + + if startsWithDirectory && not (namedByLineDirective ()) then file else Path.Combine(dir, file) + if not (FileSystem.IsPathRootedShim file) then match ccuOfItem g h with | Some ccu -> - Path.Combine(ccu.SourceCodeDirectory, file) + underDirectory ccu.SourceCodeDirectory | None -> match qualProjectDir with | None -> file - | Some dir -> Path.Combine(dir, file) + | Some dir -> underDirectory dir else file let ParamNameAndTypesOfUnaryCustomOperation g minfo = diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 8cd5ee0e922..2b473df5e1b 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -1,4 +1,4 @@ - + @@ -549,6 +549,7 @@ + diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMap.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMap.fs new file mode 100644 index 00000000000..a72b279bdcb --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMap.fs @@ -0,0 +1,245 @@ +module FSharpChecker.PathMap + +open System.IO +open System.Threading.Tasks +open Xunit +open FSharp.Test +open FSharp.Test.Compiler +open FSharp.Test.ProjectGeneration +open FSharp.Test.Utilities +open FSharp.Compiler.CodeAnalysis +open FSharp.Compiler.EditorServices +open FSharp.Compiler.Symbols +open FSharp.Compiler.Text + +let private checkWith (checker: FSharpChecker) (project: SyntheticProject) = + ProjectWorkflowBuilder(project, checker = checker).Yield() |> Async.Ignore + +/// The file an App names as declaring the function it uses from the library, once another project has filled the +/// cache: the framework imports, and the TcGlobals with them, are cached per framework set. +let private libraryFileSeenAfter (useTransparentCompiler: bool) (cacheFiller: SyntheticProject) (library: SyntheticProject) = + task { + let checker = FSharpChecker.Create(useTransparentCompiler = useTransparentCompiler) + + let app = + { SyntheticProject.Create("App", sourceFile "App" [ "Library" ]) with + DependsOn = [ library ] } + + do! checkWith checker cacheFiller + do! checkWith checker app + + let appFile = app.GetFilePath "App" + + let! _, answer = + checker.ParseAndCheckFileInProject( + appFile, + 0, + SourceText.ofString (File.ReadAllText appFile), + app.GetProjectOptions checker + ) + + let checkResults = + match answer with + | FSharpCheckFileAnswer.Succeeded checkResults -> checkResults + | FSharpCheckFileAnswer.Aborted -> failwith "the check was aborted" + + let libraryFunction = + checkResults.GetAllUsesOfAllSymbolsInFile() + |> Seq.tryFind (fun symbolUse -> symbolUse.Symbol.FullName = $"{library.Name}.ModuleLibrary.f") + |> Option.defaultWith (fun () -> + failwith + $"""ModuleLibrary.f not used; symbols: %A{checkResults.GetAllUsesOfAllSymbolsInFile() |> Seq.map _.Symbol.FullName |> Seq.distinct |> List.ofSeq}""") + + return libraryFunction.Symbol.DeclarationLocation.Value.FileName + } + +/// The path map of the project that filled the cache must not reach the ranges a sibling exposes to its consumers. +[] +[] +[] +let ``a sibling's path map does not reach the ranges of a project without one`` (useTransparentCompiler: bool) : Task = + task { + let library = SyntheticProject.Create("Library", sourceFile "Library" []) + + let mapped = + { SyntheticProject.Create("Mapped", sourceFile "Mapped" []) with + OtherOptions = [ $"--pathmap:{Path.GetDirectoryName library.ProjectDir}=.\\" ] } + + let! declaredIn = libraryFileSeenAfter useTransparentCompiler mapped library + + Assert.Equal(library.GetFilePath "Library", declaredIn) + } + +/// Nor may a project lose its own path map to a sibling without one that filled the cache first. +[] +[] +[] +let ``a project's path map reaches its ranges after a sibling without one filled the cache`` (useTransparentCompiler: bool) : Task = + task { + let unmapped = SyntheticProject.Create("Unmapped", sourceFile "Unmapped" []) + let library = SyntheticProject.Create("Library", sourceFile "Library" []) + let root = Path.GetDirectoryName library.ProjectDir + + let library = + { library with + OtherOptions = [ $"--pathmap:{root}=." ] } + + let! declaredIn = libraryFileSeenAfter useTransparentCompiler unmapped library + + Assert.Equal("." + (library.GetFilePath "Library").Substring(root.Length), declaredIn) + } + +/// Compiles the library from its own directory, as a build does: the assembly records that directory as the one it +/// was compiled in. +let private emitLibrary (root: string) (pathMap: string list) (withSignature: bool) = + let projectDir = Directory.CreateDirectory(Path.Combine(root, "src", "Domain")).FullName + File.WriteAllText(Path.Combine(projectDir, "Library.fs"), "module Library\n\nlet f x = x + 1\n") + + if withSignature then + File.WriteAllText(Path.Combine(projectDir, "Library.fsi"), "module Library\n\nval f: x: int -> int\n") + + // The arguments are joined into one command line, where a reference under "Program Files" or a root with a + // space in it would fall apart + let quoted (option: string) = $"\"{option}\"" + + let result = + runFscProcessIn projectDir [ + "--target:library" + for option in CompilerAssert.DefaultProjectOptions(TargetFramework.Current).OtherOptions do + quoted option + for option in pathMap do + quoted option + "-o:Library.dll" + if withSignature then + "Library.fsi" + "Library.fs" + ] + + Assert.True((result.ExitCode = 0), $"fsc exited with {result.ExitCode}:\n{result.StdOut}\n{result.StdErr}") + projectDir + +/// Read back through the path map, a declaration names the file the library was compiled from, whatever the map. +/// The compiler writes each name under a path map with the mapped directory it compiled in at its start; a #line +/// name, which compilers before F# 10 wrote instead, is relative to that directory. With a signature, the module +/// is read from the one file and the implementation of its function is in the other. +[] +[] +[] +[] +[] +let ``a declaration in a referenced assembly names the file it was compiled from`` (mappedRoot: string) (withSignature: bool) : Task = + task { + let root = TestFramework.createTemporaryDirectory().FullName + let pathMap = if mappedRoot = "" then [] else [ $"--pathmap:{root}={mappedRoot}" ] + let projectDir = emitLibrary root pathMap withSignature + + let usageLine = "let _ = Library.f 1" + let source = $"module Consumer\n{usageLine}\n" + let consumer = Path.Combine(root, "Consumer.fs") + File.WriteAllText(consumer, source) + + let options = + let defaults = CompilerAssert.DefaultProjectOptions(TargetFramework.Current) + + { defaults with + SourceFiles = [| consumer |] + OtherOptions = [| yield! defaults.OtherOptions; $"""-r:{Path.Combine(projectDir, "Library.dll")}""" |] } + + let! _, answer = FSharpChecker.Create().ParseAndCheckFileInProject(consumer, 0, SourceText.ofString source, options) + + let checkResults = + match answer with + | FSharpCheckFileAnswer.Succeeded checkResults -> checkResults + | FSharpCheckFileAnswer.Aborted -> failwith "the check was aborted" + + let endOfName = usageLine.Length - " 1".Length + let names = [ "Library"; "f" ] + + let declarationOf (preferSignature: bool) = + match checkResults.GetDeclarationLocation(2, endOfName, usageLine, names, preferSignature) with + | FindDeclResult.DeclFound range -> range + | result -> failwith $"expected the declaration of Library.f, got %A{result}" + + let declaration = declarationOf false + + // Since F# 10 the compiler writes the real file for a #line directive, so the name an older one wrote is + // resolved against this assembly's recorded directory directly + let fromLineDirective = + let symbol = checkResults.GetSymbolUseAtLocation(2, endOfName, usageLine, names).Value.Symbol + let range = Range.mkRange "Original.fs" declaration.Start declaration.End + SymbolHelpers.fileNameOfItem symbol.SymbolEnv.g None range symbol.Item + + // A reader that knows the map turns a name back into the file it stands for, reading a rooted name as the + // full path GetDeclarationLocation gives + let compiledFrom (fileName: string) = + let asRead (path: string) = + if Path.IsPathRooted path then Path.GetFullPath path else path + + if mappedRoot = "" then + Path.GetFullPath fileName + else + let fileName, mappedRoot = asRead fileName, asRead mappedRoot + Assert.StartsWith(mappedRoot, fileName) + Path.GetFullPath(Path.Combine(root, fileName.Substring(mappedRoot.Length).TrimStart('/', '\\'))) + + Assert.Equal(Path.Combine(projectDir, "Library.fs"), compiledFrom declaration.FileName) + Assert.Equal(Path.Combine(projectDir, "Original.fs"), compiledFrom fromLineDirective) + + if withSignature then + Assert.Equal(Path.Combine(projectDir, "Library.fsi"), compiledFrom (declarationOf true).FileName) + } + +/// PathMapFSharp9.dll, next to this file, is what the F# 9 compiler (FSharp.Compiler.Service 43.9.300) made of +/// +/// module Library +/// +/// let ordinary x = x + 1 +/// +/// #line 1 ".\src\Domain\Original.fs" +/// let fromLine x = x + 2 +/// +/// module Nested = +/// let fromLineNested x = x + 3 +/// +/// in a root's src\Domain, with --pathmap:root=. and --deterministic+. Before F# 10 the compiler named whatever +/// follows a #line directive by the directive: here that name starts with the mapped directory, as the name of +/// the compiled file does, and yet is relative to it. +[] +[] +[] +[] +let ``a #line name an older compiler wrote keeps the directory that the name of a compiled file already starts with`` + (name: string) + (recorded: string) + (keepsDirectory: bool) + : Task = + task { + let usageLine = $"let _ = Library.{name}" + let source = $"module Consumer\n{usageLine} 1\n" + let consumer = Path.Combine(TestFramework.createTemporaryDirectory().FullName, "Consumer.fs") + File.WriteAllText(consumer, source) + + let options = + let defaults = CompilerAssert.DefaultProjectOptions(TargetFramework.Current) + + { defaults with + SourceFiles = [| consumer |] + OtherOptions = [| yield! defaults.OtherOptions; $"""-r:{Path.Combine(__SOURCE_DIRECTORY__, "PathMapFSharp9.dll")}""" |] } + + let! _, answer = FSharpChecker.Create().ParseAndCheckFileInProject(consumer, 0, SourceText.ofString source, options) + + let checkResults = + match answer with + | FSharpCheckFileAnswer.Succeeded checkResults -> checkResults + | FSharpCheckFileAnswer.Aborted -> failwith "the check was aborted" + + let declaration = + match checkResults.GetDeclarationLocation(2, usageLine.Length, $"{usageLine} 1", "Library" :: List.ofArray (name.Split '.')) with + | FindDeclResult.DeclFound range -> range + | result -> failwith $"expected the declaration of Library.{name}, got %A{result}" + + let expected = + if keepsDirectory then Path.Combine(@".\src\Domain", recorded) else recorded + + Assert.Equal(expected, declaration.FileName) + } diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMapFSharp9.dll b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMapFSharp9.dll new file mode 100644 index 00000000000..01fb458cb57 Binary files /dev/null and b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMapFSharp9.dll differ diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index f7a22ac6c64..f93165a83e0 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -2552,7 +2552,7 @@ $ code --diff {outFile} {expectedFile} type ProcessResult = { ExitCode: int; StdOut: string; StdErr: string } /// Run an F# tool (FSI or FSC) as a subprocess. Shared helper for runFsiProcess / runFscProcess. - let private runToolProcess (toolPath: string) (args: string list) : ProcessResult = + let private runToolProcess (toolPath: string) (workingDirectory: string) (args: string list) : ProcessResult = #if NETCOREAPP let exe = TestFramework.initialConfig.DotNetExe let arguments = toolPath + " " + (args |> String.concat " ") @@ -2560,16 +2560,20 @@ $ code --diff {outFile} {expectedFile} let exe = toolPath let arguments = args |> String.concat " " #endif - let exitCode, stdout, stderr = Commands.executeProcess exe arguments (Directory.GetCurrentDirectory()) + let exitCode, stdout, stderr = Commands.executeProcess exe arguments workingDirectory { ExitCode = exitCode; StdOut = stdout; StdErr = stderr } /// Run FSI as a subprocess with the given arguments. For CLI-level tests only (--help, exit codes, etc.). let runFsiProcess (args: string list) : ProcessResult = - runToolProcess TestFramework.initialConfig.FSI args + runToolProcess TestFramework.initialConfig.FSI (Directory.GetCurrentDirectory()) args + + /// Run FSC as a subprocess in the given directory, which the compiled assembly records as the one it was compiled in. + let runFscProcessIn (workingDirectory: string) (args: string list) : ProcessResult = + runToolProcess TestFramework.initialConfig.FSC workingDirectory args /// Run FSC as a subprocess with the given arguments. For CLI-level tests only (missing files, exit codes, etc.). let runFscProcess (args: string list) : ProcessResult = - runToolProcess TestFramework.initialConfig.FSC args + runFscProcessIn (Directory.GetCurrentDirectory()) args /// Compile-and-run a compilation unit that depends on a FSharp.Core attribute /// which may not yet be shipped in the SDK's NuGet package.