Skip to content

Commit ef205ae

Browse files
Copilotabonie
andauthored
Fix #7776: Detect missing implementations for C# 'abstract override' methods
The IgnoreOverrides filter in InfoReader.fs 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. Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/1d6d6e17-b57c-435f-b144-0d3146d9cbd8 Co-authored-by: abonie <20281641+abonie@users.noreply.github.com>
1 parent a8565e9 commit ef205ae

File tree

2 files changed

+132
-2
lines changed

2 files changed

+132
-2
lines changed

src/Compiler/Checking/InfoReader.fs

Lines changed: 5 additions & 2 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

@@ -665,6 +667,7 @@ type InfoReader(g: TcGlobals, amap: ImportMap) as this =
665667
(fun pinfo -> pinfo.IsNewSlot),
666668
(fun pinfo -> pinfo.IsDefiniteFSharpOverride),
667669
(fun _ -> false),
670+
(fun _ -> false),
668671
PropsGetterSetterEquiv (PropInfosEquivByNameAndSig EraseNone g amap m),
669672
(fun pinfo -> pinfo.PropertyName))
670673

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,130 @@ 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+
"""
319+
|> withName "CSharpAbstractOverrideLib"
320+
321+
// https://github.com/dotnet/fsharp/issues/7776
322+
[<Fact>]
323+
let ``Abstract override ToString - missing implementation should error`` () =
324+
FSharp """
325+
module Test
326+
327+
open CSharpLib
328+
329+
type T() =
330+
inherit AbstractClass()
331+
"""
332+
|> asLibrary
333+
|> withReferences [csLibWithAbstractOverride]
334+
|> compile
335+
|> shouldFail
336+
|> withErrorCode 365
337+
338+
// https://github.com/dotnet/fsharp/issues/7776
339+
[<Fact>]
340+
let ``Abstract override ToString - with implementation should succeed`` () =
341+
FSharp """
342+
module Test
343+
344+
open CSharpLib
345+
346+
type T() =
347+
inherit AbstractClass()
348+
override _.ToString() = "T"
349+
"""
350+
|> asLibrary
351+
|> withReferences [csLibWithAbstractOverride]
352+
|> compile
353+
|> shouldSucceed
354+
355+
// https://github.com/dotnet/fsharp/issues/7776
356+
[<Fact>]
357+
let ``Abstract override custom method - missing implementation should error`` () =
358+
FSharp """
359+
module Test
360+
361+
open CSharpLib
362+
363+
type T() =
364+
inherit ReAbstractCustomMethod()
365+
"""
366+
|> asLibrary
367+
|> withReferences [csLibWithAbstractOverride]
368+
|> compile
369+
|> shouldFail
370+
|> withErrorCode 365
371+
372+
// https://github.com/dotnet/fsharp/issues/7776
373+
[<Fact>]
374+
let ``Abstract override custom method - with implementation should succeed`` () =
375+
FSharp """
376+
module Test
377+
378+
open CSharpLib
379+
380+
type T() =
381+
inherit ReAbstractCustomMethod()
382+
override _.GetValue() = 100
383+
"""
384+
|> asLibrary
385+
|> withReferences [csLibWithAbstractOverride]
386+
|> compile
387+
|> shouldSucceed
388+
389+
// https://github.com/dotnet/fsharp/issues/7776
390+
[<Fact>]
391+
let ``Abstract override - F# abstract subclass should be allowed`` () =
392+
FSharp """
393+
module Test
394+
395+
open CSharpLib
396+
397+
[<AbstractClass>]
398+
type T() =
399+
inherit AbstractClass()
400+
"""
401+
|> asLibrary
402+
|> withReferences [csLibWithAbstractOverride]
403+
|> compile
404+
|> shouldSucceed
405+
406+
// https://github.com/dotnet/fsharp/issues/7776
407+
[<Fact>]
408+
let ``Abstract override ToString - object expression must implement`` () =
409+
FSharp """
410+
module Test
411+
412+
open CSharpLib
413+
414+
let x = { new AbstractClass() with
415+
override _.ToString() = "obj" }
416+
"""
417+
|> asLibrary
418+
|> withReferences [csLibWithAbstractOverride]
419+
|> compile
420+
|> shouldSucceed

0 commit comments

Comments
 (0)