node icon indicating copy to clipboard operation
node copied to clipboard

Explicitly use int64 value of StructuredData

Open koraykoska opened this issue 8 years ago • 3 comments

Currently StructuredData.Number is limited to int, uint and double, which is perfectly fine for people who use 32 bit integers and 64 bit integers on 64 bit platforms.

But if you are on a 32 bit platform and you expect aStructuredData value to be e.g. an unsigned 64 bit integer, you will probably have some unexpected silent problems as Int and UInt are limited to 32 bits andStructuredData will return Int.max for integers bigger than 32 bits.

I am currently working on a vapor library for Telegram chatbots and they have stated in their documentation that some values are actually bigger than 32 bits which means I must ignore 32 bit platforms and add a warning to the README that this library should not be used on 32 bit platforms.

What do you guys think about that? Do we want to ignore 32 bit platforms as they already become extinct or do we want to add support for explicit int64 and uint64 values inside StructuredData.Number?

Reference (Number.swift file in this library): Number.swift

koraykoska avatar Aug 08 '17 21:08 koraykoska

If an application requires native integers to be 64 bits in width then I think it makes sense to not allow that application to run on 32 bit machines. I don't know exactly how Int64 works on a 32 bit machine (does it?). If it does, there's probably a considerable performance impact at least.

tanner0101 avatar Aug 09 '17 17:08 tanner0101

Int64 works just fine on 32 bit systems too. Consider C, the long long type has been available since C99, years before 64 bit systems became generally available 😛.

Indeed, there might be performance penalty involved, but some applications really require 64 bit integers.

vzsg avatar Aug 09 '17 17:08 vzsg

tl;dr: use strings for now, this issue raises deeper questions about the viability of StructuredData that need to be addressed.


@vzsg good point. :)

As a workaround for now, 64 bit integers could be stored as strings in the structured data and converted to 64 bit integers in an extension.

I think this issue will be made easier to tackle in upcoming updates to Vapor that will make the StructuredData type much less prevalent.

@Ybrin what type specifically are you using in this case? Are you using a Node or StructuredData directly or are you using one of the sub types like Row or JSON? In 3.0 we're hoping to transition the subtypes to being their own types not reliant on StructureData. This means that it would be much easier to add say Int64 support to just Row if it's needed there.

Right now StructuredData is a bit of a god object. Changing it even slightly affects a ton of code. For example, JSON currently extends StructuredData and all JSON can just be Doubles. Adding Int64 support there (or having Int support in the first place) could potentially cause interoperability problems, unexpected behavior, or unnecessary overhead .

tanner0101 avatar Aug 10 '17 16:08 tanner0101