SSBMTonberry / tileson

A modern and helpful cross-platform json-parser for C++, used for parsing Tiled maps.
BSD 2-Clause "Simplified" License
189 stars 29 forks source link

enable warnings & warnings as errors in tests #90

Closed dmlary closed 1 year ago

dmlary commented 1 year ago

Add -Wall -Wextra -Werror to tileson_tests build target, along with (I think) the eqivalent for MSVC.

Include quick fixes for existing tests, which may not be the best solution. Committing this to see what comes out of CI.

fixed #86

SSBMTonberry commented 1 year ago

I can have a look at the C4244 warning from MSVC. :slightly_smiling_face:
Can push a change to your branch when I figure out a solution. Unless you want to figure it out yourself, of course :stuck_out_tongue:

SSBMTonberry commented 1 year ago

Well great... Seems like some of the stuff I added was not completely supported by all compilers. I'll fix that. Sorry 😛

SSBMTonberry commented 1 year ago

Seems like the rest is GCC related. Since that's my main compiler anyways I can have a quick look 🙂

SSBMTonberry commented 1 year ago

Last error seems to be related to [[maybe_unused]] not being fully implemented for structured bindings in GCC7. Ref: https://stackoverflow.com/questions/47005032/structured-bindings-and-range-based-for-supress-unused-warning-in-gcc

dmlary commented 1 year ago

oh, nice! Looks like you've knocked out most of it.

The dumb fix for gcc7 is to just use that other variable. I wonder if the old-school cast-to-void ((void)var) will get us past the hump. Failing that, there's a C++ construct we can use: https://stackoverflow.com/a/3418951

SSBMTonberry commented 1 year ago

oh, nice! Looks like you've knocked out most of it.

The dumb fix for gcc7 is to just use that other variable. I wonder if the old-school cast-to-void ((void)var) will get us past the hump. Failing that, there's a C++ construct we can use: https://stackoverflow.com/a/3418951

Seems like the (void) cast "hack" does the job. It's not pretty, but honestly it's just to make it shut up when a variable is intentionally unused, and GCC7 has a missing support for a case that should work. Just to make the tests work I think it's okay, though it's very ugly :stuck_out_tongue_closed_eyes:

SSBMTonberry commented 1 year ago

All tests passes now!

To summarize:

@dmlary : If you got anything to add, feel free to do so. :slightly_smiling_face:

dmlary commented 1 year ago

Everything looks good. My goal is achieved here, and I have nothing more to add.

Great work sorting through all those msvc and gcc errors!

SSBMTonberry commented 1 year ago

Everything looks good. My goal is achieved here, and I have nothing more to add.

Great work sorting through all those msvc and gcc errors!

Then I guess it ready for a merge! This ended up being a nice cooperation task :smile: Thank you very much for your help, and a great initiative :partying_face: