[libcxx-commits] [libcxx] [libc++] Workaround clang bug in __has_unique_object_representations (PR #95314)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jun 17 08:19:06 PDT 2024
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/95314
>From c493de96a88f8b2346f4480a4d575af6248ff1b8 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 12 Jun 2024 17:10:06 -0400
Subject: [PATCH 1/3] [libc++] Workaround clang bug in
__has_unique_object_representations
Clang currently has a bug in the __has_unique_object_representations
builtin where it doesn't provide consistent answers based on the order
of instantiation of templates. This was reported as #95311.
This patch adds a workaround in libc++ to avoid breaking users until
Clang has been fixed. It also revamps the tests a bit.
---
.../has_unique_object_representation.h | 8 +-
...has_unique_object_representations.pass.cpp | 144 +++++++++---------
2 files changed, 79 insertions(+), 73 deletions(-)
diff --git a/libcxx/include/__type_traits/has_unique_object_representation.h b/libcxx/include/__type_traits/has_unique_object_representation.h
index 1aa044990032a..3b0dc625aa8ff 100644
--- a/libcxx/include/__type_traits/has_unique_object_representation.h
+++ b/libcxx/include/__type_traits/has_unique_object_representation.h
@@ -11,6 +11,7 @@
#include <__config>
#include <__type_traits/integral_constant.h>
+#include <__type_traits/remove_all_extents.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -22,7 +23,12 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp>
struct _LIBCPP_TEMPLATE_VIS has_unique_object_representations
- : public integral_constant<bool, __has_unique_object_representations(_Tp)> {};
+ // TODO: We work around a Clang bug in __has_unique_object_representations by instantiating the
+ // builtin on the non-array type first and discarding that. This is issue #95311.
+ // This workaround can be removed once the bug has been fixed in all supported Clangs.
+ : public integral_constant<bool,
+ ((void)__has_unique_object_representations(remove_all_extents_t<_Tp>),
+ __has_unique_object_representations(_Tp))> {};
template <class _Tp>
inline constexpr bool has_unique_object_representations_v = __has_unique_object_representations(_Tp);
diff --git a/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp b/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp
index b8b84bb908827..6e6791b1a747e 100644
--- a/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp
+++ b/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp
@@ -14,95 +14,95 @@
#include <type_traits>
-#include "test_macros.h"
-
-template <class T>
-void test_has_unique_object_representations()
-{
- static_assert( std::has_unique_object_representations<T>::value, "");
- static_assert( std::has_unique_object_representations<const T>::value, "");
- static_assert( std::has_unique_object_representations<volatile T>::value, "");
- static_assert( std::has_unique_object_representations<const volatile T>::value, "");
-
- static_assert( std::has_unique_object_representations_v<T>, "");
- static_assert( std::has_unique_object_representations_v<const T>, "");
- static_assert( std::has_unique_object_representations_v<volatile T>, "");
- static_assert( std::has_unique_object_representations_v<const volatile T>, "");
+template <bool ExpectedValue, class T>
+void test() {
+ static_assert(std::has_unique_object_representations<T>::value == ExpectedValue);
+ static_assert(std::has_unique_object_representations<const T>::value == ExpectedValue);
+ static_assert(std::has_unique_object_representations<volatile T>::value == ExpectedValue);
+ static_assert(std::has_unique_object_representations<const volatile T>::value == ExpectedValue);
+
+ static_assert(std::has_unique_object_representations_v<T> == ExpectedValue);
+ static_assert(std::has_unique_object_representations_v<const T> == ExpectedValue);
+ static_assert(std::has_unique_object_representations_v<volatile T> == ExpectedValue);
+ static_assert(std::has_unique_object_representations_v<const volatile T> == ExpectedValue);
}
-template <class T>
-void test_has_not_has_unique_object_representations()
-{
- static_assert(!std::has_unique_object_representations<T>::value, "");
- static_assert(!std::has_unique_object_representations<const T>::value, "");
- static_assert(!std::has_unique_object_representations<volatile T>::value, "");
- static_assert(!std::has_unique_object_representations<const volatile T>::value, "");
-
- static_assert(!std::has_unique_object_representations_v<T>, "");
- static_assert(!std::has_unique_object_representations_v<const T>, "");
- static_assert(!std::has_unique_object_representations_v<volatile T>, "");
- static_assert(!std::has_unique_object_representations_v<const volatile T>, "");
-}
-
-class Empty
-{
-};
-
-class NotEmpty
-{
- virtual ~NotEmpty();
-};
+class Empty {};
union EmptyUnion {};
-struct NonEmptyUnion {int x; unsigned y;};
-struct bit_zero
-{
- int : 0;
+struct NonEmptyUnion {
+ int x;
+ unsigned y;
};
-class Abstract
-{
- virtual ~Abstract() = 0;
+struct ZeroWidthBitfield {
+ int : 0;
};
-struct A
-{
- ~A();
- unsigned foo;
+class Virtual {
+ virtual ~Virtual();
};
-struct B
-{
- char bar;
- int foo;
+class Abstract {
+ virtual ~Abstract() = 0;
};
+struct UnsignedInt {
+ unsigned foo;
+};
-int main(int, char**)
-{
- test_has_not_has_unique_object_representations<void>();
- test_has_not_has_unique_object_representations<Empty>();
- test_has_not_has_unique_object_representations<EmptyUnion>();
- test_has_not_has_unique_object_representations<NotEmpty>();
- test_has_not_has_unique_object_representations<bit_zero>();
- test_has_not_has_unique_object_representations<Abstract>();
- test_has_not_has_unique_object_representations<B>();
-
-// I would expect all three of these to have unique representations.
-// I would also expect that there are systems where they do not.
-// test_has_not_has_unique_object_representations<int&>();
-// test_has_not_has_unique_object_representations<int *>();
-// test_has_not_has_unique_object_representations<double>();
+struct WithoutPadding {
+ int x;
+ int y;
+};
+struct WithPadding {
+ char bar;
+ int foo;
+};
- test_has_unique_object_representations<unsigned>();
- test_has_unique_object_representations<NonEmptyUnion>();
- test_has_unique_object_representations<char[3]>();
- test_has_unique_object_representations<char[3][4]>();
- test_has_unique_object_representations<char[3][4][5]>();
- test_has_unique_object_representations<char[]>();
+template <int>
+class NTTP_ClassType_WithoutPadding {
+ int x;
+};
+int main(int, char**) {
+ test<false, void>();
+ test<false, Empty>();
+ test<false, EmptyUnion>();
+ test<false, Virtual>();
+ test<false, ZeroWidthBitfield>();
+ test<false, Abstract>();
+ test<false, WithPadding>();
+ test<false, WithPadding[]>();
+ test<false, WithPadding[][3]>();
+
+ // I would also expect that there are systems where they do not.
+ // I would expect all three of these to have unique representations.
+ // test<false, int&>();
+ // test<false, int *>();
+ // test<false, double>();
+
+ test<true, unsigned>();
+ test<true, UnsignedInt>();
+ test<true, WithoutPadding>();
+ test<true, NonEmptyUnion>();
+ test<true, char[3]>();
+ test<true, char[3][4]>();
+ test<true, char[3][4][5]>();
+ test<true, char[]>();
+ test<true, char[][2]>();
+ test<true, char[][2][3]>();
+
+ // Important test case for https://github.com/llvm/llvm-project/issues/95311.
+ // Note that the order is important here, we want to instantiate the array
+ // variants before the non-array ones, otherwise we don't trigger the bug.
+ {
+ test<true, NTTP_ClassType_WithoutPadding<0>[]>();
+ test<true, NTTP_ClassType_WithoutPadding<0>[][3]>();
+ test<true, NTTP_ClassType_WithoutPadding<0>>();
+ }
return 0;
}
>From ea5e61e996180fb414f08bfd8da287f52f1f49d2 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 17 Jun 2024 11:10:08 -0400
Subject: [PATCH 2/3] Make .compile.pass.cpp
---
...cpp => has_unique_object_representations.compile.pass.cpp} | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
rename libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/{has_unique_object_representations.pass.cpp => has_unique_object_representations.compile.pass.cpp} (98%)
diff --git a/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp b/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.compile.pass.cpp
similarity index 98%
rename from libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp
rename to libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.compile.pass.cpp
index 6e6791b1a747e..bd7da40daf2bc 100644
--- a/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp
+++ b/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.compile.pass.cpp
@@ -67,7 +67,7 @@ class NTTP_ClassType_WithoutPadding {
int x;
};
-int main(int, char**) {
+void test() {
test<false, void>();
test<false, Empty>();
test<false, EmptyUnion>();
@@ -103,6 +103,4 @@ int main(int, char**) {
test<true, NTTP_ClassType_WithoutPadding<0>[][3]>();
test<true, NTTP_ClassType_WithoutPadding<0>>();
}
-
- return 0;
}
>From e51099a2fc31d7f9c67355522a453bd84a9d9c5c Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 17 Jun 2024 11:18:41 -0400
Subject: [PATCH 3/3] Apply review comment
---
.../__type_traits/has_unique_object_representation.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libcxx/include/__type_traits/has_unique_object_representation.h b/libcxx/include/__type_traits/has_unique_object_representation.h
index 3b0dc625aa8ff..98c440c16bf26 100644
--- a/libcxx/include/__type_traits/has_unique_object_representation.h
+++ b/libcxx/include/__type_traits/has_unique_object_representation.h
@@ -23,12 +23,12 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp>
struct _LIBCPP_TEMPLATE_VIS has_unique_object_representations
- // TODO: We work around a Clang bug in __has_unique_object_representations by instantiating the
- // builtin on the non-array type first and discarding that. This is issue #95311.
- // This workaround can be removed once the bug has been fixed in all supported Clangs.
- : public integral_constant<bool,
- ((void)__has_unique_object_representations(remove_all_extents_t<_Tp>),
- __has_unique_object_representations(_Tp))> {};
+ // TODO: We work around a Clang and GCC bug in __has_unique_object_representations by using remove_all_extents
+ // even though it should not be necessary. This was reported to the compilers:
+ // - Clang: https://github.com/llvm/llvm-project/issues/95311
+ // - GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115476
+ // remove_all_extents_t can be removed once all the compilers we support have fixed this bug.
+ : public integral_constant<bool, __has_unique_object_representations(remove_all_extents_t<_Tp>)> {};
template <class _Tp>
inline constexpr bool has_unique_object_representations_v = __has_unique_object_representations(_Tp);
More information about the libcxx-commits
mailing list