From 0dd198278e5010f742e447eb901f98c0d21c6fe8 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:24:19 +0800 Subject: [PATCH] overhaul hook injections (#24841) ref https://github.com/nim-lang/Nim/issues/24764 To keep destructors injected consistently, we need to transform `mAsgn` properly into `nkSinkAsgn` and `nkAsgn`. This PR is the first step towards overhauling hook injections. In this PR, hooks (except mAsgn) are treated consistently whether it is resolved in matching or instantiated by sempass2. It also fixes a spelling `=wasMoved` to its normalized version, which caused no replacing generic hook calls with lifted hook calls. (cherry picked from commit 40a1ec21d78d48a0f012d552047e24326b04fc7a) --- compiler/injectdestructors.nim | 4 +- compiler/liftdestructors.nim | 29 +++--- compiler/semcall.nim | 8 -- compiler/semdata.nim | 165 ++++++++++++++++++++++++++++++++- compiler/semexprs.nim | 100 -------------------- compiler/semmagic.nim | 37 +------- compiler/sempass2.nim | 39 +++++--- lib/pure/streamwrapper.nim | 3 +- lib/system.nim | 4 +- 9 files changed, 212 insertions(+), 177 deletions(-) diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index 57949dbc4f..0a1ae76beb 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -24,7 +24,7 @@ import std/[strtabs, tables, strutils, intsets] when defined(nimPreviewSlimSystem): import std/assertions -from trees import exprStructuralEquivalent, getRoot, whichPragma +from trees import exprStructuralEquivalent, getRoot, whichPragma, getPotentialWrites type Con = object @@ -400,7 +400,7 @@ proc genWasMoved(c: var Con, n: PNode): PNode = result = genOp(c, op, n) else: result = newNodeI(nkCall, n.info) - result.add(newSymNode(createMagic(c.graph, c.idgen, "`=wasMoved`", mWasMoved))) + result.add(newSymNode(createMagic(c.graph, c.idgen, "wasMoved", mWasMoved))) result.add copyTree(n) #mWasMoved does not take the address #if n.kind != nkSym: # message(c.graph.config, n.info, warnUser, "wasMoved(" & $n & ")") diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index 49c06ce1d5..e6b2979dbd 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -91,7 +91,7 @@ proc defaultOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = call.typ() = t body.add newAsgnStmt(x, call) elif c.kind == attachedWasMoved: - body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc genAddr(c: var TLiftCtx; x: PNode): PNode = if x.kind == nkHiddenDeref: @@ -148,7 +148,7 @@ proc destructorCall(c: var TLiftCtx; op: PSym; x: PNode): PNode = if sfNeverRaises notin op.flags: c.canRaise = true if c.addMemReset: - result = newTree(nkStmtList, destroy, genBuiltin(c, mWasMoved, "`=wasMoved`", x)) + result = newTree(nkStmtList, destroy, genBuiltin(c, mWasMoved, "wasMoved", x)) else: result = destroy @@ -168,7 +168,7 @@ proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool, defaultOp(c, f.typ, body, x.dotField(f), b) else: if enforceWasMoved: - body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x.dotField(f)) + body.add genBuiltin(c, mWasMoved, "wasMoved", x.dotField(f)) fillBody(c, f.typ, body, x.dotField(f), b) of nkNilLit: discard of nkRecCase: @@ -277,7 +277,8 @@ proc fillBodyObjT(c: var TLiftCtx; t: PType, body, x, y: PNode) = #body.add newAsgnStmt(blob, x) var wasMovedCall = newNodeI(nkCall, c.info) - wasMovedCall.add(newSymNode(createMagic(c.g, c.idgen, "`=wasMoved`", mWasMoved))) + wasMovedCall.add(newSymNode(createMagic(c.g, c.idgen, "wasMoved", mWasMoved))) + wasMovedCall.add x # mWasMoved does not take the address body.add wasMovedCall @@ -612,7 +613,7 @@ proc fillSeqOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = if canFormAcycle(c.g, t.elemType): # follow all elements: forallElements(c, t, body, x, y) - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc useSeqOrStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = createTypeBoundOps(c.g, c.c, t, body.info, c.idgen) @@ -650,7 +651,7 @@ proc useSeqOrStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = if op == nil: return # protect from recursion body.add newHookCall(c, op, x, y) - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) of attachedDup: # XXX: replace these with assertions. let op = getAttachedOp(c.g, t, c.kind) @@ -672,7 +673,7 @@ proc fillStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.add genBuiltin(c, mDestroy, "destroy", x) of attachedTrace: discard "strings are atomic and have no inner elements that are to trace" - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc cyclicType*(g: ModuleGraph, t: PType): bool = case t.kind @@ -771,7 +772,7 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = # If the ref is polymorphic we have to account for this body.add callCodegenProc(c.g, "nimTraceRefDyn", c.info, genAddrOf(x, c.idgen), y) #echo "can follow ", elemType, " static ", isFinal(elemType) - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) of attachedDup: if isCyclic: body.add newAsgnStmt(x, y) @@ -838,7 +839,7 @@ proc atomicClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = of attachedDeepCopy: assert(false, "cannot happen") of attachedTrace: body.add callCodegenProc(c.g, "nimTraceRefDyn", c.info, genAddrOf(xenv, c.idgen), y) - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc weakrefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = case c.kind @@ -866,7 +867,7 @@ proc weakrefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.sons.insert(des, 0) of attachedDeepCopy: assert(false, "cannot happen") of attachedTrace: discard - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc ownedRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = var actions = newNodeI(nkStmtList, c.info) @@ -894,7 +895,7 @@ proc ownedRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.add genIf(c, x, actions) of attachedDeepCopy: assert(false, "cannot happen") of attachedTrace: discard - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc closureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = if c.kind == attachedDeepCopy: @@ -934,7 +935,7 @@ proc closureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.sons.insert(des, 0) of attachedDeepCopy: assert(false, "cannot happen") of attachedTrace: discard - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc ownedClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = let xx = genBuiltin(c, mAccessEnv, "accessEnv", x) @@ -952,7 +953,7 @@ proc ownedClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.add genIf(c, xx, actions) of attachedDeepCopy: assert(false, "cannot happen") of attachedTrace: discard - of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) = case t.kind @@ -1021,7 +1022,7 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) = of {attachedAsgn, attachedSink, attachedDup}: body.add newAsgnStmt(x, y) of attachedWasMoved: - body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x) + body.add genBuiltin(c, mWasMoved, "wasMoved", x) else: fillBodyObjT(c, t, body, x, y) else: diff --git a/compiler/semcall.nim b/compiler/semcall.nim index 0b1236b254..1ffe5aed4a 100644 --- a/compiler/semcall.nim +++ b/compiler/semcall.nim @@ -244,14 +244,6 @@ proc effectProblem(f, a: PType; result: var string; c: PContext) = if not c.graph.compatibleProps(c.graph, f, a): result.add "\n The `.requires` or `.ensures` properties are incompatible." -proc renderNotLValue(n: PNode): string = - result = $n - let n = if n.kind == nkHiddenDeref: n[0] else: n - if n.kind == nkHiddenCallConv and n.len > 1: - result = $n[0] & "(" & result & ")" - elif n.kind in {nkHiddenStdConv, nkHiddenSubConv} and n.len == 2: - result = typeToString(n.typ.skipTypes(abstractVar)) & "(" & result & ")" - proc presentFailedCandidates(c: PContext, n: PNode, errors: CandidateErrors): (TPreferedDesc, string) = var prefer = preferName diff --git a/compiler/semdata.nim b/compiler/semdata.nim index 6e256b3d32..1719d75404 100644 --- a/compiler/semdata.nim +++ b/compiler/semdata.nim @@ -9,14 +9,15 @@ ## This module contains the data structures for the semantic checking phase. -import std/[tables, intsets, sets] +import std/[tables, intsets, sets, strutils] when defined(nimPreviewSlimSystem): import std/assertions import options, ast, msgs, idents, renderer, - magicsys, vmdef, modulegraphs, lineinfos, pathutils, layeredtable + magicsys, vmdef, modulegraphs, lineinfos, pathutils, layeredtable, + types, lowerings, trees, parampatterns import ic / ic @@ -635,3 +636,163 @@ proc rememberExpansion*(c: PContext; info: TLineInfo; expandedSym: PSym) = ## delegated to the "rod" file mechanism. if c.config.symbolFiles != disabledSf: storeExpansion(c.encoder, c.packedRepr, info, expandedSym) + +const + errVarForOutParamNeededX = "for a 'var' type a variable needs to be passed; but '$1' is immutable" + errXStackEscape = "address of '$1' may not escape its stack frame" + +proc renderNotLValue*(n: PNode): string = + result = $n + let n = if n.kind == nkHiddenDeref: n[0] else: n + if n.kind == nkHiddenCallConv and n.len > 1: + result = $n[0] & "(" & result & ")" + elif n.kind in {nkHiddenStdConv, nkHiddenSubConv} and n.len == 2: + result = typeToString(n.typ.skipTypes(abstractVar)) & "(" & result & ")" + +proc isAssignable(c: PContext, n: PNode): TAssignableResult = + result = parampatterns.isAssignable(c.p.owner, n) + +proc newHiddenAddrTaken(c: PContext, n: PNode, isOutParam: bool): PNode = + if n.kind == nkHiddenDeref and not (c.config.backend == backendCpp or + sfCompileToCpp in c.module.flags): + checkSonsLen(n, 1, c.config) + result = n[0] + else: + result = newNodeIT(nkHiddenAddr, n.info, makeVarType(c, n.typ)) + result.add n + let aa = isAssignable(c, n) + let sym = getRoot(n) + if aa notin {arLValue, arLocalLValue}: + if aa == arDiscriminant and c.inUncheckedAssignSection > 0: + discard "allow access within a cast(unsafeAssign) section" + elif strictDefs in c.features and aa == arAddressableConst and + sym != nil and sym.kind == skLet and isOutParam: + discard "allow let varaibles to be passed to out parameters" + else: + localError(c.config, n.info, errVarForOutParamNeededX % renderNotLValue(n)) + +proc analyseIfAddressTaken(c: PContext, n: PNode, isOutParam: bool): PNode = + result = n + case n.kind + of nkSym: + # n.sym.typ can be nil in 'check' mode ... + if n.sym.typ != nil and + skipTypes(n.sym.typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}: + incl(n.sym.flags, sfAddrTaken) + result = newHiddenAddrTaken(c, n, isOutParam) + of nkDotExpr: + checkSonsLen(n, 2, c.config) + if n[1].kind != nkSym: + internalError(c.config, n.info, "analyseIfAddressTaken") + return + if skipTypes(n[1].sym.typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}: + incl(n[1].sym.flags, sfAddrTaken) + result = newHiddenAddrTaken(c, n, isOutParam) + of nkBracketExpr: + checkMinSonsLen(n, 1, c.config) + if skipTypes(n[0].typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}: + if n[0].kind == nkSym: incl(n[0].sym.flags, sfAddrTaken) + result = newHiddenAddrTaken(c, n, isOutParam) + else: + result = newHiddenAddrTaken(c, n, isOutParam) + +proc analyseIfAddressTakenInCall*(c: PContext, n: PNode, isConverter = false) = + checkMinSonsLen(n, 1, c.config) + if n[0].typ == nil: + # n[0] might be erroring node in nimsuggest + return + const + FakeVarParams = {mNew, mNewFinalize, mInc, ast.mDec, mIncl, mExcl, + mSetLengthStr, mSetLengthSeq, mAppendStrCh, mAppendStrStr, mSwap, + mAppendSeqElem, mNewSeq, mShallowCopy, mDeepCopy, mMove, mWasMoved} + + template checkIfConverterCalled(c: PContext, n: PNode) = + ## Checks if there is a converter call which wouldn't be checked otherwise + # Call can sometimes be wrapped in a deref + let node = if n.kind == nkHiddenDeref: n[0] else: n + if node.kind == nkHiddenCallConv: + analyseIfAddressTakenInCall(c, node, true) + # get the real type of the callee + # it may be a proc var with a generic alias type, so we skip over them + var t = n[0].typ.skipTypes({tyGenericInst, tyAlias, tySink}) + if n[0].kind == nkSym and n[0].sym.magic in FakeVarParams: + # BUGFIX: check for L-Value still needs to be done for the arguments! + # note sometimes this is eval'ed twice so we check for nkHiddenAddr here: + for i in 1.. 0: + discard "allow access within a cast(unsafeAssign) section" + else: + localError(c.config, it.info, errVarForOutParamNeededX % $it) + # Make sure to still check arguments for converters + c.checkIfConverterCalled(n[i]) + # bug #5113: disallow newSeq(result) where result is a 'var T': + if n[0].sym.magic in {mNew, mNewFinalize, mNewSeq}: + var arg = n[1] #.skipAddr + if arg.kind == nkHiddenDeref: arg = arg[0] + if arg.kind == nkSym and arg.sym.kind == skResult and + arg.typ.skipTypes(abstractInst).kind in {tyVar, tyLent}: + localError(c.config, n.info, errXStackEscape % renderTree(n[1], {renderNoComments})) + + return + for i in 1.. 0: - discard "allow access within a cast(unsafeAssign) section" - elif strictDefs in c.features and aa == arAddressableConst and - sym != nil and sym.kind == skLet and isOutParam: - discard "allow let varaibles to be passed to out parameters" - else: - localError(c.config, n.info, errVarForOutParamNeededX % renderNotLValue(n)) - -proc analyseIfAddressTaken(c: PContext, n: PNode, isOutParam: bool): PNode = - result = n - case n.kind - of nkSym: - # n.sym.typ can be nil in 'check' mode ... - if n.sym.typ != nil and - skipTypes(n.sym.typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}: - incl(n.sym.flags, sfAddrTaken) - result = newHiddenAddrTaken(c, n, isOutParam) - of nkDotExpr: - checkSonsLen(n, 2, c.config) - if n[1].kind != nkSym: - internalError(c.config, n.info, "analyseIfAddressTaken") - return - if skipTypes(n[1].sym.typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}: - incl(n[1].sym.flags, sfAddrTaken) - result = newHiddenAddrTaken(c, n, isOutParam) - of nkBracketExpr: - checkMinSonsLen(n, 1, c.config) - if skipTypes(n[0].typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}: - if n[0].kind == nkSym: incl(n[0].sym.flags, sfAddrTaken) - result = newHiddenAddrTaken(c, n, isOutParam) - else: - result = newHiddenAddrTaken(c, n, isOutParam) - -proc analyseIfAddressTakenInCall(c: PContext, n: PNode, isConverter = false) = - checkMinSonsLen(n, 1, c.config) - if n[0].typ == nil: - # n[0] might be erroring node in nimsuggest - return - const - FakeVarParams = {mNew, mNewFinalize, mInc, ast.mDec, mIncl, mExcl, - mSetLengthStr, mSetLengthSeq, mAppendStrCh, mAppendStrStr, mSwap, - mAppendSeqElem, mNewSeq, mShallowCopy, mDeepCopy, mMove, - mWasMoved} - - template checkIfConverterCalled(c: PContext, n: PNode) = - ## Checks if there is a converter call which wouldn't be checked otherwise - # Call can sometimes be wrapped in a deref - let node = if n.kind == nkHiddenDeref: n[0] else: n - if node.kind == nkHiddenCallConv: - analyseIfAddressTakenInCall(c, node, true) - # get the real type of the callee - # it may be a proc var with a generic alias type, so we skip over them - var t = n[0].typ.skipTypes({tyGenericInst, tyAlias, tySink}) - if n[0].kind == nkSym and n[0].sym.magic in FakeVarParams: - # BUGFIX: check for L-Value still needs to be done for the arguments! - # note sometimes this is eval'ed twice so we check for nkHiddenAddr here: - for i in 1.. 0: - discard "allow access within a cast(unsafeAssign) section" - else: - localError(c.config, it.info, errVarForOutParamNeededX % $it) - # Make sure to still check arguments for converters - c.checkIfConverterCalled(n[i]) - # bug #5113: disallow newSeq(result) where result is a 'var T': - if n[0].sym.magic in {mNew, mNewFinalize, mNewSeq}: - var arg = n[1] #.skipAddr - if arg.kind == nkHiddenDeref: arg = arg[0] - if arg.kind == nkSym and arg.sym.kind == skResult and - arg.typ.skipTypes(abstractInst).kind in {tyVar, tyLent}: - localError(c.config, n.info, errXStackEscape % renderTree(n[1], {renderNoComments})) - - return - for i in 1.. 0 and a.sym.name.s[0] == '=' and tracked.owner.kind != skMacro: - var opKind = find(AttachedOpToStr, a.sym.name.s.normalize) - if a.sym.name.s == "=": opKind = attachedAsgn.int - if opKind != -1: + var (isHook, opKind) = findHookKind(a.sym.name.s) + if isHook: # rebind type bounds operations after createTypeBoundOps call let t = n[1].typ.skipTypes({tyAlias, tyVar}) - if a.sym != getAttachedOp(tracked.graph, t, TTypeAttachedOp(opKind)): + if a.sym != getAttachedOp(tracked.graph, t, opKind): createTypeBoundOps(tracked, t, n.info, explicit = true) - let op = getAttachedOp(tracked.graph, t, TTypeAttachedOp(opKind)) - if op != nil: - n[0].sym = op - if TTypeAttachedOp(opKind) == attachedDestructor and - op.typ.len == 2 and op.typ.firstParamType.kind != tyVar: - if n[1].kind == nkSym and n[1].sym.kind == skParam and - n[1].typ.kind == tyVar: - n[1] = genDeref(n[1]) - else: - n[1] = skipAddr(n[1]) + # replace builtin hooks with lifted ones + n = replaceHookMagic(tracked.c, n, opKind) if op != nil and op.kind == tyProc: for i in 1..