Add ByteStream and generic read proc to streams

[PMunch]

This adds a ByteStream that reads and writes seq[byte] to and from a stream. Might be useful for cryptography, or serialization.

Along with that it also adds a generic read proc that takes the type you want to read instead of the old pattern of read<typeName>. This means that things like macros can easier create read statements, and it means that type aliases can be used as well like this type MyInt = int16; stream.read(MyInt). The old read procs have been marked as deprecated, however since these are used in so many different libraries around it would be helpful to after a while put them behind a compile-time switch instead of removing them outright.

[zielmicha]

I'm for generic "read".

ByteStream is identical to StringStream (which, as I understand, is intentional), so I don't really see how it is useful. Maybe StringStream should just expose getByteArray?

[PMunch]

Well, they aren't exactly equal. They work on different types, but both seq[byte] and string behave almost exactly the same (it might even be possible to do a dodgy cast between them). The reason to have seq[byte] as well is that all the crypto libraries are switching to it, along with probably many low-level serialisation and other IO related libraries. It's just a nice convenience.

[data-man]

Maybe to add variant with Endian?

[metagn]

I don't know if this is members of the D community trying to avoid Nim's square bracket generics syntax or not, but I've been seeing lots of use of typedesc recently. I always saw typedesc as a side feature for edge cases and cool tricks. Anyway, this isn't lovable:

proc read*(s: Stream, kind: typedesc[bool]): bool =
  read(s, result)

proc peek*(s: Stream, kind: typedesc[bool]): bool =
  peek(s, result)

proc read*(s: Stream, kind: typedesc[int8]): int8 =
  read(s, result)

proc peek*(s: Stream, kind: typedesc[int8]): int8 =
  peek(s, result)

proc read*(s: Stream, kind: typedesc[int16]): int16 =
  read(s, result)

proc peek*(s: Stream, kind: typedesc[int16]): int16 =
  peek(s, result)

# goes for about 100 lines

Instead, you can leverage generics:

proc read*[T: bool|int8|int16|...](s: Stream): T =
  read(s, result)

proc peek*[T: bool|int8|int16|...](s: Stream): T =
  peek(s, result)

On top of that, you can keep the char procs:

proc read*[T: bool|int8|int16|...](s: Stream): T =
  read(s, result)

proc peek*[T: bool|int8|int16|...](s: Stream): T =
  peek(s, result)

proc read*[T: char](s: Stream): T =
  if readData(s, addr(result), sizeof(result)) != 1: result = '\0'

proc peek*[T: char](s: Stream): T =
  if peekData(s, addr(result), sizeof(result)) != 1: result = '\0'

[GULPF]

@hlaaftana I don't think it's is about D, it just subjective. It seems some people think typedesc is the primary way to send type arguments, and some people think generics is the primary way. Until #3502 is resolved generics are flawed for this use case though. Being forced to write read[int](s) instead of the more natural s.read[int] or s.read(int) is just painful.

It's possible to avoid the repetition even when using typedesc, e.g proc read*[T: int8|int16](s: Stream, kind: typedesc[T]): T, so that's not an issue.

[PMunch]

It's also a matter of runtime vs. compile time. Generics with [] has to be resolved on compile-time, but with typedesc it can be left to runtime. This has some benefits and some drawbacks. I agree though that it could have been solved better with not repeating the same proc a bunch of times. Maybe the right answer there is to just use a simple macro that writes out a copy of the procs for each type in a list.

[zielmicha]

It's also a matter of runtime vs. compile time. Generics with [] has to be resolved on compile-time, but with typedesc it can be left to runtime.

It's not true, typedesc exists only during compile time (e.g. you can't do let a: typedesc = int).

[metagn]

You can't claim generics are flawed just because the current syntax has ambiguities. In fact, I'd argue using typedesc in the call arguments adds more ambiguities.

Some programmers could probably have trouble remembering whether if it was read(int, s) or read(s, int). read[int](s) adds the flavor of knowing which part is the compile time arguments and which part is the method arguments. On topic of compiletime/runtime arguments, as demonstrated by this PR author's comment in this current comment thread, typedesc could be mistaken for a runtime value for people from dynamic language backgrounds.

I also don't think you can write a proc variable like readInt or read[int] with typedesc. You could write proc (s: Stream): int = s.read(int) however.

Speaking of : int = s.read(int), some people like adding type annotations to assignments, so that when they see its usage later in the code, they can scroll up and see what its type is by looking right next to its name in its definition. In that case, having to write let b: int8 = s.read(int8) instead of succinctly let b: int8 = s.read() could be painful. You might argue that they could just type let b = s.read(int8), but the type takes some looking from the variable name to see, nor could it always be easy to directly understand that the variable is an int8 from one of its method arguments being int8. More uses of type inference:

var data: seq[int] = @[s.read(), s.read(), s.read()]
let name = s.readStr(length = s.read())
data.add(s.read())

Also;

Maybe the right answer there is to just use a simple macro that writes out a copy of the procs for each type in a list.

This is essentially what generics are.

Unrelated: about the line var s = ByteStream(s), since ByteStream is a reference, if you're not gonna change what reference s points to and just gonna change its content, let is allowed in this case, but I personally don't know if this is a good idea

[PMunch]

@zielmicha, sorry I was wrong about typedesc. To be honest most of this implementation is based on https://github.com/nim-lang/Nim/issues/7430#comment-376804300 and following the discussion there made it seem like typedesc was the best way to go.

@hlaftaana, when it comes to ByteStreams most of the code is a verbatim copy of StringStreams. I didn't actually look at the code all that much other than making sure it would behave the same as a StringStream.

[dom96]

My two cents: I also dislike this sudden trend to use typedesc for everything. As I've said here, typedesc should be used in limited cases.

[timotheecour]

let's move the side-discussion on when to use generics vs typedesc to here: #7517 ([RFC] guidelines for when to use typedesc vs generics)

[data-man]

@PMunch

Can you add ByteStream only?

[PMunch]

@data-man you can just add it yourself :) Just rip the code from this commit or my branch and create a new PR

[data-man]

@PMunch
You're so lazy! :)

[dom96]

Shouldn't we get a readBytes or something for ByteStream that returns seq[byte]?

[narimiran]

So, what is the conclusion about this?

streams.nim has received some documentation improvements so this PR would need to solve all the merge conflicts, but maybe there is no point in that if in the end it is going to be rejected.

[Araq]

Sorry, rejected.