Skip to content

Commit

Permalink
sem: replace addr calls with nkAddr (#1465)
Browse files Browse the repository at this point in the history
## Summary

In typed AST, address-of operations are now *always* represented by
`nkAddr` trees. This simplifies some compiler logic, makes processing
for typed macros easier, and fixes an effect tracking bug with `addr`.

## Details

This is effectively a revert of
nim-lang/Nim#10814.
Not turning calls to `mAddr` into `nkAddr` was done to prevent the
unsafe address semantics from being lost, but those no longer exist.

Lowering the call into an `nkAddr` tree has the benefit that it
simplifies AST analysis and processing, as address-of operation can now
always be detected by `PNode.kind == nkAddr`. Old code for detecting
`mAddr` magic calls is removed.

The effect tracking in `sempass2` had no special case for `mAddr`
calls, thus treating them as normal calls, which led to `addr(x)` being
treated as an indirect invocation of `x`, when `x` is of procedural
type. With the `mAddr` call now being lowered earlier, this is no
longer the case.
  • Loading branch information
zerbina authored Oct 19, 2024
1 parent c61e95b commit 39317bf
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 23 deletions.
9 changes: 2 additions & 7 deletions compiler/ast/astalgo.nim
Original file line number Diff line number Diff line change
Expand Up @@ -599,13 +599,8 @@ proc iiTablePut(t: var TIITable, key, val: int) =
iiTableRawInsert(t.data, key, val)
inc(t.counter)

proc isAddrNode*(n: PNode): bool =
case n.kind
of nkAddr, nkHiddenAddr: true
of nkCallKinds:
if n[0].kind == nkSym and n[0].sym.magic == mAddr: true
else: false
else: false
func isAddrNode*(n: PNode): bool =
n.kind in {nkAddr, nkHiddenAddr}

proc listSymbolNames*(symbols: openArray[PSym]): string =
for sym in symbols:
Expand Down
4 changes: 1 addition & 3 deletions compiler/sem/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2814,9 +2814,7 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode =
of mAddr:
markUsed(c, n.info, s)
checkSonsLen(n, 2, c.config)
result[0] = newSymNode(s, n[0].info)
result[1] = semAddrArg(c, n[1])
result.typ = makePtrType(c, result[1].typ)
result = semAddrCall(c, n)
of mTypeOf:
markUsed(c, n.info, s)
result = semTypeOf(c, n)
Expand Down
15 changes: 11 additions & 4 deletions compiler/sem/semmagic.nim
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ proc semAddrArg(c: PContext; n: PNode): PNode =
else:
result = newError(c.config, n, PAstDiag(kind: adSemExprHasNoAddress))

proc semAddrCall(c: PContext, n: PNode): PNode =
## Analyzes a well-formed call of the ``system.addr`` procedure (`n`),
## returning either an error or an ``nkAddr`` expression.
result = newTreeI(nkAddr, n.info, semAddrArg(c, n[1]))
if result[0].kind == nkError:
result = c.config.wrapError(result)
else:
result.typ = makePtrType(c, result[0].typ)

proc semTypeOf(c: PContext; n: PNode): PNode =
addInNimDebugUtils(c.config, "semTypeOf", n, result)
var m = BiggestInt 1 # typeOfIter
Expand Down Expand Up @@ -421,11 +430,9 @@ proc magicsAfterOverloadResolution(c: PContext, n: PNode,

case n[0].sym.magic
of mAddr:
# XXX: wasn't this magic already processed in ``semMagic``?
# 'addr' was overloaded, hence ``semMagic`` not handling the magic already
checkSonsLen(n, 2, c.config)
result = n
result[1] = semAddrArg(c, n[1])
result.typ = makePtrType(c, result[1].typ)
result = semAddrCall(c, n)
of mTypeOf:
result = semTypeOf(c, n)
of mSizeOf:
Expand Down
2 changes: 0 additions & 2 deletions compiler/sem/sempass2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1143,8 +1143,6 @@ proc allowCStringConv(n: PNode): bool =
of nkStrLiterals: result = true
of nkSym: result = n.sym.kind in {skConst, skParam}
of nkAddr: result = isCharArrayPtr(n.typ, true)
of nkCallKinds:
result = isCharArrayPtr(n.typ, n[0].kind == nkSym and n[0].sym.magic == mAddr)
else: result = isCharArrayPtr(n.typ, false)

proc reportErrors(c: ConfigRef, n: PNode) =
Expand Down
3 changes: 0 additions & 3 deletions compiler/sem/transf.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1110,9 +1110,6 @@ proc transformCall(c: PTransf, n: PNode): PNode =
inc(j)
result.add(a)
if result.len == 2: result = result[1]
elif magic == mAddr:
result = newTreeIT(nkAddr, n.info, n.typ): n[1]
result = transformAddr(c, result)
elif magic == mTypeOf:
result = n
elif magic == mRunnableExamples:
Expand Down
11 changes: 11 additions & 0 deletions tests/effects/teffects11.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
discard """
description: '''
Taking the address of a location storing a procedural value does not
incur the procedure's effects.
'''
action: compile
"""

proc p() {.raises: [].} =
var a: proc() {.raises: ValueError.}
discard addr(a)
4 changes: 0 additions & 4 deletions tests/lang_callable/macros/tmacrostmt.nim
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ proc fn6() =

#------------------------------------
# bug #10807
proc fn_unsafeaddr(x: int): int =
cast[int](unsafeAddr(x))

static:
let fn1s = "proc fn1(x, y: int): int =\n result = 2 * (x + y)\n"
Expand All @@ -124,15 +122,13 @@ static:
let fn4s = "proc fn4(x: int): int =\n if x mod 2 == 0:\n return x + 2\n else:\n return 0\n"
let fn5s = "proc fn5(a, b: float): float =\n result = -a * a / (b * b)\n"
let fn6s = "proc fn6() =\n var a = @[1.0, 2.0]\n let z = a{0, 1}\n a{2} = 5.0\n"
let fnAddr = "proc fn_unsafeaddr(x: int): int =\n result = cast[int](unsafeAddr(x))\n"

doAssert fn1.repr_to_string == fn1s
doAssert fn2.repr_to_string == fn2s
doAssert fn3.repr_to_string == fn3s
doAssert fn4.repr_to_string == fn4s
doAssert fn5.repr_to_string == fn5s
doAssert fn6.repr_to_string == fn6s
doAssert fn_unsafeaddr.repr_to_string == fnAddr

#------------------------------------
# bug #8763
Expand Down

0 comments on commit 39317bf

Please sign in to comment.