Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgirgen: remove the var expression fixup #1482

Merged
merged 7 commits into from
Dec 27, 2024
Merged

cgirgen: remove the var expression fixup #1482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b010279
cgirgen: remove the `var` parameter argument fixup
zerbina Dec 25, 2024
2fe1d70
liftdestructors: remove the incorrect addr/deref folding
zerbina Dec 25, 2024
4c8d983
semexprs: fix argument hoisting producing ill-typed expressions
zerbina Dec 25, 2024
b6cbe85
liftdestructors: fix the call AST construction
zerbina Dec 26, 2024
410630b
cgmeth: fix the call AST construction
zerbina Dec 26, 2024
0544fed
cgirgen: inline the `wrapInHiddenAddr` procedure
zerbina Dec 26, 2024
49ade0e
small style adjustments and spelling fixes
zerbina Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions compiler/backend/cgirgen.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,18 +166,6 @@ proc newDefaultCall(info: TLineInfo, typ: PType): CgNode =
## Produces the tree for a ``default`` magic call.
newExpr(cnkCall, info, typ, [newMagicNode(mDefault, info)])

proc wrapInHiddenAddr(cl: TranslateCl, n: CgNode): CgNode =
## Restores the ``cnkHiddenAddr`` around lvalue expressions passed to ``var``
## parameters. The code-generators operating on ``CgNode``-IR depend on the
## hidden addr to be present
if n.typ.skipTypes(abstractInst).kind != tyVar:
newOp(cnkHiddenAddr, n.info, makeVarType(cl.owner, n.typ, cl.idgen), n)
else:
# XXX: is this case ever reached? It should not be. Raw ``var`` values
# must never be passed directly to ``var`` parameters at the MIR
# level
n

proc genObjConv(n: CgNode, to: PType, info: TLineInfo): CgNode =
## Depending on the type relationship between `n` and `to`, wraps `n` in
## either an up- or down-conversion. Returns `nil` if no up- or down-
Expand Down Expand Up @@ -419,7 +407,10 @@ proc callToIr(tree: MirBody, cl: var TranslateCl, n: MirNode,
# XXX: prevent this case from happening
arg = newOp(cnkDerefView, arg.info, arg.typ.base, arg)
elif mutable:
arg = wrapInHiddenAddr(cl, arg)
# much like in PNode AST, the CGIR AST also needs a ``cnkHiddenAddr``
# tree wrapped around expressions in var argument positions
arg = newOp(cnkHiddenAddr, arg.info,
makeVarType(cl.owner, arg.typ, cl.idgen), arg)

result.add arg

Expand Down
16 changes: 12 additions & 4 deletions compiler/backend/cgmeth.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,23 @@ proc genDispatcher(g: ModuleGraph; methods: seq[PSym], relevantCols: IntSet) =
if param.typ.skipTypes(abstractInst).kind in {tyRef, tyPtr}:
nilchecks.add newTree(nkCall,
newSymNode(getCompilerProc(g, "chckNilDisp")), newSymNode(param))

proc param(s: PSym): PNode {.nimcall.} =
if s.typ.kind == tyVar:
newTreeIT(nkHiddenAddr, s.info, s.typ,
newTreeIT(nkHiddenDeref, s.info, s.typ.base,
newSymNode(s)))
else:
newSymNode(s)

for meth in 0..high(methods):
var curr = methods[meth] # generate condition:
var cond: PNode = nil
for col in 1..<paramLen:
if contains(relevantCols, col):
var isn = newNodeIT(nkCall, base.info, boolType)
isn.add newSymNode(iss)
let param = base.typ.n[col].sym
isn.add newSymNode(param)
isn.add param(base.typ.n[col].sym)
isn.add newNodeIT(nkType, base.info, curr.typ[col])
if cond != nil:
var a = newNodeIT(nkCall, base.info, boolType)
Expand All @@ -306,8 +314,8 @@ proc genDispatcher(g: ModuleGraph; methods: seq[PSym], relevantCols: IntSet) =
let call = newNodeIT(nkCall, base.info, retTyp)
call.add newSymNode(curr)
for col in 1..<paramLen:
call.add genConv(newSymNode(base.typ.n[col].sym),
curr.typ[col], true, g.config)
call.add genConv(param(base.typ.n[col].sym),
curr.typ[col], true, g.config)
var ret: PNode
if retTyp != nil:
var a = newNodeI(nkFastAsgn, base.info)
Expand Down
16 changes: 4 additions & 12 deletions compiler/sem/liftdestructors.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,7 @@ proc defaultOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
body.add newAsgnStmt(x, call)

proc genAddr(c: var TLiftCtx; x: PNode): PNode =
if x.kind == nkHiddenDeref:
checkSonsLen(x, 1, c.g.config)
result = x[0]
else:
result = newTreeIT(nkHiddenAddr, x.info, makeVarType(x.typ.owner, x.typ, c.idgen)): x
newTreeIT(nkHiddenAddr, x.info, makeVarType(x.typ.owner, x.typ, c.idgen), x)

proc genWhileLoop(c: var TLiftCtx; i, dest: PNode): PNode =
let cmp = genBuiltin(c, mLtI, "<", i)
Expand Down Expand Up @@ -268,12 +264,7 @@ proc getCycleParam(c: TLiftCtx): PNode =
proc newHookCall(c: var TLiftCtx; op: PSym; x, y: PNode): PNode =
#if sfError in op.flags:
# localReport(c.config, x.info, "usage of '$1' is a user-defined error" % op.name.s)
result = newNodeI(nkCall, x.info)
result.add newSymNode(op)
if op.typ.sons[1].kind == tyVar:
result.add genAddr(c, x)
else:
result.add x
result = newTreeI(nkCall, x.info, newSymNode(op), genAddr(c, x))
if y != nil:
result.add y
if op.typ.len == 4:
Expand Down Expand Up @@ -889,7 +880,8 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
if kind == attachedSink and destructorOverriden(g, typ):
## compiler can use a combination of `=destroy` and memCopy for sink op
dest.flags.incl sfCursor
result.ast[bodyPos].add newOpCall(a, getAttachedOp(g, typ, attachedDestructor), d[0])
result.ast[bodyPos].add newOpCall(a, getAttachedOp(g, typ, attachedDestructor),
genAddr(a, d))
result.ast[bodyPos].add newAsgnStmt(d, src)
else:
let (tk, baseTyp) =
Expand Down
18 changes: 15 additions & 3 deletions compiler/sem/semexprs.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3491,10 +3491,22 @@ proc hoistParamsUsedInDefault(c: PContext, call, letSection, defExpr: var PNode)
newNodeI(nkEmpty, letSection.info),
call[paramPos])

call[paramPos] = newSymNode(hoistedVarSym) # Refer the original arg to its hoisted sym
if hoistedVarSym.typ.kind == tyVar:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to consider any skipTypes/unwrapping because this is after generic resolution etc, yes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In part, yes. There's also the assumption throughout parts of the compiler that tyVar cannot appear be nested in a tyAlias or the like.

# only ``HiddenAddr`` expressions may be of ``var`` type in argument
# positions
call[paramPos] =
newTreeIT(nkHiddenAddr, letSection.info, hoistedVarSym.typ,
newDeref(newSymNode(hoistedVarSym)))
else:
# Refer the original arg to its hoisted sym
call[paramPos] = newSymNode(hoistedVarSym)

# arg we refer to is a sym, wether introduced by hoisting or not doesn't matter, we simply reuse it
defExpr = call[paramPos]
# arg is either a sym, whether introduced by hoisting or not doesn't
# matter, or a ``(HiddenAddr (HiddenDeref sym))`` introduced by hoisting
if call[paramPos].kind == nkHiddenAddr:
defExpr = call[paramPos][0][0] # retrieve the symbol
else:
defExpr = call[paramPos] # must be a symbol
else:
for i in 0..<defExpr.safeLen:
hoistParamsUsedInDefault(c, call, letSection, defExpr[i])
Expand Down
Loading