Fix Firestore compilation under Xcode < 9.2 (#1367)

* Don't rely on specialization failure to determine when std::hash is unavailable.

Instead manually declare the conditions under which std::hash should be
defined.

* Fix detection of Objective-C classes in Xcode < 9.2

std::is_base_of<NSObject, NSString>{} is false there so the overloads
defined for Objective-C types weren't getting enabled.

* Add explicit tests for StringFormat using Objective-C objects

* Add explicit tests for HasStdHash

8 files changed, 274 insertions, 21 deletions

diff --git a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt
index 043713f..ed3a301 100644
--- a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt
+++ b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt
@@ -200,6 +200,7 @@ cc_library(
     statusor_internals.h
     string_util.cc
     string_util.h
+    type_traits.h
   DEPENDS
     absl_base
     firebase_firestore_util_base
diff --git a/Firestore/core/src/firebase/firestore/util/hashing.h b/Firestore/core/src/firebase/firestore/util/hashing.h
index d8058c8..21c0bd6 100644
--- a/Firestore/core/src/firebase/firestore/util/hashing.h
+++ b/Firestore/core/src/firebase/firestore/util/hashing.h
@@ -18,6 +18,7 @@
 #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_HASHING_H_
 
 #include <iterator>
+#include <string>
 #include <type_traits>
 
 namespace firebase {
@@ -49,6 +50,41 @@ namespace util {
 namespace impl {
 
 /**
+ * A type trait that identifies whether or not std::hash is available for a
+ * given type.
+ *
+ * This type should not be necessary since specialization failure on an
+ * expression like `decltype(std::hash<K>{}(value)` should be enough to disable
+ * overloads that require `std::hash` to be defined but unfortunately some
+ * standard libraries ship with std::hash defined for all types that only
+ * fail later (e.g. via static_assert). One such implementation is the libc++
+ * that ships with Xcode 8.3.3, which is a supported platform.
+ */
+template <typename T>
+struct has_std_hash {
+  // There may be other types for which std::hash is defined but they don't
+  // matter for our purposes.
+  enum {
+    value = std::is_arithmetic<T>{} || std::is_pointer<T>{} ||
+            std::is_same<T, std::string>{}
+  };
+
+  constexpr operator bool() const {
+    return value;
+  }
+};
+
+/**
+ * A type that's equivalent to size_t if std::hash<T> is defined or a compile
+ * error otherwise.
+ *
+ * This is effectively just a safe implementation of
+ * `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;
+
+/**
  * Combines a hash_value with whatever accumulated state there is so far.
  */
 inline size_t Combine(size_t state, size_t hash_value) {
@@ -100,8 +136,7 @@ auto RankedInvokeHash(const K& value, HashChoice<0>) -> decltype(value.Hash()) {
  * @return The result of `std::hash<K>{}(value)`
  */
 template <typename K>
-auto RankedInvokeHash(const K& value, HashChoice<1>)
-    -> decltype(std::hash<K>{}(value)) {
+std_hash_type<K> RankedInvokeHash(const K& value, HashChoice<1>) {
   return std::hash<K>{}(value);
 }
 
diff --git a/Firestore/core/src/firebase/firestore/util/string_format.h b/Firestore/core/src/firebase/firestore/util/string_format.h
index d691984..01776a9 100644
--- a/Firestore/core/src/firebase/firestore/util/string_format.h
+++ b/Firestore/core/src/firebase/firestore/util/string_format.h
@@ -22,6 +22,7 @@
 #include <utility>
 
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
+#include "Firestore/core/src/firebase/firestore/util/type_traits.h"
 #include "absl/base/attributes.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
@@ -43,7 +44,7 @@ template <int I>
 struct FormatChoice : FormatChoice<I + 1> {};
 
 template <>
-struct FormatChoice<4> {};
+struct FormatChoice<5> {};
 
 }  // namespace internal
 
@@ -87,8 +88,8 @@ class FormatArg : public absl::AlphaNum {
    */
   template <
       typename T,
-      typename = typename std::enable_if<std::is_base_of<NSObject, T>{}>::type>
-  FormatArg(T* object, internal::FormatChoice<0>)
+      typename = typename std::enable_if<is_objective_c_pointer<T>{}>::type>
+  FormatArg(T object, internal::FormatChoice<1>)
       : AlphaNum{MakeStringView([object description])} {
   }
 
@@ -96,20 +97,9 @@ class FormatArg : public absl::AlphaNum {
    * Creates a FormatArg from any Objective-C Class type. Objective-C Class
    * types are a special struct that aren't of a type derived from NSObject.
    */
-  FormatArg(Class object, internal::FormatChoice<0>)
+  FormatArg(Class object, internal::FormatChoice<1>)
       : AlphaNum{MakeStringView(NSStringFromClass(object))} {
   }
-
-  /**
-   * Creates a FormatArg from any id pointer. Note that instances of `id<Foo>`
-   * (which means "pointer conforming to the protocol Foo") do not match this
-   * without first casting to type `id`. There's no way to express a template of
-   * `id<T>` since `id<Foo>` isn't actually a C++ template and `id` isn't a
-   * parameterized C++ class.
-   */
-  FormatArg(id object, internal::FormatChoice<0>)
-      : AlphaNum{MakeStringView([object description])} {
-  }
 #endif
 
   /**
@@ -117,7 +107,7 @@ class FormatArg : public absl::AlphaNum {
    * handled specially to avoid ambiguity with generic pointers, which are
    * handled differently.
    */
-  FormatArg(std::nullptr_t, internal::FormatChoice<1>) : AlphaNum{"null"} {
+  FormatArg(std::nullptr_t, internal::FormatChoice<2>) : AlphaNum{"null"} {
   }
 
   /**
@@ -125,7 +115,7 @@ class FormatArg : public absl::AlphaNum {
    * handled specially to avoid ambiguity with generic pointers, which are
    * handled differently.
    */
-  FormatArg(const char* string_value, internal::FormatChoice<2>)
+  FormatArg(const char* string_value, internal::FormatChoice<3>)
       : AlphaNum{string_value == nullptr ? "null" : string_value} {
   }
 
@@ -134,7 +124,7 @@ class FormatArg : public absl::AlphaNum {
    * hexidecimal integer literal.
    */
   template <typename T>
-  FormatArg(T* pointer_value, internal::FormatChoice<3>)
+  FormatArg(T* pointer_value, internal::FormatChoice<4>)
       : AlphaNum{absl::Hex{reinterpret_cast<uintptr_t>(pointer_value)}} {
   }
 
@@ -143,7 +133,7 @@ class FormatArg : public absl::AlphaNum {
    * absl::AlphaNum accepts.
    */
   template <typename T>
-  FormatArg(T&& value, internal::FormatChoice<4>)
+  FormatArg(T&& value, internal::FormatChoice<5>)
       : AlphaNum{std::forward<T>(value)} {
   }
 };
diff --git a/Firestore/core/src/firebase/firestore/util/type_traits.h b/Firestore/core/src/firebase/firestore/util/type_traits.h
new file mode 100644
index 0000000..52feb6b
--- /dev/null
+++ b/Firestore/core/src/firebase/firestore/util/type_traits.h
@@ -0,0 +1,90 @@
+/*
+ * 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.
+ */
+
+#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_TYPE_TRAITS_H_
+#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_TYPE_TRAITS_H_
+
+#if __OBJC__
+#import <objc/objc.h>  // for id
+#endif
+
+#include <type_traits>
+
+namespace firebase {
+namespace firestore {
+namespace util {
+
+#if __OBJC__
+
+/**
+ * A type trait that identifies whether or not the given pointer points to an
+ * Objective-C object.
+ *
+ * is_objective_c_pointer<NSObject*>::value == true
+ * is_objective_c_pointer<NSArray<NSString*>*>::value == true
+ *
+ * // id is a dynamically typed pointer to an Objective-C object.
+ * is_objective_c_pointer<id>::value == true
+ *
+ * // pointers to C++ classes are not Objective-C pointers.
+ * is_objective_c_pointer<void*>::value == false
+ * is_objective_c_pointer<std::string*>::value == false
+ * is_objective_c_pointer<std::unique_ptr<int>>::value == false
+ */
+template <typename T>
+struct is_objective_c_pointer {
+ private:
+  using yes_type = char (&)[10];
+  using no_type = char (&)[1];
+
+  /**
+   * A non-existent function declared to produce a pointer to type T (which is
+   * consistent with the way Objective-C objects are referenced).
+   *
+   * Note that there is no definition for this function but that's okay because
+   * we only need it to reason about the function's type at compile type.
+   */
+  static T Pointer();
+
+  static yes_type Choose(id value);
+  static no_type Choose(...);
+
+ public:
+  using value_type = bool;
+
+  enum { value = sizeof(Choose(Pointer())) == sizeof(yes_type) };
+
+  constexpr operator bool() const {
+    return value;
+  }
+
+  constexpr bool operator()() const {
+    return value;
+  }
+};
+
+// Hard-code the answer for `void` because you can't pass arguments of type
+// `void` to another function.
+template <>
+struct is_objective_c_pointer<void> : public std::false_type {};
+
+#endif  // __OBJC__
+
+}  // namespace util
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_TYPE_TRAITS_H_
diff --git a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
index bcb1c84..c07a4ea 100644
--- a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
+++ b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
@@ -137,3 +137,12 @@ cc_test(
     firebase_firestore_util
     gmock
 )
+
+if(APPLE)
+  target_sources(
+    firebase_firestore_util_test
+    PUBLIC
+      string_format_apple_test.mm
+      type_traits_apple_test.mm
+  )
+endif()
diff --git a/Firestore/core/test/firebase/firestore/util/hashing_test.cc b/Firestore/core/test/firebase/firestore/util/hashing_test.cc
index e5d9ff8..2c5c2f7 100644
--- a/Firestore/core/test/firebase/firestore/util/hashing_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/hashing_test.cc
@@ -16,6 +16,9 @@
 
 #include "Firestore/core/src/firebase/firestore/util/hashing.h"
 
+#include <map>
+#include <string>
+
 #include "absl/strings/string_view.h"
 #include "gtest/gtest.h"
 
@@ -29,6 +32,21 @@ struct HasHashMember {
   }
 };
 
+TEST(HashingTest, HasStdHash) {
+  EXPECT_TRUE(impl::has_std_hash<float>::value);
+  EXPECT_TRUE(impl::has_std_hash<double>::value);
+  EXPECT_TRUE(impl::has_std_hash<int>::value);
+  EXPECT_TRUE(impl::has_std_hash<int64_t>::value);
+  EXPECT_TRUE(impl::has_std_hash<std::string>::value);
+  EXPECT_TRUE(impl::has_std_hash<void*>::value);
+  EXPECT_TRUE(impl::has_std_hash<const char*>::value);
+
+  struct Foo {};
+  EXPECT_FALSE(impl::has_std_hash<Foo>::value);
+  EXPECT_FALSE(impl::has_std_hash<absl::string_view>::value);
+  EXPECT_FALSE((impl::has_std_hash<std::map<std::string, std::string>>::value));
+}
+
 TEST(HashingTest, Int) {
   ASSERT_EQ(std::hash<int>{}(0), Hash(0));
 }
diff --git a/Firestore/core/test/firebase/firestore/util/string_format_apple_test.mm b/Firestore/core/test/firebase/firestore/util/string_format_apple_test.mm
new file mode 100644
index 0000000..f0bcd35
--- /dev/null
+++ b/Firestore/core/test/firebase/firestore/util/string_format_apple_test.mm
@@ -0,0 +1,60 @@
+/*
+ * 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.
+ */
+
+#include "Firestore/core/src/firebase/firestore/util/string_format.h"
+
+#import <Foundation/NSString.h>
+
+#include "gtest/gtest.h"
+
+@interface FSTDescribable : NSObject
+@end
+
+@implementation FSTDescribable
+
+- (NSString*)description {
+  return @"description";
+}
+
+@end
+
+namespace firebase {
+namespace firestore {
+namespace util {
+
+TEST(StringFormatTest, NSString) {
+  EXPECT_EQ("Hello World", StringFormat("Hello %s", @"World"));
+
+  NSString* hello = [NSString stringWithUTF8String:"Hello"];
+  EXPECT_EQ("Hello World", StringFormat("%s World", hello));
+
+  // NOLINTNEXTLINE false positive on "string"
+  NSMutableString* world = [NSMutableString string];
+  [world appendString:@"World"];
+  EXPECT_EQ("Hello World", StringFormat("Hello %s", world));
+}
+
+TEST(StringFormatTest, FSTDescribable) {
+  FSTDescribable* desc = [[FSTDescribable alloc] init];
+  EXPECT_EQ("Hello description", StringFormat("Hello %s", desc));
+
+  id desc_id = desc;
+  EXPECT_EQ("Hello description", StringFormat("Hello %s", desc_id));
+}
+
+}  //  namespace util
+}  //  namespace firestore
+}  //  namespace firebase
diff --git a/Firestore/core/test/firebase/firestore/util/type_traits_apple_test.mm b/Firestore/core/test/firebase/firestore/util/type_traits_apple_test.mm
new file mode 100644
index 0000000..dfb03bb
--- /dev/null
+++ b/Firestore/core/test/firebase/firestore/util/type_traits_apple_test.mm
@@ -0,0 +1,50 @@
+/*
+ * 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.
+ */
+
+#include "Firestore/core/src/firebase/firestore/util/type_traits.h"
+
+#import <Foundation/NSArray.h>
+#import <Foundation/NSObject.h>
+#import <Foundation/NSString.h>
+
+#include "gtest/gtest.h"
+
+namespace firebase {
+namespace firestore {
+namespace util {
+
+TEST(TypeTraitsTest, IsObjectiveCPointer) {
+  static_assert(is_objective_c_pointer<NSObject*>{}, "NSObject");
+  static_assert(is_objective_c_pointer<NSString*>{}, "NSString");
+  static_assert(is_objective_c_pointer<NSArray<NSString*>*>{},
+                "NSArray<NSString*>");
+
+  static_assert(is_objective_c_pointer<id>{}, "id");
+  static_assert(is_objective_c_pointer<id<NSCopying>>{}, "id<NSCopying>");
+
+  static_assert(!is_objective_c_pointer<int*>{}, "int*");
+  static_assert(!is_objective_c_pointer<void*>{}, "void*");
+  static_assert(!is_objective_c_pointer<int>{}, "int");
+  static_assert(!is_objective_c_pointer<void>{}, "void");
+
+  struct Foo {};
+  static_assert(!is_objective_c_pointer<Foo>{}, "Foo");
+  static_assert(!is_objective_c_pointer<Foo*>{}, "Foo");
+}
+
+}  // namespace util
+}  // namespace firestore
+}  // namespace firebase