Enable warnings in the CMake build (#669)

* Enable warnings when building with GCC or clang

* Fix warnings

5 files changed, 108 insertions, 30 deletions

diff --git a/Firestore/CMakeLists.txt b/Firestore/CMakeLists.txt
index 4e009e0..553f59c 100644
--- a/Firestore/CMakeLists.txt
+++ b/Firestore/CMakeLists.txt
@@ -36,27 +36,12 @@ if(APPLE)
   find_package(FirebaseCore REQUIRED)
 endif()
 
-# We use C++11
-set(CMAKE_CXX_STANDARD 11)
-set(CMAKE_CXX_STANDARD_REQUIRED ON)
-set(CMAKE_CXX_EXTENSIONS OFF)
+enable_testing()
+add_subdirectory(third_party/abseil-cpp)
+
+include(CompilerSetup)
 
 # Fully qualified imports, project wide
 include_directories(${FIREBASE_SOURCE_DIR})
 
-if(APPLE)
-  # CMake has no special support for Objective-C as a distinct language but enabling modules and
-  # other clang extensions would apply even to regular C++ sources which is nonportable. Keep these
-  # flags separate to avoid misuse.
-  set(
-    OBJC_FLAGS
-    -fobjc-arc
-    -fmodules
-    -fno-autolink
-    -F${FIREBASE_INSTALL_DIR}/Frameworks
-  )
-endif(APPLE)
-
-enable_testing()
-add_subdirectory(third_party/abseil-cpp)
 add_subdirectory(core)
diff --git a/Firestore/core/src/firebase/firestore/util/string_printf.cc b/Firestore/core/src/firebase/firestore/util/string_printf.cc
index 60cc564..9c4e31c 100644
--- a/Firestore/core/src/firebase/firestore/util/string_printf.cc
+++ b/Firestore/core/src/firebase/firestore/util/string_printf.cc
@@ -38,7 +38,7 @@ void StringAppendV(std::string* dst, const char* format, va_list ap) {
   if (result < kSpaceLength) {
     if (result >= 0) {
       // Normal case -- everything fit.
-      dst->append(space, result);
+      dst->append(space, static_cast<size_t>(result));
       return;
     }
 
@@ -49,28 +49,29 @@ void StringAppendV(std::string* dst, const char* format, va_list ap) {
     result = vsnprintf(nullptr, 0, format, backup_ap);
     va_end(backup_ap);
 #endif
+  }
 
-    if (result < 0) {
-      // Just an error.
-      return;
-    }
+  if (result < 0) {
+    // Just an error.
+    return;
   }
+  size_t result_size = static_cast<size_t>(result);
 
   // Increase the buffer size to the size requested by vsnprintf,
   // plus one for the closing \0.
   size_t initial_size = dst->size();
-  size_t target_size = initial_size + result;
+  size_t target_size = initial_size + result_size;
 
   dst->resize(target_size + 1);
   char* buf = &(*dst)[initial_size];
-  int buf_remain = result + 1;
+  size_t buf_remain = result_size + 1;
 
   // Restore the va_list before we use it again
   va_copy(backup_ap, ap);
   result = vsnprintf(buf, buf_remain, format, backup_ap);
   va_end(backup_ap);
 
-  if (result >= 0 && result < buf_remain) {
+  if (result >= 0 && static_cast<size_t>(result) < buf_remain) {
     // It fit and vsnprintf copied in directly. Resize down one to
     // remove the trailing \0.
     dst->resize(target_size);
diff --git a/Firestore/core/test/firebase/firestore/util/autoid_test.cc b/Firestore/core/test/firebase/firestore/util/autoid_test.cc
index e55a8e9..079b990 100644
--- a/Firestore/core/test/firebase/firestore/util/autoid_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/autoid_test.cc
@@ -25,8 +25,8 @@ using firebase::firestore::util::CreateAutoId;
 TEST(AutoId, IsSane) {
   for (int i = 0; i < 50; i++) {
     std::string auto_id = CreateAutoId();
-    EXPECT_EQ(20, auto_id.length());
-    for (int pos = 0; pos < 20; pos++) {
+    EXPECT_EQ(20u, auto_id.length());
+    for (size_t pos = 0; pos < 20; pos++) {
       char c = auto_id[pos];
       EXPECT_TRUE(isalpha(c) || isdigit(c))
           << "Should be printable ascii character: '" << c << "' in \""
diff --git a/Firestore/core/test/firebase/firestore/util/string_printf_test.cc b/Firestore/core/test/firebase/firestore/util/string_printf_test.cc
index 76f7cde..085be84 100644
--- a/Firestore/core/test/firebase/firestore/util/string_printf_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/string_printf_test.cc
@@ -64,7 +64,7 @@ TEST(StringPrintf, DontOverwriteErrno) {
 
 TEST(StringPrintf, LargeBuf) {
   // Check that the large buffer is handled correctly.
-  int n = 2048;
+  size_t n = 2048;
   char* buf = new char[n + 1];
   memset(buf, ' ', n);
   buf[n] = 0;
diff --git a/cmake/CompilerSetup.cmake b/cmake/CompilerSetup.cmake
new file mode 100644
index 0000000..8bcfa76
--- /dev/null
+++ b/cmake/CompilerSetup.cmake
@@ -0,0 +1,92 @@
+# Copyright 2018 Google
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# C++ Compiler setup
+
+# We use C++11
+set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
+
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+  set(CLANG ON)
+endif()
+
+if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+  set(GNU ON)
+endif()
+
+if(CLANG OR GNU)
+  set(
+    common_flags
+    -Wall -Wextra -Wconversion -Werror
+
+    # Be super pedantic about format strings
+    -Wformat
+
+    # Avoid use of uninitialized values
+    -Wuninitialized
+    -fno-common
+
+    # Delete unused things
+    -Wunused-function -Wunused-value -Wunused-variable
+
+    # Cut down on symbol clutter
+    # TODO(wilhuff) try -fvisibility=hidden
+    -fvisibility-inlines-hidden
+  )
+
+  set(
+    c_flags
+    -Wstrict-prototypes
+  )
+
+  if(CLANG)
+    list(
+      APPEND common_flags
+     -Wconditional-uninitialized -Werror=return-type -Winfinite-recursion -Wmove
+     -Wrange-loop-analysis -Wunreachable-code
+    )
+  endif()
+
+  foreach(flag ${common_flags} ${c_flags})
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${flag}")
+  endforeach()
+
+  foreach(flag ${common_flags})
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
+  endforeach()
+endif()
+
+if(APPLE)
+  # CMake has no special support for Objective-C as a distinct language but
+  # enabling modules and other clang extensions would apply even to regular C++
+  # sources which is nonportable. Keep these flags separate to avoid misuse.
+  set(
+    OBJC_FLAGS
+    -Werror=deprecated-objc-isa-usage
+    -Werror=non-modular-include-in-framework-module
+    -Werror=objc-root-class
+
+    -Wblock-capture-autoreleasing
+    -Wimplicit-atomic-properties
+    -Wnon-modular-include-in-framework-module
+
+    -fobjc-arc
+    -fmodules
+    -fno-autolink
+
+    -F${FIREBASE_INSTALL_DIR}/Frameworks
+  )
+endif(APPLE)