From 2447dfdc7d48308675b8fdc852bf898ab2fe49ac Mon Sep 17 00:00:00 2001 From: araq Date: Sat, 29 Aug 2026 09:05:44 +0200 Subject: [PATCH] cgen: name the PType child instead of subscripting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same treatment the `PNode` side just got, for the reason that applies to types: `t[0]` is the return type, the base class, the index type or the generic head depending on the kind, and the subscript says none of that. Every child access in the cgen files that has a named accessor now uses it — `baseClass` for the eleven object-hierarchy walks, `elementType` for the seq/openArray element, `returnType`, `genericHead`, `firstGenericParam` — and the two loops that walked a type's children become `paramTypes` and `kids`. Left indexed on purpose: a parameter reached by ARGUMENT position (`typ[i]` in ccgcalls/ccgstmts), a tuple field, and a generic parameter at an explicit index. There the index is the clearest thing to write. Every substitution is exact rather than merely close. `[]` with index 0 is unconditionally `sonsImpl[0]`, so `baseClass`/`returnType`/`genericHead` cannot diverge; `elementType` is `sonsImpl[^1]` and is used only where the type has a single son; `paramTypes` and `kids` are literally the loops they replace. `ast.sons(t: PType)` gets the warning it has been missing. Despite the name it is not the counterpart of the `sons` ITERATOR over a `PNode`: it returns the raw seq, and a `tyProc` keeps its parameter types in `n`, so that seq holds only the return type while `[]`/`len`/`kids` route parameters through `n[i].sym.typ`. `for x in t.sons` therefore compiles, reads exactly like the `PNode` idiom, and visits a different set of types — which is what `ccgutils.encodeType` would have started doing had it been converted to `sons` rather than `kids`. Marking the proc deprecated and rebuilding shows one call site in the whole compiler (`previouslyInferred`), so the trap is latent, not active. `bnode.nim` also records that there is deliberately no `BType` beside `BNode`: types stay `PType`s under `newIcBackend` — `BNode.typ` returns one — because the backend asks them questions (`skipTypes`, `getSize`, `lengthOrd`, the record walk over `t.n`) that a raw cursor cannot answer. Pure refactor: all 216 generated `.c` files byte-identical to the parent commit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE --- compiler/ast.nim | 12 ++++++++++++ compiler/bnode.nim | 21 +++++++++++++++++---- compiler/ccgexprs.nim | 10 +++++----- compiler/ccgtypes.nim | 24 ++++++++++++------------ compiler/ccgutils.nim | 7 +++---- compiler/cgen.nim | 8 ++++---- 6 files changed, 53 insertions(+), 29 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 8f48dacef8..80de2317f5 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -359,6 +359,18 @@ proc `flags=`*(t: PType, val: TTypeFlags) {.inline.} = t.flagsImpl = val proc sons*(t: PType): var TTypeSeq {.inline.} = + ## The RAW child seq. Despite the name this is NOT the counterpart of the + ## `sons` ITERATOR over a `PNode`, and it is not the way to walk a type's + ## children — use `kids` / `ikids` / `paramTypes` / `signature`, or the named + ## accessors (`returnType`, `baseClass`, `elementType`, `indexType`, + ## `genericHead`, ...), which say WHICH child they mean. + ## + ## The difference is not cosmetic. A `tyProc` keeps its parameter types in + ## `n`, not here — `setSons` asserts `sonsImpl.len <= 1` for one — so `[]`, + ## `len` and every iterator built on them route parameters through + ## `n[i].sym.typ`, while this seq holds only the return type. `for x in + ## t.sons` therefore compiles, looks like the `PNode` idiom, and silently + ## visits a different set of types. if t.state == Partial: loadType(t) result = t.sonsImpl diff --git a/compiler/bnode.nim b/compiler/bnode.nim index 794d672a0e..1a22d0dedf 100644 --- a/compiler/bnode.nim +++ b/compiler/bnode.nim @@ -61,10 +61,23 @@ ## remaining subscripts are writes that build a fresh `nkProcDef` ## (`theProc[namePos] = ...`), which a `Cursor` backend will not do at all, and ## accesses to a `PType`, a `string`, a `seq` or a `Table`, none of which are -## `BNode`s. `PType` is the trap to watch for: `ast.sons(t: PType)` is a `proc` -## returning `var TTypeSeq`, NOT the iterator of the same name, so `t[i]` there -## means something else entirely. The `firstSon`/`secondSon`/`lastSon`/`son` -## family is defined for `PNode` only, so a mistaken base does not compile. +## `BNode`s. The `firstSon`/`secondSon`/`lastSon`/`son` family is defined for +## `PNode` only, so a mistaken base does not compile. +## +## A `PType` has its own vocabulary and its own reason for preferring it: `t[0]` +## is the return type, the base class, the index type or the generic head +## depending on the kind, and `ast.sons(t: PType)` is a `proc` returning the raw +## seq — NOT the iterator of the same name — which for a `tyProc` does not hold +## the parameters at all. Reach for `returnType` / `baseClass` / `elementType` / +## `genericHead` and the `kids` / `ikids` / `paramTypes` / `signature` +## iterators, which name the child and go through `[]`. +## +## There is deliberately no `BType` alongside `BNode`. Types stay `PType`s even +## under `newIcBackend` — `typ` below returns one — because the backend asks +## them semantic questions (`skipTypes`, `getSize`, `lengthOrd`, the record +## walk over `t.n`) that a raw cursor cannot answer. `ast2nif` already +## materializes them lazily from the module's type index, which is the seam +## that matters on that side. import ast, lineinfos diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 744b187d99..85dc776454 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -412,7 +412,7 @@ proc genAssignment(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) = elif not isObjLackingTypeField(ty): genGenericAsgn(p, dest, src, flags) elif containsGarbageCollectedRef(ty): - if ty[0].isNil and asgnComplexity(ty.n) <= 4 and + if ty.baseClass.isNil and asgnComplexity(ty.n) <= 4 and needAssignCall notin flags: # calls might contain side effects discard getTypeDesc(p.module, ty) internalAssert p.config, ty.n != nil @@ -1040,7 +1040,7 @@ proc lookupFieldAgain(p: BProc, ty: PType; field: PSym; r: var Rope; break if not p.module.compileToCpp: r = dotField(r, "Sup") - ty = ty[0] + ty = ty.baseClass if result == nil: internalError(p.config, field.info, "genCheckedRecordField") proc genRecordField(p: BProc, e: PNode, d: var TLoc) = @@ -2336,7 +2336,7 @@ proc genSetLengthSeq(p: BProc, e: PNode, d: var TLoc, noinit = false) = let name = if noinit: "setLengthSeqUninit" else: "setLengthSeqV2" call.snippet = cCast(rt, cgCall(p, name, pExpr, rti, rb, - isTrivialTypesToSnippet(t.skipTypes(abstractInst)[0]))) + isTrivialTypesToSnippet(t.skipTypes(abstractInst).elementType))) genAssignment(p, a, call, {}) gcUsage(p.config, e) @@ -4127,7 +4127,7 @@ proc genConstTuple(p: BProc, n: PNode; isConst: bool; tup: PType; result: var Bu genBracedInit(p, it, isConst, tup[i], result) proc genConstSeq(p: BProc, n: PNode, t: PType; isConst: bool; result: var Builder) = - let base = t.skipTypes(abstractInst)[0] + let base = t.skipTypes(abstractInst).elementType let tmpName = getTempName(p.module) # genBracedInit can modify cfsStrData, we need an intermediate builder: @@ -4160,7 +4160,7 @@ proc genConstSeq(p: BProc, n: PNode, t: PType; isConst: bool; result: var Builde result.add cCast(typ = getTypeDesc(p.module, t), value = cAddr(tmpName)) proc genConstSeqV2(p: BProc, n: PNode, t: PType; isConst: bool; result: var Builder) = - let base = t.skipTypes(abstractInst)[0] + let base = t.skipTypes(abstractInst).elementType let payload = getTempName(p.module) # genBracedInit can modify cfsStrData, we need an intermediate builder: diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index d6f7c14f6c..005170e19e 100644 --- a/compiler/ccgtypes.nim +++ b/compiler/ccgtypes.nim @@ -59,10 +59,10 @@ proc mangleProc(m: BModule; s: PSym; makeUnique: bool): string = result = "_Z" # Common prefix in Itanium ABI var params = "" var staticLists = "" - if s.typ.len > 1: #we dont care about the return param - for i in 1.. 0: # we dont care about the return param + for _, pt in paramTypes(s.typ): + if pt.isNil: continue + params.add encodeType(m, pt, staticLists) result.add encodeSym(m, s, makeUnique, staticLists) result.add params @@ -311,7 +311,7 @@ proc isInvalidReturnType(conf: ConfigRef; typ: PType, isProc = true): bool = var rettype = typ var isAllowedCall = true if isProc: - rettype = rettype[0] + rettype = rettype.returnType isAllowedCall = typ.callConv in {ccClosure, ccInline, ccNimCall} if rettype == nil or (isAllowedCall and getSize(conf, rettype) > conf.target.floatSize*3): @@ -480,7 +480,7 @@ proc getTypeDescWeak(m: BModule; t: PType; check: var IntSet; kind: TypeDescKind of tySequence: let sig = hashType(t, m.config) if optSeqDestructors in m.config.globalOptions: - if skipTypes(etB[0], typedescInst).kind == tyEmpty: + if skipTypes(etB.elementType, typedescInst).kind == tyEmpty: internalError(m.config, "cannot map the empty seq type to a C type") result = cacheGetType(m.forwTypeCache, sig) @@ -524,7 +524,7 @@ proc seqV2ContentType(m: BModule; t: PType; check: var IntSet) = if result == "": discard getTypeDescAux(m, t, check, dkVar) else: - let dataTyp = getTypeDescAux(m, t.skipTypes(abstractInst)[0], check, dkVar) + let dataTyp = getTypeDescAux(m, t.skipTypes(abstractInst).elementType, check, dkVar) m.s[cfsTypes].addSimpleStruct(m, name = result & "_Content", baseType = ""): m.s[cfsTypes].addField(name = "cap", typ = NimInt) m.s[cfsTypes].addField(name = "data", @@ -715,7 +715,7 @@ proc genProcParams(m: BModule; t: PType, rettype: var Rope, params: var Builder, # need to pass hidden parameter: params.addParam(paramBuilder, name = param.locImpl.snippet & "Len_" & $j, typ = NimInt) inc(j) - arr = arr[0].skipTypes({tySink}) + arr = arr.elementType.skipTypes({tySink}) if t.returnType != nil and isInvalidReturnType(m.config, t): var arr = t.returnType var typ: Snippet @@ -915,7 +915,7 @@ proc resolveStarsInCppType(typ: PType, idx, stars: int): PType = result = typ[idx] for i in 1..stars: if result != nil and result.kidsLen > 0: - result = if result.kind == tyGenericInst: result[FirstGenericParamAt] + result = if result.kind == tyGenericInst: result.firstGenericParam else: result.elemType proc getOpenArrayDesc(m: BModule; t: PType, check: var IntSet; kind: TypeDescKind): Rope = @@ -1462,7 +1462,7 @@ proc discriminatorTableName(m: BModule; objtype: PType, d: PSym): Rope = # bugfix: we need to search the type that contains the discriminator: var objtype = objtype.skipTypes(abstractPtrs) while lookupInRecord(objtype.n, d.name) == nil: - objtype = objtype[0].skipTypes(abstractPtrs) + objtype = objtype.baseClass.skipTypes(abstractPtrs) if objtype.sym == nil: internalError(m.config, d.info, "anonymous obj with discriminator") result = "NimDT_$1_$2" % [rope($hashType(objtype, m.config)), rope(d.name.s.mangle)] @@ -1861,7 +1861,7 @@ proc getObjDepth(t: PType): int16 = result = -1 while x != nil: x = skipTypes(x, skipPtrs) - x = x[0] + x = x.baseClass inc(result) proc genDisplayElem(d: MD5Digest): uint32 = @@ -1877,7 +1877,7 @@ proc genDisplay(result: var Builder, m: BModule; t: PType, depth: int) = while x != nil: x = skipTypes(x, skipPtrs) seqs[i] = cIntValue(genDisplayElem(MD5Digest(hashType(x, m.config)))) - x = x[0] + x = x.baseClass inc i var arr: StructInitializer diff --git a/compiler/ccgutils.nim b/compiler/ccgutils.nim index 873a60b24d..42c3c34148 100644 --- a/compiler/ccgutils.nim +++ b/compiler/ccgutils.nim @@ -92,7 +92,7 @@ proc ccgIntroducedPtr*(conf: ConfigRef; s: PSym, retType: PType): bool = result = true elif (optByRef in s.options) or (getSize(conf, pt) > conf.target.floatSize * 3): result = true # requested anyway - elif (tfFinal in pt.flags) and (pt[0] == nil): + elif (tfFinal in pt.flags) and (pt.baseClass == nil): result = false # no need, because no subtyping possible else: result = true # ordinary objects are always passed by reference, @@ -148,7 +148,7 @@ proc encodeType*(m: BModule; t: PType; staticLists: var string): string = of tyObject, tyEnum, tyDistinct, tyUserTypeClass, tyGenericParam: result = encodeSym(m, t.sym) of tyGenericInst, tyUserTypeClassInst, tyGenericBody: - result = encodeName(t[0].sym.name.s) + result = encodeName(t.genericHead.sym.name.s) result.add "I" for i in 1..