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

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.

(cherry picked from commit ab18962)

Author: metagn

compiler/ast.nim

@@ -641,6 +641,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

compiler/semcall.nim

@@ -677,7 +677,7 @@ proc semOverloadedCall(c: PContext, n, nOrig: PNode,
               candidates)
     result = semResolvedCall(c, r, n, flags)
   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:

compiler/semexprs.nim

@@ -1029,7 +1029,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
@@ -1135,7 +1135,12 @@ proc semIndirectOp(c: PContext, n: PNode, flags: TExprFlags; expectedType: PType
       result.transitionSonsKind(nkCall)
       result.flags.incl nfExplicitCall
       for i in 1..<n.len: result.add n[i]
-      return semExpr(c, result, flags)
+      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:
@@ -1478,17 +1483,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:
     discard
 
@@ -1651,18 +1656,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.lastSon
+  of tyRef, tyPtr: n.typ = t.elementType
+  of tyMetaTypes, tyFromExpr:
+    n.typ = makeTypeFromExpr(c, n.copyTree)
   else: result = nil
   #GlobalError(n[0].info, errCircumNeedsPointer)
 
@@ -1692,8 +1699,11 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
   ## returns nil if not a built-in subscript operator; also called for the
   ## checking of assignments
   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
@@ -3387,7 +3397,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)

compiler/semgnrc.nim

@@ -103,6 +103,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:
@@ -127,6 +139,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)

compiler/semtypes.nim

@@ -1650,6 +1650,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)
@@ -2015,11 +2019,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)

compiler/semtypinst.nim

@@ -135,15 +135,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
@@ -226,6 +272,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
@@ -596,13 +645,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

compiler/sigmatch.nim

@@ -1860,7 +1860,12 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
     let prev = PType(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):
@@ -1917,8 +1922,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

tests/generics/t23854.nim

@@ -0,0 +1,70 @@
+# issue #23854, not entirely fixed
+
+import std/bitops
+
+const WordBitWidth = sizeof(pointer) * 8
+
+func wordsRequired*(bits: int): int {.inline.} =
+  const divShiftor = fastLog2(uint32(WordBitWidth))
+  result = (bits + WordBitWidth - 1) shr divShiftor
+
+type
+  Algebra* = enum
+    BLS12_381
+
+  BigInt*[bits: static int] = object
+    limbs*: array[wordsRequired(bits), uint]
+
+  Fr*[Name: static Algebra] = object
+    residue_form*: BigInt[255]
+
+  Fp*[Name: static Algebra] = object
+    residue_form*: BigInt[381]
+
+  FF*[Name: static Algebra] = Fp[Name] or Fr[Name]
+
+template getBigInt*[Name: static Algebra](T: type FF[Name]): untyped =
+  ## Get the underlying BigInt type.
+  typeof(default(T).residue_form)
+
+type
+  EC_ShortW_Aff*[F] = object
+    ## Elliptic curve point for a curve in Short Weierstrass form
+    ##   y² = x³ + a x + b
+    ##
+    ## over a field F
+    x*, y*: F
+
+type FieldKind* = enum
+  kBaseField
+  kScalarField
+
+func bits*[Name: static Algebra](T: type FF[Name]): static int =
+  T.getBigInt().bits
+
+template getScalarField*(EC: type EC_ShortW_Aff): untyped =
+  Fr[EC.F.Name]
+
+# ------------------------------------------------------------------------------
+
+type
+  ECFFT_Descriptor*[EC] = object
+    ## Metadata for FFT on Elliptic Curve
+    order*: int
+    rootsOfUnity1*: ptr UncheckedArray[BigInt[EC.getScalarField().bits()]]  # Error: in expression 'EC.getScalarField()': identifier expected, but found 'EC.getScalarField'
+    rootsOfUnity2*: ptr UncheckedArray[BigInt[getScalarField(EC).bits()]] # Compiler SIGSEGV: Illegal Storage Access
+
+func new*(T: type ECFFT_Descriptor): T =
+  discard
+
+# ------------------------------------------------------------------------------
+
+template getBits[bits: static int](x: ptr UncheckedArray[BigInt[bits]]): int = bits
+
+proc main() =
+  let ctx = ECFFT_Descriptor[EC_ShortW_Aff[Fp[BLS12_381]]].new()
+  when false: echo getBits(ctx.rootsOfUnity2) # doesn't work yet?
+  doAssert ctx.rootsOfUnity1[0].limbs.len == wordsRequired(255)
+  doAssert ctx.rootsOfUnity2[0].limbs.len == wordsRequired(255)
+
+main()