[libc++] Implement LWG3436: support for arrays in std::construct_at #132283
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
Member
Author
|
Note that this was originally opened as https://reviews.llvm.org/D114649. |
Member
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesFixes #118335 Full diff: https://github.com/llvm/llvm-project/pull/132283.diff 7 Files Affected:
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index 9bf31c417f3c9..3f1e7fef4520a 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -77,7 +77,7 @@
"`LWG4106 <https://wg21.link/LWG4106>`__","``basic_format_args`` should not be default-constructible","2024-06 (St. Louis)","|Complete|","19",""
"","","","","",""
"`LWG3216 <https://wg21.link/LWG3216>`__","Rebinding the allocator before calling ``construct``/``destroy`` in ``allocate_shared``","2024-11 (Wrocław)","","",""
-"`LWG3436 <https://wg21.link/LWG3436>`__","``std::construct_at`` should support arrays","2024-11 (Wrocław)","","",""
+"`LWG3436 <https://wg21.link/LWG3436>`__","``std::construct_at`` should support arrays","2024-11 (Wrocław)","|Complete|","21",""
"`LWG3886 <https://wg21.link/LWG3886>`__","Monad mo' problems","2024-11 (Wrocław)","","",""
"`LWG3899 <https://wg21.link/LWG3899>`__","``co_yield``\ing elements of an lvalue generator is unnecessarily inefficient","2024-11 (Wrocław)","","",""
"`LWG3900 <https://wg21.link/LWG3900>`__","The ``allocator_arg_t`` overloads of ``generator::promise_type::operator new`` should not be constrained","2024-11 (Wrocław)","","",""
diff --git a/libcxx/include/__memory/construct_at.h b/libcxx/include/__memory/construct_at.h
index 21337e766b2d0..f6f49d6daa0e6 100644
--- a/libcxx/include/__memory/construct_at.h
+++ b/libcxx/include/__memory/construct_at.h
@@ -17,6 +17,7 @@
#include <__new/placement_new_delete.h>
#include <__type_traits/enable_if.h>
#include <__type_traits/is_array.h>
+#include <__type_traits/is_unbounded_array.h>
#include <__utility/declval.h>
#include <__utility/forward.h>
#include <__utility/move.h>
@@ -34,15 +35,26 @@ _LIBCPP_BEGIN_NAMESPACE_STD
#if _LIBCPP_STD_VER >= 20
-template <class _Tp, class... _Args, class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...))>
+template <class _Tp,
+ class... _Args,
+ class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...)),
+ __enable_if_t<!is_unbounded_array_v<_Tp>, int> = 0>
_LIBCPP_HIDE_FROM_ABI constexpr _Tp* construct_at(_Tp* __location, _Args&&... __args) {
_LIBCPP_ASSERT_NON_NULL(__location != nullptr, "null pointer given to construct_at");
- return ::new (static_cast<void*>(__location)) _Tp(std::forward<_Args>(__args)...);
+ if constexpr (is_array_v<_Tp>) {
+ static_assert(sizeof...(_Args) == 0, "construction arguments cannot be passed to construct_at with an array type");
+ return ::new (static_cast<void*>(__location)) _Tp[1]();
+ } else {
+ return ::new (static_cast<void*>(__location)) _Tp(std::forward<_Args>(__args)...);
+ }
}
#endif
-template <class _Tp, class... _Args, class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...))>
+template <class _Tp,
+ class... _Args,
+ class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...)),
+ __enable_if_t<!is_unbounded_array_v<_Tp>, int> = 0>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp* __construct_at(_Tp* __location, _Args&&... __args) {
#if _LIBCPP_STD_VER >= 20
return std::construct_at(__location, std::forward<_Args>(__args)...);
diff --git a/libcxx/include/__memory/ranges_construct_at.h b/libcxx/include/__memory/ranges_construct_at.h
index b8a94678c10c8..3d98262ec3650 100644
--- a/libcxx/include/__memory/ranges_construct_at.h
+++ b/libcxx/include/__memory/ranges_construct_at.h
@@ -19,6 +19,7 @@
#include <__ranges/access.h>
#include <__ranges/concepts.h>
#include <__ranges/dangling.h>
+#include <__type_traits/is_unbounded_array.h>
#include <__utility/declval.h>
#include <__utility/forward.h>
#include <__utility/move.h>
@@ -38,7 +39,10 @@ namespace ranges {
// construct_at
struct __construct_at {
- template <class _Tp, class... _Args, class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...))>
+ template <class _Tp,
+ class... _Args,
+ class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...)),
+ __enable_if_t<!is_unbounded_array_v<_Tp>, int> = 0>
_LIBCPP_HIDE_FROM_ABI constexpr _Tp* operator()(_Tp* __location, _Args&&... __args) const {
return std::construct_at(__location, std::forward<_Args>(__args)...);
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.array.verify.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.array.verify.cpp
new file mode 100644
index 0000000000000..e732b75c4dcde
--- /dev/null
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.array.verify.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// REQUIRES: stdlib=libc++
+
+// <memory>
+
+// Test that std::construct_at provides a meaningful diagnostic when used with an
+// array type and construction arguments are provided. See LWG3436.
+
+#include <memory>
+
+using Array = int[3];
+void test(Array* a) {
+ std::construct_at(a, 1, 2, 3);
+ // expected-error-re@*:* {{static assertion failed {{.*}}construction arguments cannot be passed to construct_at with an array type}}
+}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp
index 00ec287ffeb66..10e3c7fbeecdd 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp
@@ -14,11 +14,13 @@
// constexpr T* construct_at(T* location, Args&& ...args);
#include <cassert>
+#include <concepts>
#include <cstddef>
#include <memory>
#include <utility>
#include "test_iterators.h"
+#include "test_macros.h"
struct Foo {
constexpr Foo() {}
@@ -39,25 +41,31 @@ struct Counted {
constexpr ~Counted() { --count_; }
};
+struct CountDefaultInitializations {
+ CountDefaultInitializations() { ++constructions; }
+ static int constructions;
+};
+int CountDefaultInitializations::constructions = 0;
+
constexpr bool test() {
{
- int i = 99;
- int* res = std::construct_at(&i);
+ int i = 99;
+ std::same_as<int*> auto res = std::construct_at(&i);
assert(res == &i);
assert(*res == 0);
}
{
- int i = 0;
- int* res = std::construct_at(&i, 42);
+ int i = 0;
+ std::same_as<int*> auto res = std::construct_at(&i, 42);
assert(res == &i);
assert(*res == 42);
}
{
- Foo foo = {};
- int count = 0;
- Foo* res = std::construct_at(&foo, 42, 'x', 123.89, &count);
+ Foo foo = {};
+ int count = 0;
+ std::same_as<Foo*> auto res = std::construct_at(&foo, 42, 'x', 123.89, &count);
assert(res == &foo);
assert(*res == Foo(42, 'x', 123.89));
assert(count == 1);
@@ -78,12 +86,70 @@ constexpr bool test() {
a.deallocate(p, 2);
}
+ // Test LWG3436, std::construct_at with array types
+ {
+ {
+ using Array = int[1];
+ Array array;
+ std::same_as<Array*> auto result = std::construct_at(&array);
+ assert(result == &array);
+ assert(array[0] == 0);
+ }
+ {
+ using Array = int[2];
+ Array array;
+ std::same_as<Array*> auto result = std::construct_at(&array);
+ assert(result == &array);
+ assert(array[0] == 0);
+ assert(array[1] == 0);
+ }
+ {
+ using Array = int[3];
+ Array array;
+ std::same_as<Array*> auto result = std::construct_at(&array);
+ assert(result == &array);
+ assert(array[0] == 0);
+ assert(array[1] == 0);
+ assert(array[2] == 0);
+ }
+
+ // Make sure we initialize the right number of elements. This can't be done inside
+ // constexpr since it requires a global variable.
+ if (!TEST_IS_CONSTANT_EVALUATED) {
+ {
+ using Array = CountDefaultInitializations[1];
+ CountDefaultInitializations array[1];
+ CountDefaultInitializations::constructions = 0;
+ std::construct_at(&array);
+ assert(CountDefaultInitializations::constructions == 1);
+ }
+ {
+ using Array = CountDefaultInitializations[2];
+ CountDefaultInitializations array[2];
+ CountDefaultInitializations::constructions = 0;
+ std::construct_at(&array);
+ assert(CountDefaultInitializations::constructions == 2);
+ }
+ {
+ using Array = CountDefaultInitializations[3];
+ CountDefaultInitializations array[3];
+ CountDefaultInitializations::constructions = 0;
+ std::construct_at(&array);
+ assert(CountDefaultInitializations::constructions == 3);
+ }
+ }
+ }
+
return true;
}
template <class... Args>
constexpr bool can_construct_at = requires { std::construct_at(std::declval<Args>()...); };
+struct NoDefault {
+ NoDefault() = delete;
+};
+
// Check that SFINAE works.
static_assert(can_construct_at<int*, int>);
static_assert(can_construct_at<Foo*, int, char, double>);
@@ -96,6 +162,11 @@ static_assert(!can_construct_at<contiguous_iterator<Foo*>, int, char, double>);
static_assert(!can_construct_at<int (*)()>);
static_assert(!can_construct_at<int (*)(), std::nullptr_t>);
+// LWG3436
+static_assert(can_construct_at<int (*)[3]>); // test the test
+static_assert(!can_construct_at<int (*)[]>); // unbounded arrays should SFINAE away
+static_assert(!can_construct_at<NoDefault (*)[1]>); // non default constructible shouldn't work
+
int main(int, char**) {
test();
static_assert(test());
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.array.verify.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.array.verify.cpp
new file mode 100644
index 0000000000000..1a1b8db96c8b4
--- /dev/null
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.array.verify.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// REQUIRES: stdlib=libc++
+
+// <memory>
+
+// Test that std::ranges::construct_at provides a meaningful diagnostic when used with an
+// array type and construction arguments are provided. See LWG3436.
+
+#include <memory>
+
+using Array = int[3];
+void test(Array* a) {
+ std::ranges::construct_at(a, 1, 2, 3);
+ // expected-error-re@*:* {{static assertion failed {{.*}}construction arguments cannot be passed to construct_at with an array type}}
+}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp
index 66e6f87432703..91e0597b6a6c6 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp
@@ -16,6 +16,7 @@
// }
#include <cassert>
+#include <concepts>
#include <initializer_list>
#include <memory>
#include <type_traits>
@@ -52,12 +53,18 @@ struct Counted {
constexpr ~Counted() { --count; }
};
+struct CountDefaultInitializations {
+ CountDefaultInitializations() { ++constructions; }
+ static int constructions;
+};
+int CountDefaultInitializations::constructions = 0;
+
constexpr bool test() {
// Value initialization.
{
int x = 1;
- int* result = std::ranges::construct_at(&x);
+ std::same_as<int*> auto result = std::ranges::construct_at(&x);
assert(result == &x);
assert(x == 0);
}
@@ -66,7 +73,7 @@ constexpr bool test() {
{
int x = 1;
- int* result = std::ranges::construct_at(&x, 42);
+ std::same_as<int*> auto result = std::ranges::construct_at(&x, 42);
assert(result == &x);
assert(x == 42);
}
@@ -75,7 +82,7 @@ constexpr bool test() {
{
Foo f;
- Foo* result = std::ranges::construct_at(std::addressof(f), 42, 123);
+ std::same_as<Foo*> auto result = std::ranges::construct_at(std::addressof(f), 42, 123);
assert(result == std::addressof(f));
assert(f.x == 42);
assert(f.y == 123);
@@ -87,7 +94,7 @@ constexpr bool test() {
Counted* out = alloc.allocate(2);
int count = 0;
- Counted* result = std::ranges::construct_at(out, count);
+ std::same_as<Counted*> auto result = std::ranges::construct_at(out, count);
assert(result == out);
assert(count == 1);
@@ -99,28 +106,85 @@ constexpr bool test() {
alloc.deallocate(out, 2);
}
- return true;
-}
+ // Test LWG3436, std::ranges::construct_at with array types
+ {
+ {
+ using Array = int[1];
+ Array array;
+ std::same_as<Array*> auto result = std::ranges::construct_at(&array);
+ assert(result == &array);
+ assert(array[0] == 0);
+ }
+ {
+ using Array = int[2];
+ Array array;
+ std::same_as<Array*> auto result = std::ranges::construct_at(&array);
+ assert(result == &array);
+ assert(array[0] == 0);
+ assert(array[1] == 0);
+ }
+ {
+ using Array = int[3];
+ Array array;
+ std::same_as<Array*> auto result = std::ranges::construct_at(&array);
+ assert(result == &array);
+ assert(array[0] == 0);
+ assert(array[1] == 0);
+ assert(array[2] == 0);
+ }
+
+ // Make sure we initialize the right number of elements. This can't be done inside
+ // constexpr since it requires a global variable.
+ if (!TEST_IS_CONSTANT_EVALUATED) {
+ {
+ using Array = CountDefaultInitializations[1];
+ CountDefaultInitializations array[1];
+ CountDefaultInitializations::constructions = 0;
+ std::ranges::construct_at(&array);
+ assert(CountDefaultInitializations::constructions == 1);
+ }
+ {
+ using Array = CountDefaultInitializations[2];
+ CountDefaultInitializations array[2];
+ CountDefaultInitializations::constructions = 0;
+ std::ranges::construct_at(&array);
+ assert(CountDefaultInitializations::constructions == 2);
+ }
+ {
+ using Array = CountDefaultInitializations[3];
+ CountDefaultInitializations array[3];
+ CountDefaultInitializations::constructions = 0;
+ std::ranges::construct_at(&array);
+ assert(CountDefaultInitializations::constructions == 3);
+ }
+ }
+ }
-constexpr bool can_construct_at(auto&&... args)
- requires requires { std::ranges::construct_at(decltype(args)(args)...); }
-{
return true;
}
-constexpr bool can_construct_at(auto&&...) { return false; }
+template <class... Args>
+constexpr bool can_construct_at = requires { std::ranges::construct_at(std::declval<Args>()...); };
+
+struct NoDefault {
+ NoDefault() = delete;
+};
// Check that SFINAE works.
-static_assert(can_construct_at((Foo*)nullptr, 1, 2));
-static_assert(!can_construct_at((Foo*)nullptr, 1));
-static_assert(!can_construct_at((Foo*)nullptr, 1, 2, 3));
-static_assert(!can_construct_at(nullptr, 1, 2));
-static_assert(!can_construct_at((int*)nullptr, 1, 2));
-static_assert(!can_construct_at(contiguous_iterator<Foo*>(), 1, 2));
+static_assert(can_construct_at<Foo*, int, int>);
+static_assert(!can_construct_at<Foo*, int>);
+static_assert(!can_construct_at<Foo*, int, int, int>);
+static_assert(!can_construct_at<std::nullptr_t, int, int>);
+static_assert(!can_construct_at<int*, int, int>);
+static_assert(!can_construct_at<contiguous_iterator<Foo*>, int, int>);
// Can't construct function pointers.
-static_assert(!can_construct_at((int (*)()) nullptr));
-static_assert(!can_construct_at((int (*)()) nullptr, nullptr));
-// TODO(varconst): check that array types work once D114649 implementing LWG3639 lands.
+static_assert(!can_construct_at<int (*)()>);
+static_assert(!can_construct_at<int (*)(), std::nullptr_t>);
+
+// LWG3436
+static_assert(can_construct_at<int (*)[3]>); // test the test
+static_assert(!can_construct_at<int (*)[]>); // unbounded arrays should SFINAE away
+static_assert(!can_construct_at<NoDefault (*)[1]>); // non default constructible shouldn't work
int main(int, char**) {
test();
|
Member
Author
|
I think this is breaking in all kinds of ways because we're still missing some Clang support for arrays in placement-new expressions. I wasn't able to get my tests fully passing locally. |
| template <class _Tp, | ||
| class... _Args, | ||
| class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...)), | ||
| __enable_if_t<!is_unbounded_array_v<_Tp>, int> = 0> |
Contributor
There was a problem hiding this comment.
is_unbounded_array_v is introduced in C++20 so we shouldn't use it for __construct_at. Moreover, should we make the internal __construct_at have the same SFINAE constraints as std::construct_at?
Comment on lines
+40
to
+41
| class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...)), | ||
| __enable_if_t<!is_unbounded_array_v<_Tp>, int> = 0> |
Contributor
There was a problem hiding this comment.
Would it be better to use requires (ditto for `ranges::construct_at)?
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| // UNSUPPORTED: c++03, c++11, c++14, c++17 |
Contributor
There was a problem hiding this comment.
It seems that we can use the new style (ditto below):
Suggested change
| // UNSUPPORTED: c++03, c++11, c++14, c++17 | |
| // REQUIRES: std-at-least-c++20 |
| a.deallocate(p, 2); | ||
| } | ||
|
|
||
| // Test LWG3436, std::construct_at with array types |
Contributor
There was a problem hiding this comment.
IIRC Clang didn't support placement new for array in constant evaluation until Clang 20. (The ability was added via #104586.) So, should we skip "Clang 19 + constant evaluation" here?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #118335