Fix #5531 (#5778)

* Fix #5531

This is a working fix for the example of #5531.
However there are still some points open before I think it's a good idea to merge this:
- Discuss the change of this proposal, is it OK to prefer overrides of methods and so ignore base implementations?
- Extend the check to the complete inheritance graph instead of a single "look-back"
- Only ignore a methinfo if the signature of both match (so respect different overloads)

@dsyme It would be great if you could have a look whether this check is allowed at this location or should appear earlier.

* Fix left points.

Signed-off-by: realvictorprm <mueller.vpr@gmail.com>

* another approach to fix

* remove old approach

* Remove approach again and apply different approach.

Signed-off-by: realvictorprm <mueller.vpr@gmail.com>

* Adjusting predicate to reflect correct behaviour.

* Revert to old List.merge strategy.

* Apply review and add tests

Signed-off-by: realvictorprm <mueller.vpr@gmail.com>

* Try fixing weird CI failure

Signed-off-by: realvictorprm <mueller.vpr@gmail.com>

* Fix test.

Signed-off-by: realvictorprm <mueller.vpr@gmail.com>

src/fsharp/ConstraintSolver.fs

@@ -1491,7 +1491,14 @@ and GetRelevantMethodsForTrait (csenv:ConstraintSolverEnv) permitWeakResolution
             /// to a generic instantiation for an operator based on the right hand type. 
             
             let minfos = List.reduce (ListSet.unionFavourLeft MethInfo.MethInfosUseIdenticalDefinitions) minfos
+
+            /// Check that the available members aren't hiding a member from the parent (depth 1 only)
+            let relevantMinfos = minfos |> List.filter(fun minfo -> not minfo.IsDispatchSlot && not minfo.IsVirtual && minfo.IsInstance)
             minfos
+            |> List.filter(fun minfo1 ->
+                not(minfo1.IsDispatchSlot && 
+                    relevantMinfos
+                    |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig EraseAll true csenv.g csenv.amap m minfo2 minfo1)))
         else 
             []
     // The trait name "op_Explicit" also covers "op_Implicit", so look for that one too.

tests/fsharp/regression/5531/compilation.output.test.bsl

@@ -0,0 +1,2 @@
+
+test.fs(7,17): warning FS0864: This new member hides the abstract member 'abstract member Base.Foo : unit -> unit'. Rename the member or use 'override' instead.

tests/fsharp/regression/5531/compilation.output.test.txt

@@ -0,0 +1,2 @@
+
+test.fs(7,17): warning FS0864: This new member hides the abstract member 'abstract member Base.Foo : unit -> unit'. Rename the member or use 'override' instead.

tests/fsharp/regression/5531/output.test.bsl

@@ -0,0 +1,7 @@
+Base
+Base
+Derived
+Base
+Base
+Base
+Derived

tests/fsharp/regression/5531/output.test.txt

@@ -0,0 +1,7 @@
+Base
+Base
+Derived
+Base
+Base
+Base
+Derived

tests/fsharp/regression/5531/test.fs

@@ -0,0 +1,23 @@
+type Base() =
+    abstract member Foo : unit -> unit
+    default this.Foo() = printfn "Base"
+
+type Derived() =
+    inherit Base()
+    member this.Foo() = printfn "Derived"
+
+let inline callFoo< ^T when ^T : (member Foo: unit -> unit) > (t: ^T) =
+    (^T : (member Foo: unit -> unit) (t))
+
+let b = Base()
+let d = Derived()
+let bd = d :> Base
+
+b.Foo()
+bd.Foo()
+d.Foo()
+
+callFoo<Base> b
+callFoo<Base> bd
+callFoo<Base> d
+callFoo<Derived> d

tests/fsharp/tests.fs

@@ -1755,6 +1755,36 @@ module RegressionTests =
 ////    [<Test >]
 ////    let ``struct-tuple-bug-1-FSI_BASIC`` () = singleTestBuildAndRun "regression/struct-tuple-bug-1" FSI_BASIC
 
+#if !FSHARP_SUITE_DRIVES_CORECLR_TESTS
+    [<Test>]
+    let ``SRTP doesn't handle calling member hiding hinherited members`` () =
+        let cfg = testConfig "regression/5531"
+
+        let outFile = "compilation.output.test.txt"
+        let expectedFile = "compilation.output.test.bsl"
+
+        fscBothToOut cfg outFile "%s --nologo -O" cfg.fsc_flags ["test.fs"]
+
+        let diff = fsdiff cfg outFile expectedFile
+
+        match diff with
+        | "" -> ()
+        | _ ->
+            Assert.Fail (sprintf "'%s' and '%s' differ; %A" (getfullpath cfg outFile) (getfullpath cfg expectedFile) diff)
+
+        let outFile2 = "output.test.txt"
+        let expectedFile2 = "output.test.bsl"
+
+        execBothToOut cfg (cfg.Directory) outFile2 (cfg.Directory ++ "test.exe") ""
+
+        let diff2 = fsdiff cfg outFile2 expectedFile2
+
+        match diff2 with
+        | "" -> ()
+        | _ ->
+            Assert.Fail (sprintf "'%s' and '%s' differ; %A" (getfullpath cfg outFile2) (getfullpath cfg expectedFile2) diff2)
+#endif
+
 #if !FSHARP_SUITE_DRIVES_CORECLR_TESTS
     // This test is disabled in coreclr builds dependent on fixing : https://github.com/Microsoft/visualfsharp/issues/2600
     [<Test>]