Fix a number of c++ build errors (firebase#715)

rsgowman · wilhuff · commit c827bf438b03 · 2018-01-27T16:16:41.000-08:00
* Move -fvisibility-inlines-hidden from common_flags to cxx_flags This option isn't supported by C (and causes the build to fail under at least rodete). Manifested error was: ``` -- Looking for include file openssl/rand.h -- Looking for include file openssl/rand.h - not found CMake Error at core/src/firebase/firestore/util/CMakeLists.txt:94 (message): No implementation for SecureRandom available. -- Configuring incomplete, errors occurred! ``` which is completely misleading. :( * Fix exception include for std::logic_error Was causing compiler error on (at least) rodete. (http://en.cppreference.com/w/cpp/error/logic_error suggests that stdexcept is the correct header.) * Remove 'const' from vector contents. vectors cannot contain const objects as the contained objects are required to be assignable and copy constructable. (Not *quite* strictly true; since c++11, this requirement now depends on the operations performed on the container. But my compiler objects at any rate.) http://en.cppreference.com/w/cpp/container/vector https://stackoverflow.com/questions/8685257/why-cant-you-put-a-const-object-into-a-stl-container * Add brackets as suggested (required) by my compiler. * Add missing include For LLONG_MIN, etc. * Enable gnu extension to c++11 for firestore/util *test*. We disable them by default (in cmake/CompilerSetup.cmake) but our tests requires hexidecimal floating point, which is not supported in c++11 (though is supported in C++17, gnu++11, and others). http://en.cppreference.com/w/cpp/language/floating_literal https://bugzilla.redhat.com/show_bug.cgi?id=1321986 * Fix printf format for uint64_t http://en.cppreference.com/w/cpp/types/integer https://stackoverflow.com/questions/9225567/how-to-print-a-int64-t-type-in-c * Allow 0 length printf template strings in tests. * Get rid of a multi line comment. The backslash is apparently interpreted by the compiler cause the next line to be ignored too. In this case, the next line is (currently) a comment, and would be ignored anyways. (But that doesn't stop the compiler from yelling.) * Run ./scripts/style.sh
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.cc b/Firestore/core/src/firebase/firestore/model/field_value.cc
@@ -95,7 +95,7 @@ FieldValue& FieldValue::operator=(const FieldValue& value) {
       break;
     case Type::Blob: {
       // copy-and-swap
-      std::vector<const uint8_t> tmp = value.blob_value_;
+      std::vector<uint8_t> tmp = value.blob_value_;
       std::swap(blob_value_, tmp);
       break;
     }
@@ -104,7 +104,7 @@ FieldValue& FieldValue::operator=(const FieldValue& value) {
       break;
     case Type::Array: {
       // copy-and-swap
-      std::vector<const FieldValue> tmp = value.array_value_;
+      std::vector<FieldValue> tmp = value.array_value_;
       std::swap(array_value_, tmp);
       break;
     }
@@ -228,7 +228,7 @@ FieldValue FieldValue::StringValue(std::string&& value) {
 FieldValue FieldValue::BlobValue(const uint8_t* source, size_t size) {
   FieldValue result;
   result.SwitchTo(Type::Blob);
-  std::vector<const uint8_t> copy(source, source + size);
+  std::vector<uint8_t> copy(source, source + size);
   std::swap(result.blob_value_, copy);
   return result;
 }
@@ -240,12 +240,12 @@ FieldValue FieldValue::GeoPointValue(const GeoPoint& value) {
   return result;
 }
 
-FieldValue FieldValue::ArrayValue(const std::vector<const FieldValue>& value) {
-  std::vector<const FieldValue> copy(value);
+FieldValue FieldValue::ArrayValue(const std::vector<FieldValue>& value) {
+  std::vector<FieldValue> copy(value);
   return ArrayValue(std::move(copy));
 }
 
-FieldValue FieldValue::ArrayValue(std::vector<const FieldValue>&& value) {
+FieldValue FieldValue::ArrayValue(std::vector<FieldValue>&& value) {
   FieldValue result;
   result.SwitchTo(Type::Array);
   std::swap(result.array_value_, value);
@@ -368,13 +368,13 @@ void FieldValue::SwitchTo(const Type type) {
       break;
     case Type::Blob:
       // Do not even bother to allocate a new array of size 0.
-      new (&blob_value_) std::vector<const uint8_t>();
+      new (&blob_value_) std::vector<uint8_t>();
       break;
     case Type::GeoPoint:
       new (&geo_point_value_) GeoPoint();
       break;
     case Type::Array:
-      new (&array_value_) std::vector<const FieldValue>();
+      new (&array_value_) std::vector<FieldValue>();
       break;
     case Type::Object:
       new (&object_value_) std::map<const std::string, const FieldValue>();
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h
@@ -105,8 +105,8 @@ class FieldValue {
   static FieldValue BlobValue(const uint8_t* source, size_t size);
   // static FieldValue ReferenceValue();
   static FieldValue GeoPointValue(const GeoPoint& value);
-  static FieldValue ArrayValue(const std::vector<const FieldValue>& value);
-  static FieldValue ArrayValue(std::vector<const FieldValue>&& value);
+  static FieldValue ArrayValue(const std::vector<FieldValue>& value);
+  static FieldValue ArrayValue(std::vector<FieldValue>&& value);
   static FieldValue ObjectValue(
       const std::map<const std::string, const FieldValue>& value);
   static FieldValue ObjectValue(
@@ -132,9 +132,9 @@ class FieldValue {
     Timestamp timestamp_value_;
     ServerTimestamp server_timestamp_value_;
     std::string string_value_;
-    std::vector<const uint8_t> blob_value_;
+    std::vector<uint8_t> blob_value_;
     GeoPoint geo_point_value_;
-    std::vector<const FieldValue> array_value_;
+    std::vector<FieldValue> array_value_;
     std::map<const std::string, const FieldValue> object_value_;
   };
 };
diff --git a/Firestore/core/src/firebase/firestore/util/assert_stdio.cc b/Firestore/core/src/firebase/firestore/util/assert_stdio.cc
@@ -16,7 +16,7 @@
 
 #include <stdarg.h>
 
-#include <exception>
+#include <stdexcept>
 #include <string>
 
 #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
diff --git a/Firestore/core/test/firebase/firestore/core/target_id_generator_test.cc b/Firestore/core/test/firebase/firestore/core/target_id_generator_test.cc
@@ -43,8 +43,8 @@ TEST(TargetIdGenerator, SkipPast) {
   for (int i = 4; i < 12; ++i) {
     TargetIdGenerator a = TargetIdGenerator::LocalStoreTargetIdGenerator(i);
     TargetIdGenerator b = TargetIdGenerator::SyncEngineTargetIdGenerator(i);
-    EXPECT_EQ(i + 2 & ~1, a.NextId());
-    EXPECT_EQ(i + 1 | 1, b.NextId());
+    EXPECT_EQ((i + 2) & ~1, a.NextId());
+    EXPECT_EQ((i + 1) | 1, b.NextId());
   }
 
   EXPECT_EQ(13, TargetIdGenerator::SyncEngineTargetIdGenerator(12).NextId());
diff --git a/Firestore/core/test/firebase/firestore/model/field_value_test.cc b/Firestore/core/test/firebase/firestore/model/field_value_test.cc
@@ -16,6 +16,8 @@
 
 #include "Firestore/core/src/firebase/firestore/model/field_value.h"
 
+#include <limits.h>
+
 #include <vector>
 
 #include "gtest/gtest.h"
@@ -152,15 +154,14 @@ TEST(FieldValue, GeoPointType) {
 }
 
 TEST(FieldValue, ArrayType) {
-  const FieldValue empty =
-      FieldValue::ArrayValue(std::vector<const FieldValue>{});
-  std::vector<const FieldValue> array{FieldValue::NullValue(),
-                                      FieldValue::BooleanValue(true),
-                                      FieldValue::BooleanValue(false)};
+  const FieldValue empty = FieldValue::ArrayValue(std::vector<FieldValue>{});
+  std::vector<FieldValue> array{FieldValue::NullValue(),
+                                FieldValue::BooleanValue(true),
+                                FieldValue::BooleanValue(false)};
   // copy the array
   const FieldValue small = FieldValue::ArrayValue(array);
-  std::vector<const FieldValue> another_array{FieldValue::BooleanValue(true),
-                                              FieldValue::BooleanValue(false)};
+  std::vector<FieldValue> another_array{FieldValue::BooleanValue(true),
+                                        FieldValue::BooleanValue(false)};
   // move the array
   const FieldValue large = FieldValue::ArrayValue(std::move(another_array));
   EXPECT_EQ(Type::Array, empty.type());
@@ -288,18 +289,17 @@ TEST(FieldValue, Copy) {
   clone = null_value;
   EXPECT_EQ(FieldValue::NullValue(), clone);
 
-  const FieldValue array_value =
-      FieldValue::ArrayValue(std::vector<const FieldValue>{
-          FieldValue::TrueValue(), FieldValue::FalseValue()});
+  const FieldValue array_value = FieldValue::ArrayValue(std::vector<FieldValue>{
+      FieldValue::TrueValue(), FieldValue::FalseValue()});
   clone = array_value;
-  EXPECT_EQ(FieldValue::ArrayValue(std::vector<const FieldValue>{
+  EXPECT_EQ(FieldValue::ArrayValue(std::vector<FieldValue>{
                 FieldValue::TrueValue(), FieldValue::FalseValue()}),
             clone);
-  EXPECT_EQ(FieldValue::ArrayValue(std::vector<const FieldValue>{
+  EXPECT_EQ(FieldValue::ArrayValue(std::vector<FieldValue>{
                 FieldValue::TrueValue(), FieldValue::FalseValue()}),
             array_value);
   clone = clone;
-  EXPECT_EQ(FieldValue::ArrayValue(std::vector<const FieldValue>{
+  EXPECT_EQ(FieldValue::ArrayValue(std::vector<FieldValue>{
                 FieldValue::TrueValue(), FieldValue::FalseValue()}),
             clone);
   clone = null_value;
@@ -385,10 +385,10 @@ TEST(FieldValue, Move) {
   clone = null_value;
   EXPECT_EQ(FieldValue::NullValue(), clone);
 
-  FieldValue array_value = FieldValue::ArrayValue(std::vector<const FieldValue>{
+  FieldValue array_value = FieldValue::ArrayValue(std::vector<FieldValue>{
       FieldValue::TrueValue(), FieldValue::FalseValue()});
   clone = std::move(array_value);
-  EXPECT_EQ(FieldValue::ArrayValue(std::vector<const FieldValue>{
+  EXPECT_EQ(FieldValue::ArrayValue(std::vector<FieldValue>{
                 FieldValue::TrueValue(), FieldValue::FalseValue()}),
             clone);
   clone = FieldValue::NullValue();
@@ -417,7 +417,7 @@ TEST(FieldValue, CompareMixedType) {
   const FieldValue blob_value = FieldValue::BlobValue(Bytes("abc"), 4);
   const FieldValue geo_point_value = FieldValue::GeoPointValue({1, 2});
   const FieldValue array_value =
-      FieldValue::ArrayValue(std::vector<const FieldValue>());
+      FieldValue::ArrayValue(std::vector<FieldValue>());
   const FieldValue object_value =
       FieldValue::ObjectValue(std::map<const std::string, const FieldValue>());
   EXPECT_TRUE(null_value < true_value);
diff --git a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
@@ -12,6 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+set(CMAKE_CXX_EXTENSIONS ON)
+
+# Required to allow 0 length printf style strings for testing purposes.
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-format-zero-length")
+
 if(HAVE_ARC4RANDOM)
   cc_test(
     firebase_firestore_util_arc4random_test
diff --git a/Firestore/core/test/firebase/firestore/util/comparison_test.cc b/Firestore/core/test/firebase/firestore/util/comparison_test.cc
@@ -16,6 +16,7 @@
 
 #include "Firestore/core/src/firebase/firestore/util/comparison.h"
 
+#include <inttypes.h>
 #include <math.h>
 
 #include <limits>
@@ -84,17 +85,17 @@ TEST(Comparison, DoubleCompare) {
   ASSERT_SAME(Compare<double>(-0, 0));
 }
 
-#define ASSERT_BIT_EQUALS(expected, actual)            \
-  do {                                                 \
-    uint64_t expectedBits = DoubleBits(expected);      \
-    uint64_t actualBits = DoubleBits(actual);          \
-    if (expectedBits != actualBits) {                  \
-      std::string message = StringPrintf(              \
-          "Expected <%f> to compare equal to <%f> "    \
-          "with bits <%llX> equal to <%llX>",          \
-          actual, expected, actualBits, expectedBits); \
-      FAIL() << message;                               \
-    }                                                  \
+#define ASSERT_BIT_EQUALS(expected, actual)                 \
+  do {                                                      \
+    uint64_t expectedBits = DoubleBits(expected);           \
+    uint64_t actualBits = DoubleBits(actual);               \
+    if (expectedBits != actualBits) {                       \
+      std::string message = StringPrintf(                   \
+          "Expected <%f> to compare equal to <%f> "         \
+          "with bits <%" PRIu64 "> equal to <%" PRIu64 ">", \
+          actual, expected, actualBits, expectedBits);      \
+      FAIL() << message;                                    \
+    }                                                       \
   } while (0);
 
 #define ASSERT_MIXED_SAME(doubleValue, longValue)                              \
diff --git a/Firestore/core/test/firebase/firestore/util/log_test.cc b/Firestore/core/test/firebase/firestore/util/log_test.cc
@@ -30,7 +30,7 @@ namespace util {
 //
 // You can fix it with:
 //
-//   defaults write firebase_firestore_util_log_apple_test \
+//   defaults write firebase_firestore_util_log_apple_test
 //       /google/firebase/debug_mode NO
 TEST(Log, SetAndGet) {
   LogSetLevel(kLogLevelVerbose);
diff --git a/cmake/CompilerSetup.cmake b/cmake/CompilerSetup.cmake
@@ -41,6 +41,10 @@ if(CLANG OR GNU)
 
     # Delete unused things
     -Wunused-function -Wunused-value -Wunused-variable
+  )
+
+  set(
+    cxx_flags
 
     # Cut down on symbol clutter
     # TODO(wilhuff) try -fvisibility=hidden
@@ -64,7 +68,7 @@ if(CLANG OR GNU)
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${flag}")
   endforeach()
 
-  foreach(flag ${common_flags})
+  foreach(flag ${common_flags} ${cxx_flags})
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
   endforeach()
 endif()
