From 6000cc8c0f5c71fd0495172b5680ab6b1945428c Mon Sep 17 00:00:00 2001 From: metagn Date: Tue, 5 Sep 2023 11:30:13 +0300 Subject: [PATCH] fix sym of created generic instantiation type (#22642) fixes #22639 A `tyGenericInst` has its last son as the instantiated body of the original generic type. However this type keeps its original `sym` field from the original generic types, which means the sym's type is uninstantiated. This causes problems in the implementation of `getType`, where it uses the `sym` fields of types to represent them in AST, the relevant example for the issue being [here](https://github.com/nim-lang/Nim/blob/d13aab50cf465a7f2edf9c37a4fa30e128892e72/compiler/vmdeps.nim#L191) called from [here](https://github.com/nim-lang/Nim/blob/d13aab50cf465a7f2edf9c37a4fa30e128892e72/compiler/vmdeps.nim#L143). To fix this, create a new symbol from the original symbol for the instantiated body during the creation of `tyGenericInst`s with the appropriate type. Normally `replaceTypeVarsS` would be used for this, but here it seems to cause some recursion issue (immediately gives an error like "cannot instantiate HSlice[T, U]"), so we directly set the symbol's type to the instantiated type. Avoiding recursion means we also cannot use `replaceTypeVarsN` for the symbol AST, and the symbol not having any AST crashes the implementation of `getType` again [here](https://github.com/nim-lang/Nim/blob/d13aab50cf465a7f2edf9c37a4fa30e128892e72/compiler/vmdeps.nim#L167), so the symbol AST is set to the original generic type AST for now which is what it was before anyway. Not sure about this because not sure why the recursion issue is happening, putting it at the end of the proc doesn't help either. Also not sure if the `cl.owner != nil and s.owner != cl.owner` condition from `replaceTypeVarsS` is relevant here. This might also break some code if it depended on the original generic type symbol being given. --- compiler/semtypinst.nim | 12 +++++++++++ compiler/sigmatch.nim | 12 +++++++++-- compiler/suggest.nim | 6 ++---- tests/generics/timpl_ast.nim | 39 ++++++++++++++++++++++++++++++++---- 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index d01ca28e43..54ea9769c6 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -433,6 +433,18 @@ proc handleGenericInvocation(cl: var TReplTypeVars, t: PType): PType = # One step is enough, because the recursive nature of # handleGenericInvocation will handle the alias-to-alias-to-alias case if newbody.isGenericAlias: newbody = newbody.skipGenericAlias + + let origSym = newbody.sym + if origSym != nil and sfFromGeneric notin origSym.flags: + # same as `replaceTypeVarsS` but directly set the type without recursion: + newbody.sym = copySym(origSym, cl.c.idgen) + incl(newbody.sym.flags, sfFromGeneric) + newbody.sym.owner = origSym.owner + newbody.sym.typ = newbody + # unfortunately calling `replaceTypeVarsN` causes recursion, so this AST + # is the original generic body AST + newbody.sym.ast = copyTree(origSym.ast) + rawAddSon(result, newbody) checkPartialConstructedType(cl.c.config, cl.info, newbody) if not cl.allowMetaTypes: diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index bb99463b64..a6cda65b15 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -505,6 +505,13 @@ proc skipToObject(t: PType; skipped: var SkippedPtr): PType = if r.kind == tyObject and ptrs <= 1: result = r else: result = nil +proc getObjectSym(t: PType): PSym = + if tfFromGeneric in t.flags and t.typeInst.kind == tyGenericInst: + var dummy: SkippedPtr + result = t.typeInst[0].skipToObject(dummy).sym + else: + result = t.sym + proc isGenericSubtype(c: var TCandidate; a, f: PType, d: var int, fGenericOrigin: PType): bool = assert f.kind in {tyGenericInst, tyGenericInvocation, tyGenericBody} var askip = skippedNone @@ -512,11 +519,12 @@ proc isGenericSubtype(c: var TCandidate; a, f: PType, d: var int, fGenericOrigin var t = a.skipToObject(askip) let r = f.skipToObject(fskip) if r == nil: return false + let rSym = getObjectSym(r) var depth = 0 var last = a # XXX sameObjectType can return false here. Need to investigate # why that is but sameObjectType does way too much work here anyway. - while t != nil and r.sym != t.sym and askip == fskip: + while t != nil and rSym != getObjectSym(t) and askip == fskip: t = t[0] if t == nil: break last = t @@ -1602,7 +1610,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType, # crossing path with metatypes/aliases, so we need to separate them # by checking sym.id let genericSubtype = isGenericSubtype(c, x, f, depth, f) - if not (genericSubtype and aobj.sym.id != fobj.sym.id) and aOrig.kind != tyGenericBody: + if not (genericSubtype and getObjectSym(aobj).id != getObjectSym(fobj).id) and aOrig.kind != tyGenericBody: depth = -1 if depth >= 0: diff --git a/compiler/suggest.nim b/compiler/suggest.nim index c7efad1af6..5554991bd7 100644 --- a/compiler/suggest.nim +++ b/compiler/suggest.nim @@ -280,10 +280,8 @@ proc fieldVisible*(c: PContext, f: PSym): bool {.inline.} = for module in c.friendModules: if fmoduleId == module.id: return true if f.kind == skField: - var symObj = f.owner - if symObj.typ.skipTypes({tyGenericBody, tyGenericInst, tyGenericInvocation, tyAlias}).kind in {tyRef, tyPtr}: - symObj = symObj.typ.toObjectFromRefPtrGeneric.sym - assert symObj != nil + var symObj = f.owner.typ.toObjectFromRefPtrGeneric.sym + assert symObj != nil for scope in allScopes(c.currentScope): for sym in scope.allowPrivateAccess: if symObj.id == sym.id: return true diff --git a/tests/generics/timpl_ast.nim b/tests/generics/timpl_ast.nim index 69a59e24d4..97fe128cd2 100644 --- a/tests/generics/timpl_ast.nim +++ b/tests/generics/timpl_ast.nim @@ -1,7 +1,3 @@ -discard """ - action: compile -""" - import std/macros import std/assertions @@ -47,3 +43,38 @@ block: # issues #9899, ##14708 macro parse(s: static string) = result = parseStmt(s) parse("type " & implRepr(Option)) + +block: # issue #22639 + type + Spectrum[N: static int] = object + data: array[N, float] + AngleInterpolator = object + data: seq[Spectrum[60]] + proc initInterpolator(num: int): AngleInterpolator = + result = AngleInterpolator() + for i in 0 ..< num: + result.data.add Spectrum[60]() + macro genCompatibleTuple(t: typed): untyped = + let typ = t.getType[1].getTypeImpl[2] + result = nnkTupleTy.newTree() + for i, ch in typ: # is `nnkObjectTy` + result.add nnkIdentDefs.newTree(ident(ch[0].strVal), # ch is `nnkIdentDefs` + ch[1], + newEmptyNode()) + proc fullSize[T: object | tuple](x: T): int = + var tmp: genCompatibleTuple(T) + result = 0 + for field, val in fieldPairs(x): + result += sizeof(val) + doAssert result == sizeof(tmp) + + let reflectivity = initInterpolator(1) + for el in reflectivity.data: + doAssert fullSize(el) == sizeof(el) + doAssert fullSize(reflectivity.data[0]) == sizeof(reflectivity.data[0]) + doAssert genCompatibleTuple(Spectrum[60]) is tuple[data: array[60, float]] + doAssert genCompatibleTuple(Spectrum[120]) is tuple[data: array[120, float]] + type Foo[T] = object + data: T + doAssert genCompatibleTuple(Foo[int]) is tuple[data: int] + doAssert genCompatibleTuple(Foo[float]) is tuple[data: float]