search

status-im / nim-faststreams

Nearly zero-overhead input/output streams for Nim
Apache License 2.0
125 stars 11 forks source link

Replace unsafeAddr by baseAddr #32

Closed lchenut closed 2 years ago

lchenut commented 2 years ago

It should fix some of the https://github.com/status-im/nim-protobuf-serialization CI problems. The rest of the problems should be solved with https://github.com/status-im/nim-faststreams/issues/31

arnetheduck commented 2 years ago

hm, I'm not convinced baseAddr is well-defined for empty arrays - taking the address of an empty array is not meaningful really - I guess using baseAddr points to these places more clearly, but we should probably revisit this at some point cc @zah (the risk being that with baseAddr, we turn a Defect into runtime-undetected UB).

The alternative solution here might be to avoid copyMem using something like assign2.

arnetheduck commented 2 years ago

should also add a test for writing/finalizing with empty array

arnetheduck commented 2 years ago

devel failure unrelated cc @zah

bung87 commented 1 year ago

where is baseAddr defined ？ I get .nimble\pkgs2\faststreams-0.3.0-62f7ac8fb200a8ecb9e6c63f5553a7dad66ae613\faststreams\outputs.nim(535, 34) Error: undeclared identifier: 'baseAddr'

nim-1.6.10

arnetheduck commented 1 year ago

it's part of nim-stew in ptrops - likely, you're using some old version

bung87 commented 1 year ago

really? it seems all the status-im packages my project relys has no tag, so no version restrictions, I also checked nimble packages dir, all of them match the latest version declared in nimble file. it seems template symbol resolution problem to me.

bung87 commented 1 year ago

hmm, you're right, when not versioned nimble not trying getting the latest.