search

safesparrow / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
1 stars 0 forks source link

Latest graph version does not compile FSharp.Compiler.Service correctly #15

Closed safesparrow closed 1 year ago

safesparrow commented 1 year ago

The explicit FCS compilation test is failing due to unknown references in some .fsi files.

I checked and both the nojaf/tcSigEnv-merging HEAD and heuristic_otel from before my changes from yesterday works, so the good news is it's just me somehow introducing a bug during the merge & cleanup.

EDIT: The above was incorrect - I was testing using an old artifact built using a different revision.

The actual result is that the changes from https://github.com/safesparrow/fsharp/pull/11 introduce a type-checking error. Specifically, using the following commit:

commit 0638d62d25910d2b9d0969bba8eb38911206e085 (HEAD, nojaf/tcSigEnv-merging)
Author: nojaf <florian.verdonck@outlook.com>
Date:   Thu Nov 10 13:52:39 2022 +0100

    Use correct file name in graph.

when I run the explicit FCS compilation test, I get the following errors:

C:\projekty\fsharp\heuristic\tests\ParallelTypeCheckingTests\Tests\.fcs_test\src\compiler\Checking\PostInferenceChecks.fsi(21,13): error FS0039: The namespace or module 'ConstraintSolver' is not defined.

C:\projekty\fsharp\heuristic\tests\ParallelTypeCheckingTests\Tests\.fcs_test\src\compiler\Optimize\LowerComputedCollections.fsi(10,12): error FS0039: The namespace or module 'ConstraintSolver' is not defined.

C:\projekty\fsharp\heuristic\tests\ParallelTypeCheckingTests\Tests\.fcs_test\src\compiler\CodeGen\IlxGen.fsi(94,10): error FS0039: The namespace or module 'Import' is not defined.

C:\projekty\fsharp\heuristic\tests\ParallelTypeCheckingTests\Tests\.fcs_test\src\compiler\CodeGen\IlxGen.fsi(94,41): error FS0039: The namespace or module 'ConstraintSolver' is not defined.

C:\projekty\fsharp\heuristic\tests\ParallelTypeCheckingTests\Tests\.fcs_test\src\compiler\Legacy\LegacyMSBuildReferenceResolver.fsi(6,26): error FS0039: The type 'LegacyReferenceResolver' is not defined.

so it looks like the changes made to TcState merging are not enough to avoid the forced dependency from .fsi files to all the files above them.

safesparrow commented 1 year ago

We looked at it with @nojaf and it turns out that the dependency graph is missing required links, which leads to the error. I'll have a look at why that is.

safesparrow commented 1 year ago

Fixed in graph-tc branch (current base branch for this project).