joncmu / rapidjson

Automatically exported from code.google.com/p/rapidjson
MIT License
0 stars 0 forks source link

Suggestions on removing some MS-specific code and warnings #23

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Milo,

This is a great lib. We inspected many JSON libs for parsing purpose and 
rapidjson is the fastest one. 

However, since we need to build on multiple platforms (Linux, Sun, AIX, MS 
Windows) with multiple compilers(GCC, SunCC, XlC, MSVC), it would be even nicer 
if some of the "MS specific" extensions can be removed.

1. In document.h, Number is defined as 

#if RAPIDJSON_ENDIAN == RAPIDJSON_LITTLEENDIAN
        struct {
            int i;
            char padding[4];
        };
        struct {
            unsigned u;
            char padding2[4];
        };
#else
        struct {
            char padding[4];
            int i;
        };
        struct {
            char padding2[4];
            unsigned u;
        };
#endif

Anonymous struct is used here, which is an MS extension. 

GCC by default (without -fms-extension flag) will issue an compilation error. 

Meanwhile, you made assumptions sizeof(int) == 4 here, which is not always the 
case. One possible solution will be using uint64_t and int64_t for everything 
and let the compiler handle typecasting (which is extremely cheap on integers)

2. Line 407 of reader.h 

if ((sizeof(Ch) == 1 || e < 256) && escape[(unsigned char)e])

GCC will issue a warning:

reader.h:407: warning: comparison is always true due to limited range of data 
type

Since e is type Ch, which is instantiated by the template as (char), and char 
is always smaller than 256.

3. pack(push, <number>) and pack(pop) are not supported by all the compilers. 
However, I have no perfect alternative on how to solve the problem. 

On our case, we change it to pack(<number>) and pack(), which will reset the 
alignment to the compiler default, but that will not work correctly if you 
already have content in the alignment stack.

Thanks
Biye

Original issue reported on code.google.com by lib...@gmail.com on 27 Apr 2012 at 5:02

GoogleCodeExporter commented 9 years ago

Original comment by milo...@gmail.com on 22 May 2012 at 9:29

GoogleCodeExporter commented 9 years ago

Original comment by milo...@gmail.com on 13 Nov 2012 at 7:21

GoogleCodeExporter commented 9 years ago
These are all fixed in r67 and r68 in trunk and version0.1x branch respectively.

Original comment by milo...@gmail.com on 13 Nov 2012 at 9:40