adapt generic default parameters to recent generics changes (#24065)

fixes #16700, fixes #20916, refs #24010

Fixes the instantiation issues for proc param default values encountered
in #24010 by:

1. semchecking generic default param values with `inGenericContext` for
#22029 and followups to apply (the bigger change in semtypes),
2. rejecting explicit generic instantiations with unresolved generic
types inside `inGenericContext` (sigmatch change),
3. instantiating the default param values using `prepareNode` rather
than an insufficient manual method (the bigger change in seminst).

This had an important side effect of references to other parameters not
working since they would be resolved as a symbol belonging to the
uninstantiated original generic proc rather than the later instantiated
proc. There is a more radical way to fix this which is generating ident
nodes with `tyFromExpr` in specifically this context, but instead we
just count them as belonging to the same proc in
`hoistParamsUsedInDefault`.

Other minor bugfixes:

* To make the error message in t20883 make sense, we now give a "cannot
instantiate" error when trying to instantiate a proc generic param with
`tyFromExpr`.
* Object constructors as default param values generated default values
of private fields going through `evalConstExpr` more than once, but the
VM doesn't mark the object fields as `nfSkipFieldChecking` which gives a
"cannot access field" error. So the VM now marks object fields it
generates as `nfSkipFieldChecking`. Not sure if this affects VM
performance, don't see why it would.
* The nkRecWhen changes in #24042 didn't cover the case where all
conditions are constantly false correctly, this is fixed with a minor
change. This isn't needed for this PR now but I encountered it after
forgetting to `dec c.inGenericContext`.

9 files changed, 56 insertions, 28 deletions

diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index 9b255bdf4..bb82a7cd7 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -3084,7 +3084,11 @@ proc hoistParamsUsedInDefault(c: PContext, call, letSection, defExpr: var PNode)
   # duty is activated by returning a non-nil value. The caller is responsible
   # for replacing the input to the function with the returned non-nil value.
   # (which is the hoisted symbol)
-  if defExpr.kind == nkSym and defExpr.sym.kind == skParam and defExpr.sym.owner == call[0].sym:
+  if defExpr.kind == nkSym and defExpr.sym.kind == skParam and
+      (defExpr.sym.owner == call[0].sym or
+        # symbol was resolved before proc was instantiated:
+        (sfFromGeneric in call[0].sym.flags and
+          defExpr.sym.owner == call[0].sym.instantiatedFrom)):
     let paramPos = defExpr.sym.position + 1
 
     if call[paramPos].skipAddr.kind != nkSym and not (
diff --git a/compiler/seminst.nim b/compiler/seminst.nim
index 5bfc01bb7..ff1592573 100644
--- a/compiler/seminst.nim
+++ b/compiler/seminst.nim
@@ -53,7 +53,7 @@ iterator instantiateGenericParamList(c: PContext, n: PNode, pt: TypeMapping): PS
           if q.typ.kind != tyCompositeTypeClass:
             localError(c.config, a.info, errCannotInstantiateX % s.name.s)
           t = errorType(c)
-      elif t.kind in {tyGenericParam, tyConcept}:
+      elif t.kind in {tyGenericParam, tyConcept, tyFromExpr}:
         localError(c.config, a.info, errCannotInstantiateX % q.name.s)
         t = errorType(c)
       elif isUnresolvedStatic(t) and (q.typ.kind == tyStatic or
@@ -277,9 +277,10 @@ proc instantiateProcType(c: PContext, pt: TypeMapping,
     # call head symbol, because this leads to infinite recursion.
     if oldParam.ast != nil:
       var def = oldParam.ast.copyTree
-      if def.kind in nkCallKinds:
-        for i in 1..<def.len:
-          def[i] = replaceTypeVarsN(cl, def[i], 1)
+      if def.typ.kind == tyFromExpr:
+        def.typ.flags.incl tfNonConstExpr
+      if not isIntLit(def.typ):
+        def = prepareNode(cl, def)
 
       # allow symchoice since node will be fit later
       # although expectedType should cover it
diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim
index 8d293a753..4c7affd27 100644
--- a/compiler/semtypes.nim
+++ b/compiler/semtypes.nim
@@ -829,7 +829,7 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
         it[idx] = if newf.len == 1: newf[0] else: newf
     if branch != nil:
       semRecordNodeAux(c, branch, check, pos, father, rectype, hasCaseFields)
-    elif c.inGenericContext > 0:
+    elif cannotResolve:
       father.add a
     elif father.kind in {nkElse, nkOfBranch}:
       father.add newNodeI(nkRecList, n.info)
@@ -1365,15 +1365,10 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode,
           "either use ';' (semicolon) or explicitly write each default value")
         message(c.config, a.info, warnImplicitDefaultValue, msg)
       block determineType:
-        var defTyp = typ
-        if isCurrentlyGeneric():
-          defTyp = nil
-          def = semGenericStmt(c, def)
-          if hasUnresolvedArgs(c, def):
-            def.typ = makeTypeFromExpr(c, def.copyTree)
-            break determineType
-
-        def = semExprWithType(c, def, {efDetermineType, efAllowSymChoice}, defTyp)
+        let isGeneric = isCurrentlyGeneric()
+        inc c.inGenericContext, ord(isGeneric)
+        def = semExprWithType(c, def, {efDetermineType, efAllowSymChoice}, typ)
+        dec c.inGenericContext, ord(isGeneric)
         if def.referencesAnotherParam(getCurrOwner(c)):
           def.flags.incl nfDefaultRefsParam
 
diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim
index c2599c6f7..3c39e846e 100644
--- a/compiler/semtypinst.nim
+++ b/compiler/semtypinst.nim
@@ -118,7 +118,7 @@ proc replaceTypeVarsT*(cl: var TReplTypeVars, t: PType): PType =
   result = replaceTypeVarsTAux(cl, t)
   checkMetaInvariants(cl, result)
 
-proc prepareNode(cl: var TReplTypeVars, n: PNode): PNode =
+proc prepareNode*(cl: var TReplTypeVars, n: PNode): PNode =
   let t = replaceTypeVarsT(cl, n.typ)
   if t != nil and t.kind == tyStatic and t.n != nil:
     return if tfUnresolved in t.flags: prepareNode(cl, t.n)
diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index c7a8c496c..e519f17c8 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -135,6 +135,13 @@ proc typeRel*(c: var TCandidate, f, aOrig: PType,
 
 proc matchGenericParam(m: var TCandidate, formal: PType, n: PNode) =
   var arg = n.typ
+  if m.c.inGenericContext > 0:
+    # don't match yet-unresolved generic instantiations
+    while arg != nil and arg.kind == tyGenericParam:
+      arg = idTableGet(m.bindings, arg)
+    if arg == nil or arg.containsGenericType:
+      m.state = csNoMatch
+      return
   # fix up the type to get ready to match formal:
   var formalBase = formal
   while formalBase.kind == tyGenericParam and
diff --git a/compiler/vm.nim b/compiler/vm.nim
index 879675d9f..a9a25e4ea 100644
--- a/compiler/vm.nim
+++ b/compiler/vm.nim
@@ -894,8 +894,10 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
         stackTrace(c, tos, pc, errNilAccess)
       elif dest[shiftedRb].kind == nkExprColonExpr:
         writeField(dest[shiftedRb][1], regs[rc])
+        dest[shiftedRb][1].flags.incl nfSkipFieldChecking
       else:
         writeField(dest[shiftedRb], regs[rc])
+        dest[shiftedRb].flags.incl nfSkipFieldChecking
     of opcWrStrIdx:
       decodeBC(rkNode)
       let idx = regs[rb].intVal.int
diff --git a/tests/misc/t20883.nim b/tests/misc/t20883.nim
index d98feaa14..92e7929f4 100644
--- a/tests/misc/t20883.nim
+++ b/tests/misc/t20883.nim
@@ -1,8 +1,9 @@
 discard """
   action: reject
-  errormsg: "type mismatch: got <float64> but expected 'typeof(U(0.000001))'"
-  line: 8
-  column: 22
+nimout: '''
+t20883.nim(13, 4) template/generic instantiation of `foo` from here
+t20883.nim(9, 11) Error: cannot instantiate: 'U'
+'''
 """
 
 proc foo*[U](x: U = U(1e-6)) =
diff --git a/tests/proc/texplicitgenerics.nim b/tests/proc/texplicitgenerics.nim
index c0bdfe513..8a90f4266 100644
--- a/tests/proc/texplicitgenerics.nim
+++ b/tests/proc/texplicitgenerics.nim
@@ -36,14 +36,10 @@ block: # ditto but may be wrong minimization
   # minimized from measuremancer
   type Foo[T] = object
   proc foo[T](): Foo[T] = Foo[T]()
-  when false:
-    # this is the actual issue but there are other instantiation problems
-    proc bar[T](x = foo[T]()) = discard
-  else:
-    proc bar[T](x: Foo[T] = foo[T]()) = discard
+  # this is the actual issue but there are other instantiation problems
+  proc bar[T](x = foo[T]()) = discard
   bar[int](Foo[int]())
   bar[int]()
-  when false:
-    # alternative version, also causes instantiation issue
-    proc baz[T](x: typeof(foo[T]())) = discard
-    baz[int](Foo[int]())
+  # alternative version, also causes instantiation issue
+  proc baz[T](x: typeof(foo[T]())) = discard
+  baz[int](Foo[int]())
diff --git a/tests/proc/tgenericdefaultparam.nim b/tests/proc/tgenericdefaultparam.nim
new file mode 100644
index 000000000..f8e08e409
--- /dev/null
+++ b/tests/proc/tgenericdefaultparam.nim
@@ -0,0 +1,22 @@
+block: # issue #16700
+  type MyObject[T] = object
+    x: T
+  proc initMyObject[T](value = T.default): MyObject[T] =
+    MyObject[T](x: value)
+  var obj = initMyObject[int]()
+
+block: # issue #20916
+  type
+    SomeX = object
+      v: int
+  var val = 0
+  proc f(_: type int, x: SomeX, v = x.v) =
+    doAssert v == 42
+    val = v
+  proc a(): proc() =
+    let v = SomeX(v: 42)
+    var tmp = proc() =
+      int.f(v)
+    tmp
+  a()()
+  doAssert val == 42