Param match relax (#23033)

#23032

---------

Co-authored-by: Nikolay Nikolov <nickysn@gmail.com>
Co-authored-by: Pylgos <43234674+Pylgos@users.noreply.github.com>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
Co-authored-by: Jason Beetham <beefers331@gmail.com>

compiler/ast.nim

@@ -2033,13 +2033,15 @@ proc skipGenericOwner*(s: PSym): PSym =
   ## Generic instantiations are owned by their originating generic
   ## symbol. This proc skips such owners and goes straight to the owner
   ## of the generic itself (the module or the enclosing proc).
-  result = if s.kind in skProcKinds and sfFromGeneric in s.flags and s.owner.kind != skModule:
+  result = if s.kind == skModule:
+            s 
+           elif s.kind in skProcKinds and sfFromGeneric in s.flags and s.owner.kind != skModule:
              s.owner.owner
            else:
              s.owner
 
 proc originatingModule*(s: PSym): PSym =
-  result = s.owner
+  result = s
   while result.kind != skModule: result = result.owner
 
 proc isRoutine*(s: PSym): bool {.inline.} =

compiler/lookups.nim

@@ -256,6 +256,18 @@ proc searchInScopesFilterBy*(c: PContext, s: PIdent, filter: TSymKinds): seq[PSy
         if s.kind in filter:
           result.add s
 
+proc cmpScopes*(ctx: PContext, s: PSym): int =
+  # Do not return a negative number
+  if s.originatingModule == ctx.module:
+    result = 2
+    var owner = s
+    while true:
+      owner = owner.skipGenericOwner
+      if owner.kind == skModule: break
+      inc result
+  else:
+    result = 1
+
 proc isAmbiguous*(c: PContext, s: PIdent, filter: TSymKinds, sym: var PSym): bool =
   result = false
   block outer:

compiler/sigmatch.nim

@@ -135,15 +135,7 @@ proc initCandidate*(ctx: PContext, callee: PSym,
   result = initCandidateAux(ctx, callee.typ)
   result.calleeSym = callee
   if callee.kind in skProcKinds and calleeScope == -1:
-    if callee.originatingModule == ctx.module:
-      result.calleeScope = 2
-      var owner = callee
-      while true:
-        owner = owner.skipGenericOwner
-        if owner.kind == skModule: break
-        inc result.calleeScope
-    else:
-      result.calleeScope = 1
+    result.calleeScope = cmpScopes(ctx, callee)
   else:
     result.calleeScope = calleeScope
   result.diagnostics = @[] # if diagnosticsEnabled: @[] else: nil
@@ -297,7 +289,7 @@ proc writeMatches*(c: TCandidate) =
   echo "  conv matches: ", c.convMatches
   echo "  inheritance: ", c.inheritancePenalty
 
-proc cmpCandidates*(a, b: TCandidate): int =
+proc cmpCandidates*(a, b: TCandidate, isFormal=true): int =
   result = a.exactMatches - b.exactMatches
   if result != 0: return
   result = a.genericMatches - b.genericMatches
@@ -311,13 +303,14 @@ proc cmpCandidates*(a, b: TCandidate): int =
   # the other way round because of other semantics:
   result = b.inheritancePenalty - a.inheritancePenalty
   if result != 0: return
-  # check for generic subclass relation
-  result = checkGeneric(a, b)
+  if isFormal:
+    # check for generic subclass relation
+    result = checkGeneric(a, b)
+    if result != 0: return
+    # prefer more specialized generic over more general generic:
+    result = complexDisambiguation(a.callee, b.callee)
   if result != 0: return
-  # prefer more specialized generic over more general generic:
-  result = complexDisambiguation(a.callee, b.callee)
   # only as a last resort, consider scoping:
-  if result != 0: return
   result = a.calleeScope - b.calleeScope
 
 proc argTypeToString(arg: PNode; prefer: TPreferedDesc): string =
@@ -2353,56 +2346,76 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType,
   if arg == nil or arg.kind notin nkSymChoices:
     result = paramTypesMatchAux(m, f, a, arg, argOrig)
   else:
-    # CAUTION: The order depends on the used hashing scheme. Thus it is
-    # incorrect to simply use the first fitting match. However, to implement
-    # this correctly is inefficient. We have to copy `m` here to be able to
-    # roll back the side effects of the unification algorithm.
-    let c = m.c
-    var
-      x = newCandidate(c, m.callee)
-      y = newCandidate(c, m.callee)
-      z = newCandidate(c, m.callee)
-    x.calleeSym = m.calleeSym
-    y.calleeSym = m.calleeSym
-    z.calleeSym = m.calleeSym
+    let matchSet = {skProc, skFunc, skMethod, skConverter,skIterator, skMacro,
+                    skTemplate, skEnumField}
+    
     var best = -1
-    for i in 0..<arg.len:
-      if arg[i].sym.kind in {skProc, skFunc, skMethod, skConverter,
-                             skIterator, skMacro, skTemplate, skEnumField}:
-        copyCandidate(z, m)
-        z.callee = arg[i].typ
-        if tfUnresolved in z.callee.flags: continue
-        z.calleeSym = arg[i].sym
-        # XXX this is still all wrong: (T, T) should be 2 generic matches
-        # and  (int, int) 2 exact matches, etc. Essentially you cannot call
-        # typeRel here and expect things to work!
-        let r = staticAwareTypeRel(z, f, arg[i])
-        incMatches(z, r, 2)
-        if r != isNone:
-          z.state = csMatch
-          case x.state
-          of csEmpty, csNoMatch:
-            x = z
+    result = arg
+    
+    if f.kind in {tyTyped, tyUntyped}:
+      var
+        bestScope = -1
+        counts = 0
+      for i in 0..<arg.len:
+        if arg[i].sym.kind in matchSet:
+          let thisScope = cmpScopes(m.c, arg[i].sym)
+          if thisScope > bestScope:
             best = i
-          of csMatch:
-            let cmp = cmpCandidates(x, z)
-            if cmp < 0:
-              best = i
-              x = z
-            elif cmp == 0:
-              y = z           # z is as good as x
-
-    if x.state == csEmpty:
-      result = nil
-    elif y.state == csMatch and cmpCandidates(x, y) == 0:
-      if x.state != csMatch:
-        internalError(m.c.graph.config, arg.info, "x.state is not csMatch")
-      # ambiguous: more than one symbol fits!
-      # See tsymchoice_for_expr as an example. 'f.kind == tyUntyped' should match
-      # anyway:
-      if f.kind in {tyUntyped, tyTyped}: result = arg
-      else: result = nil
+            bestScope = thisScope
+            counts = 0
+          elif thisScope == bestScope:
+            inc counts
+      if best == -1:
+        result = nil
+      elif counts > 0:
+        best = -1
     else:
+      # CAUTION: The order depends on the used hashing scheme. Thus it is
+      # incorrect to simply use the first fitting match. However, to implement
+      # this correctly is inefficient. We have to copy `m` here to be able to
+      # roll back the side effects of the unification algorithm.
+      let c = m.c
+      var
+        x = newCandidate(c, m.callee)
+        y = newCandidate(c, m.callee)
+        z = newCandidate(c, m.callee)
+      x.calleeSym = m.calleeSym
+      y.calleeSym = m.calleeSym
+      z.calleeSym = m.calleeSym
+      
+      for i in 0..<arg.len:
+        if arg[i].sym.kind in matchSet:
+          copyCandidate(z, m)
+          z.callee = arg[i].typ
+          if tfUnresolved in z.callee.flags: continue
+          z.calleeSym = arg[i].sym
+          z.calleeScope = cmpScopes(m.c, arg[i].sym)
+          # XXX this is still all wrong: (T, T) should be 2 generic matches
+          # and  (int, int) 2 exact matches, etc. Essentially you cannot call
+          # typeRel here and expect things to work!
+          let r = staticAwareTypeRel(z, f, arg[i])
+          incMatches(z, r, 2)
+          if r != isNone:
+            z.state = csMatch
+            case x.state
+            of csEmpty, csNoMatch:
+              x = z
+              best = i
+            of csMatch:
+              let cmp = cmpCandidates(x, z, isFormal=false)
+              if cmp < 0:
+                best = i
+                x = z
+              elif cmp == 0:
+                y = z           # z is as good as x
+
+      if x.state == csEmpty:
+        result = nil
+      elif y.state == csMatch and cmpCandidates(x, y, isFormal=false) == 0:
+        if x.state != csMatch:
+          internalError(m.c.graph.config, arg.info, "x.state is not csMatch")
+        result = nil
+    if best > -1 and result != nil:
       # only one valid interpretation found:
       markUsed(m.c, arg.info, arg[best].sym)
       onUse(arg.info, arg[best].sym)

lib/pure/sugar.nim

@@ -345,9 +345,10 @@ proc collectImpl(init, body: NimNode): NimNode {.since: (1, 1).} =
   let res = genSym(nskVar, "collectResult")
   var bracketExpr: NimNode
   if init != nil:
-    expectKind init, {nnkCall, nnkIdent, nnkSym}
+    expectKind init, {nnkCall, nnkIdent, nnkSym, nnkClosedSymChoice, nnkOpenSymChoice}
     bracketExpr = newTree(nnkBracketExpr,
-      if init.kind == nnkCall: freshIdentNodes(init[0]) else: freshIdentNodes(init))
+      if init.kind in {nnkCall, nnkClosedSymChoice, nnkOpenSymChoice}:
+        freshIdentNodes(init[0]) else: freshIdentNodes(init))
   else:
     bracketExpr = newTree(nnkBracketExpr)
   let (resBody, keyType, valueType) = trans(body, res, bracketExpr)

testament/important_packages.nim

@@ -60,7 +60,7 @@ pkg "compactdict"
 pkg "comprehension", "nimble test", "https://github.com/alehander92/comprehension"
 pkg "cowstrings"
 pkg "criterion", allowFailure = true # needs testing binary
-pkg "datamancer"
+pkg "datamancer", url="https://github.com/Graveflo/Datamancer.git"
 pkg "dashing", "nim c tests/functional.nim"
 pkg "delaunay"
 pkg "docopt"

tests/lookups/issue_23032/deep_scope.nim

@@ -0,0 +1,2 @@
+type A*[T] = object
+proc foo*(a: A[int]): bool = false

tests/lookups/t23032.nim

@@ -0,0 +1,13 @@
+discard """
+action: "run"
+outputsub: "proc (a: A[system.float]): bool{.noSideEffect, gcsafe.}"
+"""
+
+import issue_23032/deep_scope
+
+proc foo(a: A[float]):bool = true
+
+let p: proc = foo
+echo p.typeof
+doAssert p(A[float]()) == true
+doAssert compiles(doAssert p(A[int]()) == true) == false

tests/macros/t23032_1.nim

@@ -0,0 +1,19 @@
+import std/macros
+
+type A[T, H] = object
+
+proc `%*`(a: A): bool = true
+proc `%*`[T](a: A[int, T]): bool = false
+
+macro collapse(s: untyped) =
+  result = newStmtList()
+  result.add quote do:
+    doAssert(`s`(A[float, int]()) == true)
+
+macro startHere(n: untyped): untyped =
+  result = newStmtList()
+  let s = n[0]
+  result.add quote do:
+    `s`.collapse()
+
+startHere(`a` %* `b`)

tests/macros/t23032_2.nim

@@ -0,0 +1,20 @@
+discard """
+  action: "reject"
+  errormsg: "ambiguous identifier '%*'"
+"""
+import std/macros
+
+type A[T, H] = object
+
+proc `%*`[T](a: A) = discard
+proc `%*`[T](a: A[int, T]) = discard
+
+macro collapse(s: typed) = discard
+
+macro startHere(n: untyped): untyped =
+  result = newStmtList()
+  let s = n[0]
+  result.add quote do:
+    collapse(`s`.typeof())
+
+startHere(`a` %* `b`)

tests/macros/tgetimpl.nim

@@ -44,8 +44,11 @@ static:
   doAssert checkOwner(poo, 2) == "nskProc"
   doAssert checkOwner(poo, 3) == "nskModule"
   doAssert isSameOwner(foo, poo)
-  doAssert isSameOwner(foo, echo) == false
-  doAssert isSameOwner(poo, len) == false
+  proc wrappedScope() =
+    proc dummyproc() = discard
+    doAssert isSameOwner(foo, dummyproc) == false
+    doAssert isSameOwner(poo, dummyproc) == false
+  wrappedScope()
 
 #---------------------------------------------------------------