[libc++] Workaround clang bug in __has_unique_object_representations

[ldionne]

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.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/95314.diff

2 Files Affected:

• (modified) libcxx/include/__type_traits/has_unique_object_representation.h (+7-1)
• (modified) libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/has_unique_object_representations.pass.cpp (+72-72)

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;
 }

[ldionne]

I think CI is passing, the failures are either preemptions or the bootstrapping build which is known to fail right now.