diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index fbc072b7..41694f54 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -257,6 +257,7 @@ #include #include +#include #include #include #include @@ -2097,11 +2098,11 @@ class WhenDynamicCastToMatcher : public WhenDynamicCastToMatcherBase { template class FieldMatcher { public: - FieldMatcher(FieldType Class::*field, + FieldMatcher(FieldType Class::* field, const Matcher& matcher) : field_(field), matcher_(matcher), whose_field_("whose given field ") {} - FieldMatcher(const std::string& field_name, FieldType Class::*field, + FieldMatcher(const std::string& field_name, FieldType Class::* field, const Matcher& matcher) : field_(field), matcher_(matcher), @@ -2145,7 +2146,7 @@ class FieldMatcher { return MatchAndExplainImpl(std::false_type(), *p, listener); } - const FieldType Class::*field_; + const FieldType Class::* field_; const Matcher matcher_; // Contains either "whose given field " if the name of the field is unknown @@ -3291,8 +3292,8 @@ class PairMatcher { }; template -auto UnpackStructImpl(const T& t, std::index_sequence, - int) -> decltype(std::tie(get(t)...)) { +auto UnpackStructImpl(const T& t, std::index_sequence, int) + -> decltype(std::tie(get(t)...)) { static_assert(std::tuple_size::value == sizeof...(I), "Number of arguments doesn't match the number of fields."); return std::tie(get(t)...); @@ -3581,7 +3582,7 @@ class ElementsAreMatcherImpl : public MatcherInterface { StlContainerReference stl_container = View::ConstReference(container); auto it = stl_container.begin(); size_t exam_pos = 0; - bool mismatch_found = false; // Have we found a mismatched element yet? + bool unmatched_found = false; // Go through the elements and matchers in pairs, until we reach // the end of either the elements or the matchers, or until we find a @@ -3597,11 +3598,23 @@ class ElementsAreMatcherImpl : public MatcherInterface { } if (!match) { - mismatch_found = true; + unmatched_found = true; + // We cannot store the iterator for the unmatched element to be used + // later, as some users use ElementsAre() with a "container" whose + // iterator is not copy-constructible or copy-assignable. + // + // We cannot store a pointer to the element either, as some container's + // iterators return a temporary. + // + // We cannot store the element itself either, as the element may not be + // copyable. + // + // Therefore, we just remember the index of the unmatched element, + // and use it later to print the unmatched element. break; } } - // If mismatch_found is true, 'exam_pos' is the index of the mismatch. + // If unmatched_found is true, exam_pos is the index of the mismatch. // Find how many elements the actual container has. We avoid // calling size() s.t. this code works for stream-like "containers" @@ -3622,10 +3635,27 @@ class ElementsAreMatcherImpl : public MatcherInterface { return false; } - if (mismatch_found) { + if (unmatched_found) { // The element count matches, but the exam_pos-th element doesn't match. if (listener_interested) { - *listener << "whose element #" << exam_pos << " doesn't match"; + // Find the unmatched element. + auto unmatched_it = stl_container.begin(); + // We cannot call std::advance() on the iterator, as some users use + // ElementsAre() with a "container" whose iterator is incompatible with + // std::advance() (e.g. it may not have the difference_type member + // type). + for (size_t i = 0; i != exam_pos; ++i) { + ++unmatched_it; + } + + // If the array is long or the elements' print-out is large, it may be + // hard for the user to find the mismatched element and its + // corresponding matcher description. Therefore we print the index, the + // value of the mismatched element, and the corresponding matcher + // description to ease debugging. + *listener << "whose element #" << exam_pos << " (" + << PrintToString(*unmatched_it) << ") "; + matchers_[exam_pos].DescribeNegationTo(listener->stream()); PrintIfNotEmpty(explanations[exam_pos], listener->stream()); } return false; @@ -4031,15 +4061,15 @@ GTEST_API_ std::string FormatMatcherDescription( // Overloads to support `OptionalMatcher` being used with a type that either // supports implicit conversion to bool or a `has_value()` method. template -auto IsOptionalEngaged(const Optional& optional, - Rank1) -> decltype(!!optional) { +auto IsOptionalEngaged(const Optional& optional, Rank1) + -> decltype(!!optional) { // The use of double-negation here is to preserve historical behavior where // the matcher used `operator!` rather than directly using `operator bool`. return !static_cast(!optional); } template -auto IsOptionalEngaged(const Optional& optional, - Rank0) -> decltype(!optional.has_value()) { +auto IsOptionalEngaged(const Optional& optional, Rank0) + -> decltype(!optional.has_value()) { return optional.has_value(); } @@ -4567,7 +4597,7 @@ WhenDynamicCastTo(const Matcher& inner_matcher) { // matches a Foo object x if and only if x.number >= 5. template inline PolymorphicMatcher> Field( - FieldType Class::*field, const FieldMatcher& matcher) { + FieldType Class::* field, const FieldMatcher& matcher) { return MakePolymorphicMatcher(internal::FieldMatcher( field, MatcherCast(matcher))); // The call to MatcherCast() is required for supporting inner @@ -4580,7 +4610,7 @@ inline PolymorphicMatcher> Field( // messages. template inline PolymorphicMatcher> Field( - const std::string& field_name, FieldType Class::*field, + const std::string& field_name, FieldType Class::* field, const FieldMatcher& matcher) { return MakePolymorphicMatcher(internal::FieldMatcher( field_name, field, MatcherCast(matcher))); diff --git a/googlemock/test/gmock-matchers-containers_test.cc b/googlemock/test/gmock-matchers-containers_test.cc index 751fb60e..e9f1a02d 100644 --- a/googlemock/test/gmock-matchers-containers_test.cc +++ b/googlemock/test/gmock-matchers-containers_test.cc @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -1271,10 +1272,11 @@ TEST(WhenSortedByTest, CanDescribeSelf) { TEST(WhenSortedByTest, ExplainsMatchResult) { const int a[] = {2, 1}; - EXPECT_EQ("which is { 1, 2 } when sorted, whose element #0 doesn't match", - Explain(WhenSortedBy(less(), ElementsAre(2, 3)), a)); - EXPECT_EQ("which is { 1, 2 } when sorted", - Explain(WhenSortedBy(less(), ElementsAre(1, 2)), a)); + EXPECT_EQ( + Explain(WhenSortedBy(less(), ElementsAre(2, 3)), a), + "which is { 1, 2 } when sorted, whose element #0 (1) isn't equal to 2"); + EXPECT_EQ(Explain(WhenSortedBy(less(), ElementsAre(1, 2)), a), + "which is { 1, 2 } when sorted"); } // WhenSorted() is a simple wrapper on WhenSortedBy(). Hence we don't @@ -1775,6 +1777,295 @@ TEST(IsSubsetOfTest, WorksWithMoveOnly) { helper.Call(MakeUniquePtrs({2})); } +// A container whose iterator returns a temporary. This can iterate over the +// characters in a string. +class CharString { + public: + using value_type = char; + + class const_iterator { + public: + using iterator_category = std::input_iterator_tag; + using value_type = char; + using difference_type = std::ptrdiff_t; + using pointer = const char*; + using reference = const char&; + + // Create an iterator that points to the given character. + explicit const_iterator(const char* ptr) : ptr_(ptr) {} + + // Returns the current character. IMPORTANT: this must return a temporary, + // not a reference, to test that ElementsAre() works with containers whose + // iterators return temporaries. + char operator*() const { return *ptr_; } + + // Advances to the next character. + const_iterator& operator++() { + ++ptr_; + return *this; + } + + // Compares two iterators. + bool operator==(const const_iterator& other) const { + return ptr_ == other.ptr_; + } + bool operator!=(const const_iterator& other) const { + return ptr_ != other.ptr_; + } + + private: + const char* ptr_ = nullptr; + }; + + // Creates a CharString that contains the given string. + explicit CharString(const std::string& s) : s_(s) {} + + // Returns an iterator pointing to the first character in the string. + const_iterator begin() const { return const_iterator(s_.c_str()); } + + // Returns an iterator pointing past the last character in the string. + const_iterator end() const { return const_iterator(s_.c_str() + s_.size()); } + + private: + std::string s_; +}; + +// Tests using ElementsAre() with a container whose iterator returns a +// temporary. +TEST(ElementsAreTest, WorksWithContainerThatReturnsTempInIterator) { + CharString s("abc"); + EXPECT_THAT(s, ElementsAre('a', 'b', 'c')); + EXPECT_THAT(s, Not(ElementsAre('a', 'b', 'd'))); +} + +// Tests using ElementsAreArray() with a container whose iterator returns a +// temporary. +TEST(ElementsAreArrayTest, WorksWithContainerThatReturnsTempInIterator) { + CharString s("abc"); + EXPECT_THAT(s, ElementsAreArray({'a', 'b', 'c'})); + EXPECT_THAT(s, Not(ElementsAreArray({'a', 'b', 'd'}))); +} + +// A container whose iterator returns a temporary and is not copy-assignable. +// This simulates the behavior of the proxy object returned by absl::StrSplit(). +class CharString2 { + public: + using value_type = char; + + class const_iterator { + public: + using iterator_category = std::input_iterator_tag; + using value_type = char; + using difference_type = std::ptrdiff_t; + using pointer = const char*; + using reference = const char&; + + // Make const_iterator copy-constructible but not copy-assignable, + // simulating the behavior of the proxy object returned by absl::StrSplit(). + const_iterator(const const_iterator&) = default; + const_iterator& operator=(const const_iterator&) = delete; + + // Create an iterator that points to the given character. + explicit const_iterator(const char* ptr) : ptr_(ptr) {} + + // Returns the current character. IMPORTANT: this must return a temporary, + // not a reference, to test that ElementsAre() works with containers whose + // iterators return temporaries. + char operator*() const { return *ptr_; } + + // Advances to the next character. + const_iterator& operator++() { + ++ptr_; + return *this; + } + + // Compares two iterators. + bool operator==(const const_iterator& other) const { + return ptr_ == other.ptr_; + } + bool operator!=(const const_iterator& other) const { + return ptr_ != other.ptr_; + } + + private: + const char* ptr_ = nullptr; + }; + + // Creates a CharString that contains the given string. + explicit CharString2(const std::string& s) : s_(s) {} + + // Returns an iterator pointing to the first character in the string. + const_iterator begin() const { return const_iterator(s_.c_str()); } + + // Returns an iterator pointing past the last character in the string. + const_iterator end() const { return const_iterator(s_.c_str() + s_.size()); } + + private: + std::string s_; +}; + +// Tests using ElementsAre() with a container whose iterator returns a +// temporary and is not copy-assignable. +TEST(ElementsAreTest, WorksWithContainerThatReturnsTempInUnassignableIterator) { + CharString2 s("abc"); + EXPECT_THAT(s, ElementsAre('a', 'b', 'c')); + EXPECT_THAT(s, Not(ElementsAre('a', 'b', 'd'))); +} + +// Tests using ElementsAreArray() with a container whose iterator returns a +// temporary and is not copy-assignable. +TEST(ElementsAreArrayTest, + WorksWithContainerThatReturnsTempInUnassignableIterator) { + CharString2 s("abc"); + EXPECT_THAT(s, ElementsAreArray({'a', 'b', 'c'})); + EXPECT_THAT(s, Not(ElementsAreArray({'a', 'b', 'd'}))); +} + +// A container whose iterator returns a temporary and is neither +// copy-constructible nor copy-assignable. +class CharString3 { + public: + using value_type = char; + + class const_iterator { + public: + using iterator_category = std::input_iterator_tag; + using value_type = char; + using difference_type = std::ptrdiff_t; + using pointer = const char*; + using reference = const char&; + + // Make const_iterator neither copy-constructible nor copy-assignable. + const_iterator(const const_iterator&) = delete; + const_iterator& operator=(const const_iterator&) = delete; + + // Create an iterator that points to the given character. + explicit const_iterator(const char* ptr) : ptr_(ptr) {} + + // Returns the current character. IMPORTANT: this must return a temporary, + // not a reference, to test that ElementsAre() works with containers whose + // iterators return temporaries. + char operator*() const { return *ptr_; } + + // Advances to the next character. + const_iterator& operator++() { + ++ptr_; + return *this; + } + + // Compares two iterators. + bool operator==(const const_iterator& other) const { + return ptr_ == other.ptr_; + } + bool operator!=(const const_iterator& other) const { + return ptr_ != other.ptr_; + } + + private: + const char* ptr_ = nullptr; + }; + + // Creates a CharString that contains the given string. + explicit CharString3(const std::string& s) : s_(s) {} + + // Returns an iterator pointing to the first character in the string. + const_iterator begin() const { return const_iterator(s_.c_str()); } + + // Returns an iterator pointing past the last character in the string. + const_iterator end() const { return const_iterator(s_.c_str() + s_.size()); } + + private: + std::string s_; +}; + +// Tests using ElementsAre() with a container whose iterator returns a +// temporary and is neither copy-constructible nor copy-assignable. +TEST(ElementsAreTest, WorksWithContainerThatReturnsTempInUncopyableIterator) { + CharString3 s("abc"); + EXPECT_THAT(s, ElementsAre('a', 'b', 'c')); + EXPECT_THAT(s, Not(ElementsAre('a', 'b', 'd'))); +} + +// Tests using ElementsAreArray() with a container whose iterator returns a +// temporary and is neither copy-constructible nor copy-assignable. +TEST(ElementsAreArrayTest, + WorksWithContainerThatReturnsTempInUncopyableIterator) { + CharString3 s("abc"); + EXPECT_THAT(s, ElementsAreArray({'a', 'b', 'c'})); + EXPECT_THAT(s, Not(ElementsAreArray({'a', 'b', 'd'}))); +} + +// A container whose iterator returns a temporary, is neither +// copy-constructible nor copy-assignable, and has no member types. +class CharString4 { + public: + using value_type = char; + + class const_iterator { + public: + // Do not define difference_type, etc. + + // Make const_iterator neither copy-constructible nor copy-assignable. + const_iterator(const const_iterator&) = delete; + const_iterator& operator=(const const_iterator&) = delete; + + // Create an iterator that points to the given character. + explicit const_iterator(const char* ptr) : ptr_(ptr) {} + + // Returns the current character. IMPORTANT: this must return a temporary, + // not a reference, to test that ElementsAre() works with containers whose + // iterators return temporaries. + char operator*() const { return *ptr_; } + + // Advances to the next character. + const_iterator& operator++() { + ++ptr_; + return *this; + } + + // Compares two iterators. + bool operator==(const const_iterator& other) const { + return ptr_ == other.ptr_; + } + bool operator!=(const const_iterator& other) const { + return ptr_ != other.ptr_; + } + + private: + const char* ptr_ = nullptr; + }; + + // Creates a CharString that contains the given string. + explicit CharString4(const std::string& s) : s_(s) {} + + // Returns an iterator pointing to the first character in the string. + const_iterator begin() const { return const_iterator(s_.c_str()); } + + // Returns an iterator pointing past the last character in the string. + const_iterator end() const { return const_iterator(s_.c_str() + s_.size()); } + + private: + std::string s_; +}; + +// Tests using ElementsAre() with a container whose iterator returns a +// temporary, is neither copy-constructible nor copy-assignable, and has no +// member types. +TEST(ElementsAreTest, WorksWithContainerWithIteratorWithNoMemberTypes) { + CharString4 s("abc"); + EXPECT_THAT(s, ElementsAre('a', 'b', 'c')); + EXPECT_THAT(s, Not(ElementsAre('a', 'b', 'd'))); +} + +// Tests using ElementsAreArray() with a container whose iterator returns a +// temporary, is neither copy-constructible nor copy-assignable, and has no +// member types. +TEST(ElementsAreArrayTest, WorksWithContainerWithIteratorWithNoMemberTypes) { + CharString4 s("abc"); + EXPECT_THAT(s, ElementsAreArray({'a', 'b', 'c'})); + EXPECT_THAT(s, Not(ElementsAreArray({'a', 'b', 'd'}))); +} + // Tests using ElementsAre() and ElementsAreArray() with stream-like // "containers". @@ -2155,7 +2446,7 @@ TEST_P(EachTestP, ExplainsMatchResultCorrectly) { Matcher> m = Each(2); EXPECT_EQ("", Explain(m, a)); - Matcher n = Each(1); // NOLINT + Matcher n = Each(1); // NOLINT const int b[1] = {1}; EXPECT_EQ("", Explain(n, b)); @@ -2290,7 +2581,7 @@ TEST(PointwiseTest, MakesCopyOfRhs) { rhs.push_back(4); int lhs[] = {1, 2}; - const Matcher m = Pointwise(IsHalfOf(), rhs); + const Matcher m = Pointwise(IsHalfOf(), rhs); EXPECT_THAT(lhs, m); // Changing rhs now shouldn't affect m, which made a copy of rhs. @@ -2418,7 +2709,7 @@ TEST(UnorderedPointwiseTest, MakesCopyOfRhs) { rhs.push_back(4); int lhs[] = {2, 1}; - const Matcher m = UnorderedPointwise(IsHalfOf(), rhs); + const Matcher m = UnorderedPointwise(IsHalfOf(), rhs); EXPECT_THAT(lhs, m); // Changing rhs now shouldn't affect m, which made a copy of rhs. @@ -2669,11 +2960,11 @@ TEST_P(ElementsAreTestP, CanExplainMismatchRightSize) { vector v; v.push_back(2); v.push_back(1); - EXPECT_EQ("whose element #0 doesn't match", Explain(m, v)); + EXPECT_EQ(Explain(m, v), "whose element #0 (2) isn't equal to 1"); v[0] = 1; - EXPECT_EQ("whose element #1 doesn't match, which is 4 less than 5", - Explain(m, v)); + EXPECT_EQ(Explain(m, v), + "whose element #1 (1) is <= 5, which is 4 less than 5"); } TEST(ElementsAreTest, MatchesOneElementVector) { @@ -3073,7 +3364,7 @@ TEST(ContainsTest, SetDoesNotMatchWhenElementIsNotInContainer) { TEST_P(ContainsTestP, ExplainsMatchResultCorrectly) { const int a[2] = {1, 2}; - Matcher m = Contains(2); + Matcher m = Contains(2); EXPECT_EQ("whose element #1 matches", Explain(m, a)); m = Contains(3);