diff --git a/compiler/ast.nim b/compiler/ast.nim index b6cef5c6ed..7476d7025c 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -1735,15 +1735,64 @@ const magicsThatCanRaise* = { # templates and `bnode.nim` instantiates them for its own node type: one source # of truth, no import cycle, and no second copy to keep in sync. # -# `fn.typ.n` below is a *type's* formal-params node, not a routine body: it is -# always fully materialised, so indexing it is not the hazard that indexing a -# body node is. +# The effect list is reached through `effectsOf` / `raisesNothing` rather than +# by subscripting `fn.typ.n`, so the templates below contain no knowledge of the +# layout and the `BNode` instantiation inherits none. `fn.typ` stays a `PType` +# in both spellings -- there is deliberately no `BType` (see `bnode.nim`) -- so +# what "works on a `.bif`" means for these two is that the type the decoder +# materialises must carry the same effect list the from-source one did. That is +# a claim about the WRITER, not about the vocabulary, and it is checked +# separately: `-d:icCanRaiseLog` logs every answer, and the same program built +# with and without `--ic:on` must produce the same verdicts. + +when defined(icCanRaiseLog): + var canRaiseBranch* = 0 + ## Which branch decided the last answer: 1 = the symbol's magic/flags, + ## 2 = `mEcho`, 3 = the EFFECT LIST reached through `effectsOf`, 4 = the + ## conservative predicate, 5 = short-circuited in `canRaiseDisp` before + ## either predicate ran, 0 = fell through. Only branch 3 reads anything + ## that had to survive a `.bif` round trip, so a differential in which no + ## callee reaches it would prove nothing about the writer — which is the + ## whole point of running the differential. See `-d:icCanRaiseLog`. + +template markCanRaiseBranch*(n: int) = + when defined(icCanRaiseLog): canRaiseBranch = n template canRaiseConservativeImpl*(fnArg: typed): bool = block: let fn = fnArg + markCanRaiseBranch 4 not (fn.kind == nkSym and fn.sym.magic notin magicsThatCanRaise) +proc effectsOf*(t: PType): PNode {.inline.} = + ## The `nkEffectList` a proc type carries as child 0 of its formal-params + ## node, with the parameters following from index 1 (`newProcType` builds it + ## that way; `cgen` reads the params back with `sonsFrom(prc.typ.n, 1)`). + ## + ## Named rather than subscripted so that the layout is written down in ONE + ## place. `.n` here is a TYPE's node, never a routine body, so it is always + ## fully materialised and `firstSon` is safe — the `nfLazyBody` hazard that + ## makes raw child access dangerous elsewhere (see `astdef.sons`) cannot reach + ## it. A proc type always has this child; `t.n` with no children is not a + ## shape the writer or sem produces, and this deliberately does not paper over + ## one appearing. + result = if t.n == nil: nil else: t.n.firstSon + +proc raisesNothing*(effects: PNode): bool = + ## Whether an effect list says DEFINITIVELY that nothing is raised: it is long + ## enough to have a raises slot at all, the slot is present, and it is empty. + ## + ## Every other shape — a list too short to carry the slot, an absent slot, a + ## non-empty one — means the effects are unspecified or non-empty, and a + ## caller must assume a raise. Stating it as the NEGATIVE is the point: the + ## safe default has to be "can raise", so the one narrow case that licenses + ## dropping an exception check is the one spelled out here, and a shape nobody + ## anticipated falls on the conservative side by construction rather than by + ## luck. + result = effects != nil and effects.len >= effectListLen and + effects[exceptionEffects] != nil and + effects[exceptionEffects].safeLen == 0 + template canRaiseImpl*(fnArg: typed): bool = block: let fn = fnArg @@ -1751,22 +1800,27 @@ template canRaiseImpl*(fnArg: typed): bool = if fn.kind == nkSym and (fn.sym.magic notin magicsThatCanRaise or {sfImportc, sfInfixCall} * fn.sym.flags == {sfImportc} or sfGeneratedOp in fn.sym.flags): + markCanRaiseBranch 1 res = false elif fn.kind == nkSym and fn.sym.magic == mEcho: + markCanRaiseBranch 2 res = true elif fn.typ != nil and fn.typ.kind == tyProc and fn.typ.n != nil: - # TODO check for n having sons? or just return false for now if not - if fn.typ.n[0].kind == nkSym: + markCanRaiseBranch 3 + let effects = effectsOf(fn.typ) + if effects.kind == nkSym: + # The historical shape: slot 0 used to be an `nkType` before the effects + # moved in (see `newProcType`). Nothing to read, so nothing licenses a + # raise. res = false else: # A proc-typed value with no explicit raises slot still has # unspecified effects, which sempass2 treats conservatively. # Codegen needs to do the same in order to keep goto-exception # checks after indirect/closure calls. - res = ((fn.typ.n[0].len < effectListLen) or - fn.typ.n[0][exceptionEffects] == nil or - fn.typ.n[0][exceptionEffects].safeLen > 0) + res = not raisesNothing(effects) else: + markCanRaiseBranch 0 res = false res diff --git a/compiler/ast2nif.nim b/compiler/ast2nif.nim index 3183b37bc2..dae74b2c87 100644 --- a/compiler/ast2nif.nim +++ b/compiler/ast2nif.nim @@ -3280,18 +3280,23 @@ proc loadNode(c: var DecodeContext; n: var Cursor; thisModule: string; s = c.loadSymStub(n, thisModule, localSyms) result = newSymNode(s, info) result.typField = typ - if typ == nil: - # `(ht . )` — an EXPLICITLY nil node type. Without this the - # node's type is LOAD-ORDER DEPENDENT: `newSymNode` above marks the - # node lazy only when the symbol was still an unloaded stub at this - # moment, so the very same `.bif` node answers `sym.typ` or `nil` - # for `n.typ` depending on whether something else happened to touch - # that symbol first. Pin it to the lazy reading — the one `newSymNode` - # exists to provide (see `nifcBackendActive` in astdef: snapshotting - # a nil leaves the node permanently typeless and the backend then - # reads `t.flags` off it) — so the answer is a property of the file, - # not of the traversal order. - result.flags.incl nfLazyType + # `(ht . )` — an EXPLICITLY nil node type — is left exactly as the + # writer meant it: NIL. The wrapper is only emitted when the node's own + # type differed from its symbol's (`writeSymNode`), so a nil here says + # the node genuinely had no type while the symbol had one, and that is + # load-bearing: a type symbol used as a VALUE (`newException(KeyError, + # ...)`) is exactly that shape, and handing it `sym.typ` makes sem read + # the typedesc as an expression of the type it denotes ("only a 'ref + # object' can be raised"). + # + # There IS a load-order dependence here — `newSymNode` above marks the + # node lazy when the symbol was still an unloaded stub, so `ast.typ` + # answers `sym.typ` for that population and `nil` for the rest — and it + # is NOT fixed by pinning the flag either way: setting it breaks sem as + # above, and clearing it would strip the fallback from the stub + # population that `nifcBackendActive` exists to serve. Left alone + # deliberately; `bnode.typ` answers the faithful `nil` and the grinder + # excludes this one shape with the reason recorded there. elif tagIs(n, symDefTagName): let info = c.infos.oldLineInfo(n.info, cursorPool(n)) let name = n.firstSon diff --git a/compiler/bnode.nim b/compiler/bnode.nim index 1521412324..36a5fc7dc6 100644 --- a/compiler/bnode.nim +++ b/compiler/bnode.nim @@ -460,19 +460,20 @@ when defined(newIcBackend): # `(ht )`: the node type is spelled out because it differed # from the symbol's at write time. result = typeAt(currentNav()[], childCursor(c)) - if result == nil: - # `(ht . )` — an EXPLICITLY nil node type, which the writer tries - # hard not to emit but does for a sym whose own type was nil. The - # loader does not read it as nil either: `newSymNode(sym, info)` runs - # first and sets `nfLazyType` whenever the symbol was still an - # unloaded stub (`typImpl == nil`), and only then is `typField` - # overwritten with this nil — so `ast.typ` falls back to `sym.typ`. - # Mirror that. (The AST's answer is strictly speaking load-order - # dependent — a sym already loaded at that moment would leave - # `nfLazyType` clear and yield nil — which is a fragility of the - # loader, not of this mirror. The grinder walks every body in the - # dependency closure and this is the branch it lands on.) - result = symTyp(n) + # A nil here is `(ht . )`, an EXPLICITLY nil node type, and it is + # answered as nil — the writer only emits the wrapper when the node's + # type differed from its symbol's, so nil means the node really had + # none. Do NOT fall back to `sym.typ`: a type symbol used as a value + # (`newException(KeyError, ...)`) is exactly this shape, and giving it + # the symbol's type makes sem read the typedesc as an expression of the + # type it denotes. That was tried, and it broke `--ic:on` compilation of + # anything instantiating `tables.[]`. + # + # `ast.typ` may still answer `sym.typ` here, because the loader's + # `nfLazyType` marking depends on whether the symbol happened to be + # loaded already (see `ast2nif`). That is a pre-existing load-order + # dependence in the AST, not a disagreement this side can resolve, and + # the grinder excludes this shape for that reason. elif name == symNodeFlagsTagName: var inner = childCursor(c) skip inner @@ -488,6 +489,24 @@ when defined(newIcBackend): else: result = nil + proc hasExplicitNilType*(n: BNode): bool = + ## Whether this is the `(ht . )` shape — a sym node the writer gave an + ## EXPLICITLY nil type — after peeling any `(nflags ...)` wrapper, which is + ## how it usually arrives. See `typ` for why nil is the faithful answer and + ## why `ast.typ` may nonetheless say otherwise. + var c = n.raw + while nifcore.kind(c) == TagLit: + let tag = c.tags.tagName(cursorTagId(c)) + if tag == symNodeFlagsTagName: + var inner = childCursor(c) + skip inner # the node flags + c = inner + elif tag == hiddenTypeTagName: + return nifcore.kind(childCursor(c)) == DotToken + else: + return false + result = false + proc rawDesc*(n: BNode): string = ## What the token stream literally says here — the NIF token kind and, for a ## tag, its name. Diagnostics only: the vocabulary above is the interface, diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index 24e0cab2ee..28819f607c 100644 --- a/compiler/ccgcalls.nim +++ b/compiler/ccgcalls.nim @@ -11,6 +11,12 @@ proc canRaiseDisp(p: BProc; n: AnyNode): bool = # we assume things like sysFatal cannot raise themselves + # 5 = "decided here, neither predicate ran". Without resetting, the marker + # keeps whatever the PREVIOUS call left in it and the early return below + # attributes this answer to a branch that did not execute — which is how the + # first run of this differential came to claim effect-list coverage it did + # not have. + markCanRaiseBranch 5 if n.kind == nkSym and {sfNeverRaises, sfImportc, sfCompilerProc} * n.sym.flags != {}: result = false elif optPanics in p.config.globalOptions or @@ -21,6 +27,15 @@ proc canRaiseDisp(p: BProc; n: AnyNode): bool = else: # we have to be *very* conservative: result = canRaiseConservative(n) + when defined(icCanRaiseLog): + # `canRaise` reads the raises spec off `fn.typ.n`, and under `--ic:on` that + # node came back from a `.bif`. Whether it came back INTACT is not something + # the `BNode`/`PNode` grinder can answer — both spellings ask the same + # `PType` and so agree however wrong it is. The only oracle is the same + # program built without IC. Log the verdict per callee; the two builds must + # produce the same one. + if n.kind == nkSym: + logCanRaise(n.sym, result) proc preventNrvo(p: BProc; dest, le, ri: PNode): bool = proc locationEscapes(p: BProc; le: PNode; inTryStmt: bool): bool = diff --git a/compiler/cgen.nim b/compiler/cgen.nim index f8463de4d8..5f2c5a6ffc 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -1053,6 +1053,22 @@ proc initLocExprSingleUse(p: BProc, e: PNode): TLoc = result.flags.incl lfSingleUse expr(p, e, result) +when defined(icCanRaiseLog): + import std / syncio + + proc logCanRaise(s: PSym; verdict: bool) = + ## One line per verdict, keyed by name + disamb + OWNING MODULE, and carrying + ## the magic that usually decides the answer. + ## + ## The module is not decoration: `disamb` is a per-module counter, so `len.0` + ## names a different routine in every module that has one, and a key without + ## the module reports a collision as a disagreement. NOT the itemId — that is + ## a per-build counter and would make every line differ for no reason. + let m = getModule(s) + stderr.writeLine "CANRAISE " & s.name.s & "." & $s.disamb & "." & + (if m == nil: "?" else: m.name.s) & "|" & $verdict & "|" & $s.magic & + "|b" & $canRaiseBranch + include ccgcalls, "ccgstmts.nim" proc initFrame(p: BProc, procname, filename: Rope): Rope = @@ -1593,7 +1609,17 @@ when defined(newIcBackend): let ct = c.typ let at = a.typ - if (ct == nil) != (at == nil): + # `(ht . )` is the one shape where the two spellings may legitimately + # differ: the cursor answers the faithful `nil`, while `ast.typ` answers + # `sym.typ` for whichever nodes the loader happened to mark `nfLazyType` + # (see `bnode.typ`). Excluded rather than papered over — and narrowly: only + # when the cursor says nil AND the AST is saying exactly the symbol's type. + let htNilTyp = ct == nil and at != nil and a.kind == nkSym and + a.typField == nil and a.sym != nil and at == a.sym.typ and + c.hasExplicitNilType + if htNilTyp: + discard + elif (ct == nil) != (at == nil): bail("typ nil-ness", (if ct == nil: "nil" else: $ct.kind) & " raw=" & c.rawDesc, (if at == nil: "nil" else: $at.kind) & " kind=" & $a.kind &