[libc++][mdspan] Fix extents CTAD #68737
Merged
Conversation
This file contains 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++
|
@llvm/pr-subscribers-libcxx Author: Christian Trott (crtrott) Changesextents CTAD was requiring default constructibility of the extent arguments due to the way we implemented a pack expansion. This requirement is not in the standard. Reported in issue #68671 #68671 by @hewillk. Fixes #68671 Full diff: https://github.com/llvm/llvm-project/pull/68737.diff 2 Files Affected:
diff --git a/libcxx/include/__mdspan/extents.h b/libcxx/include/__mdspan/extents.h
index a510220d4096a2f..f6bcd940ee6077d 100644
--- a/libcxx/include/__mdspan/extents.h
+++ b/libcxx/include/__mdspan/extents.h
@@ -456,7 +456,7 @@ using dextents = typename __mdspan_detail::__make_dextents<_IndexType, _Rank>::t
// Deduction guide for extents
template <class... _IndexTypes>
-extents(_IndexTypes...) -> extents<size_t, size_t((_IndexTypes(), dynamic_extent))...>;
+extents(_IndexTypes...) -> extents<size_t, size_t(((void)sizeof(_IndexTypes), dynamic_extent))...>;
namespace __mdspan_detail {
diff --git a/libcxx/test/std/containers/views/mdspan/extents/ctad.pass.cpp b/libcxx/test/std/containers/views/mdspan/extents/ctad.pass.cpp
index 2a3da30bb936600..fed16b3fc91b459 100644
--- a/libcxx/test/std/containers/views/mdspan/extents/ctad.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/extents/ctad.pass.cpp
@@ -21,6 +21,12 @@
#include "../ConvertibleToIntegral.h"
#include "test_macros.h"
+struct S {
+ size_t value;
+ constexpr S(size_t val):value(val) {};
+ constexpr operator size_t() const noexcept { return value; }
+};
+
template <class E, class Expected>
constexpr void test(E e, Expected expected) {
ASSERT_SAME_TYPE(E, Expected);
@@ -35,6 +41,7 @@ constexpr bool test() {
test(std::extents(1, 2u), std::extents<std::size_t, D, D>(1, 2u));
test(std::extents(1, 2u, 3, 4, 5, 6, 7, 8, 9),
std::extents<std::size_t, D, D, D, D, D, D, D, D, D>(1, 2u, 3, 4, 5, 6, 7, 8, 9));
+ test(std::extents(S{1}, S{2}), std::extents<std::size_t, D, D>(1, 2u));
return true;
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
crtrott
force-pushed
the
fix-extents-ctad-68671
branch
from
eda9969 to
845d280
Compare
|
Ah I see clang-format is now also checked for tests. Good to know. |
ldionne
requested changes
Oct 10, 2023
dalg24
reviewed
Oct 10, 2023
crtrott
force-pushed
the
fix-extents-ctad-68671
branch
from
845d280 to
b8f2ab6
Compare
dalg24
approved these changes
Oct 10, 2023
ldionne
approved these changes
Oct 10, 2023
|
How do we retest? Looks like it failed in generated output due to some string file formatting issue which likely was caused elsewhere? |
|
Retest this please. |
extents CTAD was requiring default constructibility of the extent arguments due to the way we implemented a pack expansion. This requirement is not in the standard. Reported in issue llvm#68671 llvm#68671 by @hewillk. Fixes llvm#68671
crtrott
force-pushed
the
fix-extents-ctad-68671
branch
from
b8f2ab6 to
134d609
Compare
|
I just rebased on main again, which retriggered the build |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
extents CTAD was requiring default constructibility of the extent arguments due to the way we implemented a pack expansion. This requirement is not in the standard.
Reported in issue #68671 #68671 by @hewillk.
Fixes #68671