From e38cbd3c846a8e95bed0d709e4d61cc1ad5279b2 Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Tue, 20 Aug 2024 22:32:35 +0300
Subject: [PATCH] consider ambiguity for qualified symbols (#23989)

fixes #23893

When type symbols are ambiguous, calls to them aren't allowed to be type
conversions and only routine symbols are considered instead. But the
compiler doesn't acknowledge that qualified symbols can be ambiguous,
`qualifiedLookUp` directly tries to access the identifier from the
module string table. Now it checks the relevant symbol iterators for any
symbol after the first received symbol, in which case the symbol is
considered ambiguous. `nkDotExpr` is also included in the whitelist of
node kinds for ambiguous type symbols (not entirely sure why this
exists, it's missing `nkAccQuoted` as well).
---
 compiler/lookups.nim             | 11 ++++++++---
 compiler/modulegraphs.nim        | 19 +++++++++++++++++++
 compiler/semexprs.nim            |  9 +++++----
 tests/lookups/mqualifiedamb1.nim |  1 +
 tests/lookups/mqualifiedamb2.nim |  4 ++++
 tests/lookups/tqualifiedamb.nim  |  4 ++++
 6 files changed, 41 insertions(+), 7 deletions(-)
 create mode 100644 tests/lookups/mqualifiedamb1.nim
 create mode 100644 tests/lookups/mqualifiedamb2.nim
 create mode 100644 tests/lookups/tqualifiedamb.nim

diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index 75915383003b3..c6940a4dcbd59 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -679,13 +679,18 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
         ident = considerQuotedIdent(c, n[1])
       if ident != nil:
         if m == c.module:
-          result = strTableGet(c.topLevelScope.symbols, ident)
+          var ti: TIdentIter = default(TIdentIter)
+          result = initIdentIter(ti, c.topLevelScope.symbols, ident)
+          if result != nil and nextIdentIter(ti, c.topLevelScope.symbols) != nil:
+            # another symbol exists with same name
+            c.isAmbiguous = true
         else:
+          var amb: bool = false
           if c.importModuleLookup.getOrDefault(m.name.id).len > 1:
-            var amb: bool = false
             result = errorUseQualifier(c, n.info, m, amb)
           else:
-            result = someSym(c.graph, m, ident)
+            result = someSymAmb(c.graph, m, ident, amb)
+            if amb: c.isAmbiguous = true
         if result == nil and checkUndeclared in flags:
           result = errorUndeclaredIdentifierHint(c, ident, n[1].info)
       elif n[1].kind == nkSym:
diff --git a/compiler/modulegraphs.nim b/compiler/modulegraphs.nim
index 749cb0cc49c19..77762d23aa1bf 100644
--- a/compiler/modulegraphs.nim
+++ b/compiler/modulegraphs.nim
@@ -274,6 +274,25 @@ proc someSym*(g: ModuleGraph; m: PSym; name: PIdent): PSym =
   else:
     result = strTableGet(g.ifaces[m.position].interfSelect(importHidden), name)
 
+proc someSymAmb*(g: ModuleGraph; m: PSym; name: PIdent; amb: var bool): PSym =
+  let importHidden = optImportHidden in m.options
+  if isCachedModule(g, m):
+    result = nil
+    for s in interfaceSymbols(g.config, g.cache, g.packed, FileIndex(m.position), name, importHidden):
+      if result == nil:
+        # set result to the first symbol
+        result = s
+      else:
+        # another symbol found
+        amb = true
+        break
+  else:
+    var ti: TIdentIter = default(TIdentIter)
+    result = initIdentIter(ti, g.ifaces[m.position].interfSelect(importHidden), name)
+    if result != nil and nextIdentIter(ti, g.ifaces[m.position].interfSelect(importHidden)) != nil:
+      # another symbol exists with same name
+      amb = true
+
 proc systemModuleSym*(g: ModuleGraph; name: PIdent): PSym =
   result = someSym(g, g.systemModule, name)
 
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index bc66ba3d50706..9293f8497b150 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -3321,12 +3321,13 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
       of skType:
         # XXX think about this more (``set`` procs)
         let ambig = c.isAmbiguous
-        if not (n[0].kind in {nkClosedSymChoice, nkOpenSymChoice, nkIdent} and ambig) and n.len == 2:
+        if not (n[0].kind in nkSymChoices + {nkIdent, nkDotExpr} and ambig) and n.len == 2:
           result = semConv(c, n, flags, expectedType)
-        elif ambig and n.len == 1:
-          errorUseQualifier(c, n.info, s)
         elif n.len == 1:
-          result = semObjConstr(c, n, flags, expectedType)
+          if ambig:
+            errorUseQualifier(c, n.info, s)
+          else:
+            result = semObjConstr(c, n, flags, expectedType)
         elif s.magic == mNone: result = semDirectOp(c, n, flags, expectedType)
         else: result = semMagic(c, n, s, flags, expectedType)
       of skProc, skFunc, skMethod, skConverter, skIterator:
diff --git a/tests/lookups/mqualifiedamb1.nim b/tests/lookups/mqualifiedamb1.nim
new file mode 100644
index 0000000000000..47046142e397f
--- /dev/null
+++ b/tests/lookups/mqualifiedamb1.nim
@@ -0,0 +1 @@
+type K* = object
diff --git a/tests/lookups/mqualifiedamb2.nim b/tests/lookups/mqualifiedamb2.nim
new file mode 100644
index 0000000000000..3ea5bd04fd6aa
--- /dev/null
+++ b/tests/lookups/mqualifiedamb2.nim
@@ -0,0 +1,4 @@
+import ./mqualifiedamb1
+export mqualifiedamb1
+template K*(kind: static int): auto = typedesc[mqualifiedamb1.K]
+template B*(kind: static int): auto = typedesc[mqualifiedamb1.K]
diff --git a/tests/lookups/tqualifiedamb.nim b/tests/lookups/tqualifiedamb.nim
new file mode 100644
index 0000000000000..a5e1955f3e997
--- /dev/null
+++ b/tests/lookups/tqualifiedamb.nim
@@ -0,0 +1,4 @@
+import ./mqualifiedamb2
+discard default(K(0))       # works
+discard default(mqualifiedamb2.B(0))     # works
+discard default(mqualifiedamb2.K(0))     # doesn't work