jangko / msgpack4nim

MessagePack serializer/deserializer implementation for Nim / msgpack.org[Nim]
http://msgpack.org/
119 stars 21 forks source link

Warnings when unpacking to an object with range properties #64

Open termermc opened 2 years ago

termermc commented 2 years ago

Warnings (that will eventually turn into errors) are raised when unpacking to an object that has properties which are ranges.

Here is my object and the type definition for its property:

type
    StreamId* = uint32 ## A stream ID
    NonBlankStreamId* = range[((StreamId) 1)..high(StreamId)] ## A non-blank stream ID
    CSendStreamData* = object
        ## Sent to convert the connection into a stream data input pipe.

        streamId*: NonBlankStreamId ## The stream to send data to

Unpacking it with the following code:

body.unpack(CSendStreamData)

The compiler generates the following warning:

<home>/.nimble/pkgs/msgpack4nim-0.3.1/msgpack4nim.nim(1217, 9) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]

While this doesn't stop the code from compiling (for now), it does show as an error in the VS Code extension and is annoying in that way. The main issue here is the implication that this code may not compile in later versions of Nim. I believe that if the data being unpacked violates the range, that it should raise a ObjectConversionDefect, since that would be consistent.

My nim -v output:

Nim Compiler Version 1.6.2 [Linux: amd64]
Compiled at 2021-12-17
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 9084d9bc02bcd983b81a4c76a05f27b9ce2707dd
active boot switches: -d:release

Thanks! Other than this, your project has been working amazingly for me.

jangko commented 1 year ago

reminder for me: the solution for this is to add a macro that can traverse from parent object to every inner fields and then construct a range initialization code if one of the fields in the hierarchy contains fields with requires init attribute. explicit {.requiresInit.} or implicit one like range should be taken into account.