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

deprecate unsafeAddr; extend addr #19373

Merged
merged 9 commits into from
Jan 16, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
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
become an alias for `addr`.
ringabout marked this conversation as resolved.
Show resolved Hide resolved

## 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 an alias for the addr operator and is deprecated:
ringabout marked this conversation as resolved.
Show resolved Hide resolved

.. 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 @@ -592,7 +592,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
6 changes: 3 additions & 3 deletions lib/impure/db_sqlite.nim
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
## var id = 1
## ## Data needs to be converted to seq[byte] to be interpreted as binary by bindParams
## 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)
##
## ## Use prepared statement to insert binary data into database
## var insertStmt = db.prepare("INSERT INTO test (id, data) VALUES (?, ?)")
Expand All @@ -138,7 +138,7 @@
## let seqSize = int(dataTest.len*sizeof(byte)/sizeof(float64))
## ## Copy binary string data in dataTest into a seq
## 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)
##
## ## Check datas obtained is identical
## doAssert res == orig
Expand Down Expand Up @@ -827,7 +827,7 @@ proc bindParam*(ps: SqlPrepared, paramIdx: int,val: openArray[byte], copy = true
## binds a blob to the specified paramIndex.
## if copy is true then SQLite makes its own private copy of the data immediately
let len = val.len
if bind_blob(ps.PStmt, paramIdx.int32, val[0].unsafeAddr, len.int32, if copy: SQLITE_TRANSIENT else: SQLITE_STATIC) != SQLITE_OK:
if bind_blob(ps.PStmt, paramIdx.int32, val[0].addr, len.int32, if copy: SQLITE_TRANSIENT else: SQLITE_STATIC) != SQLITE_OK:
dbBindParamError(paramIdx, val)

macro bindParams*(ps: SqlPrepared, params: varargs[untyped]): untyped {.since: (1, 3).} =
Expand Down
4 changes: 2 additions & 2 deletions lib/impure/re.nim
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ proc bufSubstr(b: cstring, sPos, ePos: int): string {.inline.} =
## Don't assume cstring is '\0' terminated
let sz = ePos - sPos
result = newString(sz+1)
copyMem(addr(result[0]), unsafeAddr(b[sPos]), sz)
copyMem(addr(result[0]), addr(b[sPos]), sz)
result.setLen(sz)

proc matchOrFind(buf: cstring, pattern: Regex, matches: var openArray[string],
Expand Down Expand Up @@ -427,7 +427,7 @@ iterator findAll*(buf: cstring, pattern: Regex, start = 0, bufSize: int): string
let b = rawMatches[1]
if a == b and a == i: break
var str = newString(b-a)
copyMem(str[0].addr, unsafeAddr(buf[a]), b-a)
copyMem(str[0].addr, addr(buf[a]), b-a)
yield str
i = b

Expand Down
2 changes: 1 addition & 1 deletion lib/pure/asyncdispatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1920,7 +1920,7 @@ proc send*(socket: AsyncFD, data: string,
## data has been sent.
var retFuture = newFuture[void]("send")
if data.len > 0:
let sendFut = socket.send(unsafeAddr data[0], data.len, flags)
let sendFut = socket.send(addr data[0], data.len, flags)
sendFut.callback =
proc () =
keepAlive(data)
Expand Down
2 changes: 1 addition & 1 deletion lib/pure/base64.nim
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ template encodeImpl() {.dirty.} =
encodeInternal(s, lookupTableVM)
else:
block:
let lookupTable = if safe: unsafeAddr(cb64safe) else: unsafeAddr(cb64)
let lookupTable = if safe: addr(cb64safe) else: addr(cb64)
encodeInternal(s, lookupTable)

proc encode*[T: SomeInteger|char](s: openArray[T], safe = false): string =
Expand Down
2 changes: 1 addition & 1 deletion lib/pure/hashes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ proc murmurHash(x: openArray[byte]): Hash =
dec j
k1 = (k1 shl 8) or (ord(x[i+j])).uint32
else:
k1 = cast[ptr uint32](unsafeAddr x[i])[]
k1 = cast[ptr uint32](addr x[i])[]
inc i, stepSize

k1 = imul(k1, c1)
Expand Down
2 changes: 1 addition & 1 deletion lib/pure/nativesockets.nim
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ when useNimNetLite:
return ""

proc sockAddrToStr(sa: var Sockaddr_in | var Sockaddr_in6): string =
result = sockAddrToStr(cast[ptr SockAddr](unsafeAddr(sa)))
result = sockAddrToStr(cast[ptr SockAddr](addr(sa)))

proc getAddrString*(sockAddr: ptr SockAddr): string =
result = sockAddrToStr(sockAddr)
Expand Down
6 changes: 3 additions & 3 deletions lib/pure/net.nim
Original file line number Diff line number Diff line change
Expand Up @@ -484,14 +484,14 @@ proc toSockAddr*(address: IpAddress, port: Port, sa: var Sockaddr_storage,
let s = cast[ptr Sockaddr_in](addr sa)
s.sin_family = typeof(s.sin_family)(toInt(AF_INET))
s.sin_port = port
copyMem(addr s.sin_addr, unsafeAddr address.address_v4[0],
copyMem(addr s.sin_addr, addr address.address_v4[0],
sizeof(s.sin_addr))
of IpAddressFamily.IPv6:
sl = sizeof(Sockaddr_in6).SockLen
let s = cast[ptr Sockaddr_in6](addr sa)
s.sin6_family = typeof(s.sin6_family)(toInt(AF_INET6))
s.sin6_port = port
copyMem(addr s.sin6_addr, unsafeAddr address.address_v6[0],
copyMem(addr s.sin6_addr, addr address.address_v6[0],
sizeof(s.sin6_addr))

proc fromSockAddrAux(sa: ptr Sockaddr_storage, sl: SockLen,
Expand All @@ -516,7 +516,7 @@ proc fromSockAddr*(sa: Sockaddr_storage | SockAddr | Sockaddr_in | Sockaddr_in6,
sl: SockLen, address: var IpAddress, port: var Port) {.inline.} =
## Converts `SockAddr` and `SockLen` to `IpAddress` and `Port`. Raises
## `ObjectConversionDefect` in case of invalid `sa` and `sl` arguments.
fromSockAddrAux(cast[ptr Sockaddr_storage](unsafeAddr sa), sl, address, port)
fromSockAddrAux(cast[ptr Sockaddr_storage](addr sa), sl, address, port)

when defineSsl:
CRYPTO_malloc_init()
Expand Down
6 changes: 3 additions & 3 deletions lib/pure/streams.nim
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,15 @@ proc write*[T](s: Stream, x: T) =
##
## .. code-block:: Nim
##
## s.writeData(s, unsafeAddr(x), sizeof(x))
## s.writeData(s, addr(x), sizeof(x))
runnableExamples:
var strm = newStringStream("")
strm.write("abcde")
strm.setPosition(0)
doAssert strm.readAll() == "abcde"
strm.close()

writeData(s, unsafeAddr(x), sizeof(x))
writeData(s, addr(x), sizeof(x))

proc write*(s: Stream, x: string) =
## Writes the string `x` to the stream `s`. No length field or
Expand Down Expand Up @@ -1196,7 +1196,7 @@ else: # after 1.3 or JS not defined
jsOrVmBlock:
buffer[slice.a..<slice.a+result] = s.data[s.pos..<s.pos+result]
do:
copyMem(unsafeAddr buffer[slice.a], addr s.data[s.pos], result)
copyMem(addr buffer[slice.a], addr s.data[s.pos], result)
inc(s.pos, result)
else:
result = 0
Expand Down
4 changes: 2 additions & 2 deletions lib/pure/strutils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1875,7 +1875,7 @@ func find*(s: string, sub: char, start: Natural = 0, last = 0): int {.rtl,
when hasCStringBuiltin:
let L = last-start+1
if L > 0:
let found = c_memchr(s[start].unsafeAddr, sub, cast[csize_t](L))
let found = c_memchr(s[start].addr, sub, cast[csize_t](L))
if not found.isNil:
return cast[ByteAddress](found) -% cast[ByteAddress](s.cstring)
else:
Expand Down Expand Up @@ -1928,7 +1928,7 @@ func find*(s, sub: string, start: Natural = 0, last = 0): int {.rtl,
else:
when hasCStringBuiltin:
if last == 0 and s.len > start:
let found = c_strstr(s[start].unsafeAddr, sub)
let found = c_strstr(s[start].addr, sub)
if not found.isNil:
result = cast[ByteAddress](found) -% cast[ByteAddress](s.cstring)
else:
Expand Down
4 changes: 2 additions & 2 deletions lib/std/private/digitsutils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const
proc utoa2Digits*(buf: var openArray[char]; pos: int; digits: uint32) {.inline.} =
buf[pos] = digits100[2 * digits]
buf[pos+1] = digits100[2 * digits + 1]
#copyMem(buf, unsafeAddr(digits100[2 * digits]), 2 * sizeof((char)))
#copyMem(buf, addr(digits100[2 * digits]), 2 * sizeof((char)))

proc trailingZeros2Digits*(digits: uint32): int32 {.inline.} =
return trailingZeros100[digits]
Expand All @@ -49,7 +49,7 @@ func addChars[T](result: var string, x: T, start: int, n: int) {.inline.} =
when defined(js) or defined(nimscript): impl
else:
{.noSideEffect.}:
copyMem result[old].addr, x[start].unsafeAddr, n
copyMem result[old].addr, x[start].addr, n

func addChars[T](result: var string, x: T) {.inline.} =
addChars(result, x, 0, x.len)
Expand Down
4 changes: 2 additions & 2 deletions lib/std/sha1.nim
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ proc update*(ctx: var Sha1State, data: openArray[char]) =
# Gather 64-bytes worth of data in order to perform a round with the leftover
# data we had stored (but not processed yet)
if len > 64 - i:
copyMem(addr ctx.buf[i], unsafeAddr data[j], 64 - i)
copyMem(addr ctx.buf[i], addr data[j], 64 - i)
len -= 64 - i
j += 64 - i
transform(ctx)
Expand All @@ -161,7 +161,7 @@ proc update*(ctx: var Sha1State, data: openArray[char]) =
i = 0
# Process the bulk of the payload
while len >= 64:
copyMem(addr ctx.buf[0], unsafeAddr data[j], 64)
copyMem(addr ctx.buf[0], addr data[j], 64)
len -= 64
j += 64
transform(ctx)
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.
ringabout marked this conversation as resolved.
Show resolved Hide resolved
##
## 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.
ringabout marked this conversation as resolved.
Show resolved Hide resolved
##
## Cannot be overloaded.
discard
Expand Down
4 changes: 2 additions & 2 deletions lib/system/ansi_c.nim
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,11 @@ elif defined(nimBuiltinSetjmp):
proc c_builtin_longjmp(jmpb: ptr pointer, retval: cint) {.
importc: "__builtin_longjmp", nodecl.}
# The second parameter needs to be 1 and sometimes the C/C++ compiler checks it.
c_builtin_longjmp(unsafeAddr jmpb[0], 1)
c_builtin_longjmp(addr jmpb[0], 1)
proc c_setjmp*(jmpb: C_JmpBuf): cint =
proc c_builtin_setjmp(jmpb: ptr pointer): cint {.
importc: "__builtin_setjmp", nodecl.}
c_builtin_setjmp(unsafeAddr jmpb[0])
c_builtin_setjmp(addr jmpb[0])
elif defined(nimRawSetjmp) and not defined(nimStdSetjmp):
when defined(windows):
# No `_longjmp()` on Windows.
Expand Down
2 changes: 1 addition & 1 deletion lib/system/atomics.nim
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ static int __tcc_cas(int *ptr, int oldVal, int newVal)
tcc_cas(cast[ptr int](p), cast[int](oldValue), cast[int](newValue))
elif declared(atomicCompareExchangeN):
proc cas*[T: bool|int|ptr](p: ptr T; oldValue, newValue: T): bool =
atomicCompareExchangeN(p, oldValue.unsafeAddr, newValue, false, ATOMIC_SEQ_CST, ATOMIC_SEQ_CST)
atomicCompareExchangeN(p, oldValue.addr, newValue, false, ATOMIC_SEQ_CST, ATOMIC_SEQ_CST)
else:
# this is valid for GCC and Intel C++
proc cas*[T: bool|int|ptr](p: ptr T; oldValue, newValue: T): bool
Expand Down
4 changes: 2 additions & 2 deletions lib/system/channels_builtin.nim
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ proc sendImpl(q: PRawChannel, typ: PNimType, msg: pointer, noBlock: bool): bool

proc send*[TMsg](c: var Channel[TMsg], msg: sink TMsg) {.inline.} =
## Sends a message to a thread. `msg` is deeply copied.
discard sendImpl(cast[PRawChannel](addr c), cast[PNimType](getTypeInfo(msg)), unsafeAddr(msg), false)
discard sendImpl(cast[PRawChannel](addr c), cast[PNimType](getTypeInfo(msg)), addr(msg), false)
when defined(gcDestructors):
wasMoved(msg)

Expand All @@ -377,7 +377,7 @@ proc trySend*[TMsg](c: var Channel[TMsg], msg: sink TMsg): bool {.inline.} =
##
## Returns `false` if the message was not sent because number of pending items
## in the channel exceeded `maxItems`.
result = sendImpl(cast[PRawChannel](addr c), cast[PNimType](getTypeInfo(msg)), unsafeAddr(msg), true)
result = sendImpl(cast[PRawChannel](addr c), cast[PNimType](getTypeInfo(msg)), addr(msg), true)
when defined(gcDestructors):
if result:
wasMoved(msg)
Expand Down
4 changes: 2 additions & 2 deletions lib/system/io.nim
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ when defined(windows):
break
inc i
else:
let w = c_fprintf(f, "%s", unsafeAddr s[i])
let w = c_fprintf(f, "%s", addr s[i])
if w <= 0:
if doRaise: raiseEIO("cannot write string to file")
break
Expand Down Expand Up @@ -896,7 +896,7 @@ proc writeFile*(filename: string, content: openArray[byte]) {.since: (1, 1).} =
var f: File = nil
if open(f, filename, fmWrite):
try:
f.writeBuffer(unsafeAddr content[0], content.len)
f.writeBuffer(addr content[0], content.len)
finally:
close(f)
else:
Expand Down
2 changes: 1 addition & 1 deletion lib/system/repr_v2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ proc repr*(p: pointer): string =

proc repr*(p: proc): string =
## repr of a proc as its address
repr(cast[ptr pointer](unsafeAddr p)[])
repr(cast[ptr pointer](addr p)[])

template repr*(x: distinct): string =
repr(distinctBase(typeof(x))(x))
Expand Down
2 changes: 1 addition & 1 deletion lib/system/sets.nim
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ proc cardSet(s: NimSet, len: int): int {.compilerproc, inline.} =
result = 0
when defined(x86) or defined(amd64):
while i < len - 8:
inc(result, countBits64((cast[ptr uint64](s[i].unsafeAddr))[]))
inc(result, countBits64((cast[ptr uint64](s[i].addr))[]))
inc(i, 8)

while i < len:
Expand Down
Loading