[libc++][span] LWG4243: as_bytes/as_writable_bytes is broken with span<volatile T>#200993
Merged
Conversation
llvmorg-github-actions
Bot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: eiytoq (eiytoq) ChangesCloses #171317 Full diff: https://github.com/llvm/llvm-project/pull/200993.diff 5 Files Affected:
diff --git a/libcxx/include/span b/libcxx/include/span
index 230ae3fa2b198..1a484478e19da 100644
--- a/libcxx/include/span
+++ b/libcxx/include/span
@@ -172,6 +172,7 @@ template<class R>
# include <__type_traits/is_convertible.h>
# include <__type_traits/is_integral.h>
# include <__type_traits/is_same.h>
+# include <__type_traits/is_volatile.h>
# include <__type_traits/remove_const.h>
# include <__type_traits/remove_cv.h>
# include <__type_traits/remove_cvref.h>
@@ -585,12 +586,13 @@ inline constexpr bool ranges::enable_view<span<_ElementType, _Extent>> = true;
// as_bytes & as_writable_bytes
template <class _Tp, size_t _Extent>
+ requires(!is_volatile_v<_Tp>)
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI auto as_bytes(span<_Tp, _Extent> __s) noexcept {
return __s.__as_bytes();
}
template <class _Tp, size_t _Extent>
- requires(!is_const_v<_Tp>)
+ requires(!is_const_v<_Tp> && !is_volatile_v<_Tp>)
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI auto as_writable_bytes(span<_Tp, _Extent> __s) noexcept {
return __s.__as_writable_bytes();
}
diff --git a/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.pass.cpp b/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.pass.cpp
index 1b62fb94e9ab2..5c5892bc2567c 100644
--- a/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.pass.cpp
+++ b/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.pass.cpp
@@ -9,12 +9,12 @@
// <span>
-// template <class ElementType, size_t Extent>
-// span<const byte,
-// Extent == dynamic_extent
-// ? dynamic_extent
-// : sizeof(ElementType) * Extent>
+// template<class ElementType, size_t Extent>
+// span<const byte, Extent == dynamic_extent ? dynamic_extent : sizeof(ElementType) * Extent>
// as_bytes(span<ElementType, Extent> s) noexcept;
+//
+// Constraints:
+// is_volatile_v<ElementType> is false.
#include <cassert>
#include <cstddef>
diff --git a/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.verify.cpp b/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.verify.cpp
new file mode 100644
index 0000000000000..cdc830884f23c
--- /dev/null
+++ b/libcxx/test/std/containers/views/views.span/span.objectrep/as_bytes.verify.cpp
@@ -0,0 +1,70 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <span>
+
+// template<class ElementType, size_t Extent>
+// span<const byte, Extent == dynamic_extent ? dynamic_extent : sizeof(ElementType) * Extent>
+// as_bytes(span<ElementType, Extent> s) noexcept;
+//
+// Constraints:
+// is_volatile_v<ElementType> is false.
+
+#include <span>
+#include <string>
+
+#include "test_macros.h"
+
+struct A {};
+
+void f() {
+ std::as_bytes(std::span<volatile int>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile long>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile double>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile A>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile std::string>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+
+ std::as_bytes(std::span<const volatile int>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile long>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile double>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile A>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile std::string>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+
+ std::as_bytes(std::span<volatile int, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile long, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile double, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile A, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<volatile std::string, (std::size_t)0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+
+ std::as_bytes(std::span<const volatile int, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile long, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile double, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile A, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+ std::as_bytes(std::span<const volatile std::string, (std::size_t)0>());
+ // expected-error@-1 {{no matching function for call to 'as_bytes'}}
+}
diff --git a/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.pass.cpp b/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.pass.cpp
index 40c6a581e9acf..be58e1f39f97b 100644
--- a/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.pass.cpp
+++ b/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.pass.cpp
@@ -9,12 +9,12 @@
// <span>
-// template <class ElementType, size_t Extent>
-// span<byte,
-// Extent == dynamic_extent
-// ? dynamic_extent
-// : sizeof(ElementType) * Extent>
+// template<class ElementType, size_t Extent>
+// span<byte, Extent == dynamic_extent ? dynamic_extent : sizeof(ElementType) * Extent>
// as_writable_bytes(span<ElementType, Extent> s) noexcept;
+//
+// Constraints:
+// is_const_v<ElementType> is false and is_volatile_v<ElementType> is false.
#include <cassert>
#include <cstddef>
diff --git a/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.verify.cpp b/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.verify.cpp
index 9d22641ab2687..12a2229e130a8 100644
--- a/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.verify.cpp
+++ b/libcxx/test/std/containers/views/views.span/span.objectrep/as_writable_bytes.verify.cpp
@@ -9,12 +9,12 @@
// <span>
-// template <class ElementType, size_t Extent>
-// span<byte,
-// Extent == dynamic_extent
-// ? dynamic_extent
-// : sizeof(ElementType) * Extent>
+// template<class ElementType, size_t Extent>
+// span<byte, Extent == dynamic_extent ? dynamic_extent : sizeof(ElementType) * Extent>
// as_writable_bytes(span<ElementType, Extent> s) noexcept;
+//
+// Constraints:
+// is_const_v<ElementType> is false and is_volatile_v<ElementType> is false.
#include <span>
#include <string>
@@ -37,6 +37,28 @@ void f() {
std::as_writable_bytes(std::span<const std::string>());
// expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile int>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile long>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile double>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile A>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile std::string>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+
+ std::as_writable_bytes(std::span<const volatile int>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile long>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile double>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile A>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile std::string>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+
std::as_writable_bytes(std::span<const int, 0>());
// expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
std::as_writable_bytes(std::span<const long, 0>());
@@ -48,8 +70,35 @@ void f() {
std::as_writable_bytes(std::span<const std::string, (std::size_t)0>());
// expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile int, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile long, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile double, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile A, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<volatile std::string, (std::size_t)0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+
+ std::as_writable_bytes(std::span<const volatile int, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile long, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile double, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile A, 0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile std::string, (std::size_t)0>());
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+
std::as_writable_bytes(std::span<const int>(iArr2, 1));
// expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
std::as_writable_bytes(std::span<const int, 1>(iArr2 + 5, 1));
// expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+
+ std::as_writable_bytes(std::span<const volatile int>(iArr2, 1));
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
+ std::as_writable_bytes(std::span<const volatile int, 1>(iArr2 + 5, 1));
+ // expected-error@-1 {{no matching function for call to 'as_writable_bytes'}}
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| static_assert(hasAsBytes<const double, 0>()); | ||
| static_assert(hasAsBytes<const A, 0>()); | ||
| static_assert(hasAsBytes<const std::string, 0>()); | ||
|
|
Contributor
There was a problem hiding this comment.
Oh, I'm sorry for ambiguity. I meant "positive" by "successfully compiling". It's better to have test cases for both true and false results, related to cv-qualifiers, from hasAsBytes (same for hasAsWritableBytes).
Contributor
Author
There was a problem hiding this comment.
Can we test the false cases in the pass test and remove the verify test? Since this is a constraints test.
Contributor
There was a problem hiding this comment.
Can we test the 'false' cases in the pass test, and remove the verify test?
Yeah. I think this is conventional.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Closes #171317