[libcxx][NFC] Add tests for associative containers key_comp and value_comp

[kboyarinov]

Summary

Add missing tests to improve associative containers code coverage:

• Tests for key_comp() and value_comp() observers
• Tests for std::map and std::multimap value_compare member class

[kboyarinov]

Fixed incorrect upload with arc

[ldionne]

I did find some tests in libcxx/test/std/containers/associative/multiset/multiset.cons/compare.pass.cpp (and similarly for other container types). Could we incorporate the existing tests into yours and delete the old ones? IMO your new tests are more complete, the only thing I would do is use test_less like in the original ones.

libcxx/test/std/containers/associative/map/map.observers/key_comp.pass.cpp

28

Please return 0; at the end of the function. In freestanding mode, main is not treated specially so we'd get a warning "not returning anything from a function that returns int".

[kboyarinov]

In D113998#3134853, @ldionne wrote:

I did find some tests in libcxx/test/std/containers/associative/multiset/multiset.cons/compare.pass.cpp (and similarly for other container types). Could we incorporate the existing tests into yours and delete the old ones? IMO your new tests are more complete, the only thing I would do is use test_less like in the original ones.

I am not sure that we can just remove the test compare.pass.cpp, because it is intended to test std::map(const key_compare&) constructor and uses std::map::key_comp to test the constructor effect. New test is for std::map::key_comp effects. I agree that these tests are connected, but I do not think they are interchangeable. What do you think?

[kboyarinov]

Fix CI, add return 0 where missed

[ldionne]

In D113998#3136978, @kboyarinov wrote:

In D113998#3134853, @ldionne wrote:

I did find some tests in libcxx/test/std/containers/associative/multiset/multiset.cons/compare.pass.cpp (and similarly for other container types). Could we incorporate the existing tests into yours and delete the old ones? IMO your new tests are more complete, the only thing I would do is use test_less like in the original ones.

I am not sure that we can just remove the test compare.pass.cpp, because it is intended to test std::map(const key_compare&) constructor and uses std::map::key_comp to test the constructor effect. New test is for std::map::key_comp effects. I agree that these tests are connected, but I do not think they are interchangeable. What do you think?

Oh my, you are right, I looked too quickly and missed that we were testing the constructors. In those constructor tests, can you please remove the part of the comment that looks like:

// key_compare    key_comp() const;
// value_compare value_comp() const;

This confused me into thinking we were testing those methods there, instead of just the constructor (which is explained by the comment immediately above):

// explicit multiset(const value_compare& comp);
// value_compare and key_compare are the same type for set/multiset

LGTM with my suggested comment cleanup to the existing constructor tests.

[kboyarinov]

Clean-up for comments in compare.pass.cpp test for all associative containers

[kboyarinov]

In D113998#3140323, @ldionne wrote:

In D113998#3136978, @kboyarinov wrote:

In D113998#3134853, @ldionne wrote:

I did find some tests in libcxx/test/std/containers/associative/multiset/multiset.cons/compare.pass.cpp (and similarly for other container types). Could we incorporate the existing tests into yours and delete the old ones? IMO your new tests are more complete, the only thing I would do is use test_less like in the original ones.

I am not sure that we can just remove the test compare.pass.cpp, because it is intended to test std::map(const key_compare&) constructor and uses std::map::key_comp to test the constructor effect. New test is for std::map::key_comp effects. I agree that these tests are connected, but I do not think they are interchangeable. What do you think?

Oh my, you are right, I looked too quickly and missed that we were testing the constructors. In those constructor tests, can you please remove the part of the comment that looks like:

// key_compare    key_comp() const;
// value_compare value_comp() const;

This confused me into thinking we were testing those methods there, instead of just the constructor (which is explained by the comment immediately above):

// explicit multiset(const value_compare& comp);
// value_compare and key_compare are the same type for set/multiset

LGTM with my suggested comment cleanup to the existing constructor tests.

I have cleaned-up comments in compare.pass.cpp but I have figured out that this test was the only place where std::[multi]set::key_comp() and std::[multi]set::value_comp() observers were tested.
I have added the similar tests as for maps to have separate test for observers.

[ldionne]

Thanks for the patch! Please provide "Author Name <email@domain>" if you want me to commit this on your behalf.

[rarutyun]

In D113998#3148769, @ldionne wrote:

Thanks for the patch! Please provide "Author Name <email@domain>" if you want me to commit this on your behalf.

Thanks Louis. No worries, I will commit that.

libcxx/test/std/containers/associative/map/map.cons/compare.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <map>
 
 // class map
 
 // explicit map(const key_compare& comp);
 
-// key_compare key_comp() const;
-
 #include <map>
 #include <cassert>
 
 #include "test_macros.h"
 #include "../../../test_compare.h"
 #include "min_allocator.h"
 
 int main(int, char**)

libcxx/test/std/containers/associative/map/map.observers/key_comp.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// key_compare key_comp() const;
+
+#include <map>
+#include <cassert>
+#include <string>
+
+int main(int, char**) {
+    typedef std::map<int, std::string> map_type;
+
+    map_type m;
+    std::pair<map_type::iterator, bool> p1 = m.insert(map_type::value_type(1, "abc"));
+    std::pair<map_type::iterator, bool> p2 = m.insert(map_type::value_type(2, "abc"));
+
+    const map_type& cm = m;
+
+    assert(cm.key_comp()(p1.first->first, p2.first->first));
+    assert(!cm.key_comp()(p2.first->first, p1.first->first));
+
+    return 0;
+}

libcxx/test/std/containers/associative/map/map.observers/value_comp.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// value_compare value_comp() const;
+
+#include <map>
+#include <cassert>
+#include <string>
+
+int main(int, char**) {
+    typedef std::map<int, std::string> map_type;
+
+    map_type m;
+    std::pair<map_type::iterator, bool> p1 = m.insert(map_type::value_type(1, "abc"));
+    std::pair<map_type::iterator, bool> p2 = m.insert(map_type::value_type(2, "abc"));
+
+    const map_type& cm = m;
+
+    assert(cm.value_comp()(*p1.first, *p2.first));
+    assert(!cm.value_comp()(*p2.first, *p1.first));
+
+    return 0;
+}

libcxx/test/std/containers/associative/map/map.value_compare/invoke.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// class value_compare
+
+// bool operator()( const value_type& lhs, const value_type& rhs ) const;
+
+#include <map>
+#include <cassert>
+#include <string>
+#include <utility>
+
+template <typename Map>
+struct CallCompMember : Map::value_compare {
+    CallCompMember(const typename Map::value_compare& vc) : Map::value_compare(vc) {}
+
+    typedef typename Map::value_type value_type;
+    bool operator()(const value_type& value1, const value_type& value2) const {
+        return this->comp(value1.first, value2.first);
+    }
+};
+
+int main(int, char**) {
+    typedef std::map<int, std::string> map_type;
+
+    map_type m;
+    std::pair<map_type::iterator, bool> p1 = m.insert(map_type::value_type(1, "abc"));
+    std::pair<map_type::iterator, bool> p2 = m.insert(map_type::value_type(2, "abc"));
+
+    const map_type::value_compare vc = m.value_comp();
+    CallCompMember<map_type> call_comp = m.value_comp();
+
+    assert(vc(*p1.first, *p2.first));
+    assert(call_comp(*p1.first, *p2.first));
+
+    assert(!vc(*p2.first, *p1.first));
+    assert(!call_comp(*p2.first, *p1.first));
+
+    return 0;
+}

libcxx/test/std/containers/associative/map/map.value_compare/types.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// class value_compare
+
+// REQUIRES: c++98 || c++03 || c++11 || c++14
+
+#include <map>
+#include <string>
+
+#include "test_macros.h"
+
+int main(int, char**) {
+    typedef std::map<int, std::string> map_type;
+    typedef map_type::value_compare value_compare;
+    typedef map_type::value_type value_type;
+
+    ASSERT_SAME_TYPE(value_compare::result_type, bool);
+    ASSERT_SAME_TYPE(value_compare::first_argument_type, value_type);
+    ASSERT_SAME_TYPE(value_compare::second_argument_type, value_type);
+
+    return 0;
+}

libcxx/test/std/containers/associative/multimap/multimap.cons/compare.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <map>
 
 // class multimap
 
 // explicit multimap(const key_compare& comp);
 
-// key_compare key_comp() const;
-
 #include <map>
 #include <cassert>
 
 #include "test_macros.h"
 #include "../../../test_compare.h"
 #include "min_allocator.h"
 
 int main(int, char**)

libcxx/test/std/containers/associative/multimap/multimap.observers/key_comp.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// key_compare key_comp() const;
+
+#include <map>
+#include <cassert>
+#include <string>
+
+int main(int, char**) {
+    typedef std::multimap<int, std::string> map_type;
+
+    map_type m;
+    map_type::iterator i1 = m.insert(map_type::value_type(1, "abc"));
+    map_type::iterator i2 = m.insert(map_type::value_type(2, "abc"));
+
+    const map_type& cm = m;
+
+    assert(cm.key_comp()(i1->first, i2->first));
+    assert(!cm.key_comp()(i2->first, i1->first));
+
+    return 0;
+}

libcxx/test/std/containers/associative/multimap/multimap.observers/value_comp.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// value_compare value_comp() const;
+
+#include <map>
+#include <cassert>
+#include <string>
+
+int main(int, char**) {
+    typedef std::multimap<int, std::string> map_type;
+
+    map_type m;
+    map_type::iterator i1 = m.insert(map_type::value_type(1, "abc"));
+    map_type::iterator i2 = m.insert(map_type::value_type(2, "abc"));
+
+    const map_type& cm = m;
+
+    assert(cm.value_comp()(*i1, *i2));
+    assert(!cm.value_comp()(*i2, *i1));
+
+    return 0;
+}

libcxx/test/std/containers/associative/multimap/multimap.value_compare/invoke.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// class value_compare
+
+// bool operator()( const value_type& lhs, const value_type& rhs ) const;
+
+#include <map>
+#include <cassert>
+#include <string>
+#include <utility>
+
+template <typename MMap>
+struct CallCompMember : MMap::value_compare {
+    CallCompMember(const typename MMap::value_compare& vc) : MMap::value_compare(vc) {}
+
+    typedef typename MMap::value_type value_type;
+    bool operator()(const value_type& value1, const value_type& value2) const {
+        return this->comp(value1.first, value2.first);
+    }
+};
+
+int main(int, char**) {
+    typedef std::multimap<int, std::string> map_type;
+
+    map_type m;
+    map_type::iterator i1 = m.insert(map_type::value_type(1, "abc"));
+    map_type::iterator i2 = m.insert(map_type::value_type(2, "abc"));
+
+    const map_type::value_compare vc = m.value_comp();
+    CallCompMember<map_type> call_comp = m.value_comp();
+
+    assert(vc(*i1, *i2));
+    assert(call_comp(*i1, *i2));
+
+    assert(!vc(*i2, *i1));
+    assert(!call_comp(*i2, *i1));
+
+    return 0;
+}

libcxx/test/std/containers/associative/multimap/multimap.value_compare/types.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <map>
+
+// class value_compare
+
+// REQUIRES: c++98 || c++03 || c++11 || c++14
+
+#include <map>
+#include <string>
+
+#include "test_macros.h"
+
+int main(int, char**) {
+    typedef std::multimap<int, std::string> map_type;
+    typedef map_type::value_compare value_compare;
+    typedef map_type::value_type value_type;
+
+    ASSERT_SAME_TYPE(value_compare::result_type, bool);
+    ASSERT_SAME_TYPE(value_compare::first_argument_type, value_type);
+    ASSERT_SAME_TYPE(value_compare::second_argument_type, value_type);
+
+    return 0;
+}

libcxx/test/std/containers/associative/multiset/multiset.cons/compare.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <set>
 
 // class multiset
 
-// explicit multiset(const value_compare& comp);
-// value_compare and key_compare are the same type for set/multiset
-
-// key_compare    key_comp() const;
-// value_compare value_comp() const;
+// explicit multiset(const key_compare& comp);
 
 #include <set>
 #include <cassert>
 
 #include "test_macros.h"
 #include "../../../test_compare.h"
 
 int main(int, char**)

libcxx/test/std/containers/associative/multiset/multiset.observers/comp.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <set>
 
-// class set
-
-// explicit set(const value_compare& comp) const;
-// value_compare and key_compare are the same type for set/multiset
-
-// key_compare    key_comp() const;
+// key_compare key_comp() const;
 // value_compare value_comp() const;
 
 #include <set>
 #include <cassert>
 
-#include "test_macros.h"
-#include "../../../test_compare.h"
+int main(int, char**) {
+    typedef std::multiset<int> set_type;
+
+    set_type s;
+    set_type::iterator i1 = s.insert(1);
+    set_type::iterator i2 = s.insert(2);
+
+    const set_type& cs = s;
+
+    assert(cs.key_comp()(*i1, *i2));
+    assert(!cs.key_comp()(*i2, *i1));
 
-int main(int, char**)
-{
-    typedef test_less<int> C;
-    const std::set<int, C> m(C(3));
-    assert(m.empty());
-    assert(m.begin() == m.end());
-    assert(m.key_comp() == C(3));
-    assert(m.value_comp() == C(3));
+    assert(cs.value_comp()(*i1, *i2));
+    assert(!cs.value_comp()(*i2, *i1));
 
     return 0;
 }

libcxx/test/std/containers/associative/set/set.cons/compare.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <set>
 
 // class set
 
-// explicit set(const value_compare& comp) const;
-// value_compare and key_compare are the same type for set/multiset
-
-// key_compare    key_comp() const;
-// value_compare value_comp() const;
+// explicit set(const key_compare& comp) const;
 
 #include <set>
 #include <cassert>
 
 #include "test_macros.h"
 #include "../../../test_compare.h"
 
 int main(int, char**)

libcxx/test/std/containers/associative/set/set.observers/comp.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <set>
+
+// key_compare key_comp() const;
+// value_compare value_comp() const;
+
+#include <set>
+#include <cassert>
+
+int main(int, char**) {
+    typedef std::set<int> set_type;
+
+    set_type s;
+    std::pair<set_type::iterator, bool> p1 = s.insert(1);
+    std::pair<set_type::iterator, bool> p2 = s.insert(2);
+
+    const set_type& cs = s;
+
+    assert(cs.key_comp()(*p1.first, *p2.first));
+    assert(!cs.key_comp()(*p2.first, *p1.first));
+
+    assert(cs.value_comp()(*p1.first, *p2.first));
+    assert(!cs.value_comp()(*p2.first, *p1.first));
+
+    return 0;
+}