Skip to content
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.

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

Don't crash graph checking on invalid syntax #16588

Merged
merged 9 commits into from Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Expand Up @@ -2,11 +2,11 @@

* Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514))
* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550))
* Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575))
* Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575), [PR #16588](https://github.com/dotnet/fsharp/pull/16588))
psfinaki marked this conversation as resolved.
Show resolved Hide resolved

### Added

* The stackguard depth for ILPdbWriter.unshadowScopes can be modified via the environment variable `FSHARP_ILPdb_UnshadowScopes_StackGuardDepth`([PR #16583](https://github.com/dotnet/fsharp/pull/16583))
* The stackguard depth for ILPdbWriter.unshadowScopes can be modified via the environment variable `FSHARP_ILPdb_UnshadowScopes_StackGuardDepth`([PR #16583](https://github.com/dotnet/fsharp/pull/16583))
* Parser recovers on complex primary constructor patterns, better tree representation for primary constructor patterns. ([PR #16425](https://github.com/dotnet/fsharp/pull/16425))
* Name resolution: keep type vars in subsequent checks ([PR #16456](https://github.com/dotnet/fsharp/pull/16456))
* Higher-order-function-based API for working with the untyped abstract syntax tree. ([PR #16462](https://github.com/dotnet/fsharp/pull/16462))
Expand Down
7 changes: 3 additions & 4 deletions src/Compiler/Driver/GraphChecking/FileContentMapping.fs
Expand Up @@ -9,10 +9,9 @@ type Continuations = ((FileContentEntry list -> FileContentEntry list) -> FileCo
let collectFromOption (mapping: 'T -> 'U list) (t: 'T option) : 'U list = List.collect mapping (Option.toList t)

let longIdentToPath (skipLast: bool) (longId: LongIdent) : LongIdentifier =
if skipLast then
List.take (longId.Length - 1) longId
else
longId
match skipLast, longId with
| true, _ :: _ -> List.take (longId.Length - 1) longId
| _ -> longId
|> List.map (fun ident -> ident.idText)

let synLongIdentToPath (skipLast: bool) (synLongIdent: SynLongIdent) =
Expand Down
Expand Up @@ -122,17 +122,36 @@ module B = C
| [ TopLevelNamespace "" [ PrefixedIdentifier "C" ] ] -> Assert.Pass()
| content -> Assert.Fail($"Unexpected content: {content}")

[<Test>]
let ``Invalid nested module should just be ignored`` () =
let content =
getContent
false
"""
module A

module B.C
"""
module We_Shouldnt_Crash_With_Invalid_Syntax =
0101 marked this conversation as resolved.
Show resolved Hide resolved

match content with
| [ TopLevelNamespace "" [] ] -> Assert.Pass()
| content -> Assert.Fail($"Unexpected content: {content}")
[<Test>]
let ``Nested module`` () =
let content =
getContent
false
"""
module A

module B.C
"""

match content with
| [ TopLevelNamespace "" [] ] -> Assert.Pass()
| content -> Assert.Fail($"Unexpected content: {content}")


[<Test>]
let ``Module above namespace`` () =
let content =
getContent
false
"""
module

namespace A.B.C
"""

match content with
| [ TopLevelNamespace "" [] ] -> Assert.Pass()
| content -> Assert.Fail($"Unexpected content: {content}")