jyapayne/Nim
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

make implicit cstring conversions explicit (#19488)

Browse source 
The Nim manual says that an implicit conversion to cstring will
eventually not be allowed [1]:

    A Nim `string` is implicitly convertible to `cstring` for convenience.

    [...]

    Even though the conversion is implicit, it is not *safe*: The garbage collector
    does not consider a `cstring` to be a root and may collect the underlying
    memory. For this reason, the implicit conversion will be removed in future
    releases of the Nim compiler. Certain idioms like conversion of a `const` string
    to `cstring` are safe and will remain to be allowed.

And from Nim 1.6.0, such a conversion triggers a warning [2]:

    A dangerous implicit conversion to `cstring` now triggers a `[CStringConv]` warning.
    This warning will become an error in future versions! Use an explicit conversion
    like `cstring(x)` in order to silence the warning.

However, some files in this repo produced such a warning. For example,
before this commit, compiling `parsejson.nim` would produce:

    /foo/Nim/lib/pure/parsejson.nim(221, 37) Warning: implicit conversion to 'cstring' from a non-const location: my.buf; this will become a compile time error in the future [CStringConv]
    /foo/Nim/lib/pure/parsejson.nim(231, 39) Warning: implicit conversion to 'cstring' from a non-const location: my.buf; this will become a compile time error in the future [CStringConv]

This commit resolves the most visible `CStringConv` warnings, making the
cstring conversions explicit.

[1] d2318d9ccf/doc/manual.md (cstring-type)
[2] d2318d9ccf/changelogs/changelog_1_6_0.md (type-system)
This commit is contained in:
ee7 2022-08-19 21:40:53 +02:00 • committed by GitHub
commit e8657c7107
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 63 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

10
lib/pure/osproc.nim
View file

@ -1086,9 +1086,9 @@ elif not defined(useNimRtl):
var pid: Pid
if (poUsePath in data.options):
res = posix_spawnp(pid, data.sysCommand, fops, attr, data.sysArgs, data.sysEnv)
res = posix_spawnp(pid, data.sysCommand.cstring, fops, attr, data.sysArgs, data.sysEnv)
else:
res = posix_spawn(pid, data.sysCommand, fops, attr, data.sysArgs, data.sysEnv)
res = posix_spawn(pid, data.sysCommand.cstring, fops, attr, data.sysArgs, data.sysEnv)
discard posix_spawn_file_actions_destroy(fops)
discard posix_spawnattr_destroy(attr)
@ -1174,14 +1174,14 @@ elif not defined(useNimRtl):
when defined(uClibc) or defined(linux) or defined(haiku):
# uClibc environment (OpenWrt included) doesn't have the full execvpe
let exe = findExe(data.sysCommand)
discard execve(exe, data.sysArgs, data.sysEnv)
discard execve(exe.cstring, data.sysArgs, data.sysEnv)
else:
# MacOSX doesn't have execvpe, so we need workaround.
# On MacOSX we can arrive here only from fork, so this is safe:
environ = data.sysEnv
discard execvp(data.sysCommand, data.sysArgs)
discard execvp(data.sysCommand.cstring, data.sysArgs)
else:
discard execve(data.sysCommand, data.sysArgs, data.sysEnv)
discard execve(data.sysCommand.cstring, data.sysArgs, data.sysEnv)
startProcessFail(data)
{.pop.}