RE: fix #25627 (#25736)

Follow up PR to #25700

@demotomohiro 

This doesn't seem to mirror your suggested approach completely. I still
went with a recursive walk. Could probably add some kind of "clean
types" and "dirty types" cache through this to minimize the recursions,
but that seems like a little much.

compiler/semdata.nim

@@ -183,6 +183,9 @@ type
     forwardTypeUpdates*: seq[(PType, PNode)]
       # types that need to be updated in a type section
       # due to containing forward types, and their corresponding nodes
+    forwardFieldUpdates*: seq[(PType, PNode, PType)]
+      # object/tuple field definitions whose default values mention forward
+      # types and need delayed const checking
     inTypeofContext*: int
 
     semAsgnOpr*: proc (c: PContext; n: PNode; k: TNodeKind): PNode {.nimcall.}

compiler/semstmts.nim

@@ -1817,6 +1817,9 @@ proc typeSectionFinalPass(c: PContext, n: PNode) =
     assignType(typ, reified)
     typ.itemId = reified.itemId     # same id
   c.forwardTypeUpdates = @[]
+  for (owner, field, expectedType) in c.forwardFieldUpdates:
+    semDelayedFieldDefault(c, owner, expectedType, field)
+  c.forwardFieldUpdates = @[]
   for i in 0..<n.len:
     var a = n[i]
     if a.kind == nkCommentStmt: continue

compiler/semtypes.nim

@@ -318,6 +318,61 @@ proc fitDefaultNode(c: PContext, n: var PNode, expectedType: PType) =
     typeAllowedCheck(c, n.info, n.typ, skConst, {taProcContextIsNotMacro, taIsDefaultField})
   dec c.inStaticContext
 
+proc containsForwardTypeAux(t: PType; seen: var IntSet): bool
+
+proc containsForwardTypeAux(n: PNode; seen: var IntSet): bool =
+  result = false
+  if n.isNil or n.kind in nkLiterals + {nkNilLit, nkEmpty, nkType}:
+    return
+  if containsForwardTypeAux(n.typ, seen) or
+     (n.kind == nkSym and n.sym.typ != n.typ and containsForwardTypeAux(n.sym.typ, seen)):
+    return true
+
+  for i in 0 ..< n.safeLen:
+    if containsForwardTypeAux(n[i], seen):
+      return true
+
+proc containsForwardTypeAux(t: PType; seen: var IntSet): bool =
+  result = false
+  if t.isNil:
+    return
+  if t.kind == tyForward:
+    return true
+
+  if not containsOrIncl(seen, t.id):
+    if containsForwardTypeAux(t.n, seen):
+      return true
+
+    for i in 0 ..< t.len:
+      if containsForwardTypeAux(t[i], seen):
+        return true
+
+proc containsForwardType(arg: PNode): bool =
+  var seen = initIntSet()
+  containsForwardTypeAux(arg, seen)
+
+proc containsForwardType(t: PType): bool =
+  var seen = initIntSet()
+  containsForwardTypeAux(t, seen)
+
+proc semFieldDefault(c: PContext; owner, expectedType: PType; field: PNode): PType =
+  result = expectedType
+  field[^1] = semExprWithType(c, field[^1], {efDetermineType, efAllowSymChoice}, result)
+  if result == nil:
+    result = field[^1].typ
+
+  if c.inGenericContext == 0:
+    if containsForwardType(field[^1]):
+      c.forwardFieldUpdates.add (owner, field, result)
+    else:
+      fitDefaultNode(c, field[^1], result)
+      result = field[^1].typ.skipIntLit(c.idgen)
+      propagateToOwner(owner, result)
+
+proc semDelayedFieldDefault(c: PContext; owner, expectedType: PType; field: PNode) =
+  fitDefaultNode(c, field[^1], expectedType)
+  propagateToOwner(owner, field[^1].typ.skipIntLit(c.idgen))
+
 proc isRecursiveType*(t: PType): bool =
   # handle simple recusive types before typeFinalPass
   var cycleDetector = initIntSet()
@@ -550,13 +605,7 @@ proc semTuple(c: PContext, n: PNode, prev: PType): PType =
     var hasDefaultField = a[^1].kind != nkEmpty
     if hasDefaultField:
       typ = if a[^2].kind != nkEmpty: semTypeNode(c, a[^2], nil) else: nil
-      if c.inGenericContext > 0:
-        a[^1] = semExprWithType(c, a[^1], {efDetermineType, efAllowSymChoice}, typ)
-        if typ == nil:
-          typ = a[^1].typ
-      else:
-        fitDefaultNode(c, a[^1], typ)
-        typ = a[^1].typ.skipIntLit(c.idgen)
+      typ = semFieldDefault(c, result, typ, a)
     elif a[^2].kind != nkEmpty:
       typ = semTypeNode(c, a[^2], nil)
       if c.graph.config.isDefined("nimPreviewRangeDefault") and typ.skipTypes(abstractInst).kind == tyRange:
@@ -922,14 +971,7 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
     var hasDefaultField = n[^1].kind != nkEmpty
     if hasDefaultField:
       typ = if n[^2].kind != nkEmpty: semTypeNode(c, n[^2], nil) else: nil
-      if c.inGenericContext > 0:
-        n[^1] = semExprWithType(c, n[^1], {efDetermineType, efAllowSymChoice}, typ)
-        if typ == nil:
-          typ = n[^1].typ
-      else:
-        fitDefaultNode(c, n[^1], typ)
-        typ = n[^1].typ.skipIntLit(c.idgen)
-        propagateToOwner(rectype, typ)
+      typ = semFieldDefault(c, rectype, typ, n)
     elif n[^2].kind == nkEmpty:
       localError(c.config, n.info, errTypeExpected)
       typ = errorType(c)
@@ -1693,22 +1735,6 @@ proc containsGenericInvocationWithForward(n: PNode): bool =
           return true
   return false
 
-proc containsRecWhen(n: PNode): bool =
-  if n == nil:
-    return false
-  case n.kind
-  of nkRecWhen:
-    return true
-  else:
-    for i in 0..<n.safeLen:
-      if containsRecWhen(n[i]):
-        return true
-    return false
-
-proc requiresForwardTypeDelay(n: PNode): bool =
-  n.kind == nkSym and n.sym.ast != nil and n.sym.ast.len > 2 and
-    containsRecWhen(n.sym.ast[2])
-
 proc semGeneric(c: PContext, n: PNode, s: PSym, prev: PType): PType =
   if s.typ == nil:
     localError(c.config, n.info, "cannot instantiate the '$1' $2" %
@@ -1788,12 +1814,11 @@ proc semGeneric(c: PContext, n: PNode, s: PSym, prev: PType): PType =
         # XXX: What kind of error is this? is it still relevant?
         localError(c.config, n.info, errCannotInstantiateX % s.name.s)
         result = newOrPrevType(tyError, prev, c)
-      elif containsGenericInvocationWithForward(n[0]) or
-          (hasForwardTypeParam and requiresForwardTypeDelay(n[0])):
+      elif containsGenericInvocationWithForward(n[0]) or hasForwardTypeParam:
         # isConcrete == false means this generic type is not instanciated here because
         # it invoked with generic parameters.
-        # Even if isConcrete == true, don't instanciate it now if the type
-        # shape depends on unresolved `tyForward` type params.
+        # Even if isConcrete == true, don't instanciate it now if there are
+        # unresolved `tyForward` type params.
         # Such `tyForward` type params will be semchecked later and we can
         # instanciate this next time.
         # Some generic types like std/options.Option[T] need the kind of the

tests/objects/t25627.nim

@@ -20,3 +20,51 @@ let dir = DirStruct()
 doAssert dir.root.kind == fsoDir
 doAssert dir.root.dirName == "/"
 doAssert dir.root.files.len == 0
+
+block:
+  type
+    Opt[T] = object
+      when T is ref:
+        val: T
+        x: int
+      else:
+        val: T
+        x: string
+
+    DefaultOpt = ref object
+      files: Opt[DefaultOpt]
+
+    OptDirStruct = object
+      root = DefaultOpt()
+
+  let dir = OptDirStruct()
+  doAssert dir.root.files.x is int
+
+block:
+  type
+    Opt[T] = object
+      when T is ref:
+        x: int
+      else:
+        x: string
+
+    Foo[T] = object
+      x: Opt[T]
+
+    Nested = ref object
+      files: Foo[Nested]
+
+  let nested = Nested()
+  doAssert nested.files.x.x is int
+
+block:
+  type
+    Foo[T] = object
+      x = sizeof(T)
+
+    Sized = ref object
+      files: Foo[Sized]
+
+  let sized = Sized()
+  doAssert sized.files.x == sizeof(Sized)
+