Skip to content

Commit c709dff

Browse files
var-consta-maurice
authored andcommitted
Use a single function, FieldValue::Increment, for both integer and double arguments.
See go/firestore-cpp-field-value-increment for more background and rationale. This requires some template metaprogramming but results in a more convenient and safer API. In particular, `static_assert` allows preventing truncation with a readable error message (e.g. if a user passes `unsigned long long` as an argument). While working on this, I discovered that STLPort actually does contain `<type_traits>` header, so the STLPort implementation is pretty straightforward (the only extra complication is accounting for the nested `tr1` namespace). I checked the code from this CL manually in Android Studio using STLPort on NDK r17, and it compiled successfully. PiperOrigin-RevId: 308336176
1 parent 7e51d56 commit c709dff

File tree

4 files changed

+103
-15
lines changed

4 files changed

+103
-15
lines changed

app/meta/type_traits.h

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#define FIREBASE_APP_CLIENT_CPP_META_TYPE_TRAITS_H_
1919

2020
#include <cstdlib>
21+
#include <type_traits>
2122

2223
#if !defined(FIREBASE_NAMESPACE)
2324
#define FIREBASE_NAMESPACE firebase
@@ -65,6 +66,57 @@ struct is_lvalue_reference<T&> {
6566
static constexpr bool value = true;
6667
};
6768

69+
// STLPort does include <type_traits> header, but its contents are in `std::tr1`
70+
// namespace. To work around this, use aliases.
71+
// TODO(varconst): all of the reimplementations of traits above can be replaced
72+
// with appropriate aliases.
73+
// TODO(varconst): the traits in this file would be more conformant if they
74+
// inherited from `std::integral_constant`.
75+
#ifdef STLPORT
76+
#define FIREBASE_TYPE_TRAITS_NS std::tr1
77+
#else
78+
#define FIREBASE_TYPE_TRAITS_NS std
79+
#endif
80+
81+
template <bool value, typename T = void>
82+
using enable_if = FIREBASE_TYPE_TRAITS_NS::enable_if<value, T>;
83+
84+
template <typename T>
85+
using is_floating_point = FIREBASE_TYPE_TRAITS_NS::is_floating_point<T>;
86+
87+
template <typename T>
88+
using is_integral = FIREBASE_TYPE_TRAITS_NS::is_integral<T>;
89+
90+
template <typename T, typename U>
91+
using is_same = FIREBASE_TYPE_TRAITS_NS::is_same<T, U>;
92+
93+
template <typename T, T value>
94+
using integral_constant = FIREBASE_TYPE_TRAITS_NS::integral_constant<T, value>;
95+
96+
#undef FIREBASE_TYPE_TRAITS_NS
97+
98+
// `is_char<T>::value` is true iff `T` is a character type (including `wchar_t`
99+
// and C++11 fixed-width character types).
100+
template <typename T>
101+
struct is_char {
102+
static constexpr bool value =
103+
#if __cplusplus >= 202002L
104+
is_same<T, char8_t>::value ||
105+
#endif
106+
#if __cplusplus >= 201103L
107+
is_same<T, char16_t>::value || is_same<T, char32_t>::value ||
108+
#endif
109+
is_same<T, char>::value || is_same<T, signed char>::value ||
110+
is_same<T, unsigned char>::value || is_same<T, wchar_t>::value;
111+
};
112+
113+
// A subset of `std::is_integral`: excludes `bool` and character types.
114+
template <typename T>
115+
struct is_integer {
116+
static constexpr bool value =
117+
is_integral<T>::value && !is_same<T, bool>::value && !is_char<T>::value;
118+
};
119+
68120
// NOLINTNEXTLINE - allow namespace overridden
69121
} // namespace FIREBASE_NAMESPACE
70122

firestore/src/common/field_value.cc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -256,19 +256,15 @@ FieldValue FieldValue::ArrayRemove(std::vector<FieldValue> elements) {
256256
return FieldValueInternal::ArrayRemove(firebase::Move(elements));
257257
}
258258

259-
#if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
260259
/* static */
261260
FieldValue FieldValue::IntegerIncrement(int64_t by_value) {
262261
return FieldValueInternal::IntegerIncrement(by_value);
263262
}
264-
#endif // if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
265263

266-
#if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
267264
/* static */
268265
FieldValue FieldValue::DoubleIncrement(double by_value) {
269266
return FieldValueInternal::DoubleIncrement(by_value);
270267
}
271-
#endif // if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
272268

273269
// TODO(varconst): consider reversing the role of the two output functions, so
274270
// that `ToString` delegates to `operator<<`, not the other way round.
@@ -322,10 +318,8 @@ std::string FieldValue::ToString() const {
322318
return "FieldValue::ArrayRemove()";
323319

324320
case Type::kIncrementInteger:
325-
return "FieldValue::IntegerIncrement()";
326-
327321
case Type::kIncrementDouble:
328-
return "FieldValue::DoubleIncrement()";
322+
return "FieldValue::Increment()";
329323
}
330324

331325
FIREBASE_ASSERT_MESSAGE_RETURN("<invalid>", false,

firestore/src/include/firebase/firestore/field_value.h

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@
1919

2020
#include <cstdint>
2121
#include <iosfwd>
22+
#include <limits>
2223
#include <string>
2324
#include <vector>
2425

26+
#include "app/meta/type_traits.h"
2527
#include "firebase/firestore/map_field_value.h"
2628

2729
namespace firebase {
@@ -50,6 +52,17 @@ class GeoPoint;
5052
* foo_value() will fail (and cause a crash).
5153
*/
5254
class FieldValue final {
55+
// Helper aliases for `Increment` member functions.
56+
// Qualifying `is_integer` is to prevent ambiguity with the
57+
// `FieldValue::is_integer` member function.
58+
// Note: normally, `enable_if::type` would be included in the alias, but such
59+
// a declaration breaks SWIG (presumably, SWIG cannot handle `typename` within
60+
// an alias template).
61+
template <typename T>
62+
using EnableIfIntegral = enable_if<::firebase::is_integer<T>::value, int>;
63+
template <typename T>
64+
using EnableIfFloatingPoint = enable_if<is_floating_point<T>::value, int>;
65+
5366
public:
5467
/**
5568
* The enumeration of all valid runtime types of FieldValue.
@@ -315,7 +328,8 @@ class FieldValue final {
315328
#if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
316329
/**
317330
* Returns a special value that can be used with `Set()` or `Update()` that
318-
* tells the server to increment the field's current value by the given value.
331+
* tells the server to increment the field's current value by the given
332+
* integer value.
319333
*
320334
* If the current field value is an integer, possible integer overflows are
321335
* resolved to `LONG_MAX` or `LONG_MIN`. If the current field value is a
@@ -325,16 +339,29 @@ class FieldValue final {
325339
* If field is not an integer or a double, or if the field does not yet exist,
326340
* the transformation will set the field to the given value.
327341
*
328-
* @param by_value The integer value to increment by.
342+
* @param by_value The integer value to increment by. Should be an integer
343+
* type not larger than `int64_t`.
329344
* @return The FieldValue sentinel for use in a call to `Set()` or `Update().`
330345
*/
331-
static FieldValue IntegerIncrement(int64_t by_value);
346+
template <typename T, typename EnableIfIntegral<T>::type = 0>
347+
static FieldValue Increment(T by_value) {
348+
// Note: Doxygen will run into trouble if this function's definition is
349+
// moved outside the class body.
350+
static_assert(
351+
std::numeric_limits<T>::max() <= std::numeric_limits<int64_t>::max(),
352+
"The integer type you provided is larger than can fit in an int64_t. "
353+
"If you are sure the value will not be truncated, please explicitly "
354+
"cast to int64_t before passing it to FieldValue::Increment().");
355+
return IntegerIncrement(static_cast<int64_t>(by_value));
356+
}
357+
332358
#endif // if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
333359

334360
#if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
335361
/**
336362
* Returns a special value that can be used with `Set()` or `Update()` that
337-
* tells the server to increment the field's current value by the given value.
363+
* tells the server to increment the field's current value by the given
364+
* floating point value.
338365
*
339366
* If the current field value is an integer, possible integer overflows are
340367
* resolved to `LONG_MAX` or `LONG_MIN`. If the current field value is a
@@ -344,10 +371,23 @@ class FieldValue final {
344371
* If field is not an integer or a double, or if the field does not yet exist,
345372
* the transformation will set the field to the given value.
346373
*
347-
* @param by_value The double value to increment by.
374+
* @param by_value The double value to increment by. Should be a floating
375+
* point type no larger than `double`.
348376
* @return The FieldValue sentinel for use in a call to `Set()` or `Update().`
349377
*/
350-
static FieldValue DoubleIncrement(double by_value);
378+
template <typename T, typename EnableIfFloatingPoint<T>::type = 0>
379+
static FieldValue Increment(T by_value) {
380+
// Note: Doxygen will run into trouble if this function's definition is
381+
// moved outside the class body.
382+
static_assert(
383+
std::numeric_limits<T>::max() <= std::numeric_limits<double>::max(),
384+
"The floating point type you provided is larger than can fit in a "
385+
"double. If you are sure the value will not be truncated, please "
386+
"explicitly cast to double before passing it to "
387+
"FieldValue::Increment().");
388+
return DoubleIncrement(static_cast<double>(by_value));
389+
}
390+
351391
#endif // if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
352392

353393
/**
@@ -380,6 +420,9 @@ class FieldValue final {
380420

381421
explicit FieldValue(FieldValueInternal* internal);
382422

423+
static FieldValue IntegerIncrement(int64_t by_value);
424+
static FieldValue DoubleIncrement(double by_value);
425+
383426
FieldValueInternal* internal_ = nullptr;
384427
};
385428

firestore/src/ios/field_value_ios.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,8 @@ std::string Describe(Type type) {
236236
case Type::kArrayRemove:
237237
return "FieldValue::ArrayRemove()";
238238
case Type::kIncrementInteger:
239-
return "FieldValue::IntegerIncrement()";
240239
case Type::kIncrementDouble:
241-
return "FieldValue::DoubleIncrement()";
240+
return "FieldValue::Increment()";
242241
default: {
243242
// TODO(b/147444199): use string formatting.
244243
// HARD_FAIL("Unexpected type '%s'", type);

0 commit comments

Comments
 (0)