sem all call nodes in generic type bodies + many required fixes (#23983)

fixes #23406, closes #23854, closes #23855 (test code of both compiles
but separate issue exists), refs #23432, follows #23411

In generic bodies, previously all regular `nkCall` nodes like `foo(a,
b)` were directly treated as generic statements and delayed immediately,
but other call kinds like `a.foo(b)`, `foo a, b` etc underwent
typechecking before making sure they have to be delayed, as implemented
in #22029. Since the behavior for `nkCall` was slightly buggy (as in
#23406), the behavior for all call kinds is now to call `semTypeExpr`.

However the vast majority of calls in generic bodies out there are
`nkCall`, and while there isn't a difference in the expected behavior,
this exposes many issues with the implementation started in #22029 given
how much more code uses it now. The portion of these issues that CI has
caught are fixed in this PR but it's possible there are more.

1. Deref expressions, dot expressions and calls to dot expressions now
handle and propagate `tyFromExpr`. This is most of the changes in
`semexprs`.
2. For deref expressions to work in `typeof`, a new type flag
`tfNonConstExpr` is added for `tyFromExpr` that calls `semExprWithType`
with `efInTypeof` on the expression instead of `semConstExpr`. This type
flag is set for every `tyFromExpr` type of a node that `prepareNode`
encounters, so that the node itself isn't evaluated at compile time when
just trying to get the type of the node.
3. Unresolved `static` types matching `static` parameters is now treated
the same as unresolved generic types matching `typedesc` parameters in
generic type bodies, it causes a failed match which delays the call
instantiation.
4. `typedesc` parameters now reject all types containing unresolved
generic types like `seq[T]`, not just generic param types by themselves.
(using `containsGenericType`)
5. `semgnrc` now doesn't leave generic param symbols it encounters in
generic type contexts as just identifiers, and instead turns them into
symbol nodes. Normally in generic procs, this isn't a problem since the
generic param symbols will be provided again at instantiation time (and
in fact creating symbol nodes causes issues since `seminst` doesn't
actually instantiate proc body node types).
But generic types can try to be instantiated early in `sigmatch` which
will give an undeclared identifier error when the param is not provided.
Nodes in generic types (specifically in `tyFromExpr` which should be the
only use for `semGenericStmt`) undergo full generic type instantiation
with `prepareNode`, so there is no issue of these symbols remaining as
uninstantiated generic types.
6. `prepareNode` now has more logic for which nodes to avoid
instantiating.
Subscripts and subscripts turned into calls to `[]` by `semgnrc` need to
avoid instantiating the first operand, since it may be a generic body
type like `Generic` in an expression like `Generic[int]`.
Dot expressions cannot instantiate their RHS as it may be a generic proc
symbol or even an undeclared identifier for generic param fields, but
have to instantiate their LHS, so calls and subscripts need to still
instantiate their first node if it's a dot expression.
This logic still isn't perfect and needs the same level of detail as in
`semexprs` for which nodes can be left as "untyped" for overloading/dot
exprs/subscripts to handle, but should handle the majority of cases.

Also the `efDetermineType` requirement for which calls become
`tyFromExpr` is removed and as a result `efDetermineType` is entirely
unused again.

7 files changed, 125 insertions, 31 deletions

diff --git a/compiler/ast.nim b/compiler/ast.nim
index 648bc4392..d0d4f8db3 100644
--- a/compiler/ast.nim
+++ b/compiler/ast.nim
@@ -448,6 +448,8 @@ const
   tfGcSafe* = tfThread
   tfObjHasKids* = tfEnumHasHoles
   tfReturnsNew* = tfInheritable
+  tfNonConstExpr* = tfExplicitCallConv
+    ## tyFromExpr where the expression shouldn't be evaluated as a static value
   skError* = skUnknown
 
 var
diff --git a/compiler/semcall.nim b/compiler/semcall.nim
index a136cf4fe..3981d256f 100644
--- a/compiler/semcall.nim
+++ b/compiler/semcall.nim
@@ -751,7 +751,7 @@ proc semOverloadedCall(c: PContext, n, nOrig: PNode,
               candidates)
     result = semResolvedCall(c, r, n, flags, expectedType)
   else:
-    if efDetermineType in flags and c.inGenericContext > 0 and c.matchedConcept == nil:
+    if c.inGenericContext > 0 and c.matchedConcept == nil:
       result = semGenericStmt(c, n)
       result.typ = makeTypeFromExpr(c, result.copyTree)
     elif efExplain notin flags:
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index e812aa1f6..bc66ba3d5 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -1028,7 +1028,7 @@ proc semOverloadedCallAnalyseEffects(c: PContext, n: PNode, nOrig: PNode,
 
   if result != nil:
     if result[0].kind != nkSym:
-      if not (efDetermineType in flags and c.inGenericContext > 0):
+      if not (c.inGenericContext > 0): # see generic context check in semOverloadedCall
         internalError(c.config, "semOverloadedCallAnalyseEffects")
       return
     let callee = result[0].sym
@@ -1137,6 +1137,11 @@ proc semIndirectOp(c: PContext, n: PNode, flags: TExprFlags; expectedType: PType
       result.flags.incl nfExplicitCall
       for i in 1..<n.len: result.add n[i]
       return semExpr(c, result, flags, expectedType)
+    elif n0.typ.kind == tyFromExpr and c.inGenericContext > 0:
+      # don't make assumptions, entire expression needs to be tyFromExpr
+      result = semGenericStmt(c, n)
+      result.typ = makeTypeFromExpr(c, result.copyTree)
+      return
     else:
       n[0] = n0
   else:
@@ -1480,17 +1485,17 @@ proc tryReadingGenericParam(c: PContext, n: PNode, i: PIdent, t: PType): PNode =
   of tyTypeParamsHolders:
     result = readTypeParameter(c, t, i, n.info)
     if result == c.graph.emptyNode:
-      result = n
-      n.typ = makeTypeFromExpr(c, n.copyTree)
+      result = semGenericStmt(c, n)
+      result.typ = makeTypeFromExpr(c, result.copyTree)
   of tyUserTypeClasses:
     if t.isResolvedUserTypeClass:
       result = readTypeParameter(c, t, i, n.info)
     else:
-      n.typ = makeTypeFromExpr(c, copyTree(n))
-      result = n
-  of tyGenericParam, tyAnything:
-    n.typ = makeTypeFromExpr(c, copyTree(n))
-    result = n
+      result = semGenericStmt(c, n)
+      result.typ = makeTypeFromExpr(c, copyTree(result))
+  of tyFromExpr, tyGenericParam, tyAnything:
+    result = semGenericStmt(c, n)
+    result.typ = makeTypeFromExpr(c, copyTree(result))
   else:
     result = nil
 
@@ -1654,18 +1659,20 @@ proc buildOverloadedSubscripts(n: PNode, ident: PIdent): PNode =
   result.add(newIdentNode(ident, n.info))
   for s in n: result.add s
 
-proc semDeref(c: PContext, n: PNode): PNode =
+proc semDeref(c: PContext, n: PNode, flags: TExprFlags): PNode =
   checkSonsLen(n, 1, c.config)
   n[0] = semExprWithType(c, n[0])
   let a = getConstExpr(c.module, n[0], c.idgen, c.graph)
   if a != nil:
-    if a.kind == nkNilLit:
+    if a.kind == nkNilLit and efInTypeof notin flags:
       localError(c.config, n.info, "nil dereference is not allowed")
     n[0] = a
   result = n
   var t = skipTypes(n[0].typ, {tyGenericInst, tyVar, tyLent, tyAlias, tySink, tyOwned})
   case t.kind
   of tyRef, tyPtr: n.typ = t.elementType
+  of tyMetaTypes, tyFromExpr:
+    n.typ = makeTypeFromExpr(c, n.copyTree)
   else: result = nil
   #GlobalError(n[0].info, errCircumNeedsPointer)
 
@@ -1696,8 +1703,11 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
   ## checking of assignments
   result = nil
   if n.len == 1:
-    let x = semDeref(c, n)
+    let x = semDeref(c, n, flags)
     if x == nil: return nil
+    if x.typ.kind == tyFromExpr:
+      # depends on generic type
+      return x
     result = newNodeIT(nkDerefExpr, x.info, x.typ)
     result.add(x[0])
     return
@@ -3394,7 +3404,7 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
     result = semArrayConstr(c, n, flags, expectedType)
   of nkObjConstr: result = semObjConstr(c, n, flags, expectedType)
   of nkLambdaKinds: result = semProcAux(c, n, skProc, lambdaPragmas, flags)
-  of nkDerefExpr: result = semDeref(c, n)
+  of nkDerefExpr: result = semDeref(c, n, flags)
   of nkAddr:
     result = n
     checkSonsLen(n, 1, c.config)
diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim
index e2da56c5d..e7f36879f 100644
--- a/compiler/semgnrc.nim
+++ b/compiler/semgnrc.nim
@@ -104,6 +104,18 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     if s.typ != nil and s.typ.kind == tyStatic:
       if s.typ.n != nil:
         result = s.typ.n
+      elif c.inGenericContext > 0 and withinConcept notin flags:
+        # don't leave generic param as identifier node in generic type,
+        # sigmatch will try to instantiate generic type AST without all params
+        # fine to give a symbol node a generic type here since
+        # we are in a generic context and `prepareNode` will be called
+        result = newSymNodeTypeDesc(s, c.idgen, n.info)
+        if canOpenSym(result.sym):
+          if genericsOpenSym in c.features:
+            result = newOpenSym(result)
+          else:
+            result.flags.incl nfDisabledOpenSym
+            result.typ = nil
       else:
         result = n
     else:
@@ -128,6 +140,18 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
         else:
           result.flags.incl nfDisabledOpenSym
           result.typ = nil
+    elif c.inGenericContext > 0 and withinConcept notin flags:
+      # don't leave generic param as identifier node in generic type,
+      # sigmatch will try to instantiate generic type AST without all params
+      # fine to give a symbol node a generic type here since
+      # we are in a generic context and `prepareNode` will be called
+      result = newSymNodeTypeDesc(s, c.idgen, n.info)
+      if canOpenSym(result.sym):
+        if genericsOpenSym in c.features:
+          result = newOpenSym(result)
+        else:
+          result.flags.incl nfDisabledOpenSym
+          result.typ = nil
     else:
       result = n
     onUse(n.info, s)
diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim
index 4f2d397a6..bdf8fb8ca 100644
--- a/compiler/semtypes.nim
+++ b/compiler/semtypes.nim
@@ -1685,6 +1685,10 @@ proc semTypeExpr(c: PContext, n: PNode; prev: PType): PType =
         # unnecessary new type creation
         let alias = maybeAliasType(c, result, prev)
         if alias != nil: result = alias
+  elif n.typ.kind == tyFromExpr and c.inGenericContext > 0:
+    # sometimes not possible to distinguish type from value in generic body,
+    # for example `T.Foo`, so both are handled under `tyFromExpr`
+    result = n.typ
   else:
     localError(c.config, n.info, "expected type, but got: " & n.renderTree)
     result = errorType(c)
@@ -2053,11 +2057,7 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
       elif op.s == "owned" and optOwnedRefs notin c.config.globalOptions and n.len == 2:
         result = semTypeExpr(c, n[1], prev)
       else:
-        if c.inGenericContext > 0 and n.kind == nkCall:
-          let n = semGenericStmt(c, n)
-          result = makeTypeFromExpr(c, n.copyTree)
-        else:
-          result = semTypeExpr(c, n, prev)
+        result = semTypeExpr(c, n, prev)
   of nkWhenStmt:
     var whenResult = semWhen(c, n, false)
     if whenResult.kind == nkStmtList: whenResult.transitionSonsKind(nkStmtListType)
diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim
index 276424512..428d897ec 100644
--- a/compiler/semtypinst.nim
+++ b/compiler/semtypinst.nim
@@ -131,15 +131,61 @@ proc prepareNode(cl: var TReplTypeVars, n: PNode): PNode =
         replaceTypeVarsS(cl, n.sym, result.typ)
       else:
         replaceTypeVarsS(cl, n.sym, replaceTypeVarsT(cl, n.sym.typ))
-  let isCall = result.kind in nkCallKinds
-  # don't try to instantiate symchoice symbols, they can be
-  # generic procs which the compiler will think are uninstantiated
-  # because their type will contain uninstantiated params
-  let isSymChoice = result.kind in nkSymChoices
-  for i in 0..<n.safeLen:
-    # XXX HACK: ``f(a, b)``, avoid to instantiate `f`
-    if isSymChoice or (isCall and i == 0): result.add(n[i])
-    else: result.add(prepareNode(cl, n[i]))
+  # we need to avoid trying to instantiate nodes that can have uninstantiated
+  # types, like generic proc symbols or raw generic type symbols
+  case n.kind
+  of nkSymChoices:
+    # don't try to instantiate symchoice symbols, they can be
+    # generic procs which the compiler will think are uninstantiated
+    # because their type will contain uninstantiated params
+    for i in 0..<n.len:
+      result.add(n[i])
+  of nkCallKinds:
+    # don't try to instantiate call names since they may be generic proc syms
+    # also bracket expressions can turn into calls with symchoice [] and
+    # we need to not instantiate the Generic in Generic[int]
+    # exception exists for the call name being a dot expression since
+    # dot expressions need their LHS instantiated
+    assert n.len != 0
+    let ignoreFirst = n[0].kind != nkDotExpr
+    let name = n[0].getPIdent
+    let ignoreSecond = name != nil and name.s == "[]" and n.len > 1 and
+      (n[1].typ != nil and n[1].typ.kind == tyTypeDesc)
+    if ignoreFirst:
+      result.add(n[0])
+    else:
+      result.add(prepareNode(cl, n[0]))
+    if n.len > 1:
+      if ignoreSecond:
+        result.add(n[1])
+      else:
+        result.add(prepareNode(cl, n[1]))
+    for i in 2..<n.len:
+      result.add(prepareNode(cl, n[i]))
+  of nkBracketExpr:
+    # don't instantiate Generic body type in expression like Generic[T]
+    # exception exists for the call name being a dot expression since
+    # dot expressions need their LHS instantiated
+    assert n.len != 0
+    let ignoreFirst = n[0].kind != nkDotExpr and
+      n[0].typ != nil and n[0].typ.kind == tyTypeDesc
+    if ignoreFirst:
+      result.add(n[0])
+    else:
+      result.add(prepareNode(cl, n[0]))
+    for i in 1..<n.len:
+      result.add(prepareNode(cl, n[i]))
+  of nkDotExpr:
+    # don't try to instantiate RHS of dot expression, it can outright be
+    # undeclared, but definitely instantiate LHS
+    assert n.len >= 2
+    result.add(prepareNode(cl, n[0]))
+    result.add(n[1])
+    for i in 2..<n.len:
+      result.add(prepareNode(cl, n[i]))
+  else:
+    for i in 0..<n.safeLen:
+      result.add(prepareNode(cl, n[i]))
 
 proc isTypeParam(n: PNode): bool =
   # XXX: generic params should use skGenericParam instead of skType
@@ -222,6 +268,9 @@ proc replaceTypeVarsN(cl: var TReplTypeVars, n: PNode; start=0; expectedType: PT
   if n == nil: return
   result = copyNode(n)
   if n.typ != nil:
+    if n.typ.kind == tyFromExpr:
+      # type of node should not be evaluated as a static value
+      n.typ.flags.incl tfNonConstExpr
     result.typ = replaceTypeVarsT(cl, n.typ)
     checkMetaInvariants(cl, result.typ)
   case n.kind
@@ -592,13 +641,18 @@ proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType =
     assert t.n.typ != t
     var n = prepareNode(cl, t.n)
     if n.kind != nkEmpty:
-      n = cl.c.semConstExpr(cl.c, n)
+      if tfNonConstExpr in t.flags:
+        n = cl.c.semExprWithType(cl.c, n, flags = {efInTypeof})
+      else:
+        n = cl.c.semConstExpr(cl.c, n)
     if n.typ.kind == tyTypeDesc:
       # XXX: sometimes, chained typedescs enter here.
       # It may be worth investigating why this is happening,
       # because it may cause other bugs elsewhere.
       result = n.typ.skipTypes({tyTypeDesc})
       # result = n.typ.base
+    elif tfNonConstExpr in t.flags:
+      result = n.typ
     else:
       if n.typ.kind != tyStatic and n.kind != nkType:
         # XXX: In the future, semConstExpr should
diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index a17869b87..af9b29718 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -1869,7 +1869,12 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
     let prev = idTableGet(c.bindings, f)
     if prev == nil:
       if aOrig.kind == tyStatic:
-        if f.base.kind notin {tyNone, tyGenericParam}:
+        if c.c.inGenericContext > 0 and aOrig.n == nil and not c.isNoCall:
+          # don't match unresolved static value to static param to avoid
+          # faulty instantiations in calls in generic bodies
+          # but not for generic invocations as they only check constraints
+          result = isNone
+        elif f.base.kind notin {tyNone, tyGenericParam}:
           result = typeRel(c, f.base, a, flags)
           if result != isNone and f.n != nil:
             if not exprStructuralEquivalent(f.n, aOrig.n):
@@ -1924,8 +1929,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
       # proc foo(T: typedesc, x: T)
       # when `f` is an unresolved typedesc, `a` could be any
       # type, so we should not perform this check earlier
-      if c.c.inGenericContext > 0 and
-          a.skipTypes({tyTypeDesc}).kind == tyGenericParam:
+      if c.c.inGenericContext > 0 and a.containsGenericType:
         # generic type bodies can sometimes compile call expressions
         # prevent unresolved generic parameters from being passed to procs as
         # typedesc parameters