[libcxx] Add span includes to some mdspan tests #142693
Merged
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
Created using spr 1.3.4
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Aiden Grossman (boomanaiden154) ChangesThis is needed to get these tests to pass in the following cmake -GNinja -DCMAKE_BUILD_TYPE=Release ninja check-cxx Not exactly sure why these are needed. If there is some other way this Interestingly enough this does not fail when using a standalone runtimes This seems to be related to 5e19fd1 which Full diff: https://github.com/llvm/llvm-project/pull/142693.diff 15 Files Affected:
diff --git a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.conversion.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.conversion.pass.cpp
index 31766e4c51c3b..6ac70d9bd63df 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.conversion.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.conversion.pass.cpp
@@ -30,6 +30,7 @@
// (((Extents != dynamic_extent) && (OtherExtents == dynamic_extent)) || ... ) ||
// (numeric_limits<index_type>::max() < numeric_limits<OtherIndexType>::max())
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_array.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_array.pass.cpp
index 90cb0c84a063b..fc52424eacdf2 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_array.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_array.pass.cpp
@@ -30,6 +30,7 @@
// for every rank index r.
//
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_integral.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_integral.pass.cpp
index 37e79aabf8532..2fc57a35dacad 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_integral.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_integral.pass.cpp
@@ -32,6 +32,7 @@
// - each element of exts is nonnegative and is representable as a value of type index_type.
//
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_span.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_span.pass.cpp
index 5bf5143590180..4c3f0a7c5cc70 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_span.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_span.pass.cpp
@@ -28,6 +28,7 @@
// for every rank index r.
//
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.obs.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.obs.pass.cpp
index c473879d87b71..980d9889187a6 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/extents/assert.obs.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/extents/assert.obs.pass.cpp
@@ -24,6 +24,7 @@
//
// Returns: Di.
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.conversion.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.conversion.pass.cpp
index 7b6616f19d724..bf7a4fd398bcd 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.conversion.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.conversion.pass.cpp
@@ -21,6 +21,7 @@
//
// Preconditions: other.required_span_size() is representable as a value of type index_type.
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.extents.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.extents.pass.cpp
index 7c96f8ec9353f..8a1cf24221f8e 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.extents.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.extents.pass.cpp
@@ -19,6 +19,7 @@
//
// Effects: Direct-non-list-initializes extents_ with e.
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_right.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_right.pass.cpp
index e578bac2103b0..310b91e593cd8 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_right.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_right.pass.cpp
@@ -23,6 +23,7 @@
//
// Preconditions: other.required_span_size() is representable as a value of type index_type
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_stride.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_stride.pass.cpp
index cec8df8aba8d6..5e9c4f15133c9 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_stride.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_left/assert.ctor.layout_stride.pass.cpp
@@ -32,6 +32,7 @@
//
// Effects: Direct-non-list-initializes extents_ with other.extents().
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.conversion.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.conversion.pass.cpp
index df16edb925407..5fa6cc949e571 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.conversion.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.conversion.pass.cpp
@@ -21,6 +21,7 @@
//
// Preconditions: other.required_span_size() is representable as a value of type index_type.
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.extents.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.extents.pass.cpp
index 52095691f6d24..a2aa0be0102ca 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.extents.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.extents.pass.cpp
@@ -19,6 +19,7 @@
//
// Effects: Direct-non-list-initializes extents_ with e.
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_left.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_left.pass.cpp
index 1757ddb286b9c..79ee49e7a925c 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_left.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_left.pass.cpp
@@ -23,6 +23,7 @@
//
// Preconditions: other.required_span_size() is representable as a value of type index_type
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_stride.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_stride.pass.cpp
index b77d964540f9d..fa83d6087ec85 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_stride.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_right/assert.ctor.layout_stride.pass.cpp
@@ -32,6 +32,7 @@
//
// Effects: Direct-non-list-initializes extents_ with other.extents().
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_array.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_array.pass.cpp
index 6782a9789f89f..0ba5ffb47c7ea 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_array.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_array.pass.cpp
@@ -30,6 +30,7 @@
//
// Effects: Direct-non-list-initializes extents_ with e, and for all d in the range [0, rank_), direct-non-list-initializes strides_[d] with as_const(s[d]).
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
diff --git a/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_span.pass.cpp b/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_span.pass.cpp
index 74e4793c91372..f8e7ddc8b5770 100644
--- a/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_span.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_span.pass.cpp
@@ -30,6 +30,7 @@
//
// Effects: Direct-non-list-initializes extents_ with e, and for all d in the range [0, rank_), direct-non-list-initializes strides_[d] with as_const(s[d]).
+#include <span> // dynamic_extent
#include <mdspan>
#include <cassert>
|
|
I think the proper fix here is to |
Is there something we can do in the short term to get this working? 5e19fd1 did essentially what this patch is doing, but I'm pretty unfamiliar with how libc++ does development around WG21 decisions. |
We can implement this without requiring the LWG issue to be resolved. We're just providing more symbols than strictly required by the standard, which we're already allowed to do. I'm just asking for the LWG issue to make sure the standard gets fixed. See https://cplusplus.github.io/LWG/lwg-active.html in case you haven't filed one before. Or if you don't want to file one for whatever reason I can do that too. |
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This is needed to get these tests to pass in the following configuration: cmake -GNinja -DCMAKE_BUILD_TYPE=Release \ -DLLVM_ENABLE_PROJECTS="clang" \ -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \ -DLIBCXX_TEST_PARAMS="enable_modules=clang" \ -DLIBCXXABI_TEST_PARAMS="enable_modules=clang" \ ../llvm ninja check-cxx Not exactly sure why these are needed. If there is some other way this should be fixed, I am perfectly fine going with that. Interestingly enough this does not fail when using a standalone runtimes build (after installing clang/appropriate headers). I have not dug into why that is the case. This seems to be related to 5e19fd1 which added in some similar includes. Pull Request: llvm#142693
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This is needed to get these tests to pass in the following configuration: cmake -GNinja -DCMAKE_BUILD_TYPE=Release \ -DLLVM_ENABLE_PROJECTS="clang" \ -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \ -DLIBCXX_TEST_PARAMS="enable_modules=clang" \ -DLIBCXXABI_TEST_PARAMS="enable_modules=clang" \ ../llvm ninja check-cxx Not exactly sure why these are needed. If there is some other way this should be fixed, I am perfectly fine going with that. Interestingly enough this does not fail when using a standalone runtimes build (after installing clang/appropriate headers). I have not dug into why that is the case. This seems to be related to 5e19fd1 which added in some similar includes. Pull Request: llvm#142693
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This patch includes __fwd/span.h in <mdspan> so that we get the declaration of dynamic_extent inside <mdspan>. We also clean up quite a few tests that were manually included <span> for dynamic_extent. This is based on feedback from llvm#142693.
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This is needed to get these tests to pass in the following configuration: cmake -GNinja -DCMAKE_BUILD_TYPE=Release \ -DLLVM_ENABLE_PROJECTS="clang" \ -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \ -DLIBCXX_TEST_PARAMS="enable_modules=clang" \ -DLIBCXXABI_TEST_PARAMS="enable_modules=clang" \ ../llvm ninja check-cxx Not exactly sure why these are needed. If there is some other way this should be fixed, I am perfectly fine going with that. Interestingly enough this does not fail when using a standalone runtimes build (after installing clang/appropriate headers). I have not dug into why that is the case. This seems to be related to 5e19fd1 which added in some similar includes. Pull Request: llvm#142693
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This patch includes __fwd/span.h in <mdspan> so that we get the declaration of dynamic_extent inside <mdspan>. We also clean up quite a few tests that were manually included <span> for dynamic_extent. This is based on feedback from llvm#142693.
|
I opened #142925 to handle including
Makes sense. I'll look into getting something filed, hopefully over the weekend. |
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This is needed to get these tests to pass in the following configuration: cmake -GNinja -DCMAKE_BUILD_TYPE=Release \ -DLLVM_ENABLE_PROJECTS="clang" \ -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \ -DLIBCXX_TEST_PARAMS="enable_modules=clang" \ -DLIBCXXABI_TEST_PARAMS="enable_modules=clang" \ ../llvm ninja check-cxx (Within the premerge build). This patch specifically handles some missing <span> includes for invocations of std::span in these tests. There are some other cases where we need dynamic_extent that are handled in llvm#142925. This seems to be related to 5e19fd1 which added in some similar includes. Reviewers: ldionne, #reviewers-libcxx, philnik777 Pull Request: llvm#142693
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Jun 5, 2025
This patch includes __fwd/span.h in <mdspan> so that we get the declaration of dynamic_extent inside <mdspan>. We also clean up quite a few tests that were manually included <span> for dynamic_extent. This is based on feedback from llvm#142693. Pull Request: llvm#142925
ldionne
approved these changes
Jun 5, 2025
There was a problem hiding this comment.
LGTM with a super nit.
Historically, we would always include the header being tested first. The intent was to ensure that each header is self-standing and can be included on its own. Nowadays we have generated tests that do these checks, so the benefit of always including the header being tested first is less. I sorted the includes instead.
libcxx/test/libcxx/containers/views/mdspan/extents/assert.ctor_from_span.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_array.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/libcxx/containers/views/mdspan/layout_stride/assert.ctor.extents_span.pass.cpp
Outdated
Show resolved
Hide resolved
ldionne
deleted the
users/boomanaiden154/libcxx-add-span-includes-to-some-mdspan-tests-1
branch
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.
This is needed to get these tests to pass in the following
configuration:
cmake -GNinja -DCMAKE_BUILD_TYPE=Release
-DLLVM_ENABLE_PROJECTS="clang"
-DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"
-DLIBCXX_TEST_PARAMS="enable_modules=clang"
-DLIBCXXABI_TEST_PARAMS="enable_modules=clang"
../llvm
ninja check-cxx
(Within the premerge build).
This patch specifically handles some missing includes for
invocations of std::span in these tests. There are some other cases
where we need dynamic_extent that are handled in #142925.
This seems to be related to 5e19fd1 which
added in some similar includes.