Experiment: force enable graph checking #18990
Conversation
❗ Release notes requiredCaution No release notes found for the changed paths (see table below). Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format. The following format is recommended for this repository:
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request. You can open this PR in browser to add release notes: open in github.dev
|
majocha
force-pushed
the
graph-fsharpqa
branch
from
0ebbd8d to
c0637d9
Compare
|
The failing tests indicate that graph checking is not handling #load directives correctly or simply does not like script compilation mode at all? |
|
There were two classes of errors remaining:
In both cases the fix is to add graph edges: |
| | None -> Array.empty | ||
| | Some sigIdx -> Array.singleton sigIdx | ||
|
|
||
| let wrongOrderSignature = |
There was a problem hiding this comment.
This is a lot of ugly code to deal with a single edge case, maybe can be made nicer.
| [<Fact>] | ||
| let ``type check neg49`` () = singleNegTest (testConfig "typecheck/sigs") "neg49" | ||
|
|
||
| [<Fact>] |
There was a problem hiding this comment.
same test is enabled in ComponentTests now.
|
|
||
| neg14a.fs(9,6,9,33): typecheck error FS0343: The type 'missingInterfaceInSignature' implements 'System.IComparable' explicitly but provides no corresponding override for 'Object.Equals'. An implementation of 'Object.Equals' has been automatically provided, implemented via 'System.IComparable'. Consider implementing the override 'Object.Equals' explicitly | ||
|
|
||
| neg14a.fs(2,8,2,11): typecheck error FS0193: Module 'Lib' requires a type 'z' |
There was a problem hiding this comment.
Graph checking produces more (valid) diagnostics, why?
|
Unfortunately there is no quick way around properly handling load closure. |
| // If there is a script in the project, we just process sequentially all the files that may have been added as part of the script closure. | ||
| // That means all files up to the last script file. | ||
| let scriptCompilationLength = | ||
| files |> Array.tryFindIndexBack (fun f -> f.IsScript) |> Option.map (fun idx -> idx + 1) |> Option.defaultValue 0 |
There was a problem hiding this comment.
Alternatively, we could just drop down to fully sequential checking in such cases.
Ideally the dependency graph should take script load closure into account but that would require more redesign. Currently sources loaded by scripts are included along with those from commandline and the information is erased in fsc.fs
1 participant
No description provided.