proc typeclass accounts for iterator, call conventions + nil fix + document typeclass AST (#21629)

* test fix #16546 #16548 + another issue

* please don't tell me other packages do this

* fix CI + test typeclass callconv pragma

* better logic in parser

* docs and changelog

changelogs/changelog_2_0_0.md

@@ -175,6 +175,40 @@
 
 - - Added the `--legacy:verboseTypeMismatch` switch to get legacy type mismatch error messages.
 
+- The `proc` and `iterator` type classes now respectively only match
+  procs and iterators. Previously both type classes matched any of
+  procs or iterators.
+
+  ```nim
+  proc prc(): int =
+    123
+
+  iterator iter(): int =
+    yield 123
+  
+  proc takesProc[T: proc](x: T) = discard
+  proc takesIter[T: iterator](x: T) = discard
+
+  # always compiled:
+  takesProc(prc)
+  takesIter(iter)
+  # no longer compiles:
+  takesProc(iter)
+  takesIter(prc)
+  ```
+
+- The `proc` and `iterator` type classes now accept a calling convention pragma
+  (i.e. `proc {.closure.}`) that must be shared by matching proc or iterator
+  types. Previously pragmas were parsed but discarded if no parameter list
+  was given.
+
+  This is represented in the AST by an `nnkProcTy`/`nnkIteratorTy` node with
+  an `nnkEmpty` node in the place of the `nnkFormalParams` node, and the pragma
+  node in the same place as in a concrete `proc` or `iterator` type node. This
+  state of the AST may be unexpected to existing code, both due to the
+  replacement of the `nnkFormalParams` node as well as having child nodes
+  unlike other type class AST.
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"

compiler/parser.nim

@@ -1204,8 +1204,11 @@ proc parseProcExpr(p: var Parser; isExpr: bool; kind: TNodeKind): PNode =
     result[bodyPos] = parseStmt(p)
   else:
     result = newNodeI(if kind == nkIteratorDef: nkIteratorTy else: nkProcTy, info)
-    if hasSignature:
-      result.add(params)
+    if hasSignature or pragmas.kind != nkEmpty:
+      if hasSignature:
+        result.add(params)
+      else: # pragmas but no param list, implies typeclass with pragmas
+        result.add(p.emptyNode)
       if kind == nkFuncDef:
         parMessage(p, "func keyword is not allowed in type descriptions, use proc with {.noSideEffect.} pragma instead")
       result.add(pragmas)

compiler/semexprs.nim

@@ -469,6 +469,7 @@ proc isOpImpl(c: PContext, n: PNode, flags: TExprFlags): PNode =
       res = t.kind == tyProc and
             t.callConv == ccClosure
     of "iterator":
+      # holdover from when `is iterator` didn't work
       let t = skipTypes(t1, abstractRange)
       res = t.kind == tyProc and
             t.callConv == ccClosure and

compiler/semtypes.nim

@@ -2054,25 +2054,27 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
   of nkOutTy: result = semVarOutType(c, n, prev, {tfIsOutParam})
   of nkDistinctTy: result = semDistinct(c, n, prev)
   of nkStaticTy: result = semStaticType(c, n[0], prev)
-  of nkIteratorTy:
-    if n.len == 0:
+  of nkProcTy, nkIteratorTy:
+    if n.len == 0 or n[0].kind == nkEmpty:
+      # 0 length or empty param list with possible pragmas imply typeclass
       result = newTypeS(tyBuiltInTypeClass, c)
       let child = newTypeS(tyProc, c)
-      child.flags.incl tfIterator
+      var symKind: TSymKind
+      if n.kind == nkIteratorTy:
+        child.flags.incl tfIterator
+      if n.len > 0 and n[1].kind != nkEmpty and n[1].len > 0:
+        # typeclass with pragma
+        let symKind = if n.kind == nkIteratorTy: skIterator else: skProc
+        # dummy symbol for `pragma`:
+        var s = newSymS(symKind, newIdentNode(getIdent(c.cache, "dummy"), n.info), c)
+        s.typ = child
+        # for now only call convention pragmas supported in proc typeclass
+        pragma(c, s, n[1], {FirstCallConv..LastCallConv})
       result.addSonSkipIntLit(child, c.idgen)
-    else:
-      result = semProcTypeWithScope(c, n, prev, skIterator)
-      if result.kind == tyProc:
-        result.flags.incl(tfIterator)
-        if n.lastSon.kind == nkPragma and hasPragma(n.lastSon, wInline):
-          result.callConv = ccInline
-        else:
-          result.callConv = ccClosure
-  of nkProcTy:
-    if n.len == 0:
-      result = newConstraint(c, tyProc)
     else:
       result = semProcTypeWithScope(c, n, prev, skProc)
+      if n.kind == nkIteratorTy and result.kind == tyProc:
+        result.flags.incl(tfIterator)
   of nkEnumTy: result = semEnum(c, n, prev)
   of nkType: result = n.typ
   of nkStmtListType: result = semStmtListType(c, n, prev)

compiler/sigmatch.nim

@@ -624,6 +624,9 @@ proc procTypeRel(c: var TCandidate, f, a: PType): TTypeRelation =
     if f.len != a.len: return
     result = isEqual      # start with maximum; also correct for no
                           # params at all
+    
+    if f.flags * {tfIterator} != a.flags * {tfIterator}:
+      return isNone
 
     template checkParam(f, a) =
       result = minRel(result, procParamTypeRel(c, f, a))
@@ -1636,13 +1639,19 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
 
   of tyBuiltInTypeClass:
     considerPreviousT:
-      let targetKind = f[0].kind
+      let target = f[0]
+      let targetKind = target.kind
       let effectiveArgType = a.skipTypes({tyRange, tyGenericInst,
                                           tyBuiltInTypeClass, tyAlias, tySink, tyOwned})
-      let typeClassMatches = targetKind == effectiveArgType.kind and
-                             not effectiveArgType.isEmptyContainer
-      if typeClassMatches or
-        (targetKind in {tyProc, tyPointer} and effectiveArgType.kind == tyNil):
+      if targetKind == effectiveArgType.kind:
+        if effectiveArgType.isEmptyContainer:
+          return isNone
+        if targetKind == tyProc:
+          if target.flags * {tfIterator} != effectiveArgType.flags * {tfIterator}:
+            return isNone
+          if tfExplicitCallConv in target.flags and
+              target.callConv != effectiveArgType.callConv:
+            return isNone
         put(c, f, a)
         return isGeneric
       else:

compiler/typesrenderer.nim

@@ -64,9 +64,8 @@ proc renderType(n: PNode, toNormalize: bool): string =
       result = "ptr"
   of nkProcTy:
     assert n.len != 1
-    if n.len > 1:
+    if n.len > 1 and n[0].kind == nkFormalParams:
       let params = n[0]
-      assert params.kind == nkFormalParams
       assert params.len > 0
       result = "proc("
       for i in 1..<params.len: result.add(renderType(params[i], toNormalize) & ',')

doc/astspec.txt

@@ -1348,7 +1348,7 @@ Generic parameters are treated in the type, not the ``proc`` itself.
 Concrete syntax:
 
   ```nim
-  type MyProc[T] = proc(x: T)
+  type MyProc[T] = proc(x: T) {.nimcall.}
   ```
 
 AST:
@@ -1363,7 +1363,8 @@ AST:
     nnkProcTy( # behaves like a procedure declaration from here on
       nnkFormalParams(
         # ...
-      )
+      ),
+      nnkPragma(nnkIdent("nimcall"))
     )
   )
   ```
@@ -1371,6 +1372,37 @@ AST:
 The same syntax applies to ``iterator`` (with ``nnkIteratorTy``), but
 *does not* apply to ``converter`` or ``template``.
 
+Type class versions of these nodes generally share the same node kind but
+without any child nodes. The ``tuple`` type class is represented by
+``nnkTupleClassTy``, while a ``proc`` or ``iterator`` type class with pragmas
+has an ``nnkEmpty`` node in place of the ``nnkFormalParams`` node of a
+concrete ``proc`` or ``iterator`` type node.
+
+  ```nim
+  type TypeClass = proc {.nimcall.} | ref | tuple
+  ```
+
+AST:
+
+  ```nim
+  nnkTypeDef(
+    nnkIdent("TypeClass"),
+    nnkEmpty(),
+    nnkInfix(
+      nnkIdent("|"),
+      nnkProcTy(
+        nnkEmpty(),
+        nnkPragma(nnkIdent("nimcall"))
+      ),
+      nnkInfix(
+        nnkIdent("|"),
+        nnkRefTy(),
+        nnkTupleClassTy()
+      )
+    )
+  )
+  ```
+
 Mixin statement
 ---------------
 

doc/manual.md

@@ -5467,9 +5467,9 @@ type class           matches
 ==================   ===================================================
 `object`             any object type
 `tuple`              any tuple type
-
 `enum`               any enumeration
 `proc`               any proc type
+`iterator`           any iterator type
 `ref`                any `ref` type
 `ptr`                any `ptr` type
 `var`                any `var` type
@@ -5529,6 +5529,17 @@ as `type constraints`:idx: of the generic type parameter:
   onlyIntOrString("xy", 50) # invalid as 'T' cannot be both at the same time
   ```
 
+`proc` and `iterator` type classes also accept a calling convention pragma
+to restrict the calling convention of the matching `proc` or `iterator` type.
+
+  ```nim
+  proc onlyClosure[T: proc {.closure.}](x: T) = discard
+
+  onlyClosure(proc() = echo "hello") # valid
+  proc foo() {.nimcall.} = discard
+  onlyClosure(foo) # type mismatch
+  ```
+
 
 Implicit generics
 -----------------

lib/pure/hashes.nim

@@ -501,7 +501,7 @@ proc hashIgnoreCase*(sBuf: string, sPos, ePos: int): Hash =
     h = h !& ord(c)
   result = !$h
 
-proc hash*[T: tuple | object | proc](x: T): Hash =
+proc hash*[T: tuple | object | proc | iterator {.closure.}](x: T): Hash =
   ## Efficient `hash` overload.
   runnableExamples:
     # for `tuple|object`, `hash` must be defined for each component of `x`.

lib/system.nim

@@ -62,11 +62,11 @@ proc typeof*(x: untyped; mode = typeOfIter): typedesc {.
     doAssert type(myFoo()) is string
     doAssert typeof(myFoo()) is string
     doAssert typeof(myFoo(), typeOfIter) is string
-    doAssert typeof(myFoo3) is "iterator"
+    doAssert typeof(myFoo3) is iterator
 
     doAssert typeof(myFoo(), typeOfProc) is float
     doAssert typeof(0.0, typeOfProc) is float
-    doAssert typeof(myFoo3, typeOfProc) is "iterator"
+    doAssert typeof(myFoo3, typeOfProc) is iterator
     doAssert not compiles(typeof(myFoo2(), typeOfProc))
       # this would give: Error: attempting to call routine: 'myFoo2'
       # since `typeOfProc` expects a typed expression and `myFoo2()` can
@@ -2188,39 +2188,30 @@ when notJSnotNims:
     include "system/profiler"
   {.pop.}
 
-  proc rawProc*[T: proc](x: T): pointer {.noSideEffect, inline.} =
+  proc rawProc*[T: proc {.closure.} | iterator {.closure.}](x: T): pointer {.noSideEffect, inline.} =
     ## Retrieves the raw proc pointer of the closure `x`. This is
     ## useful for interfacing closures with C/C++, hash compuations, etc.
-    when T is "closure":
-      #[
-      The conversion from function pointer to `void*` is a tricky topic, but this
-      should work at least for c++ >= c++11, e.g. for `dlsym` support.
-      refs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57869,
-      https://stackoverflow.com/questions/14125474/casts-between-pointer-to-function-and-pointer-to-object-in-c-and-c
-      ]#
-      {.emit: """
-      `result` = (void*)`x`.ClP_0;
-      """.}
-    else:
-      {.error: "Only closure function and iterator are allowed!".}
+    #[
+    The conversion from function pointer to `void*` is a tricky topic, but this
+    should work at least for c++ >= c++11, e.g. for `dlsym` support.
+    refs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57869,
+    https://stackoverflow.com/questions/14125474/casts-between-pointer-to-function-and-pointer-to-object-in-c-and-c
+    ]#
+    {.emit: """
+    `result` = (void*)`x`.ClP_0;
+    """.}
 
-  proc rawEnv*[T: proc](x: T): pointer {.noSideEffect, inline.} =
+  proc rawEnv*[T: proc {.closure.} | iterator {.closure.}](x: T): pointer {.noSideEffect, inline.} =
     ## Retrieves the raw environment pointer of the closure `x`. See also `rawProc`.
-    when T is "closure":
-      {.emit: """
-      `result` = `x`.ClE_0;
-      """.}
-    else:
-      {.error: "Only closure function and iterator are allowed!".}
+    {.emit: """
+    `result` = `x`.ClE_0;
+    """.}
 
-  proc finished*[T: proc](x: T): bool {.noSideEffect, inline, magic: "Finished".} =
+  proc finished*[T: iterator {.closure.}](x: T): bool {.noSideEffect, inline, magic: "Finished".} =
     ## It can be used to determine if a first class iterator has finished.
-    when T is "iterator":
-      {.emit: """
-      `result` = ((NI*) `x`.ClE_0)[1] < 0;
-      """.}
-    else:
-      {.error: "Only closure iterator is allowed!".}
+    {.emit: """
+    `result` = ((NI*) `x`.ClE_0)[1] < 0;
+    """.}
 
 from std/private/digitsutils import addInt
 export addInt

tests/typerel/tproctypeclass.nim

@@ -0,0 +1,76 @@
+import std/assertions
+
+proc main =
+  iterator closureIter(): int {.closure.} =
+    yield 1
+    yield 2
+  iterator inlineIter(): int {.inline.} =
+    yield 1
+    yield 2
+  proc procNotIter(): int = 1
+
+  doAssert closureIter is iterator
+  doAssert inlineIter is iterator
+  doAssert procNotIter isnot iterator
+
+  doAssert closureIter isnot proc
+  doAssert inlineIter isnot proc
+  doAssert procNotIter is proc
+
+  doAssert typeof(closureIter) is iterator
+  doAssert typeof(inlineIter) is iterator
+  doAssert typeof(procNotIter) isnot iterator
+
+  doAssert typeof(closureIter) isnot proc
+  doAssert typeof(inlineIter) isnot proc
+  doAssert typeof(procNotIter) is proc
+
+  block:
+    proc fn1(iter: iterator {.closure.}) = discard
+    proc fn2[T: iterator {.closure.}](iter: T) = discard
+
+    fn1(closureIter)
+    fn2(closureIter)
+
+    doAssert not compiles(fn1(procNotIter))
+    doAssert not compiles(fn2(procNotIter))
+
+    doAssert not compiles(fn1(inlineIter))
+    doAssert not compiles(fn2(inlineIter))
+
+  block: # concrete iterator type
+    proc fn1(iter: iterator(): int) = discard
+    proc fn2[T: iterator(): int](iter: T) = discard
+
+    fn1(closureIter)
+    fn2(closureIter)
+
+    doAssert not compiles(fn1(procNotIter))
+    doAssert not compiles(fn2(procNotIter))
+
+    doAssert not compiles(fn1(inlineIter))
+    doAssert not compiles(fn2(inlineIter))
+
+  proc takesNimcall[T: proc {.nimcall.}](p: T) = discard
+  proc takesClosure[T: proc {.closure.}](p: T) = discard
+  proc takesAnyProc[T: proc](p: T) = discard
+
+  proc nimcallProc(): int {.nimcall.} = 1
+  proc closureProc(): int {.closure.} = 2
+
+  doAssert nimcallProc is proc {.nimcall.}
+  takesNimcall(nimcallProc)
+  doAssert closureProc isnot proc {.nimcall.}
+  doAssert not compiles(takesNimcall(closureProc))
+
+  doAssert nimcallProc isnot proc {.closure.}
+  doAssert not compiles(takesClosure(nimcallProc))
+  doAssert closureProc is proc {.closure.}
+  takesClosure(closureProc)
+  
+  doAssert nimcallProc is proc
+  takesAnyProc(nimcallProc)
+  doAssert closureProc is proc
+  takesAnyProc(closureProc)
+
+main()

tests/typerel/ttynilinstantiation.nim

@@ -0,0 +1,7 @@
+proc foo[T: proc](x: T) =
+  # old error here:
+  let y = x
+  # invalid type: 'typeof(nil)' for let
+
+foo(nil) #[tt.Error
+   ^ type mismatch: got <typeof(nil)>]#