diff --git a/compiler/ast.nim b/compiler/ast.nim index 7476d7025c..d3c43dc638 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -1464,18 +1464,28 @@ proc hasSubnodeWith*(n: PNode, kind: TNodeKind): bool = return true result = false -proc getInt*(a: PNode): Int128 = - case a.kind - of nkCharLit, nkUIntLit..nkUInt64Lit: - result = toInt128(cast[uint64](a.intVal)) - of nkInt8Lit..nkInt64Lit: - result = toInt128(a.intVal) - of nkIntLit: - # XXX: enable this assert - # assert a.typ.kind notin {tyChar, tyUint..tyUInt64} - result = toInt128(a.intVal) - else: - raiseRecoverableError("cannot extract number from invalid AST node") +template getIntImpl*(aArg: typed): Int128 = + ## The body of `getInt`, in a form `bnode.nim` can instantiate for a `BNode` + ## too — same reason as `canRaiseImpl`: `BNode` is defined there and that + ## module imports this one, so the shared logic has to live in a template + ## rather than an `AnyNode` proc. There is no second copy. + block: + let a = aArg + var res: Int128 + case a.kind + of nkCharLit, nkUIntLit..nkUInt64Lit: + res = toInt128(cast[uint64](a.intVal)) + of nkInt8Lit..nkInt64Lit: + res = toInt128(a.intVal) + of nkIntLit: + # XXX: enable this assert + # assert a.typ.kind notin {tyChar, tyUint..tyUInt64} + res = toInt128(a.intVal) + else: + raiseRecoverableError("cannot extract number from invalid AST node") + res + +proc getInt*(a: PNode): Int128 = getIntImpl(a) proc getInt64*(a: PNode): int64 {.deprecated: "use getInt".} = case a.kind diff --git a/compiler/astalgo.nim b/compiler/astalgo.nim index ba4396a335..978c9e0a83 100644 --- a/compiler/astalgo.nim +++ b/compiler/astalgo.nim @@ -13,7 +13,7 @@ import ast, astyaml, options, lineinfos, idents, rodutils, - msgs + msgs, bnode import std/[hashes, intsets] import std/strutils except addf @@ -100,7 +100,7 @@ proc skipConvCastAndClosure*(n: PNode): PNode = result = result[1] else: break -proc sameValue*(a, b: PNode): bool = +proc sameValue*[T: AnyNode](a, b: T): bool = result = false case a.kind of nkCharLit..nkUInt64Lit: diff --git a/compiler/bnode.nim b/compiler/bnode.nim index 776dcf562e..c2842a4458 100644 --- a/compiler/bnode.nim +++ b/compiler/bnode.nim @@ -430,6 +430,27 @@ when defined(newIcBackend): ## (`(nflags )`, `(ht )`) are peeled by ## `bodynav.symToken`, beside the code that derives the lookup key from them, ## so the two cannot drift apart. + ## + ## NOT IDEMPOTENT FOR OBJECT FIELDS, and anything built on this accessor has + ## to know it. Two calls on the SAME token yield two different `skField` + ## `PSym`s with consecutive item ids: field uses deliberately bypass the + ## nav's memo and go to `loadFieldStub`, which mints per use because two + ## distinct fields can share a name AND a position across types, so one + ## shared stub would mistype one of them (see `bodynav`). For every other + ## symbol kind the answer is stable — the nav memoises it — and `cgen`'s + ## grinder asserts that for the non-field case at every node. + ## + ## The consequence is not theoretical. A proc that reads a field sym twice + ## and compares IDENTITY is correct on a `PNode` and wrong on a `Cursor`: + ## `aliases.isPartOf` does exactly that (`a[1].sym.id != b[1].sym.id`, to + ## decide whether two accessor chains touch the same field) and so CANNOT be + ## migrated as written. What codegen actually consumes for a field is the + ## name it re-navigates the reclist with (`lookupFieldAgain`) plus, for + ## tuples, the position — which is also the tolerance the grinder applies — + ## so the fix is either to compare fields that way or to give a field token + ## a stable identity. The latter needs the token's own position as a key, + ## and `nifcore.Cursor` keeps that pointer private, so it is not something + ## this module can do alone. result = symAt(currentNav()[], n.raw) proc symTyp(n: BNode): PType = @@ -610,6 +631,8 @@ when defined(newIcBackend): # the body in a template and these two instantiate it. There is no second # copy of the logic — change the template and both spellings change. + proc getInt*(n: BNode): Int128 = getIntImpl(n) + proc canRaiseConservative*(fn: BNode): bool = canRaiseConservativeImpl(fn) proc canRaise*(fn: BNode): bool = canRaiseImpl(fn) diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index 440314509f..882cb03e8e 100644 --- a/compiler/ccgcalls.nim +++ b/compiler/ccgcalls.nim @@ -38,6 +38,13 @@ proc canRaiseDisp(p: BProc; n: AnyNode): bool = logCanRaise(n.sym, result) proc preventNrvo(p: BProc; dest, le, ri: PNode): bool = + ## STAYS on `PNode`, and the reason is a capability the seam does not have + ## rather than an accessor it is missing: the `warnObservableStores` message + ## interpolates `$le`, i.e. it RENDERS the node. Rendering is `renderer.nim` + ## reconstructing source text, which is a different job from reading a node's + ## kind/sym/type, and nothing needs it until a diagnostic does. The alias + ## analysis this calls (`isPartOf`) is already `AnyNode`, so only the message + ## is in the way. proc locationEscapes(p: BProc; le: PNode; inTryStmt: bool): bool = result = false var n = le diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 1db60d6025..dc400dbf55 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -1950,7 +1950,7 @@ proc genObjConstr(p: BProc, e: PNode, d: var TLoc) = proc lhsDoesAlias(a, b: PNode): bool = result = false - for y in b: + for y in sons(b): if isPartOf(a, y) != arNo: return true proc genSeqConstr(p: BProc, n: PNode, d: var TLoc) = diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 45d035af5b..b707a774e2 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -1622,6 +1622,22 @@ when defined(newIcBackend): check "isDeepConstExpr", isDeepConstExpr(n) check "stmtsContainPragma", stmtsContainPragma(n, wLinearScanEnd) check "notYetAlive", notYetAlive(n) + check "getInt", (if n.kind in nkIntLits: $getInt(n) else: "") + check "sameValue self", sameValue(n, n) + + # `sym` IS NOT A FUNCTION OF ITS ARGUMENT for object fields, so this asserts + # the property the rest of the seam quietly assumes everywhere else. Two + # calls on the SAME token mint two `skField` stubs with consecutive item + # ids (`loadFieldStub`, by design: two distinct fields can share a name and + # a position across types, so one shared stub would mistype one of them). + # Anything that reads a field sym twice and compares identity is therefore + # wrong on a cursor and right on an AST — which is exactly how the attempt + # to migrate `aliases.isPartOf` failed, and it failed LOUDLY only because + # this grinder existed. Left as a live check so the day it starts holding + # is visible. + if a.kind == nkSym and a.sym != nil and a.sym.kind != skField: + if c.sym != c.sym: + bail("sym is not idempotent", "two different PSyms", "one PSym") # `stmtsContainPragma` had to be re-derived rather than defined as # `getPragmaStmt(...) != nil`, because a `Cursor` has no nil to return (see