Skip to content

Commit 3208b83

Browse files
T-GroCopilot
andcommitted
Fix #7776: Detect missing implementations for C# 'abstract override' methods
The IgnoreOverrides filter in InfoReader.FilterOverrides incorrectly removed abstract override methods (e.g. C# 'abstract override string ToString()') because they were signature-equivalent to the base virtual method. This caused GetClassDispatchSlots to miss these as required dispatch slots. The fix adds an isAbstract check to the FilterOverrides function so that abstract methods from derived types are preserved even when they override an equivalent virtual in a supertype. This ensures the compiler correctly reports FS0365 when a non-abstract F# class inherits from a C# class with abstract override members without providing implementations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 98c4774 commit 3208b83

3 files changed

Lines changed: 178 additions & 3 deletions

File tree

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Fix DU case names matching IWSAM member names no longer cause duplicate property entries. (Issue [#14321](https://github.com/dotnet/fsharp/issues/14321), [PR #19341](https://github.com/dotnet/fsharp/pull/19341))
44
* Fix DefaultAugmentation(false) duplicate entry in method table. (Issue [#16565](https://github.com/dotnet/fsharp/issues/16565), [PR #19341](https://github.com/dotnet/fsharp/pull/19341))
55
* Fix abstract event accessors now have SpecialName flag. (Issue [#5834](https://github.com/dotnet/fsharp/issues/5834), [PR #19341](https://github.com/dotnet/fsharp/pull/19341))
6+
* Fix missing "No implementation was given" error when F# class inherits from a C# class with `abstract override` members without providing an implementation. ([Issue #7776](https://github.com/dotnet/fsharp/issues/7776), [PR #19503](https://github.com/dotnet/fsharp/pull/19503))
67
* Fix CLIEvent properties to be correctly recognized as events: `IsEvent` returns `true` and `XmlDocSig` uses `E:` prefix instead of `P:`. ([Issue #10273](https://github.com/dotnet/fsharp/issues/10273), [PR #18584](https://github.com/dotnet/fsharp/pull/18584))
78
* Fix extra sequence point at the end of match expressions. ([Issue #12052](https://github.com/dotnet/fsharp/issues/12052), [PR #19278](https://github.com/dotnet/fsharp/pull/19278))
89
* Fix wrong sequence point range for `return`/`yield`/`return!`/`yield!` inside computation expressions. ([Issue #19248](https://github.com/dotnet/fsharp/issues/19248), [PR #19278](https://github.com/dotnet/fsharp/pull/19278))

src/Compiler/Checking/InfoReader.fs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ type InfoReader(g: TcGlobals, amap: ImportMap) as this =
571571
FilterItemsInSuperTypesBasedOnItemsInSubTypes nmf (fun item1 items -> not (items |> List.exists (fun item2 -> equivTest item1 item2))) itemLists
572572

573573
/// Filter the overrides of methods or properties, either keeping the overrides or keeping the dispatch slots.
574-
static let FilterOverrides findFlag (isVirt:'a->bool, isNewSlot, isDefiniteOverride, isFinal, equivSigs, nmf:'a->string) items =
574+
static let FilterOverrides findFlag (isVirt:'a->bool, isNewSlot, isDefiniteOverride, isFinal, isAbstract, equivSigs, nmf:'a->string) items =
575575
let equivVirts x y = isVirt x && isVirt y && equivSigs x y
576576
let filterDefiniteOverrides = List.filter(isDefiniteOverride >> not)
577577

@@ -610,9 +610,10 @@ type InfoReader(g: TcGlobals, amap: ImportMap) as this =
610610
// (a) not virtual
611611
// (b) is a new slot or
612612
// (c) not equivalent
613+
// (d) is abstract (e.g. C# 'abstract override' re-abstracting a base virtual method)
613614
// We keep virtual finals around for error detection later on
614615
|> FilterItemsInSubTypesBasedOnItemsInSuperTypes nmf (fun newItem priorItem ->
615-
(isVirt newItem && isFinal newItem) || not (isVirt newItem) || isNewSlot newItem || not (equivVirts newItem priorItem) )
616+
(isVirt newItem && isFinal newItem) || not (isVirt newItem) || isNewSlot newItem || isAbstract newItem || not (equivVirts newItem priorItem) )
616617

617618
// Remove any abstract slots in supertypes that are (a) hidden by another newslot and (b) implemented
618619
// We leave unimplemented ones around to give errors, e.g. for
@@ -649,6 +650,7 @@ type InfoReader(g: TcGlobals, amap: ImportMap) as this =
649650
(fun minfo -> minfo.IsNewSlot),
650651
(fun minfo -> minfo.IsDefiniteFSharpOverride),
651652
(fun minfo -> minfo.IsFinal),
653+
(fun minfo -> minfo.IsAbstract),
652654
MethInfosEquivByNameAndSig EraseNone true g amap m,
653655
(fun minfo -> minfo.LogicalName))
654656

@@ -664,7 +666,8 @@ type InfoReader(g: TcGlobals, amap: ImportMap) as this =
664666
((fun (pinfo: PropInfo) -> pinfo.IsVirtualProperty),
665667
(fun pinfo -> pinfo.IsNewSlot),
666668
(fun pinfo -> pinfo.IsDefiniteFSharpOverride),
667-
(fun _ -> false),
669+
(fun _ -> false), // isFinal
670+
(fun _ -> false), // isAbstract
668671
PropsGetterSetterEquiv (PropInfosEquivByNameAndSig EraseNone g amap m),
669672
(fun pinfo -> pinfo.PropertyName))
670673

tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/AbstractMembers/AbstractMembers.fs

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,174 @@ let x4 = new TestLib.B<string>()
291291
|> compile
292292
|> shouldFail
293293
|> withErrorCode 759
294+
295+
// Regression tests for https://github.com/dotnet/fsharp/issues/7776
296+
297+
/// C# 'abstract override' re-abstracts a virtual method from a base class.
298+
/// F# classes inheriting from such a class must provide an implementation.
299+
let private csLibWithAbstractOverride =
300+
CSharp """
301+
namespace CSharpLib
302+
{
303+
public abstract class AbstractClass
304+
{
305+
public abstract override string ToString();
306+
}
307+
308+
public abstract class AbstractClassWithCustomMethod
309+
{
310+
public virtual int GetValue() => 42;
311+
}
312+
313+
public abstract class ReAbstractCustomMethod : AbstractClassWithCustomMethod
314+
{
315+
public abstract override int GetValue();
316+
}
317+
318+
public class BaseWithVirtualProperty
319+
{
320+
public virtual int Value => 42;
321+
}
322+
323+
public abstract class ReAbstractProperty : BaseWithVirtualProperty
324+
{
325+
public abstract override int Value { get; }
326+
}
327+
}
328+
"""
329+
|> withName "CSharpAbstractOverrideLib"
330+
331+
// https://github.com/dotnet/fsharp/issues/7776
332+
[<Fact>]
333+
let ``Abstract override ToString - missing implementation should error`` () =
334+
FSharp """
335+
module Test
336+
337+
open CSharpLib
338+
339+
type T() =
340+
inherit AbstractClass()
341+
"""
342+
|> asLibrary
343+
|> withReferences [csLibWithAbstractOverride]
344+
|> compile
345+
|> shouldFail
346+
|> withErrorCode 365
347+
348+
// https://github.com/dotnet/fsharp/issues/7776
349+
[<Fact>]
350+
let ``Abstract override ToString - with implementation should succeed`` () =
351+
FSharp """
352+
module Test
353+
354+
open CSharpLib
355+
356+
type T() =
357+
inherit AbstractClass()
358+
override _.ToString() = "T"
359+
"""
360+
|> asLibrary
361+
|> withReferences [csLibWithAbstractOverride]
362+
|> compile
363+
|> shouldSucceed
364+
365+
// https://github.com/dotnet/fsharp/issues/7776
366+
[<Fact>]
367+
let ``Abstract override custom method - missing implementation should error`` () =
368+
FSharp """
369+
module Test
370+
371+
open CSharpLib
372+
373+
type T() =
374+
inherit ReAbstractCustomMethod()
375+
"""
376+
|> asLibrary
377+
|> withReferences [csLibWithAbstractOverride]
378+
|> compile
379+
|> shouldFail
380+
|> withErrorCode 365
381+
382+
// https://github.com/dotnet/fsharp/issues/7776
383+
[<Fact>]
384+
let ``Abstract override custom method - with implementation should succeed`` () =
385+
FSharp """
386+
module Test
387+
388+
open CSharpLib
389+
390+
type T() =
391+
inherit ReAbstractCustomMethod()
392+
override _.GetValue() = 100
393+
"""
394+
|> asLibrary
395+
|> withReferences [csLibWithAbstractOverride]
396+
|> compile
397+
|> shouldSucceed
398+
399+
// https://github.com/dotnet/fsharp/issues/7776
400+
[<Fact>]
401+
let ``Abstract override - F# abstract subclass should be allowed`` () =
402+
FSharp """
403+
module Test
404+
405+
open CSharpLib
406+
407+
[<AbstractClass>]
408+
type T() =
409+
inherit AbstractClass()
410+
"""
411+
|> asLibrary
412+
|> withReferences [csLibWithAbstractOverride]
413+
|> compile
414+
|> shouldSucceed
415+
416+
// https://github.com/dotnet/fsharp/issues/7776
417+
[<Fact>]
418+
let ``Abstract override ToString - object expression must implement`` () =
419+
FSharp """
420+
module Test
421+
422+
open CSharpLib
423+
424+
let x = { new AbstractClass() with
425+
override _.ToString() = "obj" }
426+
"""
427+
|> asLibrary
428+
|> withReferences [csLibWithAbstractOverride]
429+
|> compile
430+
|> shouldSucceed
431+
432+
// https://github.com/dotnet/fsharp/issues/7776
433+
[<Fact>]
434+
let ``Abstract override property - missing implementation should error`` () =
435+
FSharp """
436+
module Test
437+
438+
open CSharpLib
439+
440+
type T() =
441+
inherit ReAbstractProperty()
442+
"""
443+
|> asLibrary
444+
|> withReferences [csLibWithAbstractOverride]
445+
|> compile
446+
|> shouldFail
447+
|> withErrorCode 365
448+
449+
// https://github.com/dotnet/fsharp/issues/7776
450+
[<Fact>]
451+
let ``Abstract override property - with implementation should succeed`` () =
452+
FSharp """
453+
module Test
454+
455+
open CSharpLib
456+
457+
type T() =
458+
inherit ReAbstractProperty()
459+
override _.Value = 100
460+
"""
461+
|> asLibrary
462+
|> withReferences [csLibWithAbstractOverride]
463+
|> compile
464+
|> shouldSucceed

0 commit comments

Comments
 (0)