ambiguous identifier resolution (#23123)

fixes #23002, fixes #22841, refs comments in #23097

When an identifier is ambiguous in scope (i.e. multiple imports contain
symbols with the same name), attempt resolving it through type inference
(by creating a symchoice). To do this efficiently, `qualifiedLookUp` had
to be broken up so that `semExpr` can access the ambiguous candidates
directly (now obtained directly via `lookUpCandidates`).

This fixes the linked issues, but an example like:

```nim
let on = 123

{.warning[ProveInit]: on.}
```

will still fail, since `on` is unambiguously the local `let` symbol here
(this is also true for `proc on` but `proc` symbols generate symchoices
anyway).

Type symbols are not considered to not confuse the type inference. This
includes the change in sigmatch, up to this point symchoices with
nonoverloadable symbols could be created, they just wouldn't be
considered during disambiguation. Now every proper symbol except types
are considered in disambiguation, so the correct symbols must be picked
during the creation of the symchoice node. I remember there being a
violating case of this in the compiler, but this was very likely fixed
by excluding type symbols as CI seems to have found no issues.

The pure enum ambiguity test was disabled because ambiguous pure enums
now behave like overloadable enums with this behavior, so we get a
longer error message for `echo amb` like `type mismatch: got <MyEnum |
OtherEnum> but expected T`

compiler/lookups.nim

@@ -303,8 +303,16 @@ proc isAmbiguous*(c: PContext, s: PIdent, filter: TSymKinds, sym: var PSym): boo
     # imports had a candidate but wasn't ambiguous
     return false
 
-proc errorSym*(c: PContext, n: PNode): PSym =
+proc errorSym*(c: PContext, ident: PIdent, info: TLineInfo): PSym =
   ## creates an error symbol to avoid cascading errors (for IDE support)
+  result = newSym(skError, ident, c.idgen, getCurrOwner(c), info, {})
+  result.typ = errorType(c)
+  incl(result.flags, sfDiscardable)
+  # pretend it's from the top level scope to prevent cascading errors:
+  if c.config.cmd != cmdInteractive and c.compilesContextId == 0:
+    c.moduleScope.addSym(result)
+
+proc errorSym*(c: PContext, n: PNode): PSym =
   var m = n
   # ensure that 'considerQuotedIdent' can't fail:
   if m.kind == nkDotExpr: m = m[1]
@@ -312,12 +320,7 @@ proc errorSym*(c: PContext, n: PNode): PSym =
       considerQuotedIdent(c, m)
     else:
       getIdent(c.cache, "err:" & renderTree(m))
-  result = newSym(skError, ident, c.idgen, getCurrOwner(c), n.info, {})
-  result.typ = errorType(c)
-  incl(result.flags, sfDiscardable)
-  # pretend it's from the top level scope to prevent cascading errors:
-  if c.config.cmd != cmdInteractive and c.compilesContextId == 0:
-    c.moduleScope.addSym(result)
+  result = errorSym(c, ident, n.info)
 
 type
   TOverloadIterMode* = enum
@@ -499,7 +502,7 @@ proc mustFixSpelling(c: PContext): bool {.inline.} =
   result = c.config.spellSuggestMax != 0 and c.compilesContextId == 0
     # don't slowdown inside compiles()
 
-proc fixSpelling(c: PContext, n: PNode, ident: PIdent, result: var string) =
+proc fixSpelling(c: PContext, ident: PIdent, result: var string) =
   ## when we cannot find the identifier, suggest nearby spellings
   var list = initHeapQueue[SpellCandidate]()
   let name0 = ident.s.nimIdentNormalize
@@ -558,7 +561,7 @@ proc errorUseQualifier*(c: PContext; info: TLineInfo; s: PSym) =
   var amb: bool
   discard errorUseQualifier(c, info, s, amb)
 
-proc errorUseQualifier(c: PContext; info: TLineInfo; candidates: seq[PSym]; prefix = "use one of") =
+proc errorUseQualifier*(c: PContext; info: TLineInfo; candidates: seq[PSym]; prefix = "use one of") =
   var err = "ambiguous identifier: '" & candidates[0].name.s & "'"
   var i = 0
   for candidate in candidates:
@@ -589,11 +592,11 @@ proc errorUndeclaredIdentifier*(c: PContext; info: TLineInfo; name: string, extr
       c.recursiveDep = ""
   localError(c.config, info, errGenerated, err)
 
-proc errorUndeclaredIdentifierHint*(c: PContext; n: PNode, ident: PIdent): PSym =
+proc errorUndeclaredIdentifierHint*(c: PContext; ident: PIdent; info: TLineInfo): PSym =
   var extra = ""
-  if c.mustFixSpelling: fixSpelling(c, n, ident, extra)
-  errorUndeclaredIdentifier(c, n.info, ident.s, extra)
-  result = errorSym(c, n)
+  if c.mustFixSpelling: fixSpelling(c, ident, extra)
+  errorUndeclaredIdentifier(c, info, ident.s, extra)
+  result = errorSym(c, ident, info)
 
 proc lookUp*(c: PContext, n: PNode): PSym =
   # Looks up a symbol. Generates an error in case of nil.
@@ -601,13 +604,13 @@ proc lookUp*(c: PContext, n: PNode): PSym =
   case n.kind
   of nkIdent:
     result = searchInScopes(c, n.ident, amb)
-    if result == nil: result = errorUndeclaredIdentifierHint(c, n, n.ident)
+    if result == nil: result = errorUndeclaredIdentifierHint(c, n.ident, n.info)
   of nkSym:
     result = n.sym
   of nkAccQuoted:
     var ident = considerQuotedIdent(c, n)
     result = searchInScopes(c, ident, amb)
-    if result == nil: result = errorUndeclaredIdentifierHint(c, n, ident)
+    if result == nil: result = errorUndeclaredIdentifierHint(c, ident, n.info)
   else:
     internalError(c.config, n.info, "lookUp")
     return nil
@@ -621,16 +624,29 @@ type
   TLookupFlag* = enum
     checkAmbiguity, checkUndeclared, checkModule, checkPureEnumFields
 
+const allExceptModule = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage}
+
+proc lookUpCandidates*(c: PContext, ident: PIdent, filter: set[TSymKind]): seq[PSym] =
+  result = searchInScopesFilterBy(c, ident, filter)
+  if result.len == 0:
+    result.add allPureEnumFields(c, ident)
+
 proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
-  const allExceptModule = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage}
   case n.kind
   of nkIdent, nkAccQuoted:
     var amb = false
     var ident = considerQuotedIdent(c, n)
     if checkModule in flags:
       result = searchInScopes(c, ident, amb)
+      if result == nil:
+        let candidates = allPureEnumFields(c, ident)
+        if candidates.len > 0:
+          result = candidates[0]
+          amb = candidates.len > 1
+          if amb and checkAmbiguity in flags:
+            errorUseQualifier(c, n.info, candidates)
     else:
-      let candidates = searchInScopesFilterBy(c, ident, allExceptModule)
+      let candidates = lookUpCandidates(c, ident, allExceptModule)
       if candidates.len > 0:
         result = candidates[0]
         amb = candidates.len > 1
@@ -638,16 +654,8 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
           errorUseQualifier(c, n.info, candidates)
       else:
         result = nil
-    if result == nil:
-      let candidates = allPureEnumFields(c, ident)
-      if candidates.len > 0:
-        result = candidates[0]
-        amb = candidates.len > 1
-        if amb and checkAmbiguity in flags:
-          errorUseQualifier(c, n.info, candidates)
-
     if result == nil and checkUndeclared in flags:
-      result = errorUndeclaredIdentifierHint(c, n, ident)
+      result = errorUndeclaredIdentifierHint(c, ident, n.info)
     elif checkAmbiguity in flags and result != nil and amb:
       result = errorUseQualifier(c, n.info, result, amb)
     c.isAmbiguous = amb
@@ -672,12 +680,12 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
           else:
             result = someSym(c.graph, m, ident)
         if result == nil and checkUndeclared in flags:
-          result = errorUndeclaredIdentifierHint(c, n[1], ident)
+          result = errorUndeclaredIdentifierHint(c, ident, n[1].info)
       elif n[1].kind == nkSym:
         result = n[1].sym
         if result.owner != nil and result.owner != m and checkUndeclared in flags:
           # dotExpr in templates can end up here
-          result = errorUndeclaredIdentifierHint(c, n[1], considerQuotedIdent(c, n[1]))
+          result = errorUndeclaredIdentifierHint(c, result.name, n[1].info)
       elif checkUndeclared in flags and
            n[1].kind notin {nkOpenSymChoice, nkClosedSymChoice}:
         localError(c.config, n[1].info, "identifier expected, but got: " &

compiler/semexprs.nim

@@ -131,11 +131,13 @@ proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode
 proc isSymChoice(n: PNode): bool {.inline.} =
   result = n.kind in nkSymChoices
 
-proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
-  result = n
+proc resolveSymChoice(c: PContext, n: var PNode, flags: TExprFlags = {}, expectedType: PType = nil) =
+  ## Attempts to resolve a symchoice `n`, `n` remains a symchoice if
+  ## it cannot be resolved (this is the case even when `n.len == 1`).
   if expectedType != nil:
-    result = fitNode(c, expectedType, result, n.info)
-  if isSymChoice(result) and efAllowSymChoice notin flags:
+    # resolve from type inference, see paramTypesMatch
+    n = fitNode(c, expectedType, n, n.info)
+  if isSymChoice(n) and efAllowSymChoice notin flags:
     # some contexts might want sym choices preserved for later disambiguation
     # in general though they are ambiguous
     let first = n[0].sym
@@ -145,17 +147,24 @@ proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: P
         foundSym == first:
       # choose the first resolved enum field, i.e. the latest in scope
       # to mirror behavior before overloadable enums
-      result = n[0]
-    else:
-      var err = "ambiguous identifier '" & first.name.s &
-        "' -- use one of the following:\n"
-      for child in n:
-        let candidate = child.sym
-        err.add "  " & candidate.owner.name.s & "." & candidate.name.s
-        err.add ": " & typeToString(candidate.typ) & "\n"
-      localError(c.config, n.info, err)
-      n.typ = errorType(c)
-      result = n
+      n = n[0]
+
+proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
+  result = n
+  resolveSymChoice(c, result, flags, expectedType)
+  if isSymChoice(result) and result.len == 1:
+    # resolveSymChoice can leave 1 sym
+    result = result[0]
+  if isSymChoice(result) and efAllowSymChoice notin flags:
+    var err = "ambiguous identifier: '" & result[0].sym.name.s &
+      "' -- use one of the following:\n"
+    for child in n:
+      let candidate = child.sym
+      err.add "  " & candidate.owner.name.s & "." & candidate.name.s
+      err.add ": " & typeToString(candidate.typ) & "\n"
+    localError(c.config, n.info, err)
+    n.typ = errorType(c)
+    result = n
   if result.kind == nkSym:
     result = semSym(c, result, result.sym, flags)
 
@@ -2989,6 +2998,55 @@ proc semPragmaStmt(c: PContext; n: PNode) =
   else:
     pragma(c, c.p.owner, n, stmtPragmas, true)
 
+proc resolveIdentToSym(c: PContext, n: PNode, resultNode: var PNode,
+                       flags: TExprFlags, expectedType: PType): PSym =
+  # result is nil on error or if a node that can't produce a sym is resolved
+  let ident = considerQuotedIdent(c, n)
+  if expectedType != nil and (
+      let expected = expectedType.skipTypes(abstractRange-{tyDistinct});
+      expected.kind == tyEnum):
+    let nameId = ident.id
+    for f in expected.n:
+      if f.kind == nkSym and f.sym.name.id == nameId:
+        return f.sym
+  var filter = {low(TSymKind)..high(TSymKind)}
+  if efNoEvaluateGeneric in flags:
+    # `a[...]` where `a` is a module or package is not possible
+    filter.excl {skModule, skPackage}
+  let candidates = lookUpCandidates(c, ident, filter)
+  if candidates.len == 0:
+    result = errorUndeclaredIdentifierHint(c, ident, n.info)
+  elif candidates.len == 1 or {efNoEvaluateGeneric, efInCall} * flags != {}:
+    # unambiguous, or we don't care about ambiguity
+    result = candidates[0]
+  else:
+    # ambiguous symbols have 1 last chance as a symchoice,
+    # but type symbols cannot participate in symchoices
+    var choice = newNodeIT(nkClosedSymChoice, n.info, newTypeS(tyNone, c))
+    for c in candidates:
+      if c.kind notin {skType, skModule, skPackage}:
+        choice.add newSymNode(c, n.info)
+    if choice.len == 0:
+      # we know candidates.len > 1, we just couldn't put any in a symchoice
+      errorUseQualifier(c, n.info, candidates)
+      return nil
+    resolveSymChoice(c, choice, flags, expectedType)
+    # choice.len == 1 can be true here but as long as it's a symchoice
+    # it's still not resolved
+    if isSymChoice(choice):
+      result = nil
+      if efAllowSymChoice in flags:
+        resultNode = choice
+      else:
+        errorUseQualifier(c, n.info, candidates)
+    else:
+      if choice.kind == nkSym:
+        result = choice.sym
+      else:
+        # resolution could have generated nkHiddenStdConv etc
+        resultNode = semExpr(c, choice, flags, expectedType)
+        result = nil
+
 proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
   when defined(nimCompilerStacktraceHints):
     setFrameMsg c.config$n.info & " " & $n.kind
@@ -3026,25 +3084,10 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
   if nfSem in n.flags: return
   case n.kind
   of nkIdent, nkAccQuoted:
-    var s: PSym = nil
-    if expectedType != nil and (
-        let expected = expectedType.skipTypes(abstractRange-{tyDistinct});
-        expected.kind == tyEnum):
-      let nameId = considerQuotedIdent(c, n).id
-      for f in expected.n:
-        if f.kind == nkSym and f.sym.name.id == nameId:
-          s = f.sym
-          break
+    let s = resolveIdentToSym(c, n, result, flags, expectedType)
     if s == nil:
-      let checks = if efNoEvaluateGeneric in flags:
-          {checkUndeclared, checkPureEnumFields}
-        elif efInCall in flags:
-          {checkUndeclared, checkModule, checkPureEnumFields}
-        else:
-          {checkUndeclared, checkModule, checkAmbiguity, checkPureEnumFields}
-      s = qualifiedLookUp(c, n, checks)
-      if s == nil:
-        return
+      # resolveIdentToSym either errored or gave a result node
+      return
     if c.matchedConcept == nil: semCaptureSym(s, c.p.owner)
     case s.kind
     of skProc, skFunc, skMethod, skConverter, skIterator:

compiler/sigmatch.nim

@@ -2338,8 +2338,8 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType,
   if arg == nil or arg.kind notin nkSymChoices:
     result = paramTypesMatchAux(m, f, a, arg, argOrig)
   else:
-    let matchSet = {skProc, skFunc, skMethod, skConverter,skIterator, skMacro,
-                    skTemplate, skEnumField}
+    # symbol kinds that don't participate in symchoice type disambiguation:
+    let matchSet = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage, skType}
     
     var best = -1
     result = arg

tests/enum/tambiguousoverloads.nim

@@ -9,7 +9,7 @@ block: # bug #21887
     EnumC = enum C
 
   doAssert typeof(EnumC(A)) is EnumC #[tt.Error
-                        ^ ambiguous identifier 'A' -- use one of the following:
+                        ^ ambiguous identifier: 'A' -- use one of the following:
   EnumA.A: EnumA
   EnumB.A: EnumB]#
 
@@ -21,6 +21,6 @@ block: # issue #22598
       red
 
   let a = red #[tt.Error
-          ^ ambiguous identifier 'red' -- use one of the following:
+          ^ ambiguous identifier: 'red' -- use one of the following:
   A.red: A
   B.red: B]#

tests/enum/tpure_enums_conflict.nim

@@ -1,4 +1,5 @@
 discard """
+  disabled: true # pure enums behave like overloaded enums on ambiguity now which gives a different error message
   errormsg: "ambiguous identifier: 'amb'"
   line: 19
 """

tests/errmsgs/t8064.nim

@@ -5,5 +5,5 @@ values
 
 discard """
   # either this or "expression has no type":
-  errormsg: "ambiguous identifier 'values' -- use one of the following:"
+  errormsg: "ambiguous identifier: 'values' -- use one of the following:"
 """

tests/lookups/tambiguousemit.nim

@@ -7,6 +7,6 @@ proc foo(x: int) = discard
 proc foo(x: float) = discard
 
 {.emit: ["// ", foo].} #[tt.Error
-                ^ ambiguous identifier 'foo' -- use one of the following:
+                ^ ambiguous identifier: 'foo' -- use one of the following:
   tambiguousemit.foo: proc (x: int){.noSideEffect, gcsafe.}
   tambiguousemit.foo: proc (x: float){.noSideEffect, gcsafe.}]#

tests/lookups/tambprocvar.nim

@@ -2,7 +2,7 @@ discard """
   action: reject
   cmd: "nim check $file"
   nimout: '''
-tambprocvar.nim(15, 11) Error: ambiguous identifier 'foo' -- use one of the following:
+tambprocvar.nim(15, 11) Error: ambiguous identifier: 'foo' -- use one of the following:
   tambprocvar.foo: proc (x: int){.noSideEffect, gcsafe.}
   tambprocvar.foo: proc (x: float){.noSideEffect, gcsafe.}
 '''
@@ -16,4 +16,4 @@ block:
 
 block:
   let x = `+` #[tt.Error
-           ^ ambiguous identifier '+' -- use one of the following:]#
+          ^ ambiguous identifier: '+' -- use one of the following:]#

tests/lookups/tambsym3.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "ambiguous identifier 'mDec' -- use one of the following:"
+  errormsg: "ambiguous identifier: 'mDec' -- use one of the following:"
   file: "tambsym3.nim"
   line: 11
 """

tests/macros/t23032_2.nim

@@ -1,6 +1,6 @@
 discard """
   action: "reject"
-  errormsg: "ambiguous identifier '%*'"
+  errormsg: "ambiguous identifier: '%*'"
 """
 import std/macros
 

tests/pragmas/monoff1.nim

@@ -0,0 +1 @@
+proc on*() = discard

tests/pragmas/tonoff1.nim

@@ -0,0 +1,8 @@
+# issue #23002
+
+import monoff1
+
+proc test() =
+  {.warning[ProveInit]: on.}
+
+test()

tests/pragmas/tonoff2.nim

@@ -0,0 +1,14 @@
+discard """
+    action: compile
+"""
+
+# issue #22841
+
+import unittest
+
+proc on() =
+    discard
+
+suite "some suite":
+    test "some test":
+        discard