sigmatch: simpler procedural type matching

Summary
=======

* simplify the logic for handling generic routines using `auto` as the
  return types
* reject un-instantiated routines used in branches and array
  construction expression earlier, which results in clearer error
  messages (`has no concrete type` vs. `cannot instantiate`)
* support lambda expressions coming from template expansions

Details
=======

Introduced by nim-lang/Nim#3234, when both the
formal and actual type are unresolved meta types, the relation is
considered to be `isBothMetaConvertible`. The intention seems to have
been supporting passing `auto`-returning routines or lambda-expressions
to generic procedure-type parameters, but the logic introduced by the
aforementioned PR is not necessary for achieving that.

`auto` return types now use dedicated handling in `procTypeRel`, both
the formal and actual type being unresolved generics results in a
mismatch again, and `typeRel` is no longer applied repeatedly for
`isBothMetaConvertible` matches.

There was also the issue of `generateTypeInstance` being misused, which
could result in spurious `cannot instantiate` or internal compiler
errors. Since meta types are possibly involved, the correct routine to
use is `prepareMetatypeForSigmatch`. Testing that the produced
type instance is not a meta type was also done incorrectly (`isMetaType`
was used instead of `containsGenericType`).

Finally, `nkStmtListExpr` nodes are now considered when instantiating
generic routine as part of parameter matching, which allows for complex
expression yielding anonymous generic procedures.

compiler/sem/sem.nim

@@ -243,6 +243,23 @@ proc fitNodeConsiderViewType(c: PContext, formal: PType, arg: PNode; info: TLine
   else:
    result = a
 
+proc genericProcCheck(c: PContext, n: PNode): PNode =
+  ## Checks that the analysed expression `n` is of generic procedural type.
+  ## Returns either `n` or an error.
+  if n.typ != nil and n.typ.kind == tyError:
+    return n
+
+  # skip all statement list wrappers:
+  var it {.cursor.} = n
+  while it.kind == nkStmtListExpr:
+    it = it.lastSon
+
+  if (it.kind == nkSym and it.sym.isGenericRoutineStrict) or
+     it.isGenericRoutine:
+    c.config.newError(n, PAstDiag(kind: adSemProcHasNoConcreteType))
+  else:
+    n
+
 proc inferWithMetatype(c: PContext, formal: PType,
                        arg: PNode, coerceDistincts = false): PNode
 

compiler/sem/semexprs.nim

@@ -782,6 +782,7 @@ proc semArrayConstr(c: PContext, n: PNode, flags: TExprFlags): PNode =
 
       if e.kind != nkError:
         e = semExprWithType(c, e, {})
+        e = genericProcCheck(c, e)
 
       if typ.isNil:
         # must be the first item; initialize the common type:

compiler/sem/semstmts.nim

@@ -80,6 +80,7 @@ proc semProc(c: PContext, n: PNode): PNode
 
 proc semExprBranch(c: PContext, n: PNode; flags: TExprFlags = {}): PNode =
   result = semExpr(c, n, flags)
+  result = genericProcCheck(c, result)
   if result.typ != nil:
     # XXX tyGenericInst here?
     if result.typ.kind in {tyVar, tyLent}: result = newDeref(result)

compiler/sem/semtypinst.nim

@@ -793,6 +793,10 @@ proc recomputeFieldPositions*(t: PType; obj: PNode; currPosition: var int) =
 
 proc generateTypeInstance*(p: PContext, pt: TIdTable, info: TLineInfo,
                            t: PType): PType =
+  ## Produces the instantiated type for the generic type `t`, using the
+  ## bindings provided by `pt`. All type variables used by `t` must have
+  ## *concrete* type bounds to them -- both meta types and missing bindings
+  ## are disallowed and will result in an instantiation failure.
   # Given `t` like Foo[T]
   # pt: Table with type mappings: T -> int
   # Desired result: Foo[int]

compiler/sem/sigmatch.nim

@@ -41,6 +41,7 @@ import
   ],
   compiler/utils/[
     debugutils,
+    idioms
   ]
 
 # xxx: reports are a code smell meaning data types are misplaced, for example
@@ -509,20 +510,20 @@ proc procParamTypeRel(c: var TCandidate, f, a: PType): TTypeRelation =
   ## For example we have:
   ##
   ## .. code-block:: nim
-  ##   proc myMap[T,S](sIn: seq[T], f: proc(x: T): S): seq[S] = ...
-  ##   proc innerProc[Q,W](q: Q): W = ...
+  ##   proc myMap[T,U,S](sIn: seq[T], f: proc(x: T, y: U): S): seq[S] = ...
+  ##   proc innerProc[Q](q: Q, b: Q): auto = ...
   ##
   ## And we want to match: myMap(@[1,2,3], innerProc)
   ## This proc (procParamTypeRel) will do the following steps in
-  ## three different calls:
-  ## - matches f=T to a=Q. Since f is metatype, we resolve it
-  ##    to int (which is already known at this point). So in this case
-  ##    Q=int mapping will be saved to c.bindings.
-  ## - matches f=S to a=W. Both of these metatypes are unknown, so we
-  ##    return with isBothMetaConvertible to ask for rerun.
-  ## - matches f=S to a=W. At this point the return type of innerProc
-  ##    is known (we get it from c.bindings). We can use that value
-  ##    to match with f, and save back to c.bindings.
+  ## two different calls:
+  ## - matches `f`=T to `a`=Q. `f` is a resolved metatype ('int' is bound to
+  ##   it already), so `a` can be inferred; a binding for Q=int is saved
+  ## - matces `f`=U to `a`=Q. `f` is an unresolved metatype, but since Q was
+  ##   already inferred as 'int', U can be inferred from it; a binding for
+  ##   U=int is saved
+  ##
+  ## The 'auto' return type doesn't reach here, but is instead handled by
+  ## ``procTypeRel``.
   var
     f = f
     a = a
@@ -531,6 +532,7 @@ proc procParamTypeRel(c: var TCandidate, f, a: PType): TTypeRelation =
     let aResolved = PType(idTableGet(c.bindings, a))
     if aResolved != nil:
       a = aResolved
+
   if a.isMetaType:
     if f.isMetaType:
       # We are matching a generic proc (as proc param)
@@ -539,12 +541,14 @@ proc procParamTypeRel(c: var TCandidate, f, a: PType): TTypeRelation =
       # type is already fully-determined, so we are
       # going to try resolve it
       if c.call != nil:
-        f = generateTypeInstance(c.c, c.bindings, c.call.info, f)
+        f = prepareMetatypeForSigmatch(c.c, c.bindings, c.call.info, f)
       else:
         f = nil
-      if f.isNil() or f.isMetaType:
-        # no luck resolving the type, so the inference fails
-        return isBothMetaConvertible
+      if f.isNil() or containsGenericType(f):
+        # no luck resolving the type, so the inference fails. Note that using
+        # ``isMetaType`` wont work, as it doesn't check for nested
+        # unresolved type variables
+        return isNone
     # Note that this typeRel call will save a's resolved type into c.bindings
     let reverseRel = typeRel(c, a, f)
     if reverseRel >= isGeneric:
@@ -577,12 +581,16 @@ proc procTypeRel(c: var TCandidate, f, a: PType): TTypeRelation =
     for i in 1..<f.len:
       checkParam(f[i], a[i])
 
-    if f[0] != nil:
-      if a[0] != nil:
-        checkParam(f[0], a[0])
+    if f[0] != nil and f[0] != nil:
+      # both have return types
+      if a[0].kind == tyUntyped:
+        # special handling for the return type: if `a` is 'auto' we first
+        # instantiate the procedure passed as the argument
+        result = isBothMetaConvertible
       else:
-        return isNone
-    elif a[0] != nil:
+        checkParam(f[0], a[0])
+    elif a[0] != f[0]:
+      # one has a void return type while the other doesn't
       return isNone
 
     result = getProcConvMismatch(c.c.config, f, a, result)[1]
@@ -2078,6 +2086,59 @@ template matchesVoidProc(t: PType): bool =
   (t.kind == tyProc and t.len == 1 and t[0].isNil()) or
     (t.kind == tyBuiltInTypeClass and t[0].kind == tyProc)
 
+proc instantiateRoutineExpr(c: PContext, bindings: TIdTable, n: PNode): PNode =
+  ## Instantiates the generic routine that the expression `n` names. Returns
+  ## the updated expression, or an error.
+  let orig = n
+  var
+    n = n
+    depth = 0
+
+  # the symbol or lambda expression might be coming from a statement list
+  # expression, so we have to unwrap it first
+  while n.kind == nkStmtListExpr:
+    n = n.lastSon
+    inc depth
+
+  # instantiate the symbol
+  case n.kind
+  of nkProcDef, nkFuncDef, nkIteratorDef, nkLambdaKinds:
+    result = c.semInferredLambda(c, bindings, n)
+  of nkSym:
+    let inferred = c.semGenerateInstance(c, n.sym, bindings, n.info)
+    result =
+      if inferred.isError:
+        inferred.ast
+      else:
+        newSymNode(inferred, n.info)
+  else:
+    # only lambda and symbol expressions are able to provide uninstantiated
+    # generic routines
+    unreachable(n.kind)
+
+  if orig.kind == nkStmtListExpr:
+    # make a copy of the tree, update the types, and fill in the instantiated
+    # lambda/symbol expression
+    let updated = copyTreeWithoutNode(orig, n)
+
+    var it {.cursor.} = updated
+    # traverse all nested statement list expressions and update their type:
+    for _ in 0..<depth-1:
+      it.typ = result.typ
+      it = it.lastSon
+
+    # set the type for last statement list and add the instantiated
+    # expression:
+    it.typ = result.typ
+    it.add result
+
+    if result.isError:
+      result = c.config.wrapError(updated)
+    else:
+      result = updated
+  else:
+    discard "result is already set"
+
 proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
                         argSemantized: PNode): PNode =
   if argSemantized.isError:
@@ -2154,42 +2215,29 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
         argSemantized
     return
 
-  var
-    bothMetaCounter = 0
-      ## bothMetaCounter is for safety to avoid any infinite loop,
-      ## we don't have any example when it is needed.
-    lastBindingsLength = -1
-      ## lastBindingsLenth use to track whether m.bindings remains the same,
-      ## because in that case there is no point in continuing.
-
-  # If r == isBothMetaConvertible then we rerun typeRel.
-  while r == isBothMetaConvertible and
-      lastBindingsLength != m.bindings.counter and
-      bothMetaCounter < 100:                        # ensure termination
-
-    lastBindingsLength = m.bindings.counter
-    inc(bothMetaCounter)
-
-    case arg.kind
-    of nkProcDef, nkFuncDef, nkIteratorDef, nkLambdaKinds:
-      result = c.semInferredLambda(c, m.bindings, arg)
-    of nkSym:
-      let inferred = c.semGenerateInstance(c, arg.sym, m.bindings, arg.info)
-      result =
-        if inferred.isError:
-          inferred.ast
-        else:
-          newSymNode(inferred, arg.info)
-    else:
-      result = nil
-      return
-
+  if r == isBothMetaConvertible:
+    result = instantiateRoutineExpr(c, m.bindings, arg)
     if result.isError:
       return
 
     inc(m.convMatches)
     arg = result
+    # now that we know the result type, re-run the match. We cannot simply
+    # match the result types unfortunately, as the formal type might be some
+    # complex generic type
     r = typeRel(m, f, arg.typ)
+    case r
+    of isConvertible, isNone:
+      discard "okay; these stay"
+    of isInferredConvertible, isInferred:
+      # there's nothing left to infer for the procedure type used as the
+      # argument, so these are not possible
+      unreachable()
+    else:
+      # some form of inference must have taken place (otherwise we wouldn't
+      # have reached here), so the match has to be counted as generic;
+      # ``isBothMetaConvertible`` is used to signal this.
+      r = isBothMetaConvertible
 
   # now check the relation result in `r`
   case r
@@ -2219,16 +2267,7 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
       else:
         implicitConv(nkHiddenStdConv, f, arg, m, c)
   of isInferred, isInferredConvertible:
-    case arg.kind
-    of nkProcDef, nkFuncDef, nkIteratorDef, nkLambdaKinds:
-      result = c.semInferredLambda(c, m.bindings, arg)
-    of nkSym:
-      let inferred = c.semGenerateInstance(c, arg.sym, m.bindings, arg.info)
-      result = newSymNode(inferred, arg.info)
-    else:
-      result = nil
-      return
-
+    result = instantiateRoutineExpr(c, m.bindings, arg)
     if result.isError:
       return
 
@@ -2255,8 +2294,10 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
     else:
       result = arg
   of isBothMetaConvertible:
-    # This is the result for the 101th time.
-    result = nil
+    # we reach here if a generic procedure with an 'auto' return type was
+    # instantiated. Regarding the counters, this is treated the same way
+    # as ``isInferred`` is
+    inc(m.genericMatches)
   of isFromIntLit:
     # too lazy to introduce another ``*matches`` field, so we conflate
     # ``isIntConv`` and ``isIntLit`` here:

tests/lang_callable/generics/tgeneric_intantiation_outside_call.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "cannot instantiate: \'T\'"
+  errormsg: "'m' doesn't have a concrete type, due to unspecified generic parameters"
   description: '''
     . From https://github.com/nim-lang/Nim/issues/8270
       SIGSEGV on pragma with array of generic proc

tests/lang_callable/generics/tgeneric_procedure_cannot_assigned.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "cannot instantiate: \'T\'"
+  errormsg: "'foo' doesn't have a concrete type, due to unspecified generic parameters"
   description: '''
     . From https://github.com/nim-lang/Nim/issues/7141
       3 lines to break the compiler: proc [T], assignment and SIGSEGV