From df71c9ad8c2751672b8ae85915cc7f74f3db08c8 Mon Sep 17 00:00:00 2001 From: Gil Date: Tue, 16 Jan 2018 10:17:27 -0800 Subject: Enable warnings in the CMake build (#669) * Enable warnings when building with GCC or clang * Fix warnings --- Firestore/CMakeLists.txt | 23 +----- .../src/firebase/firestore/util/string_printf.cc | 17 ++-- .../test/firebase/firestore/util/autoid_test.cc | 4 +- .../firebase/firestore/util/string_printf_test.cc | 2 +- cmake/CompilerSetup.cmake | 92 ++++++++++++++++++++++ 5 files changed, 108 insertions(+), 30 deletions(-) create mode 100644 cmake/CompilerSetup.cmake 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(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(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(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) -- cgit v1.2.3