Skip to content

Commit

Permalink
deprecate unsafeAddr; extend addr (nim-lang#19373)
Browse files Browse the repository at this point in the history
* deprecate unsafeAddr; extend addr

addr is now available for all addressable locations, unsafeAddr is deprecated and become an alias for addr

* follow @Vindaar's advice

* change the signature of addr

* unsafeAddr => addr (stdlib)

* Update changelog.md

* unsafeAddr => addr (tests)

* Revert "unsafeAddr => addr (stdlib)"

This reverts commit ab83c99.

* doc changes; thanks to @konsumlamm

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
  • Loading branch information
2 people authored and PMunch committed Mar 28, 2022
1 parent 3831559 commit 7cee437
Show file tree
Hide file tree
Showing 30 changed files with 70 additions and 67 deletions.
3 changes: 3 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
- `std/sharedstrings` module is removed.
- Constants `colors.colPaleVioletRed` and `colors.colMediumPurple` changed to match the CSS color standard.

- `addr` is now available for all addressable locations, `unsafeAddr` is deprecated and
becomes an alias for `addr`.

## Standard library additions and changes

- `macros.parseExpr` and `macros.parseStmt` now accept an optional
Expand Down
2 changes: 1 addition & 1 deletion compiler/isolation_check.nim
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ proc isValueOnlyType(t: PType): bool =

proc canAlias*(arg, ret: PType): bool =
if isValueOnlyType(arg):
# can alias only with unsafeAddr(arg.x) and we don't care if it is not safe
# can alias only with addr(arg.x) and we don't care if it is not safe
result = false
else:
var marker = initIntSet()
Expand Down
9 changes: 2 additions & 7 deletions compiler/semmagic.nim
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,8 @@ proc semAddrArg(c: PContext; n: PNode; isUnsafeAddr = false): PNode =
let x = semExprWithType(c, n)
if x.kind == nkSym:
x.sym.flags.incl(sfAddrTaken)
if isAssignable(c, x, isUnsafeAddr) notin {arLValue, arLocalLValue}:
# Do not suggest the use of unsafeAddr if this expression already is a
# unsafeAddr
if isUnsafeAddr:
localError(c.config, n.info, errExprHasNoAddress)
else:
localError(c.config, n.info, errExprHasNoAddress & "; maybe use 'unsafeAddr'")
if isAssignable(c, x, true) notin {arLValue, arLocalLValue}:
localError(c.config, n.info, errExprHasNoAddress)
result = x

proc semTypeOf(c: PContext; n: PNode): PNode =
Expand Down
15 changes: 5 additions & 10 deletions doc/manual.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3502,8 +3502,9 @@ location is `T`, the `addr` operator result is of the type `ptr T`. An
address is always an untraced reference. Taking the address of an object that
resides on the stack is **unsafe**, as the pointer may live longer than the
object on the stack and can thus reference a non-existing object. One can get
the address of variables, but one can't use it on variables declared through
`let` statements:
the address of variables. For easier interoperability with other compiled languages
such as C, retrieving the address of a `let` variable, a parameter,
or a `for` loop variable can be accomplished too:

.. code-block:: nim
Expand All @@ -3515,24 +3516,18 @@ the address of variables, but one can't use it on variables declared through
# --> ref 0x7fff6b71b670 --> 0x10bb81050"Hello"
echo cast[ptr string](t3)[]
# --> Hello
# The following line doesn't compile:
# The following line also works
echo repr(addr(t1))
# Error: expression has no address
The unsafeAddr operator
-----------------------

For easier interoperability with other compiled languages such as C, retrieving
the address of a `let` variable, a parameter, or a `for` loop variable can
be accomplished by using the `unsafeAddr` operation:
The `unsafeAddr` operator is a deprecated alias for the `addr` operator:

.. code-block:: nim
let myArray = [1, 2, 3]
foreignProcThatTakesAnAddr(unsafeAddr myArray)
Procedures
==========

Expand Down
2 changes: 1 addition & 1 deletion doc/manual_experimental.rst
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ has `source` as the owner. A path expression `e` is defined recursively:
- Object field access `e.field` is a path expression.
- `system.toOpenArray(e, ...)` is a path expression.
- Pointer dereference `e[]` is a path expression.
- An address `addr e`, `unsafeAddr e` is a path expression.
- An address `addr e` is a path expression.
- A type conversion `T(e)` is a path expression.
- A cast expression `cast[T](e)` is a path expression.
- `f(e, ...)` is a path expression if `f`'s return type is a view type.
Expand Down
28 changes: 19 additions & 9 deletions lib/system.nim
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,16 @@ when defined(nimHasDeclaredMagic):
else:
proc declaredInScope*(x: untyped): bool {.magic: "DefinedInScope", noSideEffect, compileTime.}

proc `addr`*[T](x: var T): ptr T {.magic: "Addr", noSideEffect.} =
proc `addr`*[T](x: T): ptr T {.magic: "Addr", noSideEffect.} =
## Builtin `addr` operator for taking the address of a memory location.
##
## .. note:: This works for `let` variables or parameters
## for better interop with C. When you use it to write a wrapper
## for a C library and take the address of `let` variables or parameters,
## you should always check that the original library
## does never write to data behind the pointer that is returned from
## this procedure.
##
## Cannot be overloaded.
##
## See also:
Expand All @@ -205,15 +213,17 @@ proc `addr`*[T](x: var T): ptr T {.magic: "Addr", noSideEffect.} =
## echo p[] # b
discard

proc unsafeAddr*[T](x: T): ptr T {.magic: "Addr", noSideEffect.} =
proc unsafeAddr*[T](x: T): ptr T {.magic: "Addr", noSideEffect,
deprecated: "'unsafeAddr' is a deprecated alias for 'addr'".} =
## Builtin `addr` operator for taking the address of a memory
## location. This works even for `let` variables or parameters
## for better interop with C and so it is considered even more
## unsafe than the ordinary `addr <#addr,T>`_.
##
## **Note**: When you use it to write a wrapper for a C library, you should
## always check that the original library does never write to data behind the
## pointer that is returned from this procedure.
## location.
##
## .. note:: This works for `let` variables or parameters
## for better interop with C. When you use it to write a wrapper
## for a C library and take the address of `let` variables or parameters,
## you should always check that the original library
## does never write to data behind the pointer that is returned from
## this procedure.
##
## Cannot be overloaded.
discard
Expand Down
2 changes: 1 addition & 1 deletion tests/arc/tcustomtrace.nim
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ proc createSeq*[T](elems: varargs[T]): myseq[T] =
result.data = cast[type(result.data)](alloc0(result.cap * sizeof(T)))
inc allocCount
when supportsCopyMem(T):
copyMem(result.data, unsafeAddr(elems[0]), result.cap * sizeof(T))
copyMem(result.data, addr(elems[0]), result.cap * sizeof(T))
else:
for i in 0..<result.len:
result.data[i] = elems[i]
Expand Down
2 changes: 1 addition & 1 deletion tests/arc/thard_alignment.nim
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type
proc set1(x: float): m256d {.importc: "_mm256_set1_pd", header: "immintrin.h".}
func `+`(a,b: m256d): m256d {.importc: "_mm256_add_pd", header: "immintrin.h".}
proc `$`(a: m256d): string =
result = $(cast[ptr float](a.unsafeAddr)[])
result = $(cast[ptr float](a.addr)[])


var res: seq[seq[m256d]]
Expand Down
2 changes: 1 addition & 1 deletion tests/async/tioselectors.nim
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ when not defined(windows):
if fd == -1:
raiseOsError(osLastError())
let length = len(data).cint
if posix.write(fd, cast[pointer](unsafeAddr data[0]),
if posix.write(fd, cast[pointer](addr data[0]),
len(data).cint) != length:
raiseOsError(osLastError())
if posix.close(fd) == -1:
Expand Down
2 changes: 1 addition & 1 deletion tests/ccgbugs/tnoalias.nim
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type
field {.noalias.}: ptr UncheckedArray[int]

proc p(x {.noalias.}: openArray[char]) =
var q {.noalias.}: pointer = unsafeAddr(x[0])
var q {.noalias.}: pointer = addr(x[0])

var bn: BigNum
p "abc"
2 changes: 1 addition & 1 deletion tests/compiles/t8630.nim
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ bar

proc test(strings: seq[string]) =
for s in strings:
var p3 = unsafeAddr(s)
var p3 = addr(s)
echo p3[]

test(@["foo", "bar"])
4 changes: 2 additions & 2 deletions tests/concepts/tconcepts_issues.nim
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,10 @@ type
var address = pointer(nil)
proc prod(r: var QuadraticExt, b: QuadraticExt) =
if address == nil:
address = unsafeAddr b
address = addr b
prod(r, b)
else:
assert address == unsafeAddr b
assert address == addr b

type
Fp2[N: static int, T] {.byref.} = object
Expand Down
6 changes: 3 additions & 3 deletions tests/destructor/t12037.nim
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ copy length and contents 1 @[42]
proc test() =
var sq1 = @[42]
echo "showing original type, length, and contents ", sq1.typeof, " ", sq1.len, " ", sq1
doAssert cast[int](sq1[0].unsafeAddr) != 0
doAssert cast[int](sq1[0].addr) != 0
var sq2 = sq1 # copy of original
echo "copy length and contents ", sq2.len, " ", sq2
doAssert cast[int](sq2[0].unsafeAddr) != 0
doAssert cast[int](sq1[0].unsafeAddr) != 0
doAssert cast[int](sq2[0].addr) != 0
doAssert cast[int](sq1[0].addr) != 0

test()

Expand Down
2 changes: 1 addition & 1 deletion tests/destructor/tarray_indexing.nim
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ proc fillPages*(mem: UserProcessMemory, start: uint32, data: seq[byte]) =
#echo cast[uint64](addr mem.pageAccess[i])
let page = mem.pageAccess[i]
assert page != nil
#copyMem(page, unsafeAddr data[i * 0x1000 - start], 0x1000)
#copyMem(page, addr data[i * 0x1000 - start], 0x1000)

const base = 0x00100000

Expand Down
2 changes: 1 addition & 1 deletion tests/destructor/tcustomseqs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ proc createSeq*[T](elems: varargs[T]): myseq[T] =
result.data = cast[type(result.data)](alloc(result.cap * sizeof(T)))
inc allocCount
when supportsCopyMem(T):
copyMem(result.data, unsafeAddr(elems[0]), result.cap * sizeof(T))
copyMem(result.data, addr(elems[0]), result.cap * sizeof(T))
else:
for i in 0..<result.len:
result.data[i] = elems[i]
Expand Down
2 changes: 1 addition & 1 deletion tests/destructor/tcustomstrings.nim
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ proc add*(self: var mystring; y: mystring) =
proc create*(lit: string): mystring =
let newLen = lit.len
ensure(result, newLen)
copyMem(addr result.data[result.len], unsafeAddr lit[0], newLen + 1)
copyMem(addr result.data[result.len], addr lit[0], newLen + 1)
result.len = newLen

proc `&`*(a, b: mystring): mystring =
Expand Down
2 changes: 1 addition & 1 deletion tests/destructor/tuse_result_prevents_sinks.nim
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ proc `=sink`(self: var Foo; other: Foo) =
proc `=destroy`(self: var Foo) = discard

template preventCursorInference(x) =
let p = unsafeAddr(x)
let p = addr(x)

proc test(): Foo =
result = Foo()
Expand Down
2 changes: 1 addition & 1 deletion tests/errmsgs/t10594.nim
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ discard """
line: 7
"""

template foo(v: varargs[int]) = unsafeAddr v
template foo(v: varargs[int]) = addr v
foo(1, 2)
2 changes: 1 addition & 1 deletion tests/errmsgs/tnon_concrete_cast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ proc write(rw: var MyReadWrite; value: SomeNumber): void =
proc write[T](rw: var MyReadWrite; value: seq[T]): void =
rw.write value.len
let dst = cast[ptr SomeNumber](cast[uint](rw.memfile.mem) + uint(rw.offset))
let src = cast[pointer](value[0].unsafeAddr)
let src = cast[pointer](value[0].addr)
let size = sizeof(T) * value.len
copyMem(dst, src, size)
rw.offset += size
Expand Down
2 changes: 1 addition & 1 deletion tests/js/tfieldchecks.nim
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ block:
block:
let a0 = addr(obj.f0)
echo a0[]
# let a1 = unsafeAddr(obj.f1)
# let a1 = addr(obj.f1)
# echo a1[]
doAssertRaises(FieldDefect):
let a2 = addr(obj.f2)
Expand Down
10 changes: 5 additions & 5 deletions tests/lent/tbasic_lent_check.nim
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ template main2 = # bug #15958
let b = @[21,23]
let ss = {1, 2, 3, 5}
doAssert byLent(a) == [11,12]
doAssert byLent(a).unsafeAddr == a.unsafeAddr
doAssert byLent(a).addr == a.addr
doAssert byLent(b) == @[21,23]
when not defined(js): # pending bug #16073
doAssert byLent(b).unsafeAddr == b.unsafeAddr
doAssert byLent(b).addr == b.addr
doAssert byLent(ss) == {1, 2, 3, 5}
doAssert byLent(ss).unsafeAddr == ss.unsafeAddr
doAssert byLent(ss).addr == ss.addr

let r = new(float)
r[] = 10.0
Expand All @@ -41,9 +41,9 @@ template main2 = # bug #15958

proc byLent2[T](a: openArray[T]): lent T = a[0]
doAssert byLent2(a) == 11
doAssert byLent2(a).unsafeAddr == a[0].unsafeAddr
doAssert byLent2(a).addr == a[0].addr
doAssert byLent2(b) == 21
doAssert byLent2(b).unsafeAddr == b[0].unsafeAddr
doAssert byLent2(b).addr == b[0].addr

proc byLent3[T](a: varargs[T]): lent T = a[1]
let
Expand Down
2 changes: 1 addition & 1 deletion tests/misc/taddr.nim
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ block:
# bug #14576
# lots of these used to give: Error: internal error: genAddr: 2
proc byLent[T](a: T): lent T = a
proc byPtr[T](a: T): ptr T = a.unsafeAddr
proc byPtr[T](a: T): ptr T = a.addr

block:
let a = (10,11)
Expand Down
4 changes: 2 additions & 2 deletions tests/overload/t8829.nim
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ block:
template `[]`[T](p: ptr T, span: Slice[int]): untyped =
toOpenArray(cast[ptr array[0, T]](p)[], span.a, span.b)

doAssert $cast[ptr uint8](txt[0].unsafeAddr)[0 ..< txt.len] ==
doAssert $cast[ptr uint8](txt[0].addr)[0 ..< txt.len] ==
"[72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100]"


Expand All @@ -14,5 +14,5 @@ block:
template `[]`[T](p: ptr T, span: Slice[int]): untyped =
toOpenArray(cast[ptr array[0, T]](p)[], span.a, span.b)

doAssert $cast[ptr uint8](txt[0].unsafeAddr)[0 ..< txt.len] ==
doAssert $cast[ptr uint8](txt[0].addr)[0 ..< txt.len] ==
"[72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100]"
2 changes: 1 addition & 1 deletion tests/overload/tstatic_with_converter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ proc `^`(x: vfloat, exp: static[float]): vfloat =
pow(x, exp)

proc `$`(x: vfloat): string =
let y = cast[ptr float](unsafeAddr x)
let y = cast[ptr float](addr x)
# xxx not sure if intentional in this issue, but this returns ""
echo y[]

Expand Down
14 changes: 7 additions & 7 deletions tests/stdlib/tdecls.nim
Original file line number Diff line number Diff line change
Expand Up @@ -48,37 +48,37 @@ when false: # pending bug #13887
## We can define custom pragmas in user code
template byUnsafeAddr(lhs, typ, expr) =
when typ is type(nil):
let tmp = unsafeAddr(expr)
let tmp = addr(expr)
else:
let tmp: ptr typ = unsafeAddr(expr)
let tmp: ptr typ = addr(expr)
template lhs: untyped = tmp[]

block:
let s = @["foo", "bar"]
let a {.byUnsafeAddr.} = s[0]
doAssert a == "foo"
doAssert a[0].unsafeAddr == s[0][0].unsafeAddr
doAssert a[0].addr == s[0][0].addr

block: # nkAccQuoted
# shows using a keyword, which requires nkAccQuoted
template `cast`(lhs, typ, expr) =
when typ is type(nil):
let tmp = unsafeAddr(expr)
let tmp = addr(expr)
else:
let tmp: ptr typ = unsafeAddr(expr)
let tmp: ptr typ = addr(expr)
template lhs: untyped = tmp[]

block:
let s = @["foo", "bar"]
let a {.`byUnsafeAddr`.} = s[0]
doAssert a == "foo"
doAssert a[0].unsafeAddr == s[0][0].unsafeAddr
doAssert a[0].addr == s[0][0].addr

block:
let s = @["foo", "bar"]
let a {.`cast`.} = s[0]
doAssert a == "foo"
doAssert a[0].unsafeAddr == s[0][0].unsafeAddr
doAssert a[0].addr == s[0][0].addr

block: # bug #15920
template foo(lhs, typ, expr) =
Expand Down
2 changes: 1 addition & 1 deletion tests/stdlib/tmemory.nim
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ block: # cmpMem

doAssert cmpMem(a.addr, b.addr, sizeof(SomeHash)) > 0
doAssert cmpMem(b.addr, a.addr, sizeof(SomeHash)) < 0
doAssert cmpMem(a.addr, c.unsafeAddr, sizeof(SomeHash)) == 0
doAssert cmpMem(a.addr, c.addr, sizeof(SomeHash)) == 0
4 changes: 2 additions & 2 deletions tests/stdlib/tsqlitebindatas.nim
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ block tsqlitebindatas: ## db_sqlite binary data
db.exec(createTableStr)

var dbuf = newSeq[byte](orig.len*sizeof(float64))
copyMem(unsafeAddr(dbuf[0]), unsafeAddr(orig[0]), dbuf.len)
copyMem(addr(dbuf[0]), addr(orig[0]), dbuf.len)

var insertStmt = db.prepare("INSERT INTO test (id, name, data) VALUES (?, ?, ?)")
insertStmt.bindParams(1, origName, dbuf)
Expand All @@ -42,7 +42,7 @@ block tsqlitebindatas: ## db_sqlite binary data
var dataTest = db.getValue(sql"SELECT data FROM test WHERE id = ?", 1)
let seqSize = int(dataTest.len*sizeof(byte)/sizeof(float64))
var res: seq[float64] = newSeq[float64](seqSize)
copyMem(unsafeAddr(res[0]), addr(dataTest[0]), dataTest.len)
copyMem(addr(res[0]), addr(dataTest[0]), dataTest.len)
doAssert res.len == orig.len
doAssert res == orig

Expand Down
Loading

0 comments on commit 7cee437

Please sign in to comment.