Make `typeRel` behave to spec (#22261)

The goal of this PR is to make `typeRel` accurate to it's definition for
generics:
```
# 3) When used with two type classes, it will check whether the types
# matching the first type class (aOrig) are a strict subset of the types matching
# the other (f). This allows us to compare the signatures of generic procs in
# order to give preferrence to the most specific one:
```

I don't want this PR to break any code, and I want to preserve all of
Nims current behaviors. I think that making this more accurate will help
serve as ground work for the future. It may not be possible to not break
anything but this is my attempt.

So that it is understood, this code was part of another PR (#22143) but
that problem statement only needed this change by extension. It's more
organized to split two problems into two PRs and this issue, being
non-breaking, should be a more immediate improvement.

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

4 files changed, 78 insertions, 29 deletions

diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index ee93321c8..3867a67b7 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -453,6 +453,38 @@ proc handleFloatRange(f, a: PType): TTypeRelation =
       else: result = isIntConv
     else: result = isNone
 
+proc getObjectTypeOrNil(f: PType): PType =
+  #[
+    Returns a type that is f's effective typeclass. This is usually just one level deeper
+    in the hierarchy of generality for a type. `object`, `ref object`, `enum` and user defined
+    tyObjects are common return values.
+  ]#
+  if f == nil: return nil
+  case f.kind:
+  of tyGenericInvocation, tyCompositeTypeClass, tyAlias:
+    if f.len <= 0 or f[0] == nil:
+      result = nil
+    else:
+      result = getObjectTypeOrNil(f[0])
+  of tyGenericBody, tyGenericInst:
+    result = getObjectTypeOrNil(f.lastSon)
+  of tyUserTypeClass:
+    if f.isResolvedUserTypeClass:
+      result = f.base  # ?? idk if this is right
+    else:
+      result = f.lastSon
+  of tyStatic, tyOwned, tyVar, tyLent, tySink:
+    result = getObjectTypeOrNil(f.base)
+  of tyInferred:
+    # This is not true "After a candidate type is selected"
+    result = getObjectTypeOrNil(f.base)
+  of tyTyped, tyUntyped, tyFromExpr:
+    result = nil
+  of tyRange:
+    result = f.lastSon
+  else:
+    result = f
+
 proc genericParamPut(c: var TCandidate; last, fGenericOrigin: PType) =
   if fGenericOrigin != nil and last.kind == tyGenericInst and
      last.len-1 == fGenericOrigin.len:
@@ -467,7 +499,8 @@ proc isObjectSubtype(c: var TCandidate; a, f, fGenericOrigin: PType): int =
   var depth = 0
   var last = a
   while t != nil and not sameObjectTypes(f, t):
-    assert t.kind == tyObject
+    if t.kind != tyObject:  # avoid entering generic params etc
+      return -1
     t = t[0]
     if t == nil: break
     last = t
@@ -997,8 +1030,8 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
   # of the designated type class.
   #
   # 3) When used with two type classes, it will check whether the types
-  # matching the first type class are a strict subset of the types matching
-  # the other. This allows us to compare the signatures of generic procs in
+  # matching the first type class (aOrig) are a strict subset of the types matching
+  # the other (f). This allows us to compare the signatures of generic procs in
   # order to give preferrence to the most specific one:
   #
   # seq[seq[any]] is a strict subset of seq[any] and hence more specific.
@@ -1154,7 +1187,6 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
       # being passed as parameters
       return isNone
   else: discard
-
   case f.kind
   of tyEnum:
     if a.kind == f.kind and sameEnumTypes(f, a): result = isEqual
@@ -1245,6 +1277,8 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
         return inferStaticsInRange(c, fRange, a)
       elif c.c.matchedConcept != nil and aRange.rangeHasUnresolvedStatic:
         return inferStaticsInRange(c, aRange, f)
+      elif result == isGeneric and concreteType(c, aa, ff) == nil:
+        return isNone
       else:
         if lengthOrd(c.c.config, fRange) != lengthOrd(c.c.config, aRange):
           result = isNone
@@ -1332,12 +1366,17 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
   of tyTuple:
     if a.kind == tyTuple: result = recordRel(c, f, a)
   of tyObject:
-    if a.kind == tyObject:
-      if sameObjectTypes(f, a):
+    let effectiveArgType = if useTypeLoweringRuleInTypeClass:
+        a
+      else:
+        getObjectTypeOrNil(a)
+    if effectiveArgType == nil: return isNone
+    if effectiveArgType.kind == tyObject:
+      if sameObjectTypes(f, effectiveArgType):
         result = isEqual
         # elif tfHasMeta in f.flags: result = recordRel(c, f, a)
       elif trIsOutParam notin flags:
-        var depth = isObjectSubtype(c, a, f, nil)
+        var depth = isObjectSubtype(c, effectiveArgType, f, nil)
         if depth > 0:
           inc(c.inheritancePenalty, depth)
           result = isSubtype
@@ -1533,6 +1572,8 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
 
   of tyGenericInvocation:
     var x = a.skipGenericAlias
+    if x.kind == tyGenericParam and x.len > 0:
+      x = x.lastSon
     let concpt = f[0].skipTypes({tyGenericBody})
     var preventHack = concpt.kind == tyConcept
     if x.kind == tyOwned and f[0].kind != tyOwned:
@@ -1543,7 +1584,6 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
       c.calleeSym != nil and
       c.calleeSym.kind in {skProc, skFunc} and c.call != nil and not preventHack:
       let inst = prepareMetatypeForSigmatch(c.c, c.bindings, c.call.info, f)
-      #echo "inferred ", typeToString(inst), " for ", f
       return typeRel(c, inst, a, flags)
 
     if x.kind == tyGenericInvocation:
@@ -1572,9 +1612,6 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
       var fskip = skippedNone
       let aobj = x.skipToObject(askip)
       let fobj = genericBody.lastSon.skipToObject(fskip)
-      var depth = -1
-      if fobj != nil and aobj != nil and askip == fskip:
-        depth = isObjectSubtype(c, aobj, fobj, f)
       result = typeRel(c, genericBody, x, flags)
       if result != isNone:
         # see tests/generics/tgeneric3.nim for an example that triggers this
@@ -1600,7 +1637,10 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
               put(c, key, x)
             elif typeRel(c, old, x, flags + {trDontBind}) == isNone:
               return isNone
-
+      var depth = -1
+      if fobj != nil and aobj != nil and askip == fskip:
+        depth = isObjectSubtype(c, aobj, fobj, f)
+      
       if result == isNone:
         # Here object inheriting from generic/specialized generic object
         # crossing path with metatypes/aliases, so we need to separate them
@@ -1662,8 +1702,9 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
     considerPreviousT:
       let target = f[0]
       let targetKind = target.kind
-      let effectiveArgType = a.skipTypes({tyRange, tyGenericInst,
-                                          tyBuiltInTypeClass, tyAlias, tySink, tyOwned})
+      var effectiveArgType = a.getObjectTypeOrNil()
+      if effectiveArgType == nil: return isNone
+      effectiveArgType = effectiveArgType.skipTypes({tyBuiltInTypeClass})
       if targetKind == effectiveArgType.kind:
         if effectiveArgType.isEmptyContainer:
           return isNone
@@ -1785,7 +1826,11 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
         if tfWildcard in a.flags:
           a.sym.transitionGenericParamToType()
           a.flags.excl tfWildcard
-        else:
+        elif doBind:
+          # The mechanics of `doBind` being a flag that also denotes sig cmp via
+          # negation is potentially problematic. `IsNone` is appropriate for
+          # preventing illegal bindings, but it is not necessarily appropriate
+          # before the bindings have been finalized.
           concrete = concreteType(c, a, f)
           if concrete == nil:
             return isNone
diff --git a/doc/manual_experimental.md b/doc/manual_experimental.md
index d05a693bb..41d463ff8 100644
--- a/doc/manual_experimental.md
+++ b/doc/manual_experimental.md
@@ -919,6 +919,7 @@ The concept matches if:
 
 a) all expressions within the body can be compiled for the tested type
 b) all statically evaluable boolean expressions in the body are true
+c) all type modifiers specified match their respective definitions
 
 The identifiers following the `concept` keyword represent instances of the
 currently matched type. You can apply any of the standard type modifiers such
diff --git a/tests/overload/tor_isnt_better.nim b/tests/overload/tor_isnt_better.nim
index 5ef8bc7c4..ce92009d0 100644
--- a/tests/overload/tor_isnt_better.nim
+++ b/tests/overload/tor_isnt_better.nim
@@ -1,18 +1,19 @@
-discard """
-  errormsg: "ambiguous call;"
-  line: 16
-"""
-
-# bug #8568
-
 type
   D[T] = object
   E[T] = object
 
-proc g(a: D|E): string = "foo D|E"
-proc g(a: D): string = "foo D"
+block: # PR #22261
+  proc d(x: D):bool= false
+  proc d(x: int | D[SomeInteger]):bool= true
+  doAssert d(D[5]()) == false
 
-proc test() =
-  let x = g D[int]()
 
-test()
+block: # bug #8568
+#[
+  Since PR #22261 and amendment has been made. Since D is a subset of D | E but
+  not the other way around `checkGeneric` should favor proc g(a: D) instead
+  of asserting ambiguity
+]#
+  proc g(a: D|E): string = "foo D|E"
+  proc g(a: D): string = "foo D"
+  doAssert g(D[int]()) == "foo D"
diff --git a/tests/stdlib/t21406.nim b/tests/stdlib/t21406.nim
index 5b96227ce..86bf7b0c7 100644
--- a/tests/stdlib/t21406.nim
+++ b/tests/stdlib/t21406.nim
@@ -1,5 +1,7 @@
 import std/[times, strformat]
 import std/assertions
 
-doAssert fmt"{getTime()}" == $getTime()
-doAssert fmt"{now()}" == $now()
+let aTime = getTime()
+doAssert fmt"{aTime}" == $aTime
+let aNow = now()
+doAssert fmt"{aNow}" == $aNow