Fix Firestore source errors under VS2017 (#1515)

* Project file updates from sync_project.rb

* Fix misc compile errors under VS2017

* Fix util/hashing under VS2017

std::hash<int> is not just a pass through in Microsoft's STL.

* Disable unsafe code warnings in VS2017

... where comparing against a reference implementation that has no easy
safe equivalent.

* Handle drive letters in paths on Windows

10 files changed, 155 insertions, 67 deletions

diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj
index 6729eab..08e4926 100644
--- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj
+++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj
@@ -610,8 +610,8 @@
 		546854A720A3681B004BDBD5 /* remote */ = {
 			isa = PBXGroup;
 			children = (
-				B6D1B68420E2AB1A00B35856 /* exponential_backoff_test.cc */,
 				546854A820A36867004BDBD5 /* datastore_test.cc */,
+				B6D1B68420E2AB1A00B35856 /* exponential_backoff_test.cc */,
 				61F72C5520BC48FD001A68CB /* serializer_test.cc */,
 			);
 			path = remote;
@@ -1750,7 +1750,6 @@
 				5492E03F2021401F00B64F25 /* FSTHelpers.mm in Sources */,
 				DE2EF0861F3D0B6E003D0CDC /* FSTImmutableSortedDictionary+Testing.m in Sources */,
 				DE2EF0871F3D0B6E003D0CDC /* FSTImmutableSortedSet+Testing.m in Sources */,
-				B6D1B68520E2AB1B00B35856 /* exponential_backoff_test.cc in Sources */,
 				5491BC721FB44593008B3588 /* FSTIntegrationTestCase.mm in Sources */,
 				5492E0A72021552D00B64F25 /* FSTLevelDBKeyTests.mm in Sources */,
 				5492E0A82021552D00B64F25 /* FSTLevelDBLocalStoreTests.mm in Sources */,
@@ -1808,6 +1807,7 @@
 				B6FB468E208F9BAB00554BA2 /* executor_libdispatch_test.mm in Sources */,
 				B6FB468F208F9BAE00554BA2 /* executor_std_test.cc in Sources */,
 				B6FB4690208F9BB300554BA2 /* executor_test.cc in Sources */,
+				B6D1B68520E2AB1B00B35856 /* exponential_backoff_test.cc in Sources */,
 				549CCA5720A36E1F00BCEB75 /* field_mask_test.cc in Sources */,
 				B686F2AF2023DDEE0028D6BE /* field_path_test.cc in Sources */,
 				54A0352620A3AED0003E0143 /* field_transform_test.mm in Sources */,
diff --git a/Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h b/Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h
index 960e3f6..20c11a7 100644
--- a/Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h
+++ b/Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h
@@ -119,7 +119,9 @@ class SortedMapIterator {
   pointer get() const {
     switch (tag_) {
       case Tag::Array:
-        return array_iter_;
+        // std::array::iterator is not guaranteed to be a bare pointer but will
+        // be a RandomAccessIterator which does have operator*().
+        return &*array_iter_;
       case Tag::Tree:
         return tree_iter_.get();
     }
diff --git a/Firestore/core/src/firebase/firestore/util/bits.h b/Firestore/core/src/firebase/firestore/util/bits.h
index 94a018f..591c306 100644
--- a/Firestore/core/src/firebase/firestore/util/bits.h
+++ b/Firestore/core/src/firebase/firestore/util/bits.h
@@ -24,13 +24,13 @@
 
 #include <cstdint>
 
-class Bits_Port32_Test;
-class Bits_Port64_Test;
-
 namespace firebase {
 namespace firestore {
 namespace util {
 
+class Bits_Port32_Test;
+class Bits_Port64_Test;
+
 class Bits {
  public:
   /** Return floor(log2(n)) for positive integer n.  Returns -1 iff n == 0. */
@@ -81,9 +81,9 @@ inline int Bits::Log2FloorNonZero64(uint64_t n) {
   return 63 ^ __builtin_clzll(n);
 }
 
-#elif defined(COMPILER_MSVC)
+#elif defined(_MSC_VER)
 
-inline int Bits::Log2FloorNonZero(uint32 n) {
+inline int Bits::Log2FloorNonZero(uint32_t n) {
 #ifdef _M_IX86
   _asm {
     bsr ebx, n
@@ -95,7 +95,7 @@ inline int Bits::Log2FloorNonZero(uint32 n) {
 #endif
 }
 
-inline int Bits::Log2Floor(uint32 n) {
+inline int Bits::Log2Floor(uint32_t n) {
 #ifdef _M_IX86
   _asm {
     xor ebx, ebx
@@ -120,7 +120,7 @@ inline int Bits::Log2FloorNonZero64(uint64_t n) {
   return Bits::Log2FloorNonZero64_Portable(n);
 }
 
-#else  // !__GNUC__ && !COMPILER_MSVC
+#else  // !__GNUC__ && !_MSC_VER
 
 inline int Bits::Log2Floor64(uint64_t n) {
   return Bits::Log2Floor64_Portable(n);
diff --git a/Firestore/core/src/firebase/firestore/util/hard_assert.h b/Firestore/core/src/firebase/firestore/util/hard_assert.h
index e60d71a..7b66f33 100644
--- a/Firestore/core/src/firebase/firestore/util/hard_assert.h
+++ b/Firestore/core/src/firebase/firestore/util/hard_assert.h
@@ -21,6 +21,12 @@
 
 #include "Firestore/core/src/firebase/firestore/util/string_format.h"
 
+#if defined(_MSC_VER)
+#define FIRESTORE_FUNCTION_NAME __FUNCSIG__
+#else
+#define FIRESTORE_FUNCTION_NAME __PRETTY_FUNCTION__
+#endif
+
 /**
  * Fails the current function if the given condition is false.
  *
@@ -30,14 +36,14 @@
  * @param format (optional) A format string suitable for util::StringFormat.
  * @param ... format arguments to pass to util::StringFormat.
  */
-#define HARD_ASSERT(condition, ...)                                       \
-  do {                                                                    \
-    if (!(condition)) {                                                   \
-      std::string _message =                                              \
-          firebase::firestore::util::StringFormat(__VA_ARGS__);           \
-      firebase::firestore::util::internal::Fail(                          \
-          __FILE__, __PRETTY_FUNCTION__, __LINE__, _message, #condition); \
-    }                                                                     \
+#define HARD_ASSERT(condition, ...)                                           \
+  do {                                                                        \
+    if (!(condition)) {                                                       \
+      std::string _message =                                                  \
+          firebase::firestore::util::StringFormat(__VA_ARGS__);               \
+      firebase::firestore::util::internal::Fail(                              \
+          __FILE__, FIRESTORE_FUNCTION_NAME, __LINE__, _message, #condition); \
+    }                                                                         \
   } while (0)
 
 /**
@@ -48,12 +54,12 @@
  * @param format A format string suitable for util::StringFormat.
  * @param ... format arguments to pass to util::StringFormat.
  */
-#define HARD_FAIL(...)                                                       \
-  do {                                                                       \
-    std::string _failure =                                                   \
-        firebase::firestore::util::StringFormat(__VA_ARGS__);                \
-    firebase::firestore::util::internal::Fail(__FILE__, __PRETTY_FUNCTION__, \
-                                              __LINE__, _failure);           \
+#define HARD_FAIL(...)                                          \
+  do {                                                          \
+    std::string _failure =                                      \
+        firebase::firestore::util::StringFormat(__VA_ARGS__);   \
+    firebase::firestore::util::internal::Fail(                  \
+        __FILE__, FIRESTORE_FUNCTION_NAME, __LINE__, _failure); \
   } while (0)
 
 /**
diff --git a/Firestore/core/src/firebase/firestore/util/hashing.h b/Firestore/core/src/firebase/firestore/util/hashing.h
index 21c0bd6..5219d64 100644
--- a/Firestore/core/src/firebase/firestore/util/hashing.h
+++ b/Firestore/core/src/firebase/firestore/util/hashing.h
@@ -82,7 +82,8 @@ struct has_std_hash {
  * `decltype(std::hash<T>{}(std::declval<T>()))`.
  */
 template <typename T>
-using std_hash_type = typename std::enable_if<has_std_hash<T>{}, size_t>::type;
+using std_hash_type =
+    typename std::enable_if<has_std_hash<T>::value, size_t>::type;
 
 /**
  * Combines a hash_value with whatever accumulated state there is so far.
@@ -151,9 +152,11 @@ auto RankedInvokeHash(const Range& range, HashChoice<2>)
   size_t size = 0;
   for (auto&& element : range) {
     ++size;
-    result = Combine(result, InvokeHash(element));
+    size_t piece = InvokeHash(element);
+    result = Combine(result, piece);
   }
-  result = Combine(result, size);
+  size_t size_hash = InvokeHash(size);
+  result = Combine(result, size_hash);
   return result;
 }
 
diff --git a/Firestore/core/src/firebase/firestore/util/path.cc b/Firestore/core/src/firebase/firestore/util/path.cc
index 940f12a..0da3b23 100644
--- a/Firestore/core/src/firebase/firestore/util/path.cc
+++ b/Firestore/core/src/firebase/firestore/util/path.cc
@@ -16,14 +16,45 @@
 
 #include "Firestore/core/src/firebase/firestore/util/path.h"
 
+#include "absl/strings/ascii.h"
+#include "absl/strings/string_view.h"
+
 namespace firebase {
 namespace firestore {
 namespace util {
 
+namespace {
+
+static constexpr absl::string_view::size_type npos = absl::string_view::npos;
+
+/** Returns the given path with its leading drive letter removed. */
+inline absl::string_view StripDriveLetter(absl::string_view path) {
+#if defined(_WIN32)
+  if (path.size() >= 2 && path[1] == ':' && absl::ascii_isalpha(path[0])) {
+    return path.substr(2);
+  }
+  return path;
+
+#else
+  return path;
+#endif  // defined(_WIN32)
+}
+
+/** Returns true if the given character is a pathname separator. */
+inline bool IsSeparator(char c) {
+#if defined(_WIN32)
+  return c == '/' || c == '\\';
+#else
+  return c == '/';
+#endif  // defined(_WIN32)
+}
+
+}  // namespace
+
 absl::string_view Path::Basename(absl::string_view pathname) {
   size_t slash = pathname.find_last_of('/');
 
-  if (slash == absl::string_view::npos) {
+  if (slash == npos) {
     // No path separator found => the whole string.
     return pathname;
   }
@@ -35,7 +66,7 @@ absl::string_view Path::Basename(absl::string_view pathname) {
 absl::string_view Path::Dirname(absl::string_view pathname) {
   size_t last_slash = pathname.find_last_of('/');
 
-  if (last_slash == absl::string_view::npos) {
+  if (last_slash == npos) {
     // No path separator found => empty string. Conformance with POSIX would
     // have us return "." here.
     return pathname.substr(0, 0);
@@ -43,7 +74,7 @@ absl::string_view Path::Dirname(absl::string_view pathname) {
 
   // Collapse runs of slashes.
   size_t nonslash = pathname.find_last_not_of('/', last_slash);
-  if (nonslash == absl::string_view::npos) {
+  if (nonslash == npos) {
     // All characters preceding the last path separator are slashes
     return pathname.substr(0, 1);
   }
@@ -55,12 +86,8 @@ absl::string_view Path::Dirname(absl::string_view pathname) {
 }
 
 bool Path::IsAbsolute(absl::string_view path) {
-#if defined(_WIN32)
-#error "Handle drive letters"
-
-#else
-  return !path.empty() && path.front() == '/';
-#endif
+  path = StripDriveLetter(path);
+  return !path.empty() && IsSeparator(path.front());
 }
 
 void Path::JoinAppend(std::string* base, absl::string_view path) {
@@ -69,7 +96,7 @@ void Path::JoinAppend(std::string* base, absl::string_view path) {
 
   } else {
     size_t nonslash = base->find_last_not_of('/');
-    if (nonslash != std::string::npos) {
+    if (nonslash != npos) {
       base->resize(nonslash + 1);
       base->push_back('/');
     }
diff --git a/Firestore/core/src/firebase/firestore/util/status.cc b/Firestore/core/src/firebase/firestore/util/status.cc
index 3e8585a..beaa88a 100644
--- a/Firestore/core/src/firebase/firestore/util/status.cc
+++ b/Firestore/core/src/firebase/firestore/util/status.cc
@@ -100,18 +100,24 @@ static FirestoreErrorCode CodeForErrno(int errno_code) {
 #if defined(EISNAM)
     case EISNAM:  // Is a named type file
 #endif
-    case ENOTBLK:    // Block device required
-    case ENOTCONN:   // The socket is not connected
-    case EPIPE:      // Broken pipe
+#if defined(ENOTBLK)
+    case ENOTBLK:  // Block device required
+#endif
+    case ENOTCONN:  // The socket is not connected
+    case EPIPE:     // Broken pipe
+#if defined(ESHUTDOWN)
     case ESHUTDOWN:  // Cannot send after transport endpoint shutdown
-    case ETXTBSY:    // Text file busy
+#endif
+    case ETXTBSY:  // Text file busy
 #if defined(EUNATCH)
     case EUNATCH:  // Protocol driver not attached
 #endif
       return FirestoreErrorCode::FailedPrecondition;
 
-    case ENOSPC:   // No space left on device
-    case EDQUOT:   // Disk quota exceeded
+    case ENOSPC:  // No space left on device
+#if defined(EDQUOT)
+    case EDQUOT:  // Disk quota exceeded
+#endif
     case EMFILE:   // Too many open files
     case EMLINK:   // Too many links
     case ENFILE:   // Too many open files in system
@@ -119,7 +125,9 @@ static FirestoreErrorCode CodeForErrno(int errno_code) {
     case ENODATA:  // No message is available on the STREAM read queue
     case ENOMEM:   // Not enough space
     case ENOSR:    // No STREAM resources
-    case EUSERS:   // Too many users
+#if defined(EUSERS)
+    case EUSERS:  // Too many users
+#endif
       return FirestoreErrorCode::ResourceExhausted;
 
 #if defined(ECHRNG)
@@ -133,13 +141,17 @@ static FirestoreErrorCode CodeForErrno(int errno_code) {
 #if defined(ENOPKG)
     case ENOPKG:  // Package not installed
 #endif
-    case ENOSYS:           // Function not implemented
-    case ENOTSUP:          // Operation not supported
-    case EAFNOSUPPORT:     // Address family not supported
-    case EPFNOSUPPORT:     // Protocol family not supported
+    case ENOSYS:        // Function not implemented
+    case ENOTSUP:       // Operation not supported
+    case EAFNOSUPPORT:  // Address family not supported
+#if defined(EPFNOSUPPORT)
+    case EPFNOSUPPORT:  // Protocol family not supported
+#endif
     case EPROTONOSUPPORT:  // Protocol not supported
+#if defined(ESOCKTNOSUPPORT)
     case ESOCKTNOSUPPORT:  // Socket type not supported
-    case EXDEV:            // Improper link
+#endif
+    case EXDEV:  // Improper link
       return FirestoreErrorCode::Unimplemented;
 
     case EAGAIN:  // Resource temporarily unavailable
@@ -150,7 +162,9 @@ static FirestoreErrorCode CodeForErrno(int errno_code) {
     case ECONNABORTED:  // Connection aborted
     case ECONNRESET:    // Connection reset
     case EINTR:         // Interrupted function call
-    case EHOSTDOWN:     // Host is down
+#if defined(EHOSTDOWN)
+    case EHOSTDOWN:  // Host is down
+#endif
     case EHOSTUNREACH:  // Host is unreachable
     case ENETDOWN:      // Network is down
     case ENETRESET:     // Connection aborted by network
@@ -163,7 +177,9 @@ static FirestoreErrorCode CodeForErrno(int errno_code) {
       return FirestoreErrorCode::Unavailable;
 
     case EDEADLK:  // Resource deadlock avoided
-    case ESTALE:   // Stale file handle
+#if defined(ESTALE)
+    case ESTALE:  // Stale file handle
+#endif
       return FirestoreErrorCode::Aborted;
 
     case ECANCELED:  // Operation cancelled
diff --git a/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc b/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc
index 75353d9..acd0642 100644
--- a/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc
+++ b/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc
@@ -14,6 +14,13 @@
  * limitations under the License.
  */
 
+// Disable warnings in the MSVC standard library about the use of std::mismatch.
+// There's no guaranteed safe way to use it in C++11 but the test verifies that
+// our implementation matches the standard so we need to call it.
+#if defined(_MSC_VER)
+#define _SCL_SECURE_NO_WARNINGS 1
+#endif
+
 #include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h"
 
 #include <numeric>
diff --git a/Firestore/core/test/firebase/firestore/util/hashing_test.cc b/Firestore/core/test/firebase/firestore/util/hashing_test.cc
index 2c5c2f7..d762bbe 100644
--- a/Firestore/core/test/firebase/firestore/util/hashing_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/hashing_test.cc
@@ -28,7 +28,7 @@ namespace util {
 
 struct HasHashMember {
   size_t Hash() const {
-    return 42;
+    return 42u;
   }
 };
 
@@ -66,22 +66,23 @@ TEST(HashingTest, StringView) {
   // that, but that requires an explicit specialization. Since we're only
   // defining this for compatibility with Objective-C and not really sensitive
   // to performance or hash quality here, this is good enough.
-  size_t expected = 'a';
-  expected = 31u * expected + 1;
+  size_t expected = std::hash<unsigned char>{}('a');
+  expected = 31u * expected + std::hash<size_t>{}(1);
   ASSERT_EQ(expected, Hash(absl::string_view{"a"}));
 }
 
 TEST(HashingTest, SizeT) {
-  ASSERT_EQ(42u, Hash(size_t{42u}));
+  size_t expected = std::hash<size_t>{}(42u);
+  ASSERT_EQ(expected, Hash(size_t{42u}));
 }
 
 TEST(HashingTest, Array) {
   int values[] = {0, 1, 2};
 
-  size_t expected = 0;
-  expected = 31 * expected + 1;
-  expected = 31 * expected + 2;
-  expected = 31 * expected + 3;  // length of array
+  size_t expected = std::hash<int>{}(0);
+  expected = 31u * expected + std::hash<int>{}(1);
+  expected = 31u * expected + std::hash<int>{}(2);
+  expected = 31u * expected + std::hash<size_t>{}(3);  // length of array
   ASSERT_EQ(expected, Hash(values));
 }
 
@@ -91,7 +92,10 @@ TEST(HashingTest, HasHashMember) {
 
 TEST(HashingTest, RangeOfStdHashable) {
   std::vector<int> values{42};
-  ASSERT_EQ(31u * 42u + 1, Hash(values));
+
+  size_t expected = std::hash<int>{}(42);
+  expected = 31u * expected + std::hash<size_t>{}(1);  // length of array
+  ASSERT_EQ(expected, Hash(values));
 
   std::vector<int> values_leading_zero{0, 42};
   std::vector<int> values_trailing_zero{42, 0};
@@ -103,18 +107,30 @@ TEST(HashingTest, RangeOfStdHashable) {
 
 TEST(HashingTest, RangeOfHashMember) {
   std::vector<HasHashMember> values{HasHashMember{}};
-  ASSERT_EQ(31u * 42u + 1, Hash(values));
+
+  // We trust the underlying Hash() member to do its thing, so unlike the other
+  // examples, the 42u here is not run through std::hash<size_t>{}().
+  size_t expected = 42u;
+  expected = 31u * expected + std::hash<size_t>{}(1);  // length of array
+  ASSERT_EQ(expected, Hash(values));
 }
 
 TEST(HashingTest, Composite) {
   // Verify the result ends up as if hand-rolled
-  EXPECT_EQ(1u, Hash(1));
-  EXPECT_EQ(31u, Hash(1, 0));
-  EXPECT_EQ(31u * 31u, Hash(1, 0, 0));
+  EXPECT_EQ(std::hash<int>{}(1), Hash(1));
+
+  size_t expected = std::hash<int>{}(1);
+  expected = 31 * expected + std::hash<int>{}(0);
+  EXPECT_EQ(expected, Hash(1, 0));
+
+  expected = std::hash<int>{}(1);
+  expected = 31 * expected + std::hash<int>{}(0);
+  expected = 31 * expected + std::hash<int>{}(0);
+  EXPECT_EQ(expected, Hash(1, 0, 0));
 
-  size_t expected = Hash(1);
-  expected = 31 * expected + Hash(2);
-  expected = 31 * expected + Hash(3);
+  expected = Hash(1);
+  expected = 31u * expected + Hash(2);
+  expected = 31u * expected + Hash(3);
   EXPECT_EQ(expected, Hash(1, 2, 3));
 }
 
diff --git a/Firestore/core/test/firebase/firestore/util/strerror_test.cc b/Firestore/core/test/firebase/firestore/util/strerror_test.cc
index 854cb08..e6e64a4 100644
--- a/Firestore/core/test/firebase/firestore/util/strerror_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/strerror_test.cc
@@ -23,9 +23,20 @@ namespace firestore {
 namespace util {
 
 TEST(StrErrorTest, ValidErrorCode) {
+#if defined(_MSC_VER)
+#pragma warning(push)
+  // strerror is unsafe generally, but it's used here as the simplest possible
+  // reference implementation.
+#pragma warning(disable : 4996)
+#endif
+
   errno = EAGAIN;
   EXPECT_EQ(StrError(EINTR), strerror(EINTR));
   EXPECT_EQ(errno, EAGAIN);
+
+#if defined(_MSC_VER)
+#pragma warning(pop)
+#endif
 }
 
 TEST(StrErrorTest, InvalidErrorCode) {