Build with more aggressive compiler warnings

[springmeyer]

My sense is we should start with enabling and fixing warnings in roughly this order:

• -Wabsolute-value
• -Wfloat-conversion
• -Wshorten-64-to-32
• -Wconversion
• -Wshadow
• -Wsign-compare
• -Wsign-conversion

[e-n-f]

-Wshadow should be fixed in f1b3f6d

[springmeyer]

@ericfischer - refreshed, recommended list is now available at mapbox/cpp#37

[e-n-f]

Thanks. Working on this in #489

[springmeyer]

Thanks @ericfischer

[springmeyer]

@ericfischer great to see #489 land. Any progress on attempting -Wconversion or is it looking too narly?

[e-n-f]

That one is going to be terrible. There are hundreds of implicit type conversions.

[springmeyer]

@ericfischer okay, then I recommend starting with a subset like:

-Wbitfield-enum-conversion, -Wbool-conversion, -Wconstant-conversion, -Wenum-conversion, - -Wliteral-conversion, -Wnon-literal-null-conversion, -Wnull-conversion, -Wshorten-64-to-32, -Wstring-conversion.

I presume that will surface much fewer warnings, primarily from -Wshorten-64-to-32.

The whole -Wconversion group is listed at https://clang.llvm.org/docs/DiagnosticsReference.html#wconversion.

After those are resolved I recommend one-by-one incrementally adding -Wsign-conversion, then -Wint-conversion, and finally -Wfloat-conversion (which is often too much in my experience, but nevertheless useful to peek at even if you don't fix them).

[springmeyer]

@ericfischer do you have thoughts on my proposed plan?

[e-n-f]

I can certainly go through and add 571 type casts. I'm just not sure what the balance will turn out to be between:

• fixing genuine bugs
• adding needless clutter
• accidentally masking genuine bugs by adding casts that make errors look intentional

which is always my anxiety around casts. I hope there will be more of the former than of the latter two though.

[e-n-f]

The fundamental tension is that the language really wants array bounds and indices to be unsigned, for efficiency, but really wants ordinary arithmetic to be signed, for intuitive results.

[e-n-f]

Trying to get more rigorous about numeric types in #506.

Situations I've encountered so far that are stuck as int but perhaps should not be:

• Tile coordinate detail (log2(extent))
• Global scaling factor for tile coordinates
• Feature flags (because that's what getopt_long wants)
• Comparator return values (because that's what qsort wants)
• Zoom levels
• File descriptors (because that's what open et al want)
• Attribute types (for coercion)
• Exit status
• Char+EOF return values from getc
• Char+EOF return from getopt_long, and optind option index

[springmeyer]

Excellent progress @ericfischer - I notice that branch fixes a number of the -Wshorten-64-to-32 warnings. Some that remain are coming from the zlib wrapper code, which reminds me that these warnings are solved in https://github.com/mapbox/gzip-hpp, which is fundamentally a port of the same code in tippecanoe into a standalone module with the warnings (and potential overflows) resolved.

[e-n-f]

What remains now is a handful of int-to-char conversions and hundreds of sign conversions.