Merge pull request #11680 from timotheecour/pr_fix_sigmatch_errmsg

fixes #8305; fixes #7808; fixes #10285; fixes #11061 + other bugs with type mismatch error msgs

2 files changed, 68 insertions, 51 deletions

diff --git a/compiler/semcall.nim b/compiler/semcall.nim
index 232a350b8..805a29303 100644
--- a/compiler/semcall.nim
+++ b/compiler/semcall.nim
@@ -107,7 +107,6 @@ proc pickBestCandidate(c: PContext, headSymbol: PNode,
       elif errorsEnabled or z.diagnosticsEnabled:
         errors.add(CandidateError(
           sym: sym,
-          unmatchedVarParam: int z.mutabilityProblem,
           firstMismatch: z.firstMismatch,
           diagnostics: z.diagnostics))
     else:
@@ -173,14 +172,14 @@ proc presentFailedCandidates(c: PContext, n: PNode, errors: CandidateErrors):
   var filterOnlyFirst = false
   if optShowAllMismatches notin c.config.globalOptions:
     for err in errors:
-      if err.firstMismatch > 1:
+      if err.firstMismatch.arg > 1:
         filterOnlyFirst = true
         break
 
   var candidates = ""
   var skipped = 0
   for err in errors:
-    if filterOnlyFirst and err.firstMismatch == 1:
+    if filterOnlyFirst and err.firstMismatch.arg == 1:
       inc skipped
       continue
     if err.sym.kind in routineKinds and err.sym.ast != nil:
@@ -189,34 +188,35 @@ proc presentFailedCandidates(c: PContext, n: PNode, errors: CandidateErrors):
     else:
       add(candidates, getProcHeader(c.config, err.sym, prefer))
     add(candidates, "\n")
-    if err.firstMismatch != 0 and n.len > 1:
-      let cond = n.len > 2
-      if cond:
-        candidates.add("  first type mismatch at position: " & $abs(err.firstMismatch))
-        if err.firstMismatch >= 0: candidates.add("\n  required type: ")
-        else: candidates.add("\n  unknown named parameter: " & $n[-err.firstMismatch][0])
-      var wanted, got: PType = nil
-      if err.firstMismatch < 0:
-        discard
-      elif err.firstMismatch < err.sym.typ.len:
-        wanted = err.sym.typ.sons[err.firstMismatch]
-        if cond: candidates.add typeToString(wanted)
-      else:
-        if cond: candidates.add "none"
-      if err.firstMismatch > 0 and err.firstMismatch < n.len:
-        if cond:
-          candidates.add "\n  but expression '"
-          candidates.add renderTree(n[err.firstMismatch])
+    let nArg = if err.firstMismatch.arg < n.len: n[err.firstMismatch.arg] else: nil
+    let nameParam = if err.firstMismatch.formal != nil: err.firstMismatch.formal.name.s else: ""
+    if n.len > 1:
+      candidates.add("  first type mismatch at position: " & $err.firstMismatch.arg)
+      # candidates.add "\n  reason: " & $err.firstMismatch.kind # for debugging
+      case err.firstMismatch.kind
+      of kUnknownNamedParam: candidates.add("\n  unknown named parameter: " & $nArg[0])
+      of kAlreadyGiven: candidates.add("\n  named param already provided: " & $nArg[0])
+      of kExtraArg: candidates.add("\n  extra argument given")
+      of kMissingParam: candidates.add("\n  missing parameter: " & nameParam)
+      of kTypeMismatch, kVarNeeded:
+        doAssert nArg != nil
+        var wanted = err.firstMismatch.formal.typ
+        doAssert err.firstMismatch.formal != nil
+        candidates.add("\n  required type for " & nameParam &  ": ")
+        candidates.add typeToString(wanted)
+        candidates.add "\n  but expression '"
+        if err.firstMismatch.kind == kVarNeeded:
+          candidates.add renderNotLValue(nArg)
+          candidates.add "' is immutable, not 'var'"
+        else:
+          candidates.add renderTree(nArg)
           candidates.add "' is of type: "
-        got = n[err.firstMismatch].typ
-        if cond: candidates.add typeToString(got)
-      if wanted != nil and got != nil:
-        effectProblem(wanted, got, candidates)
-      if cond: candidates.add "\n"
-    if err.unmatchedVarParam != 0 and err.unmatchedVarParam < n.len:
-      candidates.add("  for a 'var' type a variable needs to be passed, but '" &
-                      renderNotLValue(n[err.unmatchedVarParam]) &
-                      "' is immutable\n")
+          var got = nArg.typ
+          candidates.add typeToString(got)
+          doAssert wanted != nil
+          if got != nil: effectProblem(wanted, got, candidates)
+      of kUnknown: internalAssert(c.config, false)
+      candidates.add "\n"
     for diag in err.diagnostics:
       candidates.add(diag & "\n")
   if skipped > 0:
@@ -260,7 +260,7 @@ proc bracketNotFoundError(c: PContext; n: PNode) =
   while symx != nil:
     if symx.kind in routineKinds:
       errors.add(CandidateError(sym: symx,
-                                unmatchedVarParam: 0, firstMismatch: 0,
+                                firstMismatch: MismatchInfo(),
                                 diagnostics: @[],
                                 enabled: false))
     symx = nextOverloadIter(o, c, headSymbol)
diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index 655ee83f0..4a6bf66e7 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -19,12 +19,22 @@ when (defined(booting) or defined(nimsuggest)) and not defined(leanCompiler):
   import docgen
 
 type
+  MismatchKind* = enum
+    kUnknown, kAlreadyGiven, kUnknownNamedParam, kTypeMismatch, kVarNeeded,
+    kMissingParam, kExtraArg
+
+  MismatchInfo* = object
+    kind*: MismatchKind # reason for mismatch
+    arg*: int           # position of provided arguments that mismatches
+    formal*: PSym       # parameter that mismatches against provided argument
+                        # its position can differ from `arg` because of varargs
+
   TCandidateState* = enum
     csEmpty, csMatch, csNoMatch
 
   CandidateError* = object
     sym*: PSym
-    unmatchedVarParam*, firstMismatch*: int
+    firstMismatch*: MismatchInfo
     diagnostics*: seq[string]
     enabled*: bool
 
@@ -56,7 +66,6 @@ type
                              # a distrinct type
     typedescMatched*: bool
     isNoCall*: bool          # misused for generic type instantiations C[T]
-    mutabilityProblem*: uint8 # tyVar mismatch
     inferredTypes: seq[PType] # inferred types during the current signature
                               # matching. they will be reset if the matching
                               # is not successful. may replace the bindings
@@ -70,8 +79,7 @@ type
                               # triggered with an idetools command in the
                               # future.
     inheritancePenalty: int   # to prefer closest father object type
-    firstMismatch*: int       # position of the first type mismatch for
-                              # better error messages
+    firstMismatch*: MismatchInfo # mismatch info for better error messages
     diagnosticsEnabled*: bool
 
   TTypeRelFlag* = enum
@@ -112,6 +120,7 @@ proc initCandidateAux(ctx: PContext,
   c.intConvMatches = 0
   c.genericMatches = 0
   c.state = csEmpty
+  c.firstMismatch = MismatchInfo()
   c.callee = callee
   c.call = nil
   c.baseTypeMatch = false
@@ -2280,6 +2289,17 @@ template isVarargsUntyped(x): untyped =
 
 proc matchesAux(c: PContext, n, nOrig: PNode,
                 m: var TCandidate, marker: var IntSet) =
+  var
+    a = 1 # iterates over the actual given arguments
+    f = if m.callee.kind != tyGenericBody: 1
+        else: 0 # iterates over formal parameters
+    arg: PNode # current prepared argument
+    formal: PSym # current routine parameter
+
+  defer:
+    m.firstMismatch.arg = a
+    m.firstMismatch.formal = formal
+
   template checkConstraint(n: untyped) {.dirty.} =
     if not formal.constraint.isNil:
       if matchNodeKinds(formal.constraint, n):
@@ -2294,28 +2314,21 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
       if argConverter.kind == nkHiddenCallConv:
         if argConverter.typ.kind != tyVar:
           m.state = csNoMatch
-          m.mutabilityProblem = uint8(f-1)
+          m.firstMismatch.kind = kVarNeeded
           return
       elif not n.isLValue:
         m.state = csNoMatch
-        m.mutabilityProblem = uint8(f-1)
+        m.firstMismatch.kind = kVarNeeded
         return
 
-  var
-    # iterates over formal parameters
-    f = if m.callee.kind != tyGenericBody: 1
-        else: 0
-    # iterates over the actual given arguments
-    a = 1
-    arg: PNode # current prepared argument
-
   m.state = csMatch # until proven otherwise
+  m.firstMismatch = MismatchInfo()
   m.call = newNodeI(n.kind, n.info)
   m.call.typ = base(m.callee) # may be nil
   var formalLen = m.callee.n.len
   addSon(m.call, copyTree(n.sons[0]))
   var container: PNode = nil # constructed container
-  var formal: PSym = if formalLen > 1: m.callee.n.sons[1].sym else: nil
+  formal = if formalLen > 1: m.callee.n.sons[1].sym else: nil
 
   while a < n.len:
     if a >= formalLen-1 and f < formalLen and m.callee.n[f].typ.isVarargsUntyped:
@@ -2336,20 +2349,20 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
         addSon(container, n.sons[a])
     elif n.sons[a].kind == nkExprEqExpr:
       # named param
+      m.firstMismatch.kind = kUnknownNamedParam
       # check if m.callee has such a param:
       prepareNamedParam(n.sons[a], c)
       if n.sons[a].sons[0].kind != nkIdent:
         localError(c.config, n.sons[a].info, "named parameter has to be an identifier")
         m.state = csNoMatch
-        m.firstMismatch = -a
         return
       formal = getSymFromList(m.callee.n, n.sons[a].sons[0].ident, 1)
       if formal == nil:
         # no error message!
         m.state = csNoMatch
-        m.firstMismatch = -a
         return
       if containsOrIncl(marker, formal.position):
+        m.firstMismatch.kind = kAlreadyGiven
         # already in namedParams, so no match
         # we used to produce 'errCannotBindXTwice' here but see
         # bug #3836 of why that is not sound (other overload with
@@ -2363,9 +2376,9 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
       n.sons[a].typ = n.sons[a].sons[1].typ
       arg = paramTypesMatch(m, formal.typ, n.sons[a].typ,
                                 n.sons[a].sons[1], n.sons[a].sons[1])
+      m.firstMismatch.kind = kTypeMismatch
       if arg == nil:
         m.state = csNoMatch
-        m.firstMismatch = a
         return
       checkConstraint(n.sons[a].sons[1])
       if m.baseTypeMatch:
@@ -2392,6 +2405,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
           else:
             addSon(m.call, copyTree(n.sons[a]))
         elif formal != nil and formal.typ.kind == tyVarargs:
+          m.firstMismatch.kind = kTypeMismatch
           # beware of the side-effects in 'prepareOperand'! So only do it for
           # varargs matching. See tests/metatype/tstatic_overloading.
           m.baseTypeMatch = false
@@ -2408,6 +2422,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
             m.state = csNoMatch
             return
         else:
+          m.firstMismatch.kind = kExtraArg
           m.state = csNoMatch
           return
       else:
@@ -2415,7 +2430,9 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
           internalError(c.config, n.sons[a].info, "matches")
           return
         formal = m.callee.n.sons[f].sym
+        m.firstMismatch.kind = kTypeMismatch
         if containsOrIncl(marker, formal.position) and container.isNil:
+          m.firstMismatch.kind = kAlreadyGiven
           # already in namedParams: (see above remark)
           when false: localError(n.sons[a].info, errCannotBindXTwice, formal.name.s)
           m.state = csNoMatch
@@ -2436,7 +2453,6 @@ proc matchesAux(c: PContext, n, nOrig: PNode,
                                     n.sons[a], nOrig.sons[a])
           if arg == nil:
             m.state = csNoMatch
-            m.firstMismatch = f
             return
           if m.baseTypeMatch:
             assert formal.typ.kind == tyVarargs
@@ -2510,7 +2526,8 @@ proc matches*(c: PContext, n, nOrig: PNode, m: var TCandidate) =
         else:
           # no default value
           m.state = csNoMatch
-          m.firstMismatch = f
+          m.firstMismatch.kind = kMissingParam
+          m.firstMismatch.formal = formal
           break
       else:
         if formal.ast.kind == nkEmpty: