better errors for standalone explicit generic instantiations (#24276)

refs #8064, refs #24010

Error messages for standalone explicit generic instantiations are
revamped. Failing standalone explicit generic instantiations now only
error after overloading has finished and resolved to the default `[]`
magic (this means `[]` can now be overloaded for procs but this isn't
necessarily intentional, in #24010 it was documented that it isn't
possible). The error messages for failed instantiations are also no
longer a simple `cannot instantiate: foo` message, instead they now give
the same type mismatch error message as overloads with mismatching
explicit generic parameters.

This is now possible due to the changes in #24010 that delegate all
explicit generic proc instantiations to overload resolution. Old code
that worked around this is now removed. `maybeInstantiateGeneric` could
maybe also be removed in favor of just `explicitGenericSym`, the `result
== n` case is due to `explicitGenericInstError` which is only for niche
cases.

Also, to cause "ambiguous identifier" error messages when the explicit
instantiation is a symchoice and the expression context doesn't allow
symchoices, we semcheck the sym/symchoice created by
`explicitGenericSym` with the given expression flags.

#8064 isn't entirely fixed because the error message is still misleading
for the original case which does `values[1]`, as a consequence of
#12664.

(cherry picked from commit 0a058a6)

Author: metagn

compiler/semcall.nim

@@ -864,7 +864,7 @@ proc explicitGenericInstError(c: PContext; n: PNode): PNode =
   localError(c.config, getCallLineInfo(n), errCannotInstantiateX % renderTree(n))
   result = n
 
-proc explicitGenericSym(c: PContext, n: PNode, s: PSym): PNode =
+proc explicitGenericSym(c: PContext, n: PNode, s: PSym, errors: var CandidateErrors, doError: bool): PNode =
   if s.kind in {skTemplate, skMacro}:
     internalError c.config, n.info, "cannot get explicitly instantiated symbol of " &
       (if s.kind == skTemplate: "template" else: "macro")
@@ -874,6 +874,11 @@ proc explicitGenericSym(c: PContext, n: PNode, s: PSym): PNode =
   if m.state != csMatch:
     # state is csMatch only if *all* generic params were matched,
     # including implicit parameters
+    if doError:
+      errors.add(CandidateError(
+        sym: s,
+        firstMismatch: m.firstMismatch,
+        diagnostics: m.diagnostics))
     return nil
   var newInst = generateInstance(c, s, m.bindings, n.info)
   newInst.typ.flags.excl tfUnresolved
@@ -897,42 +902,39 @@ proc setGenericParams(c: PContext, n, expectedParams: PNode) =
     else:
       n[i].typ() = e.typ.skipTypes({tyTypeDesc})
 
-proc explicitGenericInstantiation(c: PContext, n: PNode, s: PSym): PNode =
+proc explicitGenericInstantiation(c: PContext, n: PNode, s: PSym, doError: bool): PNode =
   assert n.kind == nkBracketExpr
   setGenericParams(c, n, s.ast[genericParamsPos])
   var s = s
   var a = n[0]
+  var errors: CandidateErrors = @[]
   if a.kind == nkSym:
     # common case; check the only candidate has the right
     # number of generic type parameters:
-    if s.ast[genericParamsPos].safeLen != n.len-1:
-      let expected = s.ast[genericParamsPos].safeLen
-      localError(c.config, getCallLineInfo(n), errGenerated, "cannot instantiate: '" & renderTree(n) &
-         "'; got " & $(n.len-1) & " typeof(s) but expected " & $expected)
-      return n
-    result = explicitGenericSym(c, n, s)
-    if result == nil: result = explicitGenericInstError(c, n)
+    result = explicitGenericSym(c, n, s, errors, doError)
+    if doError and result == nil:
+      notFoundError(c, n, errors)
   elif a.kind in {nkClosedSymChoice, nkOpenSymChoice}:
     # choose the generic proc with the proper number of type parameters.
-    # XXX I think this could be improved by reusing sigmatch.paramTypesMatch.
-    # It's good enough for now.
     result = newNodeI(a.kind, getCallLineInfo(n))
     for i in 0..<a.len:
       var candidate = a[i].sym
       if candidate.kind in {skProc, skMethod, skConverter,
                             skFunc, skIterator}:
-        # it suffices that the candidate has the proper number of generic
-        # type parameters:
-        if candidate.ast[genericParamsPos].safeLen == n.len-1:
-          let x = explicitGenericSym(c, n, candidate)
-          if x != nil: result.add(x)
+        let x = explicitGenericSym(c, n, candidate, errors, doError)
+        if x != nil: result.add(x)
     # get rid of nkClosedSymChoice if not ambiguous:
-    if result.len == 1 and a.kind == nkClosedSymChoice:
-      result = result[0]
-    elif result.len == 0: result = explicitGenericInstError(c, n)
-    # candidateCount != 1: return explicitGenericInstError(c, n)
+    if result.len == 0:
+      result = nil
+      if doError:
+        notFoundError(c, n, errors)
   else:
-    result = explicitGenericInstError(c, n)
+    # probably unreachable: we are trying to instantiate `a` which is not
+    # a sym/symchoice
+    if doError:
+      result = explicitGenericInstError(c, n)
+    else:
+      result = nil
 
 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

compiler/semexprs.nim

@@ -1724,27 +1724,15 @@ proc semDeref(c: PContext, n: PNode, flags: TExprFlags): PNode =
   else: result = nil
   #GlobalError(n[0].info, errCircumNeedsPointer)
 
-proc maybeInstantiateGeneric(c: PContext, n: PNode, s: PSym): PNode =
-  ## Instantiates generic if not lacking implicit generics,
-  ## otherwise returns n.
-  let
-    neededGenParams = s.ast[genericParamsPos].len
-    heldGenParams = n.len - 1
-  var implicitParams = 0
-  for x in s.ast[genericParamsPos]:
-    if tfImplicitTypeParam in x.typ.flags:
-      inc implicitParams
-  if heldGenParams != neededGenParams and implicitParams + heldGenParams == neededGenParams:
-    # This is an implicit + explicit generic procedure without all args passed,
-    # kicking back the sem'd symbol fixes #17212
-    # Uncertain the hackiness of this solution.
-    result = n
-  else:
-    result = explicitGenericInstantiation(c, n, s)
-    if result == n:
-      n[0] = copyTree(result[0])
-
-proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
+proc maybeInstantiateGeneric(c: PContext, n: PNode, s: PSym, doError: bool): PNode =
+  ## Attempts to instantiate generic proc symbol(s) with given parameters.
+  ## If instantiation causes errors; if `doError` is `true`, a type mismatch
+  ## error is given, otherwise `nil` is returned.
+  result = explicitGenericInstantiation(c, n, s, doError)
+  if result == n:
+    n[0] = copyTree(result[0])
+
+proc semSubscript(c: PContext, n: PNode, flags: TExprFlags, afterOverloading = false): PNode =
   ## returns nil if not a built-in subscript operator; also called for the
   ## checking of assignments
   result = nil
@@ -1768,7 +1756,7 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
       result.typ() = semStaticType(c, n[1], nil)
       return
     elif arr.n != nil:
-      return semSubscript(c, arr.n, flags)
+      return semSubscript(c, arr.n, flags, afterOverloading)
     else:
       arr = arr.base
 
@@ -1825,7 +1813,10 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
       of skProc, skFunc, skMethod, skConverter, skIterator:
         # type parameters: partial generic specialization
         n[0] = semSymGenericInstantiation(c, n[0], s)
-        result = maybeInstantiateGeneric(c, n, s)
+        result = maybeInstantiateGeneric(c, n, s, doError = afterOverloading)
+        if result != nil:
+          # check newly created sym/symchoice
+          result = semExpr(c, result, flags)
       of skMacro, skTemplate:
         if efInCall in flags:
           # We are processing macroOrTmpl[] in macroOrTmpl[](...) call.

compiler/semmagic.nim

@@ -61,12 +61,12 @@ type
   SemAsgnMode = enum asgnNormal, noOverloadedSubscript, noOverloadedAsgn
 
 proc semAsgn(c: PContext, n: PNode; mode=asgnNormal): PNode
-proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode
+proc semSubscript(c: PContext, n: PNode, flags: TExprFlags, afterOverloading = false): PNode
 
 proc semArrGet(c: PContext; n: PNode; flags: TExprFlags): PNode =
   result = newNodeI(nkBracketExpr, n.info)
   for i in 1..<n.len: result.add(n[i])
-  result = semSubscript(c, result, flags)
+  result = semSubscript(c, result, flags, afterOverloading = true)
   if result.isNil:
     let x = copyTree(n)
     x[0] = newIdentNode(getIdent(c.cache, "[]"), n.info)
@@ -79,9 +79,20 @@ proc semArrGet(c: PContext; n: PNode; flags: TExprFlags): PNode =
           result.typ() = makeTypeFromExpr(c, copyTree(result))
           result.typ.flags.incl tfNonConstExpr
           return
-    bracketNotFoundError(c, x, flags)
-    #localError(c.config, n.info, "could not resolve: " & $n)
-    result = errorNode(c, n)
+    let s = # extract sym from first arg
+      if n.len > 1:
+        if n[1].kind == nkSym: n[1].sym
+        elif n[1].kind in nkSymChoices + {nkOpenSym} and n[1].len != 0:
+          n[1][0].sym
+        else: nil
+      else: nil
+    if s != nil and s.kind in routineKinds:
+      # this is a failed generic instantiation
+      # semSubscript should already error but this is better for cascading errors
+      result = explicitGenericInstError(c, n)
+    else:
+      bracketNotFoundError(c, x, flags)
+      result = errorNode(c, n)
 
 proc semArrPut(c: PContext; n: PNode; flags: TExprFlags): PNode =
   # rewrite `[]=`(a, i, x)  back to ``a[i] = x``.

compiler/sigmatch.nim

@@ -200,6 +200,9 @@ proc matchGenericParams*(m: var TCandidate, binding: PNode, callee: PSym) =
     elif tfImplicitTypeParam in paramSym.typ.flags:
       # not a mismatch, but can't create sym
       m.state = csEmpty
+      m.firstMismatch.kind = kMissingGenericParam
+      m.firstMismatch.arg = i + 1
+      m.firstMismatch.formal = paramSym
       return
     else:
       m.state = csNoMatch

tests/generics/tpointerprocs.nim

@@ -2,10 +2,17 @@ discard """
 cmd: "nim check $options --hints:off $file"
 action: "reject"
 nimout:'''
-tpointerprocs.nim(15, 11) Error: 'foo' doesn't have a concrete type, due to unspecified generic parameters.
-tpointerprocs.nim(27, 11) Error: cannot instantiate: 'foo[int]'; got 1 typeof(s) but expected 2
-tpointerprocs.nim(27, 14) Error: expression 'foo[int]' has no type (or is ambiguous)
-tpointerprocs.nim(28, 11) Error: expression 'bar' has no type (or is ambiguous)
+tpointerprocs.nim(22, 11) Error: 'foo' doesn't have a concrete type, due to unspecified generic parameters.
+tpointerprocs.nim(34, 14) Error: type mismatch: got <int>
+but expected one of:
+proc foo(x: int | float; y: int or string): float
+  first type mismatch at position: 2 in generic parameters
+  missing generic parameter: y:type
+
+expression: foo[int]
+tpointerprocs.nim(34, 14) Error: cannot instantiate: 'foo[int]'
+tpointerprocs.nim(34, 14) Error: expression 'foo[int]' has no type (or is ambiguous)
+tpointerprocs.nim(35, 11) Error: expression 'bar' has no type (or is ambiguous)
 '''
 """
 
@@ -25,4 +32,4 @@ block:
   proc foo(x: int | float, y: int or string): float = result = 1.0
   let
     bar = foo[int]
-    baz = bar
+    baz = bar

tests/overload/tambiguousexplicitgeneric.nim

@@ -0,0 +1,6 @@
+# related to issue #8064
+
+import tables
+
+let x = values[int] #[tt.Error
+        ^ ambiguous identifier: 'values' -- use one of the following:]#

tests/overload/texplicitgenericdiscard.nim

@@ -0,0 +1,7 @@
+# issue #8064
+
+import tables
+
+values[int] #[tt.Error
+^ ambiguous identifier: 'values' -- use one of the following:]#
+# this happens before discard check, so no discard error