fix #19304 Borrowing std/times.format causes Error: illformed AST (#20659)

* fix #19304 Borrowing std/times.format causes Error: illformed AST

* follow suggestions

* mitigate for #4121

* improve error message

3 files changed, 53 insertions, 26 deletions

diff --git a/compiler/semcall.nim b/compiler/semcall.nim
index e5f2e820b..073c00c37 100644
--- a/compiler/semcall.nim
+++ b/compiler/semcall.nim
@@ -771,18 +771,25 @@ proc explicitGenericInstantiation(c: PContext, n: PNode, s: PSym): PNode =
   else:
     result = explicitGenericInstError(c, n)
 
-proc searchForBorrowProc(c: PContext, startScope: PScope, fn: PSym): PSym =
+proc searchForBorrowProc(c: PContext, startScope: PScope, fn: PSym): tuple[s: PSym, state: TBorrowState] =
   # Searches for the fn in the symbol table. If the parameter lists are suitable
   # for borrowing the sym in the symbol table is returned, else nil.
   # New approach: generate fn(x, y, z) where x, y, z have the proper types
   # and use the overloading resolution mechanism:
-  result = nil
+  const desiredTypes = abstractVar + {tyCompositeTypeClass} - {tyTypeDesc, tyDistinct}
+
+  template getType(isDistinct: bool; t: PType):untyped =
+    if isDistinct: t.baseOfDistinct(c.graph, c.idgen) else: t
+
+  result = default(tuple[s: PSym, state: TBorrowState])
   var call = newNodeI(nkCall, fn.info)
   var hasDistinct = false
+  var isDistinct: bool
+  var x: PType
+  var t: PType
   call.add(newIdentNode(fn.name, fn.info))
   for i in 1..<fn.typ.n.len:
     let param = fn.typ.n[i]
-    const desiredTypes = abstractVar + {tyCompositeTypeClass} - {tyTypeDesc, tyDistinct}
     #[.
       # We only want the type not any modifiers such as `ptr`, `var`, `ref` ...
       # tyCompositeTypeClass is here for
@@ -791,22 +798,31 @@ proc searchForBorrowProc(c: PContext, startScope: PScope, fn: PSym): PSym =
       proc `$`(f: Foo): string {.borrow.}
       # We want to skip the `Foo` to get `int`
     ]#
-    let t = skipTypes(param.typ, desiredTypes)
-    if t.kind == tyDistinct or param.typ.kind == tyDistinct: hasDistinct = true
-    var x: PType
+    t = skipTypes(param.typ, desiredTypes)
+    isDistinct = t.kind == tyDistinct or param.typ.kind == tyDistinct
+    if t.kind == tyGenericInvocation and t[0].lastSon.kind == tyDistinct:
+      result.state = bsGeneric
+      return
+    if isDistinct: hasDistinct = true
     if param.typ.kind == tyVar:
       x = newTypeS(param.typ.kind, c)
-      x.addSonSkipIntLit(t.baseOfDistinct(c.graph, c.idgen), c.idgen)
+      x.addSonSkipIntLit(getType(isDistinct, t), c.idgen)
     else:
-      x = t.baseOfDistinct(c.graph, c.idgen)
-    call.add(newNodeIT(nkEmpty, fn.info, x))
+      x = getType(isDistinct, t)
+    var s = copySym(param.sym, c.idgen)
+    s.typ = x
+    s.info = param.info
+    call.add(newSymNode(s))
   if hasDistinct:
     let filter = if fn.kind in {skProc, skFunc}: {skProc, skFunc} else: {fn.kind}
     var resolved = semOverloadedCall(c, call, call, filter, {})
     if resolved != nil:
-      result = resolved[0].sym
-      if not compareTypes(result.typ[0], fn.typ[0], dcEqIgnoreDistinct):
-        result = nil
-      elif result.magic in {mArrPut, mArrGet}:
+      result.s = resolved[0].sym
+      result.state = bsMatch
+      if not compareTypes(result.s.typ[0], fn.typ[0], dcEqIgnoreDistinct):
+        result.state = bsReturnNotMatch
+      elif result.s.magic in {mArrPut, mArrGet}:
         # cannot borrow these magics for now
-        result = nil
+        result.state = bsNotSupported
+  else:
+    result.state = bsNoDistinct
diff --git a/compiler/semdata.nim b/compiler/semdata.nim
index a85f8e638..e783e2168 100644
--- a/compiler/semdata.nim
+++ b/compiler/semdata.nim
@@ -169,6 +169,8 @@ type
     inUncheckedAssignSection*: int
     importModuleLookup*: Table[int, seq[int]] # (module.ident.id, [module.id])
     skipTypes*: seq[PNode] # used to skip types between passes in type section. So far only used for inheritance and sets.
+  TBorrowState* = enum
+    bsNone, bsReturnNotMatch, bsNoDistinct, bsGeneric, bsNotSupported, bsMatch
 
 template config*(c: PContext): ConfigRef = c.graph.config
 
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index 302ccc8b9..e64cd8db6 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -1659,18 +1659,27 @@ proc addParams(c: PContext, n: PNode, kind: TSymKind) =
 
 proc semBorrow(c: PContext, n: PNode, s: PSym) =
   # search for the correct alias:
-  var b = searchForBorrowProc(c, c.currentScope.parent, s)
-  if b != nil:
-    # store the alias:
-    n[bodyPos] = newSymNode(b)
-    # Carry over the original symbol magic, this is necessary in order to ensure
-    # the semantic pass is correct
-    s.magic = b.magic
-    if b.typ != nil and b.typ.len > 0:
-      s.typ.n[0] = b.typ.n[0]
-    s.typ.flags = b.typ.flags
-  else:
-    localError(c.config, n.info, errNoSymbolToBorrowFromFound)
+  var (b, state) = searchForBorrowProc(c, c.currentScope.parent, s)
+  case state
+    of bsMatch:
+      # store the alias:
+      n[bodyPos] = newSymNode(b)
+      # Carry over the original symbol magic, this is necessary in order to ensure
+      # the semantic pass is correct
+      s.magic = b.magic
+      if b.typ != nil and b.typ.len > 0:
+        s.typ.n[0] = b.typ.n[0]
+      s.typ.flags = b.typ.flags
+    of bsNoDistinct:
+      localError(c.config, n.info, "borrow proc without distinct type parameter is meaningless")
+    of bsReturnNotMatch:
+      localError(c.config, n.info, "borrow from proc return type mismatch: '$1'" % typeToString(b.typ[0]))
+    of bsGeneric:
+      localError(c.config, n.info, "borrow with generic parameter is not supported")
+    of bsNotSupported:
+      localError(c.config, n.info, "borrow from '$1' is not supported" % $b.name.s)
+    else:
+      localError(c.config, n.info, errNoSymbolToBorrowFromFound)
 
 proc swapResult(n: PNode, sRes: PSym, dNode: PNode) =
   ## Swap nodes that are (skResult) symbols to d(estination)Node.