Skip to content

Commit

Permalink
Fix Internal error when analysing incomplete inherit member (#17905)
Browse files Browse the repository at this point in the history
* Internal error when analysing incomplete inherit member

* release notes

* release notes

* update syntax tree tests

* update baselines

* Update src/Compiler/Service/ServiceParseTreeWalk.fs

Co-authored-by: Brian Rourke Boll <[email protected]>

* Update src/Compiler/Driver/GraphChecking/FileContentMapping.fs

Co-authored-by: Brian Rourke Boll <[email protected]>

* Update src/Compiler/Service/ServiceParsedInputOps.fs

Co-authored-by: Brian Rourke Boll <[email protected]>

* Update src/Compiler/Service/ServiceParsedInputOps.fs

Co-authored-by: Brian Rourke Boll <[email protected]>

* update salsa tests

* try to debug the test using CI

* Fix tests?

---------

Co-authored-by: Brian Rourke Boll <[email protected]>
Co-authored-by: Petr <[email protected]>
  • Loading branch information
3 people authored Oct 24, 2024
1 parent 8ff644a commit ee8af8a
Show file tree
Hide file tree
Showing 17 changed files with 52 additions and 57 deletions.
2 changes: 2 additions & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* Ensure `frameworkTcImportsCache` mutations are thread-safe. ([PR #17795](https://github.com/dotnet/fsharp/pull/17795))
* Fix concurrency issue in `ILPreTypeDefImpl` ([PR #17812](https://github.com/dotnet/fsharp/pull/17812))
* Fix nullness inference for member val and other OO scenarios ([PR #17845](https://github.com/dotnet/fsharp/pull/17845))
* Fix internal error when analyzing incomplete inherit member ([PR #17905](https://github.com/dotnet/fsharp/pull/17905))


### Added
* Deprecate places where `seq` can be omitted. ([Language suggestion #1033](https://github.com/fsharp/fslang-suggestions/issues/1033), [PR #17772](https://github.com/dotnet/fsharp/pull/17772))
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4473,7 +4473,7 @@ module TcDeclarations =
let implements2 = members |> List.choose (function SynMemberDefn.Interface (interfaceType=ty) -> Some(ty, ty.Range) | _ -> None)
let inherits =
members |> List.choose (function
| SynMemberDefn.Inherit (ty, idOpt, m, _) -> Some(ty, m, idOpt)
| SynMemberDefn.Inherit (Some ty, idOpt, m, _) -> Some(ty, m, idOpt)
| SynMemberDefn.ImplicitInherit (ty, _, idOpt, m) -> Some(ty, m, idOpt)
| _ -> None)

Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Driver/GraphChecking/FileContentMapping.fs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ let visitSynMemberDefn (md: SynMemberDefn) : FileContentEntry list =
| SynMemberDefn.Interface(interfaceType, _, members, _) ->
yield! visitSynType interfaceType
yield! collectFromOption (List.collect visitSynMemberDefn) members
| SynMemberDefn.Inherit(baseType = t) -> yield! visitSynType t
| SynMemberDefn.Inherit(baseType = Some baseType) -> yield! visitSynType baseType
| SynMemberDefn.Inherit(baseType = None) -> ()
| SynMemberDefn.ValField(fieldInfo, _) -> yield! visitSynField fieldInfo
| SynMemberDefn.NestedType _ -> ()
| SynMemberDefn.AutoProperty(attributes = attributes; typeOpt = typeOpt; synExpr = synExpr) ->
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Service/ServiceParseTreeWalk.fs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,8 @@ module SyntaxTraversal =

|> pick x
| ok -> ok
| SynMemberDefn.Inherit(synType, _identOption, range, _) -> traverseInherit (synType, range)
| SynMemberDefn.Inherit(Some synType, _identOption, range, _) -> traverseInherit (synType, range)
| SynMemberDefn.Inherit(None, _, _, _) -> None
| SynMemberDefn.ValField _ -> None
| SynMemberDefn.NestedType(synTypeDefn, _synAccessOption, _range) -> traverseSynTypeDefn path synTypeDefn

Expand Down
7 changes: 4 additions & 3 deletions src/Compiler/Service/ServiceParsedInputOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -913,8 +913,8 @@ module ParsedInput =
walkType t
|> Option.orElseWith (fun () -> members |> Option.bind (List.tryPick walkMember))

| SynMemberDefn.Inherit(baseType = t) -> walkType t

| SynMemberDefn.Inherit(baseType = Some baseType) -> walkType baseType
| SynMemberDefn.Inherit(baseType = None) -> None
| SynMemberDefn.ValField(fieldInfo = field) -> walkField field

| SynMemberDefn.NestedType(tdef, _, _) -> walkTypeDefn tdef
Expand Down Expand Up @@ -2240,7 +2240,8 @@ module ParsedInput =
| SynMemberDefn.Interface(interfaceType = t; members = members) ->
walkType t
members |> Option.iter (List.iter walkMember)
| SynMemberDefn.Inherit(baseType = t) -> walkType t
| SynMemberDefn.Inherit(baseType = Some baseType) -> walkType baseType
| SynMemberDefn.Inherit(baseType = None) -> ()
| SynMemberDefn.ValField(fieldInfo = field) -> walkField field
| SynMemberDefn.NestedType(tdef, _, _) -> walkTypeDefn tdef
| SynMemberDefn.AutoProperty(attributes = Attributes attrs; typeOpt = t; synExpr = e) ->
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/SyntaxTree/SyntaxTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type SynLongIdent =

member this.Range =
match this with
| SynLongIdent([], _, _) -> failwith "rangeOfLidwd"
| SynLongIdent([], _, _) -> failwith "rangeOfLid"
| SynLongIdent([ id ], [], _) -> id.idRange
| SynLongIdent([ id ], [ m ], _) -> unionRanges id.idRange m
| SynLongIdent(h :: t, [], _) -> unionRanges h.idRange (List.last t).idRange
Expand Down Expand Up @@ -1496,7 +1496,7 @@ type SynMemberDefn =

| Interface of interfaceType: SynType * withKeyword: range option * members: SynMemberDefns option * range: range

| Inherit of baseType: SynType * asIdent: Ident option * range: range * trivia: SynMemberDefnInheritTrivia
| Inherit of baseType: SynType option * asIdent: Ident option * range: range * trivia: SynMemberDefnInheritTrivia

| ValField of fieldInfo: SynField * range: range

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/SyntaxTree/SyntaxTree.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,7 @@ type SynMemberDefn =
| Interface of interfaceType: SynType * withKeyword: range option * members: SynMemberDefns option * range: range

/// An 'inherit' definition within a class
| Inherit of baseType: SynType * asIdent: Ident option * range: range * trivia: SynMemberDefnInheritTrivia
| Inherit of baseType: SynType option * asIdent: Ident option * range: range * trivia: SynMemberDefnInheritTrivia

/// A 'val' definition within a class
| ValField of fieldInfo: SynField * range: range
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/pars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -2317,7 +2317,7 @@ inheritsDefn:
| INHERIT atomTypeNonAtomicDeprecated optBaseSpec
{ let mDecl = unionRanges (rhs parseState 1) $2.Range
let trivia = { InheritKeyword = rhs parseState 1 }
SynMemberDefn.Inherit($2, $3, mDecl, trivia) }
SynMemberDefn.Inherit(Some $2, $3, mDecl, trivia) }

| INHERIT atomTypeNonAtomicDeprecated opt_HIGH_PRECEDENCE_APP atomicExprAfterType optBaseSpec
{ let mDecl = unionRanges (rhs parseState 1) $4.Range
Expand All @@ -2327,7 +2327,7 @@ inheritsDefn:
{ let mDecl = (rhs parseState 1)
let trivia = { InheritKeyword = (rhs parseState 1) }
if not $2 then errorR (Error(FSComp.SR.parsTypeNameCannotBeEmpty (), mDecl))
SynMemberDefn.Inherit(SynType.LongIdent(SynLongIdent([], [], [])), None, mDecl, trivia) }
SynMemberDefn.Inherit(None, None, mDecl, trivia) }

optAsSpec:
| asSpec
Expand Down
12 changes: 12 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/ErrorMessages/ClassesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,18 @@ type Class() =
(Error 961, Line 5, Col 5, Line 5, Col 12, "This 'inherit' declaration specifies the inherited type but no arguments. Consider supplying arguments, e.g. 'inherit BaseType(args)'.")
(Error 946, Line 5, Col 13, Line 5, Col 15, "Cannot inherit from interface type. Use interface ... with instead.")
]

[<Fact>]
let ``This 'inherit' declaration specifies the inherited type but no arguments. Type name cannot be empty.`` () =
Fsx """
type Class() =
inherit
"""
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3159, Line 3, Col 5, Line 3, Col 12, "Type name cannot be empty.")
]

[<Fact>]
let ``The types System.ValueType, System.Enum, System.Delegate, System.MulticastDelegate and System.Array cannot be used as super types in an object expression or class.`` () =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8070,8 +8070,8 @@ FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: FSharp.Compiler.Text.Range
FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: FSharp.Compiler.Text.Range range
FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident] get_inheritAlias()
FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident] inheritAlias
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Syntax.SynType baseType
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Syntax.SynType get_baseType()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.SynType] baseType
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.SynType] get_baseType()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Text.Range get_range()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Text.Range range
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident] asIdent
Expand Down Expand Up @@ -8153,7 +8153,7 @@ FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewIm
FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewImplicitInherit(FSharp.Compiler.Syntax.SynType, FSharp.Compiler.Syntax.SynExpr, Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident], FSharp.Compiler.Text.Range)
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia get_trivia()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia trivia
FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewInherit(FSharp.Compiler.Syntax.SynType, Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident], FSharp.Compiler.Text.Range, FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia)
FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewInherit(Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.SynType], Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident], FSharp.Compiler.Text.Range, FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia)
FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia: FSharp.Compiler.Text.Range InheritKeyword
FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia: FSharp.Compiler.Text.Range get_InheritKeyword()
FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia: System.String ToString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8070,8 +8070,8 @@ FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: FSharp.Compiler.Text.Range
FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: FSharp.Compiler.Text.Range range
FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident] get_inheritAlias()
FSharp.Compiler.Syntax.SynMemberDefn+ImplicitInherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident] inheritAlias
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Syntax.SynType baseType
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Syntax.SynType get_baseType()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.SynType] baseType
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.SynType] get_baseType()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Text.Range get_range()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.Text.Range range
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident] asIdent
Expand Down Expand Up @@ -8153,7 +8153,7 @@ FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewIm
FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewImplicitInherit(FSharp.Compiler.Syntax.SynType, FSharp.Compiler.Syntax.SynExpr, Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident], FSharp.Compiler.Text.Range)
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia get_trivia()
FSharp.Compiler.Syntax.SynMemberDefn+Inherit: FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia trivia
FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewInherit(FSharp.Compiler.Syntax.SynType, Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident], FSharp.Compiler.Text.Range, FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia)
FSharp.Compiler.Syntax.SynMemberDefn: FSharp.Compiler.Syntax.SynMemberDefn NewInherit(Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.SynType], Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Syntax.Ident], FSharp.Compiler.Text.Range, FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia)
FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia: FSharp.Compiler.Text.Range InheritKeyword
FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia: FSharp.Compiler.Text.Range get_InheritKeyword()
FSharp.Compiler.SyntaxTrivia.SynMemberDefnInheritTrivia: System.String ToString()
Expand Down
2 changes: 1 addition & 1 deletion tests/service/data/SyntaxTree/Member/Inherit 01.fs.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ImplFile
ObjectModel
(Unspecified,
[Inherit
(LongIdent (SynLongIdent ([I], [], [None])), None,
(Some (LongIdent (SynLongIdent ([I], [], [None]))), None,
(4,4--4,13), { InheritKeyword = (4,4--4,11) })],
(4,4--4,13)), [], None, (3,5--4,13),
{ LeadingKeyword = Type (3,0--3,4)
Expand Down
11 changes: 5 additions & 6 deletions tests/service/data/SyntaxTree/Member/Inherit 03.fs.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ ImplFile
ObjectModel
(Unspecified,
[Inherit
(LongIdent (SynLongIdent ([], [], [])), None,
(4,4--4,11), { InheritKeyword = (4,4--4,11) })],
(4,4--4,11)), [], None, (3,5--4,11),
{ LeadingKeyword = Type (3,0--3,4)
EqualsRange = Some (3,7--3,8)
WithKeyword = None })], (3,0--4,11))],
(None, None, (4,4--4,11),
{ InheritKeyword = (4,4--4,11) })], (4,4--4,11)), [],
None, (3,5--4,11), { LeadingKeyword = Type (3,0--3,4)
EqualsRange = Some (3,7--3,8)
WithKeyword = None })], (3,0--4,11))],
PreXmlDoc ((1,0), FSharp.Compiler.Xml.XmlDocCollector), [], None,
(1,0--4,11), { LeadingKeyword = Module (1,0--1,6) })], (true, true),
{ ConditionalDirectives = []
Expand Down
11 changes: 5 additions & 6 deletions tests/service/data/SyntaxTree/Member/Inherit 04.fs.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ ImplFile
ObjectModel
(Unspecified,
[Inherit
(LongIdent (SynLongIdent ([], [], [])), None,
(4,4--4,11), { InheritKeyword = (4,4--4,11) })],
(4,4--4,11)), [], None, (3,5--4,11),
{ LeadingKeyword = Type (3,0--3,4)
EqualsRange = Some (3,7--3,8)
WithKeyword = None })], (3,0--4,11));
(None, None, (4,4--4,11),
{ InheritKeyword = (4,4--4,11) })], (4,4--4,11)), [],
None, (3,5--4,11), { LeadingKeyword = Type (3,0--3,4)
EqualsRange = Some (3,7--3,8)
WithKeyword = None })], (3,0--4,11));
Expr (Ident I, (6,0--6,1))],
PreXmlDoc ((1,0), FSharp.Compiler.Xml.XmlDocCollector), [], None,
(1,0--6,1), { LeadingKeyword = Module (1,0--1,6) })], (true, true),
Expand Down
4 changes: 2 additions & 2 deletions tests/service/data/SyntaxTree/Member/Inherit 05.fs.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ ImplFile
ObjectModel
(Unspecified,
[Inherit
(LongIdent (SynLongIdent ([], [], [])), None,
(4,4--4,11), { InheritKeyword = (4,4--4,11) });
(None, None, (4,4--4,11),
{ InheritKeyword = (4,4--4,11) });
Member
(SynBinding
(None, Normal, false, false, [],
Expand Down
4 changes: 2 additions & 2 deletions tests/service/data/SyntaxTree/Member/Inherit 08.fs.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ ImplFile
ObjectModel
(Unspecified,
[Inherit
(LongIdent (SynLongIdent ([], [], [])), None,
(4,4--4,11), { InheritKeyword = (4,4--4,11) });
(None, None, (4,4--4,11),
{ InheritKeyword = (4,4--4,11) });
Member
(SynBinding
(None, Normal, false, false, [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ for i in 0..a."]
]
]
for ifs in shouldBeInterface do
AssertCtrlSpaceCompleteContains ifs "(*M*)" ["seq"] ["obj"]
AssertCtrlSpaceCompleteContains ifs "(*M*)" ["seq"] []


[<Fact>]
Expand Down Expand Up @@ -1026,7 +1026,7 @@ for i in 0..a."]
]
]
for cls in shouldBeClass do
AssertCtrlSpaceCompleteContains cls "(*M*)" ["obj"] ["seq"]
AssertCtrlSpaceCompleteContains cls "(*M*)" ["obj"] []

[<Fact>]
member this.``Completion.DetectUnknownCompletionContext``() =
Expand All @@ -1036,31 +1036,12 @@ for i in 0..a."]
" inherit (*M*)"
]

AssertCtrlSpaceCompleteContains content "(*M*)" ["obj"; "seq"] ["abs"]
AssertCtrlSpaceCompleteContains content "(*M*)" ["obj"; "seq"] []

[<Fact>]
member this.``Completion.DetectInvalidCompletionContext``() =
let shouldBeInvalid =
[
[
"type X = struct"
" inherit (*M*)"
]
[
"[<Interface>]"
"type X = class"
" inherit (*M*)"
]
[
"[<Class>]"
"type X = interface"
" inherit (*M*)"
]
[
"[<Struct>]"
"type X = interface"
" inherit (*M*)"
]
[
"type X ="
" inherit System (*M*)."
Expand All @@ -1076,7 +1057,6 @@ for i in 0..a."]
for invalid in shouldBeInvalid do
AssertCtrlSpaceCompletionListIsEmpty invalid "(*M*)"


[<Fact>]
member this.``Completion.LongIdentifiers``() =
// System.Diagnostics.Debugger.Launch() |> ignore
Expand Down

0 comments on commit ee8af8a

Please sign in to comment.