Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 24 additions & 2 deletions src/Compiler/Driver/GraphChecking/DependencyResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ let mkGraph (filePairs: FilePairMap) (files: FileInProject array) : Graph<FileIn
if file.Idx = 0 then
// First file cannot have any dependencies.
Array.empty

else
let fileContent = fileContents[file.Idx]

Expand Down Expand Up @@ -235,20 +236,41 @@ let mkGraph (filePairs: FilePairMap) (files: FileInProject array) : Graph<FileIn
| None -> Array.empty
| Some sigIdx -> Array.singleton sigIdx

let wrongOrderSignature =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a lot of ugly code to deal with a single edge case, maybe can be made nicer.

if file.ParsedInput.IsSigFile then
match filePairs.TryGetWrongOrderSignatureToImplementationIndex file.Idx with
| Some idx -> Array.singleton idx
| None -> Array.empty
else
Array.empty

let allDependencies =
[|
yield! depsResult.FoundDependencies
yield! ghostDependencies
yield! signatureDependency
yield! wrongOrderSignature
|]
|> Array.distinct

allDependencies

let graph =
// 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
Comment on lines +258 to +261
Copy link
Contributor Author

@majocha majocha Oct 14, 2025

Choose a reason for hiding this comment

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

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


let sequentialPartForScriptCompilation =
files
|> Array.take scriptCompilationLength
|> Array.map (fun file -> file.Idx, [| if file.Idx > 0 then file.Idx - 1 |])

let normalPart =
files
|> Array.skip scriptCompilationLength
|> Array.Parallel.map (fun file -> file.Idx, findDependencies file)
|> readOnlyDict

let graph = Array.append sequentialPartForScriptCompilation normalPart |> readOnlyDict

let trie = trie |> Array.last |> snd

Expand Down
14 changes: 14 additions & 0 deletions src/Compiler/Driver/GraphChecking/Types.fs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ type internal FileInProject =
ParsedInput: ParsedInput
}

member x.IsScript =
match x.ParsedInput with
| ParsedInput.ImplFile impl -> impl.IsScript
| _ -> false

/// There is a subtle difference between a module and namespace.
/// A namespace does not necessarily expose a set of dependent files.
/// Only when the namespace exposes types that could later be inferred.
Expand Down Expand Up @@ -175,5 +180,14 @@ type internal FilePairMap(files: FileInProject array) =

member x.IsSignature(index: FileIndex) = Map.containsKey index sigToImpl

member x.TryGetWrongOrderSignatureToImplementationIndex(index: FileIndex) =
let input = files[index].ParsedInput

files
|> Array.truncate index
|> Array.tryFindIndex (fun f ->
f.ParsedInput.IsImplFile
&& f.ParsedInput.QualifiedName.Text = input.QualifiedName.Text)

/// Callback that returns a previously calculated 'Result and updates 'State accordingly.
type internal Finisher<'Node, 'State, 'Result> = Finisher of node: 'Node * finisher: ('State -> 'Result * 'State)
3 changes: 3 additions & 0 deletions src/Compiler/Driver/GraphChecking/Types.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ type internal FileInProject =
FileName: FileName
ParsedInput: ParsedInput }

member IsScript: bool

/// There is a subtle difference between a module and namespace.
/// A namespace does not necessarily expose a set of dependent files.
/// Only when the namespace exposes types that could later be inferred.
Expand Down Expand Up @@ -115,6 +117,7 @@ type internal FilePairMap =
member HasSignature: implementationIndex: FileIndex -> bool
member TryGetSignatureIndex: implementationIndex: FileIndex -> FileIndex option
member IsSignature: index: FileIndex -> bool
member TryGetWrongOrderSignatureToImplementationIndex: index: FileIndex -> FileIndex option

/// Callback that returns a previously calculated 'Result and updates 'State accordingly.
type internal Finisher<'Node, 'State, 'Result> = Finisher of node: 'Node * finisher: ('State -> 'Result * 'State)
12 changes: 8 additions & 4 deletions src/Compiler/Driver/ParseAndCheckInputs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1786,7 +1786,7 @@ let CheckMultipleInputsUsingGraphMode
(idx, friendlyFileName))
|> Graph.writeMermaidToFile graphFile)

let _ = ctok // TODO Use it
ignore ctok // TODO Use it
let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLogger

// In the first linear part of parallel checking, we use a 'checkForErrors' that checks either for errors
Expand Down Expand Up @@ -1880,8 +1880,11 @@ let CheckMultipleInputsUsingGraphMode
let CheckClosedInputSet (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tcGlobals, prefixPathOpt, tcState, eagerFormat, inputs) =
// tcEnvAtEndOfLastFile is the environment required by fsi.exe when incrementally adding definitions
let results, tcState =
match tcConfig.typeCheckingConfig.Mode with
| TypeCheckingMode.Graph when (not tcConfig.isInteractive && not tcConfig.compilingFSharpCore) ->
if
not tcConfig.deterministic
&& not tcConfig.isInteractive
&& not tcConfig.compilingFSharpCore
then
CheckMultipleInputsUsingGraphMode(
ctok,
checkForErrors,
Expand All @@ -1893,7 +1896,8 @@ let CheckClosedInputSet (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tc
eagerFormat,
inputs
)
| _ -> CheckMultipleInputsSequential(ctok, checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, tcState, inputs)
else
CheckMultipleInputsSequential(ctok, checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, tcState, inputs)

let (tcEnvAtEndOfLastFile, topAttrs, implFiles, _), tcState =
CheckMultipleInputsFinish(results, tcState)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ let ``type check neg55`` () = singleNegTest ( "typecheck/sigs") "neg55"
[<FactForDESKTOP>]
let ``type check neg56`` () = singleNegTest ( "typecheck/sigs") "neg56"

[<FactForDESKTOP(Skip = "Failing in new test framework")>]
[<Fact>]
let ``type check neg56_a`` () = singleNegTest ( "typecheck/sigs") "neg56_a"

[<FactForDESKTOP>]
Expand Down
3 changes: 0 additions & 3 deletions tests/fsharp/tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2282,9 +2282,6 @@ module TypecheckTests =
[<Fact>]
let ``type check neg49`` () = singleNegTest (testConfig "typecheck/sigs") "neg49"

[<Fact>]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same test is enabled in ComponentTests now.

let ``type check neg56_a`` () = singleNegTest (testConfig "typecheck/sigs") "neg56_a"

[<Fact>]
let ``type check neg94`` () = singleNegTest (testConfig "typecheck/sigs") "neg94"

Expand Down
8 changes: 8 additions & 0 deletions tests/fsharp/typecheck/sigs/neg14.bsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@

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'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Graph checking produces more (valid) diagnostics, why?


neg14a.fs(2,8,2,11): typecheck error FS0193: Module 'Lib' requires a type 'missingTypeVariableInSignature'

neg14a.fs(2,8,2,11): typecheck error FS0193: Module 'Lib' requires a type 'missingTypeInImplementation'

neg14a.fs(2,8,2,11): typecheck error FS0193: Module 'Lib' requires a type 'fieldsInWrongOrder'

neg14b.fs(2,13,2,14): typecheck error FS0039: The value, constructor, namespace or type 'X' is not defined.
2 changes: 2 additions & 0 deletions tests/fsharp/typecheck/sigs/neg56_a.bsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@

neg56_a.fs(11,35,11,47): typecheck error FS1125: The instantiation of the generic type 'list1' is missing and can't be inferred from the arguments or return type of this member. Consider providing a type instantiation when accessing this type, e.g. 'list1<_>'.

neg56_a.fs(15,18,15,33): typecheck error FS3068: The function or member 'toList' is used in a way that requires further type annotations at its definition to ensure consistency of inferred types. The inferred signature is 'static member private list1.toList: ('a list1 -> 'a list)'.
Loading