[libc++] Properly decay functions in CTAD for pair

[frederick-vs-ja]

This patch makes instantiation of pair in CTAD a bit lazier to avoid instantiating invalid pair specialization before the decaying explicit deduction guide works.

Fixes #133056.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

This patch makes instantiation of pair in CTAD a bit lazier to avoid instantiating invalid pair specialization before the decaying explicit deduction guide works.

Fixes #133056.

---

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

2 Files Affected:

• (modified) libcxx/include/__utility/pair.h (+59-45)
• (added) libcxx/test/std/utilities/utility/pairs/pairs.pair/explicit_deduction_guides.pass.cpp (+60)

diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 1f596a87f7cc7..c59008c5494b3 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -49,6 +49,33 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#ifndef _LIBCPP_CXX03_LANG
+
+template <class _T1, class _T2>
+struct __check_pair_construction {
+  template <int&...>
+  static _LIBCPP_HIDE_FROM_ABI constexpr bool __enable_implicit_default() {
+    return __is_implicitly_default_constructible<_T1>::value && __is_implicitly_default_constructible<_T2>::value;
+  }
+
+  template <int&...>
+  static _LIBCPP_HIDE_FROM_ABI constexpr bool __enable_default() {
+    return is_default_constructible<_T1>::value && is_default_constructible<_T2>::value;
+  }
+
+  template <class _U1, class _U2>
+  static _LIBCPP_HIDE_FROM_ABI constexpr bool __is_pair_constructible() {
+    return is_constructible<_T1, _U1>::value && is_constructible<_T2, _U2>::value;
+  }
+
+  template <class _U1, class _U2>
+  static _LIBCPP_HIDE_FROM_ABI constexpr bool __is_implicit() {
+    return is_convertible<_U1, _T1>::value && is_convertible<_U2, _T2>::value;
+  }
+};
+
+#endif
+
 template <class, class>
 struct __non_trivially_copyable_base {
   _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI __non_trivially_copyable_base() _NOEXCEPT {}
@@ -104,40 +131,16 @@ struct _LIBCPP_TEMPLATE_VIS pair
     return *this;
   }
 #else
-  struct _CheckArgs {
-    template <int&...>
-    static _LIBCPP_HIDE_FROM_ABI constexpr bool __enable_implicit_default() {
-      return __is_implicitly_default_constructible<_T1>::value && __is_implicitly_default_constructible<_T2>::value;
-    }
-
-    template <int&...>
-    static _LIBCPP_HIDE_FROM_ABI constexpr bool __enable_default() {
-      return is_default_constructible<_T1>::value && is_default_constructible<_T2>::value;
-    }
-
-    template <class _U1, class _U2>
-    static _LIBCPP_HIDE_FROM_ABI constexpr bool __is_pair_constructible() {
-      return is_constructible<first_type, _U1>::value && is_constructible<second_type, _U2>::value;
-    }
-
-    template <class _U1, class _U2>
-    static _LIBCPP_HIDE_FROM_ABI constexpr bool __is_implicit() {
-      return is_convertible<_U1, first_type>::value && is_convertible<_U2, second_type>::value;
-    }
-  };
-
-  template <bool _MaybeEnable>
-  using _CheckArgsDep _LIBCPP_NODEBUG = __conditional_t<_MaybeEnable, _CheckArgs, void>;
-
-  template <bool _Dummy = true, __enable_if_t<_CheckArgsDep<_Dummy>::__enable_default(), int> = 0>
-  explicit(!_CheckArgsDep<_Dummy>::__enable_implicit_default()) _LIBCPP_HIDE_FROM_ABI constexpr pair() noexcept(
+  template <class _CheckArgsDep                                   = __check_pair_construction<_T1, _T2>,
+            __enable_if_t<_CheckArgsDep::__enable_default(), int> = 0>
+  explicit(!_CheckArgsDep::__enable_implicit_default()) _LIBCPP_HIDE_FROM_ABI constexpr pair() noexcept(
       is_nothrow_default_constructible<first_type>::value && is_nothrow_default_constructible<second_type>::value)
       : first(), second() {}
 
-  template <bool _Dummy = true,
-            __enable_if_t<_CheckArgsDep<_Dummy>::template __is_pair_constructible<_T1 const&, _T2 const&>(), int> = 0>
+  template <class _CheckArgsDep = __check_pair_construction<_T1, _T2>,
+            __enable_if_t<_CheckArgsDep::template __is_pair_constructible<_T1 const&, _T2 const&>(), int> = 0>
   _LIBCPP_HIDE_FROM_ABI
-  _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!_CheckArgsDep<_Dummy>::template __is_implicit<_T1 const&, _T2 const&>())
+  _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!_CheckArgsDep::template __is_implicit<_T1 const&, _T2 const&>())
       pair(_T1 const& __t1, _T2 const& __t2) noexcept(is_nothrow_copy_constructible<first_type>::value &&
                                                       is_nothrow_copy_constructible<second_type>::value)
       : first(__t1), second(__t2) {}
@@ -150,41 +153,52 @@ struct _LIBCPP_TEMPLATE_VIS pair
       class _U1,
       class _U2,
 #  endif
-      __enable_if_t<_CheckArgs::template __is_pair_constructible<_U1, _U2>(), int> = 0 >
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!_CheckArgs::template __is_implicit<_U1, _U2>())
+      __enable_if_t<__check_pair_construction<_T1, _T2>::template __is_pair_constructible<_U1, _U2>(), int> = 0 >
+  _LIBCPP_HIDE_FROM_ABI
+  _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!__check_pair_construction<_T1, _T2>::template __is_implicit<_U1, _U2>())
       pair(_U1&& __u1, _U2&& __u2) noexcept(is_nothrow_constructible<first_type, _U1>::value &&
                                             is_nothrow_constructible<second_type, _U2>::value)
       : first(std::forward<_U1>(__u1)), second(std::forward<_U2>(__u2)) {
   }
 
 #  if _LIBCPP_STD_VER >= 23
-  template <class _U1, class _U2, __enable_if_t<_CheckArgs::template __is_pair_constructible<_U1&, _U2&>(), int> = 0>
-  _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_CheckArgs::template __is_implicit<_U1&, _U2&>())
+  template <class _U1,
+            class _U2,
+            __enable_if_t<__check_pair_construction<_T1, _T2>::template __is_pair_constructible<_U1&, _U2&>(), int> = 0>
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit(!__check_pair_construction<_T1, _T2>::template __is_implicit<_U1&, _U2&>())
       pair(pair<_U1, _U2>& __p) noexcept((is_nothrow_constructible<first_type, _U1&>::value &&
                                           is_nothrow_constructible<second_type, _U2&>::value))
       : first(__p.first), second(__p.second) {}
 #  endif
 
-  template <class _U1,
-            class _U2,
-            __enable_if_t<_CheckArgs::template __is_pair_constructible<_U1 const&, _U2 const&>(), int> = 0>
-  _LIBCPP_HIDE_FROM_ABI
-  _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!_CheckArgs::template __is_implicit<_U1 const&, _U2 const&>())
+  template <
+      class _U1,
+      class _U2,
+      __enable_if_t<__check_pair_construction<_T1, _T2>::template __is_pair_constructible<_U1 const&, _U2 const&>(),
+                    int> = 0>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(
+      !__check_pair_construction<_T1, _T2>::template __is_implicit<_U1 const&, _U2 const&>())
       pair(pair<_U1, _U2> const& __p) noexcept(is_nothrow_constructible<first_type, _U1 const&>::value &&
                                                is_nothrow_constructible<second_type, _U2 const&>::value)
       : first(__p.first), second(__p.second) {}
 
-  template <class _U1, class _U2, __enable_if_t<_CheckArgs::template __is_pair_constructible<_U1, _U2>(), int> = 0>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!_CheckArgs::template __is_implicit<_U1, _U2>())
+  template <class _U1,
+            class _U2,
+            __enable_if_t<__check_pair_construction<_T1, _T2>::template __is_pair_constructible<_U1, _U2>(), int> = 0>
+  _LIBCPP_HIDE_FROM_ABI
+  _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(!__check_pair_construction<_T1, _T2>::template __is_implicit<_U1, _U2>())
       pair(pair<_U1, _U2>&& __p) noexcept(is_nothrow_constructible<first_type, _U1&&>::value &&
                                           is_nothrow_constructible<second_type, _U2&&>::value)
       : first(std::forward<_U1>(__p.first)), second(std::forward<_U2>(__p.second)) {}
 
 #  if _LIBCPP_STD_VER >= 23
-  template <class _U1,
-            class _U2,
-            __enable_if_t<_CheckArgs::template __is_pair_constructible<const _U1&&, const _U2&&>(), int> = 0>
-  _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_CheckArgs::template __is_implicit<const _U1&&, const _U2&&>())
+  template <
+      class _U1,
+      class _U2,
+      __enable_if_t<__check_pair_construction<_T1, _T2>::template __is_pair_constructible<const _U1&&, const _U2&&>(),
+                    int> = 0>
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit(
+      !_Chec__check_pair_construction<_T1, _T2> Args::template __is_implicit<const _U1&&, const _U2&&>())
       pair(const pair<_U1, _U2>&& __p) noexcept(is_nothrow_constructible<first_type, const _U1&&>::value &&
                                                 is_nothrow_constructible<second_type, const _U2&&>::value)
       : first(std::move(__p.first)), second(std::move(__p.second)) {}
diff --git a/libcxx/test/std/utilities/utility/pairs/pairs.pair/explicit_deduction_guides.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.pair/explicit_deduction_guides.pass.cpp
new file mode 100644
index 0000000000000..d580e04012706
--- /dev/null
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.pair/explicit_deduction_guides.pass.cpp
@@ -0,0 +1,60 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: std-at-least-c++17
+
+// <utility>
+
+// template<class T1, class T2>
+//   pair(T1, T2) -> pair<T1, T2>;
+
+// Test that the explicit deduction guide for std::pair correctly decays function lvalues and
+// behaves different from std::make_pair.
+
+#include <cassert>
+#include <functional>
+#include <type_traits>
+#include <utility>
+
+#include "test_macros.h"
+
+void dummy() {}
+
+constexpr void test_decay() {
+  char arr[1]{};
+  std::pair pr(arr, dummy);
+
+  ASSERT_SAME_TYPE(decltype(pr), std::pair<char*, void (*)()>);
+
+  assert(pr == std::make_pair(arr, dummy));
+}
+
+TEST_CONSTEXPR_CXX20 void test_unwrap() {
+  int n = 0;
+  std::pair pr(std::ref(n), dummy);
+
+  ASSERT_SAME_TYPE(decltype(pr), std::pair<std::reference_wrapper<int>, void (*)()>);
+  static_assert(!std::is_same_v<decltype(pr), decltype(std::make_pair(std::ref(n), dummy))>);
+
+  assert(&(pr.first.get()) == &n);
+  assert(pr.second == dummy);
+}
+
+constexpr bool test() {
+  test_decay();
+  if (TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED)
+    test_unwrap();
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}

[huixie90]

question: isn't cxx03 using different header?

[frederick-vs-ja]

Currently the combination of C++03 with non-frozen headers is still being tested in CI. I don't know when the support for this will be dropped.

[huixie90]

could you please add a test for the test case in the github issue you've fixed?

[frederick-vs-ja]

I think the first case in this test file is exactly for that GitHub issue. Did you mean adding a URL?

[huixie90]

NM. In the issue it was a lambda converted function pointer and here it is just regular functions.

[huixie90]

I am trying to understand why the original code instantiate pair<T1, T2> at this point. is it because _CheckArgsDep was a member struct and it has to instantiate the enclosing class to be able to use it?

[frederick-vs-ja]

I think that's the reason.