Overload resultion with generic variables an inheritance (#23870)

The test case diff is self explanatory

compiler/sigmatch.nim

@@ -80,7 +80,8 @@ type
                               # or when the explain pragma is used. may be
                               # triggered with an idetools command in the
                               # future.
-    inheritancePenalty: int   # to prefer closest father object type
+                              # to prefer closest father object type
+    inheritancePenalty: int
     firstMismatch*: MismatchInfo # mismatch info for better error messages
     diagnosticsEnabled*: bool
 
@@ -95,6 +96,7 @@ type
 
 const
   isNilConversion = isConvertible # maybe 'isIntConv' fits better?
+  maxInheritancePenalty = high(int) div 2
 
 proc markUsed*(c: PContext; info: TLineInfo, s: PSym; checkStyle = true)
 proc markOwnerModuleAsUsed*(c: PContext; s: PSym)
@@ -107,7 +109,7 @@ proc initCandidateAux(ctx: PContext,
                       convMatches: 0, intConvMatches: 0, genericMatches: 0,
                       state: csEmpty, firstMismatch: MismatchInfo(),
                       callee: callee, call: nil, baseTypeMatch: false,
-                      genericConverter: false, inheritancePenalty: 0
+                      genericConverter: false, inheritancePenalty: -1
   )
 
 proc initCandidate*(ctx: PContext, callee: PType): TCandidate =
@@ -287,6 +289,15 @@ proc writeMatches*(c: TCandidate) =
   echo "  conv matches: ", c.convMatches
   echo "  inheritance: ", c.inheritancePenalty
 
+proc cmpInheritancePenalty(a, b: int): int =
+  var eb = b
+  var ea = a
+  if b < 0:
+    eb = maxInheritancePenalty  # defacto max penalty
+  if a < 0:
+    ea = maxInheritancePenalty
+  eb - ea
+
 proc cmpCandidates*(a, b: TCandidate, isFormal=true): int =
   result = a.exactMatches - b.exactMatches
   if result != 0: return
@@ -298,8 +309,7 @@ proc cmpCandidates*(a, b: TCandidate, isFormal=true): int =
   if result != 0: return
   result = a.convMatches - b.convMatches
   if result != 0: return
-  # the other way round because of other semantics:
-  result = b.inheritancePenalty - a.inheritancePenalty
+  result = cmpInheritancePenalty(a.inheritancePenalty, b.inheritancePenalty)
   if result != 0: return
   if isFormal:
     # check for generic subclass relation
@@ -588,10 +598,9 @@ proc recordRel(c: var TCandidate, f, a: PType, flags: TTypeRelFlags): TTypeRelat
     let firstField = if f.kind == tyTuple: 0
                      else: 1
     for _, ff, aa in tupleTypePairs(f, a):
-      let oldInheritancePenalty = c.inheritancePenalty
       var m = typeRel(c, ff, aa, flags)
       if m < isSubtype: return isNone
-      if m == isSubtype and c.inheritancePenalty > oldInheritancePenalty:
+      if m == isSubtype and aa.kind != tyNil and c.inheritancePenalty > -1:
         # we can't process individual element type conversions from a
         # type conversion for the whole tuple
         # subtype relations need type conversions when inheritance is used
@@ -1388,12 +1397,12 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
         reduceToBase(a)
     if effectiveArgType.kind == tyObject:
       if sameObjectTypes(f, effectiveArgType):
+        c.inheritancePenalty = 0
         result = isEqual
         # elif tfHasMeta in f.flags: result = recordRel(c, f, a)
       elif trIsOutParam notin flags:
-        var depth = isObjectSubtype(c, effectiveArgType, f, nil)
-        if depth > 0:
-          inc(c.inheritancePenalty, depth)
+        c.inheritancePenalty = isObjectSubtype(c, effectiveArgType, f, nil)
+        if c.inheritancePenalty > 0:
           result = isSubtype
   of tyDistinct:
     a = a.skipTypes({tyOwned, tyGenericInst, tyRange})
@@ -1562,7 +1571,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
         if aAsObject.kind == tyObject and trIsOutParam notin flags:
           let baseType = aAsObject.base
           if baseType != nil:
-            c.inheritancePenalty += 1
+            inc c.inheritancePenalty, 1 + int(c.inheritancePenalty < 0)
             let ret = typeRel(c, f, baseType, flags)
             return if ret in {isEqual,isGeneric}: isSubtype else: ret
 
@@ -1659,7 +1668,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
           depth = -1
 
       if depth >= 0:
-        c.inheritancePenalty += depth
+        inc c.inheritancePenalty, depth + int(c.inheritancePenalty < 0)
         # bug #4863: We still need to bind generic alias crap, so
         # we cannot return immediately:
         result = if depth == 0: isGeneric else: isSubtype
@@ -1677,19 +1686,21 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
     considerPreviousT:
       result = isNone
       let oldInheritancePenalty = c.inheritancePenalty
-      var maxInheritance = 0
+      var minInheritance = maxInheritancePenalty
       for branch in f.kids:
-        c.inheritancePenalty = 0
+        c.inheritancePenalty = -1
         let x = typeRel(c, branch, aOrig, flags)
-        maxInheritance = max(maxInheritance, c.inheritancePenalty)
-        # 'or' implies maximum matching result:
-        if x > result: result = x
+        if x >= result:
+          if  c.inheritancePenalty > -1:
+            minInheritance = min(minInheritance, c.inheritancePenalty)
+          result = x
       if result >= isIntConv:
+        if minInheritance < maxInheritancePenalty:
+          c.inheritancePenalty = oldInheritancePenalty + minInheritance
         if result > isGeneric: result = isGeneric
         bindingRet result
       else:
         result = isNone
-      c.inheritancePenalty = oldInheritancePenalty + maxInheritance
   of tyNot:
     considerPreviousT:
       if typeRel(c, f.elementType, aOrig, flags) != isNone:
@@ -1799,18 +1810,12 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
       else:
         # check if 'T' has a constraint as in 'proc p[T: Constraint](x: T)'
         if f.len > 0 and f[0].kind != tyNone:
-          let oldInheritancePenalty = c.inheritancePenalty
           result = typeRel(c, f[0], a, flags + {trDontBind, trBindGenericParam})
           if doBindGP and result notin {isNone, isGeneric}:
             let concrete = concreteType(c, a, f)
             if concrete == nil: return isNone
             put(c, f, concrete)
-          # bug #6526
           if result in {isEqual, isSubtype}:
-            # 'T: Class' is a *better* match than just 'T'
-            # but 'T: Subclass' is even better:
-            c.inheritancePenalty = oldInheritancePenalty - c.inheritancePenalty -
-                                  100 * ord(result == isEqual)
             result = isGeneric
         elif a.kind == tyTypeDesc:
           # somewhat special typing rule, the following is illegal:
@@ -1843,7 +1848,11 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
     elif x.kind == tyGenericParam:
       result = isGeneric
     else:
+      # This is the bound type - can't benifit from these tallies
+      let
+        inheritancePenaltyOld = c.inheritancePenalty
       result = typeRel(c, x, a, flags) # check if it fits
+      c.inheritancePenalty = inheritancePenaltyOld
       if result > isGeneric: result = isGeneric
   of tyStatic:
     let prev = idTableGet(c.bindings, f)
@@ -2256,8 +2265,7 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
     inc(m.genericMatches)
     if arg.typ == nil:
       result = arg
-    elif skipTypes(arg.typ, abstractVar-{tyTypeDesc}).kind == tyTuple or
-         m.inheritancePenalty > oldInheritancePenalty:
+    elif skipTypes(arg.typ, abstractVar-{tyTypeDesc}).kind == tyTuple or cmpInheritancePenalty(oldInheritancePenalty, m.inheritancePenalty) > 0:
       result = implicitConv(nkHiddenSubConv, f, arg, m, c)
     elif arg.typ.isEmptyContainer:
       result = arg.copyTree

tests/overload/toverload_issues.nim

@@ -77,27 +77,30 @@ testPred(1)
 
 
 
-# bug #6526
-type
-  BaseObj = ref object of RootObj
-  DerivedObj = ref object of BaseObj
-  OtherDerivate = ref object of BaseObj
-
-proc `==`*[T1, T2: BaseObj](a: T1, b: T2): bool =
-  echo "baseobj =="
-  return true
-
-let a = DerivedObj()
-let b = DerivedObj()
-echo a == b
-
-proc `==`*[T1, T2: OtherDerivate](a: T1, b: T2): bool =
-  echo "even better! =="
-  return true
-
-let a2 = OtherDerivate()
-let b2 = OtherDerivate()
-echo a2 == b2
+block: # bug #6526
+  type
+    BaseObj = ref object of RootObj
+    DerivedObj = ref object of BaseObj
+    OtherDerivate = ref object of BaseObj
+
+  proc p[T](a: T, b: T): bool =
+    assert false
+
+  proc p[T1, T2: BaseObj](a: T1, b: T2): bool =
+    echo "baseobj =="
+    return true
+
+  let a = DerivedObj()
+  let b = DerivedObj()
+  echo p(a,b)
+
+  proc p[T1, T2: OtherDerivate](a: T1, b: T2): bool =
+    echo "even better! =="
+    return true
+
+  let a2 = OtherDerivate()
+  let b2 = OtherDerivate()
+  echo p(a2, b2)
 
 
 

tests/overload/toverload_various.nim

@@ -506,3 +506,63 @@ block:
   doAssert(p2(F(float,1.0),F(float,2)) == 3.0)
   doAssert(p2(F(float,1.0),F(float,2.0)) == 3.0)
   #doAssert(p2(F(float,1),F(int,2.0)) == 3.0)
+
+block: # PR #23870
+  type
+    A {.inheritable.} = object
+    B = object of A
+    C = object of B
+
+  proc p[T: A](x: T): int = 0
+  proc p[T: B](x: T): int = 1
+
+  proc d(x: A): int = 0
+  proc d(x: B): int = 1
+
+  proc g[T:A](x: typedesc[T]): int = 0
+  proc g[T: B](x: typedesc[T]): int = 1
+
+  proc f[T](x: typedesc[T]): int = 0
+  proc f[T:B](x: typedesc[T]): int = 1
+
+  assert p(C()) == 1
+  assert d(C()) == 1
+  assert g(C) == 1
+  assert f(C) == 1
+
+block: # PR #23870
+  type
+    A = object of RootObj
+    PT = proc(ev: A) {.closure.}
+    sdt = seq[(PT, PT)]
+
+  proc encap() =
+    proc p(a: A) {.closure.} =
+      discard
+
+    var s: sdt
+    s.add (p, nil)
+
+  encap()
+
+block: # PR #23870
+  type
+    A = object of RootObj
+    B = object of A
+    C = object of B
+
+  proc p(a: B | RootObj): int =
+    0
+
+  proc p(a: A | A): int =
+    1
+
+  assert p(C()) == 0
+
+  proc d(a: RootObj | B): int =
+    0
+
+  proc d(a: A | A): int =
+    1
+
+  assert d(C()) == 0