From 2aab03bdfb0a216820541de7dcb34950a017bf18 Mon Sep 17 00:00:00 2001
From: Bung <crc32@qq.com>
Date: Thu, 10 Aug 2023 16:26:23 +0800
Subject: [PATCH] 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
---
 compiler/semcall.nim              | 44 +++++++++++++++++++++----------
 compiler/semdata.nim              |  2 ++
 compiler/semstmts.nim             | 33 ++++++++++++++---------
 tests/distinct/tinvalidborrow.nim | 23 +++++++++++++---
 tests/stdlib/t19304.nim           |  7 +++++
 5 files changed, 80 insertions(+), 29 deletions(-)
 create mode 100644 tests/stdlib/t19304.nim

diff --git a/compiler/semcall.nim b/compiler/semcall.nim
index e5f2e820b6..073c00c37e 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 a85f8e6387..e783e2168e 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 302ccc8b9e..e64cd8db63 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.
diff --git a/tests/distinct/tinvalidborrow.nim b/tests/distinct/tinvalidborrow.nim
index 08148608d6..d4b19fa8de 100644
--- a/tests/distinct/tinvalidborrow.nim
+++ b/tests/distinct/tinvalidborrow.nim
@@ -2,12 +2,19 @@ discard """
   cmd: "nim check --hints:off --warnings:off $file"
   action: "reject"
   nimout:'''
-tinvalidborrow.nim(18, 3) Error: only a 'distinct' type can borrow `.`
-tinvalidborrow.nim(19, 3) Error: only a 'distinct' type can borrow `.`
-tinvalidborrow.nim(20, 1) Error: no symbol to borrow from found
+tinvalidborrow.nim(25, 3) Error: only a 'distinct' type can borrow `.`
+tinvalidborrow.nim(26, 3) Error: only a 'distinct' type can borrow `.`
+tinvalidborrow.nim(27, 1) Error: borrow proc without distinct type parameter is meaningless
+tinvalidborrow.nim(36, 1) Error: borrow with generic parameter is not supported
+tinvalidborrow.nim(41, 1) Error: borrow from proc return type mismatch: 'T'
+tinvalidborrow.nim(42, 1) Error: borrow from '[]=' is not supported
 '''
 """
 
+
+
+
+
 # bug #516
 
 type
@@ -23,3 +30,13 @@ var
   d, e: TAtom
 
 discard( $(d == e) )
+
+# issue #4121
+type HeapQueue[T] = distinct seq[T]
+proc len*[T](h: HeapQueue[T]): int {.borrow.}
+
+# issue #3564
+type vec4[T] = distinct array[4, float32]
+
+proc `[]`(v: vec4, i: int): float32 {.borrow.}
+proc `[]=`(v: vec4, i: int, va: float32) {.borrow.}
diff --git a/tests/stdlib/t19304.nim b/tests/stdlib/t19304.nim
new file mode 100644
index 0000000000..5e8795ac56
--- /dev/null
+++ b/tests/stdlib/t19304.nim
@@ -0,0 +1,7 @@
+import times
+
+type DjangoDateTime* = distinct DateTime
+
+# proc toTime*(x: DjangoDateTime): Time  {.borrow.} # <-- works
+proc format*(x: DjangoDateTime, f: TimeFormat,
+    loc: DateTimeLocale = DefaultLocale): string {.borrow.}