From d439bbccd4b90583a89d209d2cc81308aabca8ac Mon Sep 17 00:00:00 2001 From: Gil Date: Tue, 22 May 2018 13:21:08 -0700 Subject: Add a HARD_ASSERT C++ assertion macro (#1304) * Add HARD_ASSERT * Use HARD_ASSERT * Remove FIREBASE_ASSERT * Remove StringPrintf --- .../core/src/firebase/firestore/model/base_path.h | 16 ++++++------- .../src/firebase/firestore/model/database_id.cc | 6 ++--- .../core/src/firebase/firestore/model/document.cc | 4 ++-- .../src/firebase/firestore/model/document_key.cc | 7 +++--- .../src/firebase/firestore/model/field_path.cc | 27 +++++++-------------- .../src/firebase/firestore/model/field_value.cc | 28 ++++++++++------------ .../src/firebase/firestore/model/field_value.h | 12 +++++----- .../src/firebase/firestore/model/maybe_document.cc | 2 -- .../src/firebase/firestore/model/precondition.cc | 4 ++-- .../src/firebase/firestore/model/precondition.h | 9 +++---- .../src/firebase/firestore/model/resource_path.cc | 8 +++---- .../firestore/model/transform_operations.h | 6 ++--- 12 files changed, 54 insertions(+), 75 deletions(-) (limited to 'Firestore/core/src/firebase/firestore/model') diff --git a/Firestore/core/src/firebase/firestore/model/base_path.h b/Firestore/core/src/firebase/firestore/model/base_path.h index 58df6f0..7608829 100644 --- a/Firestore/core/src/firebase/firestore/model/base_path.h +++ b/Firestore/core/src/firebase/firestore/model/base_path.h @@ -24,7 +24,7 @@ #include #include -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/hashing.h" namespace firebase { @@ -57,19 +57,18 @@ class BasePath { /** Returns i-th segment of the path. */ const std::string& operator[](const size_t i) const { - FIREBASE_ASSERT_MESSAGE(i < segments_.size(), "index %u out of range", i); + HARD_ASSERT(i < segments_.size(), "index %s out of range", i); return segments_[i]; } /** Returns the first segment of the path. */ const std::string& first_segment() const { - FIREBASE_ASSERT_MESSAGE(!empty(), - "Cannot call first_segment on empty path"); + HARD_ASSERT(!empty(), "Cannot call first_segment on empty path"); return segments_[0]; } /** Returns the last segment of the path. */ const std::string& last_segment() const { - FIREBASE_ASSERT_MESSAGE(!empty(), "Cannot call last_segment on empty path"); + HARD_ASSERT(!empty(), "Cannot call last_segment on empty path"); return segments_[size() - 1]; } @@ -117,9 +116,8 @@ class BasePath { * this path. */ T PopFirst(const size_t n = 1) const { - FIREBASE_ASSERT_MESSAGE(n <= size(), - "Cannot call PopFirst(%u) on path of length %u", n, - size()); + HARD_ASSERT(n <= size(), "Cannot call PopFirst(%s) on path of length %s", n, + size()); return T{begin() + n, end()}; } @@ -128,7 +126,7 @@ class BasePath { * this path. */ T PopLast() const { - FIREBASE_ASSERT_MESSAGE(!empty(), "Cannot call PopLast() on empty path"); + HARD_ASSERT(!empty(), "Cannot call PopLast() on empty path"); return T{begin(), end() - 1}; } diff --git a/Firestore/core/src/firebase/firestore/model/database_id.cc b/Firestore/core/src/firebase/firestore/model/database_id.cc index a756ea7..97e7d92 100644 --- a/Firestore/core/src/firebase/firestore/model/database_id.cc +++ b/Firestore/core/src/firebase/firestore/model/database_id.cc @@ -16,7 +16,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" namespace firebase { namespace firestore { @@ -27,8 +27,8 @@ constexpr const char* DatabaseId::kDefault; DatabaseId::DatabaseId(const absl::string_view project_id, const absl::string_view database_id) : project_id_(project_id), database_id_(database_id) { - FIREBASE_ASSERT(!project_id.empty()); - FIREBASE_ASSERT(!database_id.empty()); + HARD_ASSERT(!project_id.empty()); + HARD_ASSERT(!database_id.empty()); } } // namespace model diff --git a/Firestore/core/src/firebase/firestore/model/document.cc b/Firestore/core/src/firebase/firestore/model/document.cc index ae59d15..7adf684 100644 --- a/Firestore/core/src/firebase/firestore/model/document.cc +++ b/Firestore/core/src/firebase/firestore/model/document.cc @@ -18,7 +18,7 @@ #include -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" namespace firebase { namespace firestore { @@ -32,7 +32,7 @@ Document::Document(FieldValue&& data, data_(std::move(data)), has_local_mutations_(has_local_mutations) { set_type(Type::Document); - FIREBASE_ASSERT(FieldValue::Type::Object == data.type()); + HARD_ASSERT(FieldValue::Type::Object == data.type()); } bool Document::Equals(const MaybeDocument& other) const { diff --git a/Firestore/core/src/firebase/firestore/model/document_key.cc b/Firestore/core/src/firebase/firestore/model/document_key.cc index ddda4c9..7dae412 100644 --- a/Firestore/core/src/firebase/firestore/model/document_key.cc +++ b/Firestore/core/src/firebase/firestore/model/document_key.cc @@ -18,7 +18,7 @@ #include -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" namespace firebase { namespace firestore { @@ -27,9 +27,8 @@ namespace model { namespace { void AssertValidPath(const ResourcePath& path) { - FIREBASE_ASSERT_MESSAGE(DocumentKey::IsDocumentKey(path), - "invalid document key path: %s", - path.CanonicalString().c_str()); + HARD_ASSERT(DocumentKey::IsDocumentKey(path), "invalid document key path: %s", + path.CanonicalString()); } } // namespace diff --git a/Firestore/core/src/firebase/firestore/model/field_path.cc b/Firestore/core/src/firebase/firestore/model/field_path.cc index bc0e97c..5636b53 100644 --- a/Firestore/core/src/firebase/firestore/model/field_path.cc +++ b/Firestore/core/src/firebase/firestore/model/field_path.cc @@ -19,7 +19,7 @@ #include #include -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "absl/strings/str_join.h" #include "absl/strings/str_replace.h" #include "absl/strings/str_split.h" @@ -71,18 +71,11 @@ FieldPath FieldPath::FromServerFormat(const absl::string_view path) { std::string segment; segment.reserve(path.size()); - // string_view doesn't have a c_str() method, because it might not be - // null-terminated. Assertions expect C strings, so construct std::string on - // the fly, so that c_str() might be called on it. - const auto to_string = [](const absl::string_view view) { - return std::string{view.data(), view.data() + view.size()}; - }; - const auto finish_segment = [&segments, &segment, &path, &to_string] { - FIREBASE_ASSERT_MESSAGE( - !segment.empty(), - "Invalid field path (%s). Paths must not be empty, begin with " - "'.', end with '.', or contain '..'", - to_string(path).c_str()); + const auto finish_segment = [&segments, &segment, &path] { + HARD_ASSERT(!segment.empty(), + "Invalid field path (%s). Paths must not be empty, begin with " + "'.', end with '.', or contain '..'", + path); // Move operation will clear segment, but capacity will remain the same // (not, strictly speaking, required by the standard, but true in practice). segments.push_back(std::move(segment)); @@ -116,9 +109,8 @@ FieldPath FieldPath::FromServerFormat(const absl::string_view path) { case '\\': // TODO(b/37244157): Make this a user-facing exception once we // finalize field escaping. - FIREBASE_ASSERT_MESSAGE(i + 1 != path.size(), - "Trailing escape characters not allowed in %s", - to_string(path).c_str()); + HARD_ASSERT(i + 1 != path.size(), + "Trailing escape characters not allowed in %s", path); ++i; segment += path[i]; break; @@ -131,8 +123,7 @@ FieldPath FieldPath::FromServerFormat(const absl::string_view path) { } finish_segment(); - FIREBASE_ASSERT_MESSAGE(!inside_backticks, "Unterminated ` in path %s", - to_string(path).c_str()); + HARD_ASSERT(!inside_backticks, "Unterminated ` in path %s", path); return FieldPath{std::move(segments)}; } diff --git a/Firestore/core/src/firebase/firestore/model/field_value.cc b/Firestore/core/src/firebase/firestore/model/field_value.cc index 35253c8..4f92c8b 100644 --- a/Firestore/core/src/firebase/firestore/model/field_value.cc +++ b/Firestore/core/src/firebase/firestore/model/field_value.cc @@ -23,7 +23,7 @@ #include #include "Firestore/core/src/firebase/firestore/util/comparison.h" -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" using firebase::firestore::util::Comparator; @@ -130,8 +130,7 @@ FieldValue& FieldValue::operator=(const FieldValue& value) { break; } default: - FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - false, lhs.type(), "Unsupported type %d", value.type()); + HARD_FAIL("Unsupported type %s", value.type()); } return *this; } @@ -168,10 +167,10 @@ FieldValue& FieldValue::operator=(FieldValue&& value) { FieldValue FieldValue::Set(const FieldPath& field_path, FieldValue value) const { - FIREBASE_ASSERT_MESSAGE(type() == Type::Object, - "Cannot set field for non-object FieldValue"); - FIREBASE_ASSERT_MESSAGE(!field_path.empty(), - "Cannot set field for empty path on FieldValue"); + HARD_ASSERT(type() == Type::Object, + "Cannot set field for non-object FieldValue"); + HARD_ASSERT(!field_path.empty(), + "Cannot set field for empty path on FieldValue"); // Set the value by recursively calling on child object. const std::string& child_name = field_path.first_segment(); const ObjectValue::Map& object_map = object_value_.internal_value; @@ -195,10 +194,10 @@ FieldValue FieldValue::Set(const FieldPath& field_path, } FieldValue FieldValue::Delete(const FieldPath& field_path) const { - FIREBASE_ASSERT_MESSAGE(type() == Type::Object, - "Cannot delete field for non-object FieldValue"); - FIREBASE_ASSERT_MESSAGE(!field_path.empty(), - "Cannot delete field for empty path on FieldValue"); + HARD_ASSERT(type() == Type::Object, + "Cannot delete field for non-object FieldValue"); + HARD_ASSERT(!field_path.empty(), + "Cannot delete field for empty path on FieldValue"); // Delete the value by recursively calling on child object. const std::string& child_name = field_path.first_segment(); const ObjectValue::Map& object_map = object_value_.internal_value; @@ -223,8 +222,8 @@ FieldValue FieldValue::Delete(const FieldPath& field_path) const { } absl::optional FieldValue::Get(const FieldPath& field_path) const { - FIREBASE_ASSERT_MESSAGE(type() == Type::Object, - "Cannot get field for non-object FieldValue"); + HARD_ASSERT(type() == Type::Object, + "Cannot get field for non-object FieldValue"); const FieldValue* current = this; for (const auto& path : field_path) { if (current->type() != Type::Object) { @@ -435,8 +434,7 @@ bool operator<(const FieldValue& lhs, const FieldValue& rhs) { case Type::Object: return lhs.object_value_ < rhs.object_value_; default: - FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - false, lhs.type(), "Unsupported type %d", lhs.type()); + HARD_FAIL("Unsupported type %s", lhs.type()); // return false if assertion does not abort the program. We will say // each unsupported type takes only one value thus everything is equal. return false; diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h index 0f7dfc4..09c8531 100644 --- a/Firestore/core/src/firebase/firestore/model/field_value.h +++ b/Firestore/core/src/firebase/firestore/model/field_value.h @@ -28,7 +28,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "absl/types/optional.h" namespace firebase { @@ -109,27 +109,27 @@ class FieldValue { } bool boolean_value() const { - FIREBASE_ASSERT(tag_ == Type::Boolean); + HARD_ASSERT(tag_ == Type::Boolean); return boolean_value_; } int64_t integer_value() const { - FIREBASE_ASSERT(tag_ == Type::Integer); + HARD_ASSERT(tag_ == Type::Integer); return integer_value_; } Timestamp timestamp_value() const { - FIREBASE_ASSERT(tag_ == Type::Timestamp); + HARD_ASSERT(tag_ == Type::Timestamp); return timestamp_value_; } const std::string& string_value() const { - FIREBASE_ASSERT(tag_ == Type::String); + HARD_ASSERT(tag_ == Type::String); return string_value_; } ObjectValue object_value() const { - FIREBASE_ASSERT(tag_ == Type::Object); + HARD_ASSERT(tag_ == Type::Object); return ObjectValue{object_value_}; } diff --git a/Firestore/core/src/firebase/firestore/model/maybe_document.cc b/Firestore/core/src/firebase/firestore/model/maybe_document.cc index 4f3be1d..aa5e15a 100644 --- a/Firestore/core/src/firebase/firestore/model/maybe_document.cc +++ b/Firestore/core/src/firebase/firestore/model/maybe_document.cc @@ -18,8 +18,6 @@ #include -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" - namespace firebase { namespace firestore { namespace model { diff --git a/Firestore/core/src/firebase/firestore/model/precondition.cc b/Firestore/core/src/firebase/firestore/model/precondition.cc index 423d5a2..179b313 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.cc +++ b/Firestore/core/src/firebase/firestore/model/precondition.cc @@ -18,7 +18,7 @@ #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" namespace firebase { namespace firestore { @@ -59,7 +59,7 @@ bool Precondition::IsValidFor(const MaybeDocument& maybe_doc) const { case Type::None: return true; } - FIREBASE_UNREACHABLE(); + UNREACHABLE(); } } // namespace model diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h index b98bb45..b015100 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.h +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -27,7 +27,7 @@ #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" namespace firebase { namespace firestore { @@ -104,7 +104,7 @@ class Precondition { case Type::None: return true; } - FIREBASE_UNREACHABLE(); + UNREACHABLE(); } // For Objective-C++ hash; to be removed after migration. @@ -131,10 +131,7 @@ class Precondition { stringWithFormat:@"", update_time_.timestamp().ToString().c_str()]; } - // We only raise dev assertion here. This function is mainly used in - // logging. - FIREBASE_DEV_ASSERT_MESSAGE(false, "precondition invalid"); - return @""; + UNREACHABLE(); } #endif // defined(__OBJC__) diff --git a/Firestore/core/src/firebase/firestore/model/resource_path.cc b/Firestore/core/src/firebase/firestore/model/resource_path.cc index c95aa63..cdd795f 100644 --- a/Firestore/core/src/firebase/firestore/model/resource_path.cc +++ b/Firestore/core/src/firebase/firestore/model/resource_path.cc @@ -20,7 +20,7 @@ #include #include -#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "absl/strings/str_join.h" #include "absl/strings/str_split.h" @@ -33,10 +33,8 @@ ResourcePath ResourcePath::FromString(const absl::string_view path) { // sequences (e.g. __id123__) and just passes them through raw (they exist // for legacy reasons and should not be used frequently). - FIREBASE_ASSERT_MESSAGE( - path.find("//") == std::string::npos, - "Invalid path (%s). Paths must not contain // in them.", - std::string{path.data(), path.data() + path.size()}.c_str()); + HARD_ASSERT(path.find("//") == std::string::npos, + "Invalid path (%s). Paths must not contain // in them.", path); // SkipEmpty because we may still have an empty segment at the beginning or // end if they had a leading or trailing slash (which we allow). diff --git a/Firestore/core/src/firebase/firestore/model/transform_operations.h b/Firestore/core/src/firebase/firestore/model/transform_operations.h index 8eed7ae..a5c5f82 100644 --- a/Firestore/core/src/firebase/firestore/model/transform_operations.h +++ b/Firestore/core/src/firebase/firestore/model/transform_operations.h @@ -191,8 +191,8 @@ class ArrayTransform : public TransformOperation { static const std::vector& Elements( const TransformOperation& op) { - FIREBASE_ASSERT(op.type() == Type::ArrayUnion || - op.type() == Type::ArrayRemove); + HARD_ASSERT(op.type() == Type::ArrayUnion || + op.type() == Type::ArrayRemove); return static_cast(op).elements(); } @@ -225,7 +225,7 @@ class ArrayTransform : public TransformOperation { [result addObject:element]; } } else { - FIREBASE_ASSERT(type_ == Type::ArrayRemove); + HARD_ASSERT(type_ == Type::ArrayRemove); [result removeObject:element]; } } -- cgit v1.2.3