[libcxx-commits] [libcxx] ceff0b7 - [libc++] Do not require movability in __non_propagating_cache::__emplace_deref
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Aug 17 08:32:55 PDT 2021
Author: Louis Dionne
Date: 2021-08-17T11:32:40-04:00
New Revision: ceff0b7258aefc6e5299802c119b3d01545440f4
URL: https://github.com/llvm/llvm-project/commit/ceff0b7258aefc6e5299802c119b3d01545440f4
DIFF: https://github.com/llvm/llvm-project/commit/ceff0b7258aefc6e5299802c119b3d01545440f4.diff
LOG: [libc++] Do not require movability in __non_propagating_cache::__emplace_deref
As explained in http://eel.is/c++draft/range.nonprop.cache#note-1, we
should allow copy and move elision to happen when calling emplace_deref
in non-propagating-cache. Before this change, the only way to emplace
into the non-propagating-cache was to call `__set(*it)`, which materialized
`*it` when binding it to the reference argument of `__set` and disabled
move elision.
As a fly-by change, this also renames `__set` to `__emplace` for consistency
and adds tests for it.
Differential Revision: https://reviews.llvm.org/D107932
Added:
libcxx/test/libcxx/ranges/range.nonprop.cache/emplace.pass.cpp
libcxx/test/libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp
Modified:
libcxx/include/__ranges/drop_view.h
libcxx/include/__ranges/join_view.h
libcxx/include/__ranges/non_propagating_cache.h
libcxx/include/__ranges/reverse_view.h
libcxx/test/libcxx/ranges/range.nonprop.cache/assign.copy.pass.cpp
libcxx/test/libcxx/ranges/range.nonprop.cache/assign.move.pass.cpp
libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.copy.pass.cpp
libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.move.pass.cpp
libcxx/test/libcxx/ranges/range.nonprop.cache/deref.pass.cpp
libcxx/test/libcxx/ranges/range.nonprop.cache/has_value.pass.cpp
Removed:
################################################################################
diff --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index 6d1a5a2efff66..d732d02f2f2a0 100644
--- a/libcxx/include/__ranges/drop_view.h
+++ b/libcxx/include/__ranges/drop_view.h
@@ -77,7 +77,7 @@ namespace ranges {
auto __tmp = ranges::next(ranges::begin(__base_), __count_, ranges::end(__base_));
if constexpr (_UseCache)
- __cached_begin_.__set(__tmp);
+ __cached_begin_.__emplace(__tmp);
return __tmp;
}
diff --git a/libcxx/include/__ranges/join_view.h b/libcxx/include/__ranges/join_view.h
index 44aa1d0264e6d..9aa69da76cf0b 100644
--- a/libcxx/include/__ranges/join_view.h
+++ b/libcxx/include/__ranges/join_view.h
@@ -191,7 +191,7 @@ namespace ranges {
if constexpr (__ref_is_glvalue)
return *__outer_;
else
- return __parent_->__cache_.__emplace_deref(__outer_);
+ return __parent_->__cache_.__emplace_from([&]() -> decltype(auto) { return *__outer_; });
}();
__inner_ = ranges::begin(__inner);
if (*__inner_ != ranges::end(__inner))
diff --git a/libcxx/include/__ranges/non_propagating_cache.h b/libcxx/include/__ranges/non_propagating_cache.h
index 76577f47a5ad4..456e08d8c971a 100644
--- a/libcxx/include/__ranges/non_propagating_cache.h
+++ b/libcxx/include/__ranges/non_propagating_cache.h
@@ -13,6 +13,7 @@
#include <__iterator/concepts.h> // indirectly_readable
#include <__iterator/iterator_traits.h> // iter_reference_t
#include <__memory/addressof.h>
+#include <__utility/forward.h>
#include <concepts> // constructible_from
#include <optional>
#include <type_traits>
@@ -21,13 +22,8 @@
#pragma GCC system_header
#endif
-_LIBCPP_PUSH_MACROS
-#include <__undef_macros>
-
_LIBCPP_BEGIN_NAMESPACE_STD
-// clang-format off
-
#if !defined(_LIBCPP_HAS_NO_RANGES)
namespace ranges {
@@ -42,7 +38,20 @@ namespace ranges {
template<class _Tp>
requires is_object_v<_Tp>
class _LIBCPP_TEMPLATE_VIS __non_propagating_cache {
- optional<_Tp> __value_ = nullopt;
+ struct __from_tag { };
+ struct __forward_tag { };
+
+ // This helper class is needed to perform copy and move elision when
+ // constructing the contained type from an iterator.
+ struct __wrapper {
+ template<class ..._Args>
+ constexpr explicit __wrapper(__forward_tag, _Args&& ...__args) : __t_(_VSTD::forward<_Args>(__args)...) { }
+ template<class _Fn>
+ constexpr explicit __wrapper(__from_tag, _Fn const& __f) : __t_(__f()) { }
+ _Tp __t_;
+ };
+
+ optional<__wrapper> __value_ = nullopt;
public:
_LIBCPP_HIDE_FROM_ABI __non_propagating_cache() = default;
@@ -75,23 +84,23 @@ namespace ranges {
}
_LIBCPP_HIDE_FROM_ABI
- constexpr _Tp& operator*() { return *__value_; }
+ constexpr _Tp& operator*() { return __value_->__t_; }
_LIBCPP_HIDE_FROM_ABI
- constexpr _Tp const& operator*() const { return *__value_; }
+ constexpr _Tp const& operator*() const { return __value_->__t_; }
_LIBCPP_HIDE_FROM_ABI
constexpr bool __has_value() const { return __value_.has_value(); }
+
+ template<class _Fn>
_LIBCPP_HIDE_FROM_ABI
- constexpr void __set(_Tp const& __value) { __value_.emplace(__value); }
- _LIBCPP_HIDE_FROM_ABI
- constexpr void __set(_Tp&& __value) { __value_.emplace(_VSTD::move(__value)); }
+ constexpr _Tp& __emplace_from(_Fn const& __f) {
+ return __value_.emplace(__from_tag{}, __f).__t_;
+ }
- template<class _Other>
+ template<class ..._Args>
_LIBCPP_HIDE_FROM_ABI
- constexpr _Tp& __emplace_deref(const _Other& __value) {
- __value_.reset();
- __value_.emplace(*__value);
- return *__value_;
+ constexpr _Tp& __emplace(_Args&& ...__args) {
+ return __value_.emplace(__forward_tag{}, _VSTD::forward<_Args>(__args)...).__t_;
}
};
@@ -102,6 +111,4 @@ namespace ranges {
_LIBCPP_END_NAMESPACE_STD
-_LIBCPP_POP_MACROS
-
#endif // _LIBCPP___RANGES_NON_PROPAGATING_CACHE_H
diff --git a/libcxx/include/__ranges/reverse_view.h b/libcxx/include/__ranges/reverse_view.h
index 5953f74fd77d5..ad88dc7138053 100644
--- a/libcxx/include/__ranges/reverse_view.h
+++ b/libcxx/include/__ranges/reverse_view.h
@@ -64,7 +64,7 @@ namespace ranges {
auto __tmp = _VSTD::make_reverse_iterator(ranges::next(ranges::begin(__base_), ranges::end(__base_)));
if constexpr (_UseCache)
- __cached_begin_.__set(__tmp);
+ __cached_begin_.__emplace(__tmp);
return __tmp;
}
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.copy.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.copy.pass.cpp
index 393cde1199b21..108422fa4e228 100644
--- a/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.copy.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.copy.pass.cpp
@@ -47,7 +47,7 @@ constexpr void test() {
// Assign to an empty cache
{
- Cache a; a.__set(T{3});
+ Cache a; a.__emplace(3);
Cache b;
Cache& result = (b = a);
@@ -60,8 +60,8 @@ constexpr void test() {
// Assign to a non-empty cache
{
- Cache a; a.__set(T{3});
- Cache b; b.__set(T{5});
+ Cache a; a.__emplace(3);
+ Cache b; b.__emplace(5);
Cache& result = (b = a);
assert(&result == &b);
@@ -81,7 +81,7 @@ constexpr void test() {
// Self-assignment should not do anything (case with non-empty cache)
{
- Cache b; b.__set(T{5});
+ Cache b; b.__emplace(5);
Cache& result = (b = b);
assert(&result == &b);
assert(b.__has_value());
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.move.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.move.pass.cpp
index 6c5ccb100293a..5f04619832fbc 100644
--- a/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.move.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/assign.move.pass.cpp
@@ -46,7 +46,7 @@ constexpr void test() {
// Assign to an empty cache
{
- Cache a; a.__set(T{3});
+ Cache a; a.__emplace(3);
Cache b;
Cache& result = (b = std::move(a));
@@ -57,8 +57,8 @@ constexpr void test() {
// Assign to a non-empty cache
{
- Cache a; a.__set(T{3});
- Cache b; b.__set(T{5});
+ Cache a; a.__emplace(3);
+ Cache b; b.__emplace(5);
Cache& result = (b = std::move(a));
assert(&result == &b);
@@ -77,7 +77,7 @@ constexpr void test() {
// Self-assignment should clear the cache (case with non-empty cache)
{
- Cache b; b.__set(T{5});
+ Cache b; b.__emplace(5);
Cache& result = (b = std::move(b));
assert(&result == &b);
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.copy.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.copy.pass.cpp
index fae7a3e92939a..762222058e1a9 100644
--- a/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.copy.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.copy.pass.cpp
@@ -39,7 +39,7 @@ constexpr void test() {
using Cache = std::ranges::__non_propagating_cache<T>;
static_assert(std::is_nothrow_copy_constructible_v<Cache>);
Cache a;
- a.__set(T{3});
+ a.__emplace(3);
// Test with direct initialization
{
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.move.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.move.pass.cpp
index 1fa454f7acc17..b28c751281d67 100644
--- a/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.move.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/ctor.move.pass.cpp
@@ -34,7 +34,7 @@ constexpr void test() {
// Test with direct initialization
{
Cache a;
- a.__set(T{3});
+ a.__emplace(3);
Cache b(std::move(a));
assert(!b.__has_value()); // make sure we don't propagate
@@ -44,7 +44,7 @@ constexpr void test() {
// Test with copy initialization
{
Cache a;
- a.__set(T{3});
+ a.__emplace(3);
Cache b = std::move(a);
assert(!b.__has_value()); // make sure we don't propagate
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/deref.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/deref.pass.cpp
index 55b288028af3d..51508c59a22aa 100644
--- a/libcxx/test/libcxx/ranges/range.nonprop.cache/deref.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/deref.pass.cpp
@@ -23,14 +23,14 @@ constexpr void test() {
// non-const version
{
- Cache cache; cache.__set(T{3});
+ Cache cache; cache.__emplace(3);
T& result = *cache;
assert(result == T{3});
}
// const version
{
- Cache cache; cache.__set(T{3});
+ Cache cache; cache.__emplace(3);
T const& result = *static_cast<Cache const&>(cache);
assert(result == T{3});
}
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/emplace.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/emplace.pass.cpp
new file mode 100644
index 0000000000000..636eda8aa6d91
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/emplace.pass.cpp
@@ -0,0 +1,97 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
+// template<class ...Args>
+// constexpr T& __emplace(Args&& ...);
+
+#include <ranges>
+
+#include <cassert>
+#include <tuple>
+
+template<int I>
+struct X {
+ int value = -1;
+ template<int J>
+ friend constexpr bool operator==(X const& a, X<J> const& b) { return I == J && a.value == b.value; }
+};
+
+struct NonMovable {
+ int value = -1;
+ NonMovable() = default;
+ constexpr explicit NonMovable(int v) : value(v) { }
+ NonMovable(NonMovable&&) = delete;
+ NonMovable& operator=(NonMovable&&) = delete;
+};
+
+constexpr bool test() {
+ {
+ using T = std::tuple<>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace();
+ assert(&result == &*cache);
+ assert(result == T());
+ }
+
+ {
+ using T = std::tuple<X<0>>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace();
+ assert(&result == &*cache);
+ assert(result == T());
+ }
+ {
+ using T = std::tuple<X<0>>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace(X<0>{0});
+ assert(&result == &*cache);
+ assert(result == T(X<0>{0}));
+ }
+
+ {
+ using T = std::tuple<X<0>, X<1>>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace();
+ assert(&result == &*cache);
+ assert(result == T());
+ }
+ {
+ using T = std::tuple<X<0>, X<1>>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace(X<0>{0}, X<1>{1});
+ assert(&result == &*cache);
+ assert(result == T(X<0>{0}, X<1>{1}));
+ }
+
+ // Make sure that we do not require the type to be movable when we emplace it.
+ // Move elision should be performed instead, see http://eel.is/c++draft/range.nonprop.cache#note-1.
+ {
+ using Cache = std::ranges::__non_propagating_cache<NonMovable>;
+ Cache cache;
+ NonMovable& result = cache.__emplace();
+ assert(&result == &*cache);
+ assert(result.value == -1);
+ }
+
+ return true;
+}
+
+int main(int, char**) {
+ static_assert(test());
+ test();
+ return 0;
+}
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp
new file mode 100644
index 0000000000000..d87db6570cdb9
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp
@@ -0,0 +1,79 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
+// template<class Fn>
+// constexpr T& __emplace_from(Fn const&);
+
+#include <ranges>
+
+#include <cassert>
+#include <tuple>
+
+template<int I>
+struct X {
+ int value = -1;
+ template<int J>
+ friend constexpr bool operator==(X const& a, X<J> const& b) { return I == J && a.value == b.value; }
+};
+
+struct NonMovable {
+ int value = -1;
+ NonMovable() = default;
+ constexpr explicit NonMovable(int v) : value(v) { }
+ NonMovable(NonMovable&&) = delete;
+ NonMovable& operator=(NonMovable&&) = delete;
+};
+
+constexpr bool test() {
+ {
+ using T = std::tuple<>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace_from([] { return T(); });
+ assert(&result == &*cache);
+ assert(result == T());
+ }
+ {
+ using T = std::tuple<X<0>>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace_from([] { return T(X<0>{0}); });
+ assert(&result == &*cache);
+ assert(result == T(X<0>{0}));
+ }
+ {
+ using T = std::tuple<X<0>, X<1>>;
+ using Cache = std::ranges::__non_propagating_cache<T>;
+ Cache cache;
+ T& result = cache.__emplace_from([] { return T(X<0>{0}, X<1>{1}); });
+ assert(&result == &*cache);
+ assert(result == T(X<0>{0}, X<1>{1}));
+ }
+
+ // Make sure that we do not require the type to be movable when we emplace it.
+ // Move elision should be performed instead, see http://eel.is/c++draft/range.nonprop.cache#note-1.
+ {
+ using Cache = std::ranges::__non_propagating_cache<NonMovable>;
+ Cache cache;
+ NonMovable& result = cache.__emplace_from([] { return NonMovable(3); });
+ assert(&result == &*cache);
+ assert(result.value == 3);
+ }
+
+ return true;
+}
+
+int main(int, char**) {
+ static_assert(test());
+ test();
+ return 0;
+}
diff --git a/libcxx/test/libcxx/ranges/range.nonprop.cache/has_value.pass.cpp b/libcxx/test/libcxx/ranges/range.nonprop.cache/has_value.pass.cpp
index 4268cf9abe903..2b30e99ce06e7 100644
--- a/libcxx/test/libcxx/ranges/range.nonprop.cache/has_value.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.nonprop.cache/has_value.pass.cpp
@@ -28,7 +28,7 @@ constexpr void test() {
// __has_value on a non-empty cache
{
- Cache cache; cache.__set(T{});
+ Cache cache; cache.__emplace();
assert(cache.__has_value());
}
}
More information about the libcxx-commits
mailing list