generate symchoice for ambiguous types in templates & generics + hand…

…le types in symchoices (#23997)

fixes #23898, supersedes #23966 and #23990

Since #20631 ambiguous type symbols in templates are rejected outright,
now we generate a symchoice for type nodes if they're ambiguous, a
generalization of what was done in #22375. This is done for generics as
well. Symchoices also handle type symbols better now, ensuring their
type is a `typedesc` type; this probably isn't necessary for everything
to work but it makes the logic more robust.

Similar to #23989, we have to prepare for the fact that ambiguous type
symbols behave differently than normal type symbols and either error
normally or relegate to other routine symbols if the symbol is being
called. Generating a symchoice emulates this behavior, `semExpr` will
find the type symbol first, but since the symchoice has other symbols,
it will count as an ambiguous type symbol.

I know it seems spammy to carry around an ambiguity flag everywhere, but
in the future when we have something like #23104 we could just always
generate a symchoice, and the symchoice itself would carry the info of
whether the first symbol was ambiguous. But this could harm compiler
performance/memory use, it might be better to generate it only when we
have to, which in the case of type symbols is only when they're
ambiguous.

(cherry picked from commit 09dcff7)

changelog.md

@@ -7,6 +7,13 @@
 - The default user-agent in `std/httpclient` has been changed to `Nim-httpclient/<version>` instead of `Nim httpclient/<version>` which was incorrect according to the HTTP spec.
 - With `-d:nimPreviewNonVarDestructor`, non-var destructors become the default.
 
+- Ambiguous type symbols in generic procs and templates now generate symchoice nodes.
+  Previously; in templates they would error immediately at the template definition,
+  and in generic procs a type symbol would arbitrarily be captured, losing the
+  information of the other symbols. This means that generic code can now give
+  errors for ambiguous type symbols, and macros operating on generic proc AST
+  may encounter symchoice nodes instead of the arbitrarily resolved type symbol nodes.
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"

compiler/semexprs.nim

@@ -3124,12 +3124,16 @@ proc resolveIdentToSym(c: PContext, n: PNode, resultNode: var PNode,
     # 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
+    # ambiguous symbols have 1 last chance as a symchoice
     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)
+    for cand in candidates:
+      case cand.kind
+      of skModule, skPackage:
+        discard
+      of skType:
+        choice.add newSymNodeTypeDesc(cand, c.idgen, n.info)
+      else:
+        choice.add newSymNode(cand, 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)

compiler/semgnrc.nim

@@ -64,6 +64,7 @@ proc newOpenSym*(n: PNode): PNode {.inline.} =
 
 proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
                           ctx: var GenericCtx; flags: TSemGenericFlags,
+                          isAmbiguous: bool,
                           fromDotExpr=false): PNode =
   semIdeForTemplateOrGenericCheck(c.config, n, ctx.cursorInBody)
   incl(s.flags, sfUsed)
@@ -132,6 +133,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
   of skType:
     if (s.typ != nil) and
        (s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}):
+      if isAmbiguous:
+        # ambiguous types should be symchoices since lookup behaves
+        # differently for them in regular expressions
+        maybeDotChoice(c, n, s, fromDotExpr)
+        return
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
         if genericsOpenSym in c.features:
@@ -183,7 +189,7 @@ proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags,
     elif s.isMixedIn:
       result = symChoice(c, n, s, scForceOpen)
     else:
-      result = semGenericStmtSymbol(c, n, s, ctx, flags)
+      result = semGenericStmtSymbol(c, n, s, ctx, flags, amb)
   # else: leave as nkIdent
 
 proc newDot(n, b: PNode): PNode =
@@ -199,10 +205,11 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags,
 
   let luf = if withinMixin notin flags: {checkUndeclared, checkModule} else: {checkModule}
 
+  c.isAmbiguous = false
   var s = qualifiedLookUp(c, n, luf)
   if s != nil:
     isMacro = s.kind in {skTemplate, skMacro}
-    result = semGenericStmtSymbol(c, n, s, ctx, flags)
+    result = semGenericStmtSymbol(c, n, s, ctx, flags, c.isAmbiguous)
   else:
     n[0] = semGenericStmt(c, n[0], flags, ctx)
     result = n
@@ -216,24 +223,21 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags,
       isMacro = s.kind in {skTemplate, skMacro}
       if withinBind in flags or s.id in ctx.toBind:
         if s.kind == skType: # don't put types in sym choice
-          result = newDot(result, semGenericStmtSymbol(c, n, s, ctx, flags, fromDotExpr=true))
+          var ambig = false
+          if candidates.len > 1:
+            let s2 = searchInScopes(c, ident, ambig)
+          result = newDot(result, semGenericStmtSymbol(c, n, s, ctx, flags,
+            isAmbiguous = ambig, fromDotExpr = true))
         else:
           result = newDot(result, symChoice(c, n, s, scClosed))
       elif s.isMixedIn:
         result = newDot(result, symChoice(c, n, s, scForceOpen))
       else:
+        var ambig = false
         if s.kind == skType and candidates.len > 1:
-          var ambig = false
-          let s2 = searchInScopes(c, ident, ambig) 
-          if ambig:
-            # this is a type conversion like a.T where T is ambiguous with
-            # other types or routines
-            # in regular code, this never considers a type conversion and
-            # skips to routine overloading
-            # so symchoices are used which behave similarly with type symbols
-            result = newDot(result, symChoice(c, n, s, scForceOpen))
-            return
-        let syms = semGenericStmtSymbol(c, n, s, ctx, flags, fromDotExpr=true)
+          discard searchInScopes(c, ident, ambig)
+        let syms = semGenericStmtSymbol(c, n, s, ctx, flags,
+          isAmbiguous = ambig, fromDotExpr = true)
         result = newDot(result, syms)
 
 proc addTempDecl(c: PContext; n: PNode; kind: TSymKind) =
@@ -300,7 +304,9 @@ proc semGenericStmt(c: PContext, n: PNode,
     # check if it is an expression macro:
     checkMinSonsLen(n, 1, c.config)
     let fn = n[0]
+    c.isAmbiguous = false
     var s = qualifiedLookUp(c, fn, {})
+    let ambig = c.isAmbiguous
     if s == nil and
         {withinMixin, withinConcept}*flags == {} and
         fn.kind in {nkIdent, nkAccQuoted} and
@@ -353,7 +359,12 @@ proc semGenericStmt(c: PContext, n: PNode,
       of skType:
         # bad hack for generics:
         if (s.typ != nil) and (s.typ.kind != tyGenericParam):
-          result[0] = newSymNodeTypeDesc(s, c.idgen, fn.info)
+          if ambig:
+            # ambiguous types should be symchoices since lookup behaves
+            # differently for them in regular expressions
+            result[0] = sc
+          else:
+            result[0] = newSymNodeTypeDesc(s, c.idgen, fn.info)
           onUse(fn.info, s)
           first = 1
       else:

compiler/semtempl.nim

@@ -217,7 +217,7 @@ proc addLocalDecl(c: var TemplCtx, n: var PNode, k: TSymKind) =
         if k == skParam and c.inTemplateHeader > 0:
           local.flags.incl sfTemplateParam
 
-proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode =
+proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField, isAmbiguous: bool): PNode =
   incl(s.flags, sfUsed)
   # bug #12885; ideally sem'checking is performed again afterwards marking
   # the symbol as used properly, but the nfSem mechanism currently prevents
@@ -244,6 +244,10 @@ proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode =
     result = n
   of skType:
     if isField and sfGenSym in s.flags: result = n
+    elif isAmbiguous:
+      # ambiguous types should be symchoices since lookup behaves
+      # differently for them in regular expressions
+      result = symChoice(c, n, s, scOpen, isField)
     else: result = newSymNodeTypeDesc(s, c.idgen, n.info)
   else:
     if isField and sfGenSym in s.flags: result = n
@@ -344,6 +348,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
   case n.kind
   of nkIdent:
     if n.ident.id in c.toInject: return n
+    c.c.isAmbiguous = false
     let s = qualifiedLookUp(c.c, n, {})
     if s != nil:
       if s.owner == c.owner and s.kind == skParam and sfTemplateParam in s.flags:
@@ -361,9 +366,9 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
         result = newSymNode(s, n.info)
         onUse(n.info, s)
       else:
-        if s.kind in {skType, skVar, skLet, skConst}:
+        if s.kind in {skVar, skLet, skConst}:
           discard qualifiedLookUp(c.c, n, {checkAmbiguity, checkModule})
-        result = semTemplSymbol(c.c, n, s, c.noGenSym > 0)
+        result = semTemplSymbol(c.c, n, s, c.noGenSym > 0, c.c.isAmbiguous)
   of nkBind:
     result = semTemplBody(c, n[0])
   of nkBindStmt:
@@ -557,6 +562,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
   of nkDotExpr, nkAccQuoted:
     # dotExpr is ambiguous: note that we explicitly allow 'x.TemplateParam',
     # so we use the generic code for nkDotExpr too
+    c.c.isAmbiguous = false
     let s = qualifiedLookUp(c.c, n, {})
     if s != nil:
       # mirror the nkIdent case
@@ -571,9 +577,9 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
       elif contains(c.toMixin, s.name.id):
         return symChoice(c.c, n, s, scForceOpen, c.noGenSym > 0)
       else:
-        if s.kind in {skType, skVar, skLet, skConst}:
+        if s.kind in {skVar, skLet, skConst}:
           discard qualifiedLookUp(c.c, n, {checkAmbiguity, checkModule})
-        return semTemplSymbol(c.c, n, s, c.noGenSym > 0)
+        return semTemplSymbol(c.c, n, s, c.noGenSym > 0, c.c.isAmbiguous)
     if n.kind == nkDotExpr:
       result = n
       result[0] = semTemplBody(c, n[0])

compiler/sigmatch.nim

@@ -2364,8 +2364,8 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType,
     result = paramTypesMatchAux(m, f, a, arg, argOrig)
   else:
     # symbol kinds that don't participate in symchoice type disambiguation:
-    let matchSet = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage, skType}
-    
+    let matchSet = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage}
+
     var best = -1
     result = arg
 
@@ -2404,6 +2404,12 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType,
         if arg[i].sym.kind in matchSet:
           copyCandidate(z, m)
           z.callee = arg[i].typ
+          if arg[i].sym.kind == skType and z.callee.kind != tyTypeDesc:
+            # creating the symchoice with the type sym having typedesc type
+            # breaks a lot of stuff, so we make the typedesc type here
+            # mirrored from `newSymNodeTypeDesc`
+            z.callee = newType(tyTypeDesc, nextTypeId(c.idgen), arg[i].sym.owner)
+            z.callee.addSonSkipIntLit(arg[i].sym.typ, c.idgen)
           if tfUnresolved in z.callee.flags: continue
           z.calleeSym = arg[i].sym
           z.calleeScope = cmpScopes(m.c, arg[i].sym)

lib/pure/ioselects/ioselectors_select.nim

@@ -314,7 +314,7 @@ proc selectInto*[T](s: Selector[T], timeout: int,
 
   if timeout != -1:
     when defined(genode) or defined(freertos) or defined(zephyr) or defined(nuttx):
-      tv.tv_sec = Time(timeout div 1_000)
+      tv.tv_sec = posix.Time(timeout div 1_000)
     else:
       tv.tv_sec = timeout.int32 div 1_000
     tv.tv_usec = (timeout.int32 %% 1_000) * 1_000

tests/lookups/mambtype1.nim

@@ -0,0 +1 @@
+type K* = object

tests/lookups/mambtype2.nim

@@ -0,0 +1,4 @@
+import ./mambtype1
+export mambtype1
+template K*(kind: static int): auto = typedesc[mambtype1.K]
+template B*(kind: static int): auto = typedesc[mambtype1.K]

tests/lookups/tambtype.nim

@@ -0,0 +1,20 @@
+import ./mambtype2
+
+block: # issue #23893
+  discard default(K(0))       # works
+  discard default(mambtype2.B(0))     # works
+  discard default(mambtype2.K(0))     # doesn't work
+
+block: # issue #23898, in template
+  template r() =
+    discard default(B(0))     # compiles
+    discard default(mambtype2.B(0))   # compiles
+    discard default(K(0))     # does not compile
+  r()
+
+block: # in generics
+  proc foo[T]() =
+    discard default(B(0))     # compiles
+    discard default(mambtype2.B(0))   # compiles
+    discard default(K(0))     # does not compile
+  foo[int]()