google / googletest

GoogleTest - Google Testing and Mocking Framework
https://google.github.io/googletest/
BSD 3-Clause "New" or "Revised" License
33.78k stars 10k forks source link

[Bug]: Container matchers fail to build on containers that do not expose value_type #4375

Closed m-pilia closed 9 months ago

m-pilia commented 9 months ago

Describe the issue

Trying to use container matchers on containers that do not expose value_type as a member type results in a compiler error. This prevents from using the matchers on custom user containers that do not follow STL naming conventions for member types.

The value type could be deduced from the iterator type using std::iterator_traits, so the container should not need to expose value_type as a member.

Steps to reproduce the problem

Minimal reproducible example:

// A minimal container that does not expose value_type
class ContainerWithoutValueType {
  using UnderlyingContainer = std::vector<int>;

 public:
  ContainerWithoutValueType(std::initializer_list<int> const args) : _v{args} {}

  UnderlyingContainer::const_iterator begin() const { return _v.begin(); }
  UnderlyingContainer::const_iterator end() const { return _v.end(); }
  UnderlyingContainer::size_type size() const { return _v.size(); }

  friend bool operator==(ContainerWithoutValueType const& lhs,
                         ContainerWithoutValueType const& rhs) {
    return lhs._v == rhs._v;
  }

 private:
  UnderlyingContainer _v;
};

TEST(PointwiseTest, DeducesValueType) {
  const ContainerWithoutValueType container{1, 2, 3};
  EXPECT_THAT(container, Pointwise(Eq(), container));
}

What version of GoogleTest are you using?

e40661d89b051e9ef4eb8a2420b74bf78b39ef41

What operating system and version are you using?

Arch Linux (kernel 6.1.39-1-lts)

What compiler and version are you using?

gcc version 13.1.1 20230714 (GCC)

What build system are you using?

cmake version 3.27.1

Additional context

No response

dinord commented 9 months ago

This isn't hard to do in googletest, but it would expand a bunch of utilities that have "STL container" in their name to work with containers that do not satisfy this definition: https://en.cppreference.com/w/cpp/named_req/Container

It doesn't seem like this change would break anything right now, but it would make internals slightly more difficult to read.

Can't we just add value_type to the custom container instead?

dinord commented 9 months ago

The problem with std::iterator_traits<It> is that if It does not provide all five expected member types, then std::iterator_traits<It> will not have any member types: https://en.cppreference.com/w/cpp/iterator/iterator_traits

We already have some code that would break if we started depending on iterator_traits. For example, absl::container_internal::BitMask has value_type, but it doesn't have difference_type: https://github.com/abseil/abseil-cpp/blob/e3114cc5744393c5d8e514d9f3323ef194f3bcb5/absl/container/internal/raw_hash_set.h#L416

Implementing this proposal would require substantial cleanup in our open source and internal libraries. We suggest defining value_type instead.

m-pilia commented 9 months ago

Ok I understand, thank you for the explanation!