JSONKit
JSONKit copied to clipboard
Fixes warnings when compiling with -Wconversion and -pedantic etc.
We compile with very strict compiler options, most notibly -Wconversion which will warn about any implicit conversions between signed and unsigned as well as when an integer is casted down in size.
I added a bunch of explicit casts to get rid of these warnings. I took care to examine the code and make sure the casts make sense. In all cases the behaviour is unchanged, although I don't have any unit tests to run (see Issue #56). We're just making the implicit casts explicit.
I also fixed a few other problems:
The commits:
Commit 1 adds an additional check for STRICT_ANSI to GNUC to decide between the stubbed out macros like JK_ATTRIBUTES and the real versions. clang will define GNUC even when compiling with -pedantic mode where some of the GNUC features are not allowed (like certain comma pasting and macro varargs stuff)
Commit 2 fixes a warning about unused varargs in the stub version of the JK_ATTRIBUTES macro. This is a silly warning but we can just remove the first argument since the whole thing is a NOP anyway.
Commit 3 Fixes a bunch of implicit sign conversion warnings when using ~ to remove a flag. ~ will return a signed integer so we cast the result to the flag type we want. An alternative would be to declare JKFlags as NSInteger but I thought this is was better choice since all the flag values should be generated as unsigned by the compiler.
Commit 4 replaces the non-standard triple-dot '...' style cases. Ugly but follows standards.
Commit 5 fixes all the implicit conversions between different pointer sizes and signedness.
Cheers,
Mike
I agree in principle with all of this, but since I haven't had the time to properly review them, I'm not going to merge them just right now.
The only thing that immediately stands out is trying to get -pedantic
to compile cleaning... that's a tough nut to crack. :)
I also know there are some corner cases where you're screwed no matter what you do (mostly have to do with "toll-free bridging" between Cocoa / Foundation and Core Foundation). I made an effort to keep this as clean as possible from a pedantic ANSI C standard perspective, but there's clearly places where you need to make practical trade-offs due to the fact that you're using Objective-C (hello type-punning!) combined with the craziness of Objective-C <-> Core Foundation toll-free bridging.
From a purely pedantic perspective, the whole thing amounts to a giant mess of "undefined behavior" due to the type-punning from type pointer casts... Just search for APPLE LOCAL begin disable strict aliasing;
in gcc-5646/gcc/config/i386/i386.c. Good times!
Yeah I didn't really expect this to get merged into the official repo. I can understand it takes a while to review all this and for most people it is unnecessary. I just thought I'd put it out there since I had done the work.
Cheers,
Mike
@MikeWeller
-Wconversion will always raise warnings even on valid ISO conversions, your code will always have third party dependencies e.g your OS (don't expect...), even some functions from the libC and the STL will give you such warnings (take it as a backtrace not all the warnings are violations, there are indications about what the compiler does, especially regarding floating point conversions and const casting), using pedantic with std=c99 is a good thing but you'll still get warnings in obj-c, you may use it to debug and QA your own code but not as a requirement, if you can compile with Wextra (some opaque struct definitions, static initialization and unused callbacks could give u a hard time, moreover the output might differ according to the target) Werror you are already good to go (you are my friend, if every project creator could have this expectation, it would be nice). Perfection is not attainable, but if we chase perfection we can catch excellence.
thus chase perfection -Wall -Werror
(chasing unused var, that is also a good thing 99% of the time it exposes bogus parameters forwarding, clang is a good tool for that purpose)
moreover some iso c99 warnings do not reflect anymore the reality of modern hardware, there are simply deprecated.
-- the bad man