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.

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

fix: crash when passing NimNode to static parameter #1449

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions compiler/mir/mirgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2490,6 +2490,8 @@ proc constDataToMir*(env: var MirEnv, n: PNode): MirTree =
bu.use toFloatLiteral(env, n)
of nkStrLiterals:
bu.use strLiteral(env, n.strVal, typ)
of nkNimNodeLit:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yay, nkNimNodeLit is becoming more pervasive, this means fewer places where AST is ambiguously passed around.

bu.use astLiteral(env, n[0], n.typ)
of nkHiddenStdConv, nkHiddenSubConv:
# doesn't translate to a MIR node itself, but the type overrides
# that of the sub-expression
Expand Down
15 changes: 11 additions & 4 deletions compiler/vm/vm.nim
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,11 @@ proc regToNode*(c: TCtx, x: TFullReg; typ: PType, info: TLineInfo): PNode =
# TODO: validate the address
result = c.deserialize(c.allocator.makeLocHandle(x.addrVal, x.addrTyp), typ, info)
of rkHandle, rkLocation: result = c.deserialize(x.handle, typ, info)
of rkNimNode: result = x.nimNode
of rkNimNode:
if typ.sym != nil and typ.sym.magic == mPNimrodNode:
result = c.deserializeNimNode(x.nimNode, typ, info)
else:
result = x.nimNode

# ---- exception handling ----

Expand Down Expand Up @@ -2245,7 +2249,6 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason =

of opcRepr:
# Turn the provided value into its string representation. Used for:
# - implementing the general ``repr`` when not using the ``repr`` v2
# - rendering an AST to its text representation (``repr`` for
# ``NimNode``)
# - rendering the discriminant value for a ``FieldDefect``'s message
Expand All @@ -2261,9 +2264,13 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason =

checkHandle(regs[rb])

let str = renderTree(c.regToNode(regs[rb], typ.nimType, c.debug[pc]),
{renderNoComments, renderDocComments})
let n =
if typ.nimType.sym != nil and typ.nimType.sym.magic == mPNimrodNode:
regs[rb].nimNode
else:
c.regToNode(regs[rb], typ.nimType, c.debug[pc])

let str = renderTree(n, {renderNoComments, renderDocComments})
regs[ra].strVal.newVmString(str, c.allocator)
of opcQuit:
return YieldReason(kind: yrkQuit, exitCode: regs[ra].intVal.int)
Expand Down
19 changes: 11 additions & 8 deletions compiler/vm/vmcompilerserdes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,16 @@ proc deserializeArray*(
if hasError:
result = c.config.wrapError(result)

proc deserializeNimNode*(c: TCtx, n: PNode, formal: PType,
info: TLineInfo): PNode =
## Makes sure `n` (which represent a NimNode value) is valid and turns it
## into a proper NimNode literal.
if cyclicTree(n):
result = c.config.newError(
newNodeIT(nkEmpty, info, formal),
PAstDiag(kind: adCyclicTree, cyclic: n))
else:
result = newTreeIT(nkNimNodeLit, info, formal): n
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't see this as an issue now, and probably ever, but it looks like we're going to treat nkNimNodeLit to mean either a literal/static AST value or dynamic AST value. Where the latter one is generated on the fly, at compile time.


proc deserialize(c: TCtx, m: VmMemoryRegion, vt: PVmType, formal, t: PType, info: TLineInfo): PNode =
let
Expand Down Expand Up @@ -270,14 +280,7 @@ proc deserialize(c: TCtx, m: VmMemoryRegion, vt: PVmType, formal, t: PType, info
result = deserializeRef(c, atom.refVal, vt.targetType, formal, t, info)
else:
assert vt.kind == akPNode

if unlikely(cyclicTree(atom.nodeVal)):
result = c.config.newError(
wrongNode(),
PAstDiag(kind: adCyclicTree, cyclic: atom.nodeVal))
else:
# XXX: not doing a full tree-copy here might lead to issues
result = newTreeIT(nkNimNodeLit, info, formal): atom.nodeVal
result = deserializeNimNode(c, atom.nodeVal, formal, info)

of tyProc:
case t.callConv
Expand Down
14 changes: 14 additions & 0 deletions tests/lang_callable/macros/tstatic_nim_node_parameter.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
discard """
description: '''
Ensure that a NimNode can be passed as a static parameter to a macro.
'''
action: compile
"""

import std/macros

macro m(n: static NimNode) =
doAssert n.kind == nnkStmtList
doAssert n.len == 0

m(newStmtList())
Loading