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

add io.readChars overload (simpler, less error prone) #16044

Merged
merged 4 commits into from
Feb 22, 2021

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Nov 19, 2020

add io.readChars overload (simpler, less error prone)

  • see tests tests/stdlib/tio.nim; I'm noticing that io.nim isn't tested (at least lots of procs aren't), so tio.nim is a start

  • removed

## **Warning:** The buffer `a` must be pre-allocated. This can be done	
  ## using, for example, ``newString``.

which doesn't make sense to me; it's impossible to not have a pre-allocated unless I'm missing something

  • deprecate (possibly indefinitely) existing readChars overload since the new one is simpler, less error prone (impossible to misuse the way the other one was, as evidenced by lack of raiseEIO("buffer overflow: (start+len) > length of openarray buffer")
  • show in deprecation msg how to use new overload via readChars(toOpenArray(buf, start, len-1), making the old one superfluous (and 99% of the time you'd simply use readChars(f, buf) without toOpenArray anyways)

note regarding since

I didn't use since: (1,5,1) for simplicity, and so that stdlib (lib/std/sha1.nim and ./lib/wrappers/openssl.nim) can use the new readChars proc, simplifying code, promoting the new preferred one, and avoiding the warning.

as usual, if/when #11865 (recently rebased to avoid conflict bitrot) is merged I'd be able to use since: (1,5,1) for readChars and stdlib code could use:

# in io.nim
proc readChars(f: File, a: var openArray[char]): int = ...
since (1,5,1): export readChars

# or even:
proc readChars(f: File, a: var openArray[char]): int {.exportSince: (1.5.1).} = ...


# in lib/std/sha1.nim
from std/io {.privateImport.} import readChars

note regarding test

I've used defer, but it's a test, not compiler code. See also #16048

lib/wrappers/openssl.nim Outdated Show resolved Hide resolved
@Araq
Copy link
Member

Araq commented Nov 19, 2020

note regarding since

since has a serious design flaw, it should only disable the export marker when used with --useVersion so that it can be used regardless in the same module. I know it doesn't help stdlib clients of the module but it would help. This probably means that since should come a compiler builtin...

@Araq Araq merged commit ce7caec into nim-lang:devel Feb 22, 2021
@timotheecour timotheecour deleted the pr_fix_readChars branch February 22, 2021 19:16
ringabout added a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
* add simpler to use readChars overload

* use new readChars overload

* Update lib/wrappers/openssl.nim

Co-authored-by: Andreas Rumpf <[email protected]>
Co-authored-by: flywind <[email protected]>
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
* add simpler to use readChars overload

* use new readChars overload

* Update lib/wrappers/openssl.nim

Co-authored-by: Andreas Rumpf <[email protected]>
Co-authored-by: flywind <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants