Several compiler warnings when compiling with clang6 and all warnings enabled (-Wall)

[pieter3d]

When compiling abseil's cctz libraries, I get the following warnings:

G../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.cc:166:22: warning: declaration shadows a local variable [-Wshadow]
        if (std::tm* tmp = local_time(&lo, &tm)) {
                     ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.cc:156:18: note: previous declaration is here
    if (std::tm* tmp = local_time(&mid, &tm)) {
                 ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.cc:191:9: warning: result of comparison 'const std::int_fast64_t' (aka 'const long') < -9223372036854775808 is always false [-Wtautological-type-limit-compare]
  if (s < std::numeric_limits<std::time_t>::min()) {
      ~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.cc:195:9: warning: result of comparison 'const std::int_fast64_t' (aka 'const long') > 9223372036854775807 is always false [-Wtautological-type-limit-compare]
  if (s > std::numeric_limits<std::time_t>::max()) {
      ~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.cc:84:6: warning: unused function template 'tm_gmtoff' [-Wunused-template]
auto tm_gmtoff(const T& tm) -> decltype(tm.__tm_gmtoff) {
     ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.cc:100:6: warning: unused function template 'tm_zone' [-Wunused-template]
auto tm_zone(const T& tm) -> decltype(tm.__tm_zone) {
     ^
5 warnings generated.
[140/165] Building CXX object external/absei...ne.dir/internal/cctz/src/time_zone_info.cc.o
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:644:10: warning: declaration shadows a local variable [-Wshadow]
    long pos = ftell(fp);
         ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:618:21: note: previous declaration is here
  const std::size_t pos = (name.compare(0, 5, "file:") == 0) ? 5 : 0;
                    ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:721:35: warning: declaration shadows a local variable [-Wshadow-uncaptured-local]
      name, [](const std::string& name) -> std::unique_ptr<ZoneInfoSource> {
                                  ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:709:44: note: previous declaration is here
bool TimeZoneInfo::Load(const std::string& name) {
                                           ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:722:18: warning: declaration shadows a local variable [-Wshadow-uncaptured-local]
        if (auto zip = FileZoneInfoSource::Open(name)) return zip;
                 ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:720:8: note: previous declaration is here
  auto zip = cctz_extension::zone_info_source_factory(
       ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:723:18: warning: declaration shadows a local variable [-Wshadow-uncaptured-local]
        if (auto zip = AndroidZoneInfoSource::Open(name)) return zip;
                 ^
../external/abseil-cpp/absl/time/internal/cctz/src/time_zone_info.cc:720:8: note: previous declaration is here
  auto zip = cctz_extension::zone_info_source_factory(
       ^
4 warnings generated.

[devbww]

Thanks Pieter.

In all but perhaps one(*) of these cases, the status quo is intentional:

• In the -Wshadow/-Wshadow-uncaptured-local cases the use of the same name affirms that the objects play the same role. Inventing new names would be confusing.
• In the -Wtautological-type-limit-compare case, the warning depends on how your toolchain represents std::time_t. If the latter is not 64 bits, the comparisons are not tautological.
• In the -Wunused-template case, the warning depends on the shape of your std::tm and its compatibility defines.

In general we don't agree with all the diagnostics enabled by -Wall. My suggestion would be judicious use of additional -Wno... flags when compiling this module if you want to stick with -Wall. (I see that you're picking CCTZ up via Abseil however, so I'm not sure what their position is on changing flags, or what warnings they support in general.)

[Aside: You and/or Abseil could probably omit the time_zone_libc support altogether. It is limited to localtime/UTC, and is mostly intended as a testing mechanism.]

(*) The one case we may want to change is where pos is used as a std::string position, and then as a stdio file position in an enclosed block. Those objects don't play the same role.

[devbww]

#133 was just merged to address the unintentional -Wshadow issues.

[devbww]

#156 and #159 were recently merged to address the remaining -Wshadow issues.

[devbww]

Given the merged PRs listed above, I'm going to close this. But please feel free to re-open if issues remain.