mirror of
https://github.com/nim-lang/Nim.git
synced 2026-09-17 10:34:53 +00:00
compiler: derived type flags are exempt from IC (#26198)
`propagateToOwner` aborted with
ast.nim(1314, 9) `mask <= o2.flags` IC bug: sealed type missing
propagated flags
whenever a consumer module propagated `tfHasAsgn`/`tfHasOwned` into a
type another module had already sealed. The assert encoded the hope that
a producer's derivation is complete at seal time. It is not: these flags
are DERIVED -- a fixpoint over kind, MM config, elements and the
attached-op table -- so a consumer can legitimately discover one later.
The trigger in the wild is a generic alias, `Channel[TMsg] {.gcsafe.} =
RawChannel`, whose object only acquires `tfHasAsgn` at the first
`Channel[T]` instantiation, in another module and (under IC) another
process. Any program that opens a channel crashed the compiler under
`nim ic` as soon as a `=copy` hook was declared for the alias
(nim-lang/Nim#26182).
Partition the flags instead. `derivedTypeFlags` names the bookkeeping
bits -- tfHasAsgn, tfHasOwned, tfHasGCedMem, tfCheckedForDestructor --
and `ast.inclDerived` / `ast.exclDerived` write exactly those without
the `Sealed` assert. The exemption is sound: none of them is in
`eqTypeFlags`, the `typekeys` content key or the NIF name, so they
cannot rename a type, move it in the cache or change `sameType`; and
`ast2nif.writeType` emits a `SymUse` for any type the current module
does not own, so a consumer's write can never reach a NIF. They stay
serialized, so a reload starts from the producer's derivation and a
consumer only ever adds to it.
`tfGenericHasDestructor` is deliberately NOT in the set: it is an alias
for `tfExplicitCallConv`, and exempting it would exempt a real proc-type
property.
This also gives the four existing `flagsImpl` writes a name. They wrote
the raw field precisely to dodge this assert -- liftdestructors' one
carried `# ^ XXX Breaks IC!` -- and are now `inclDerived`/`exclDerived`
calls, so the exemption is typed, asserted against `derivedTypeFlags`,
and greppable. `-d:icDerivedBarrier` reports every derived-flag write
onto a sealed type; a full `nim ic` build of a channel program produces
25, of which 24 are the `tfCheckedForDestructor` writes that were
already happening silently.
This commit is contained in:
@@ -459,6 +459,29 @@ proc excl*(t: PType; flags: set[TTypeFlag]) {.inline.} =
|
||||
if t.state == Partial: loadType(t)
|
||||
t.flagsImpl.excl(flags)
|
||||
|
||||
proc inclDerived*(t: PType; flags: TTypeFlags) {.inline.} =
|
||||
## Add DERIVED bookkeeping flags (see `derivedTypeFlags`). Unlike `incl` this
|
||||
## is allowed on a `Sealed` type: the bit is not part of the type's identity,
|
||||
## its NIF name or its content key, and a consumer never rewrites a foreign
|
||||
## type's def (`ast2nif.writeType` emits a `SymUse` for anything it does not
|
||||
## own), so the write cannot reach any NIF. It is process-local bookkeeping
|
||||
## that a consumer re-derives from data the producer already serialized.
|
||||
assert flags <= derivedTypeFlags
|
||||
if t.state == Partial: loadType(t)
|
||||
when defined(icDerivedBarrier):
|
||||
if t.state == Sealed and not (flags <= t.flagsImpl):
|
||||
echo "[derived-barrier] +", flags - t.flagsImpl, " on sealed ", t.kind,
|
||||
(if t.symImpl != nil: "/" & t.symImpl.name.s else: ""),
|
||||
" @", t.itemId.module, ".", t.itemId.item
|
||||
t.flagsImpl.incl(flags)
|
||||
|
||||
proc exclDerived*(t: PType; flags: TTypeFlags) {.inline.} =
|
||||
## Counterpart of `inclDerived`. Same reasoning; `tfCheckedForDestructor` is
|
||||
## genuinely cleared again (`injectdestructors`), so this is not monotone.
|
||||
assert flags <= derivedTypeFlags
|
||||
if t.state == Partial: loadType(t)
|
||||
t.flagsImpl.excl(flags)
|
||||
|
||||
proc typ*(n: PNode): lent PType {.inline.} =
|
||||
result = n.typField
|
||||
if result == nil and nfLazyType in n.flags:
|
||||
@@ -1308,13 +1331,15 @@ proc propagateToOwner*(owner, elem: PType; propagateHasAsgn = true) =
|
||||
let o2 = owner.skipTypes({tyGenericInst, tyAlias, tySink})
|
||||
if o2.kind in {tyTuple, tyObject, tyArray,
|
||||
tySequence, tyString, tySet, tyDistinct}:
|
||||
if o2.state == Sealed:
|
||||
# During the original compilation, propagateToOwner set tfHasAsgn/tfHasOwned on the type before it was sealed
|
||||
# On IC reload, the sealed type already has those flags
|
||||
assert mask <= o2.flags, "IC bug: sealed type missing propagated flags"
|
||||
else:
|
||||
o2.incl mask
|
||||
owner.incl mask
|
||||
# `o2` may be `Sealed`: these are DERIVED flags, and a consumer can reach
|
||||
# a foreign type here that the producer sealed without them. The classic
|
||||
# case is a generic alias -- `Channel[TMsg] = RawChannel` -- where
|
||||
# `normalizeTypeHook` leaves `tfHasAsgn` on the `tyAlias` and the object
|
||||
# behind it only acquires it at the first instantiation, in another
|
||||
# module and, under IC, another process. Adding the bit is sound (see
|
||||
# `inclDerived`); asserting here just crashed on legal code.
|
||||
o2.inclDerived mask
|
||||
owner.inclDerived mask
|
||||
|
||||
if owner.kind notin {tyProc, tyGenericInst, tyGenericBody,
|
||||
tyGenericInvocation, tyPtr}:
|
||||
@@ -1322,7 +1347,7 @@ proc propagateToOwner*(owner, elem: PType; propagateHasAsgn = true) =
|
||||
if elemB.isGCedMem or tfHasGCedMem in elemB.flags:
|
||||
# for simplicity, we propagate this flag even to generics. We then
|
||||
# ensure this doesn't bite us in sempass2.
|
||||
owner.incl tfHasGCedMem
|
||||
owner.inclDerived {tfHasGCedMem}
|
||||
|
||||
proc rawAddSon*(father, son: PType; propagateHasAsgn = true) =
|
||||
ensureMutable father
|
||||
|
||||
@@ -473,6 +473,26 @@ const
|
||||
## tyGenericBody where an instance has a generated destructor
|
||||
skError* = skUnknown
|
||||
|
||||
const
|
||||
derivedTypeFlags* = {tfHasAsgn, tfHasOwned, tfHasGCedMem, tfCheckedForDestructor}
|
||||
## Codegen/lifting BOOKKEEPING bits, as opposed to the flags that make a
|
||||
## type what it is. They are *derived*: a fixpoint over the type's own kind,
|
||||
## the memory management config, its elements and the attached-op table --
|
||||
## never something the source said. They are in none of `eqTypeFlags`, the
|
||||
## `typekeys` content key or the NIF name, so changing one cannot rename a
|
||||
## type, move it in the cache, or alter `sameType`.
|
||||
##
|
||||
## Because they are derived rather than declared, a consumer module can
|
||||
## legitimately discover one *after* the defining module sealed the type
|
||||
## (the classic case: an alias carries `tfHasAsgn` into the NIF but the
|
||||
## object behind it only gets it at the first generic instantiation, which
|
||||
## happens in another module -- and, under IC, another process). Writing
|
||||
## one is therefore exempt from the `Sealed` assert: see `ast.inclDerived`.
|
||||
##
|
||||
## NOT in this set even though it looks like it belongs:
|
||||
## `tfGenericHasDestructor`, which is an ALIAS for `tfExplicitCallConv`.
|
||||
## Exempting it would exempt a real proc-type property from the seal.
|
||||
|
||||
var
|
||||
eqTypeFlags* = {tfIterator, tfNotNil, tfVarIsPtr, tfGcSafe, tfNoSideEffect, tfIsOutParam}
|
||||
## type flags that are essential for type equality.
|
||||
|
||||
@@ -242,7 +242,7 @@ proc genOp(c: var Con; t: PType; kind: TTypeAttachedOp; dest, ri: PNode): PNode
|
||||
# closure-env identity resolves via `attachedOps[itemId]`/env-erased typeKey,
|
||||
# env objects load complete, and atomicRefOp's type-erased path covers any
|
||||
# still-incomplete env (so the lift never walks a nil field).
|
||||
excl t.flagsImpl, tfCheckedForDestructor
|
||||
t.exclDerived {tfCheckedForDestructor}
|
||||
createTypeBoundOps(c.graph, nil, t, dest.info, c.idgen)
|
||||
op = getAttachedOp(c.graph, t, kind)
|
||||
if op == nil:
|
||||
|
||||
@@ -1463,7 +1463,7 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
|
||||
## The later 'injectdestructors' pass depends on it.
|
||||
if orig == nil or {tfCheckedForDestructor, tfHasMeta} * orig.flags != {}: return
|
||||
# IC: review this solution again later
|
||||
incl orig.flagsImpl, tfCheckedForDestructor
|
||||
orig.inclDerived {tfCheckedForDestructor}
|
||||
# for user defined generic destructors:
|
||||
let origRoot = genericRoot(orig)
|
||||
if origRoot != nil:
|
||||
@@ -1513,6 +1513,8 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
|
||||
if not isTrivial(getAttachedOp(g, orig, attachedDestructor)):
|
||||
#or not isTrivial(orig.assignment) or
|
||||
# not isTrivial(orig.sink):
|
||||
# IC: review this solution again later
|
||||
orig.flagsImpl.incl tfHasAsgn
|
||||
# ^ XXX Breaks IC!
|
||||
# A hook was lifted for `orig` in THIS module, which may be a module that
|
||||
# merely uses the type. `inclDerived` is the sanctioned way to record that
|
||||
# on a possibly-`Sealed` foreign type (it used to write `flagsImpl` behind
|
||||
# the accessor's back precisely to dodge the seal assert).
|
||||
orig.inclDerived {tfHasAsgn}
|
||||
|
||||
@@ -486,7 +486,7 @@ proc makeVarType*(c: PContext, baseType: PType; kind = tyVar): PType =
|
||||
|
||||
proc makeTypeSymNode*(c: PContext, typ: PType, info: TLineInfo): PNode =
|
||||
let typedesc = newTypeS(tyTypeDesc, c)
|
||||
incl typedesc.flagsImpl, tfCheckedForDestructor
|
||||
typedesc.inclDerived {tfCheckedForDestructor}
|
||||
internalAssert(c.config, typ != nil)
|
||||
typedesc.addSonSkipIntLit(typ, c.idgen)
|
||||
let sym = newSym(skType, c.cache.idAnon, c.idgen, getCurrOwner(c), info,
|
||||
@@ -565,7 +565,7 @@ template rangeHasUnresolvedStatic*(t: PType): bool =
|
||||
proc errorType*(c: PContext): PType =
|
||||
## creates a type representing an error state
|
||||
result = newTypeS(tyError, c)
|
||||
result.flagsImpl.incl tfCheckedForDestructor
|
||||
result.inclDerived {tfCheckedForDestructor}
|
||||
|
||||
proc errorNode*(c: PContext, n: PNode): PNode =
|
||||
result = newNodeI(nkEmpty, n.info)
|
||||
|
||||
@@ -24,7 +24,7 @@ when defined(nimPreviewSlimSystem):
|
||||
proc errorType*(g: ModuleGraph): PType =
|
||||
## creates a type representing an error state
|
||||
result = newType(tyError, g.idgen, g.owners[^1])
|
||||
result.flagsImpl.incl tfCheckedForDestructor
|
||||
result.inclDerived {tfCheckedForDestructor}
|
||||
|
||||
proc getIntLitTypeG(g: ModuleGraph; literal: PNode; idgen: IdGenerator): PType =
|
||||
# we cache some common integer literal types for performance:
|
||||
|
||||
Reference in New Issue
Block a user