[libcxx] workaround PR 28385 in __find_exactly_one_checked

[CaseyCarter]

Summary

Fixes #35578.

(This is https://github.com/CaseyCarter/libcxx/tree/fix_35578)

[lebedev.ri]

Test?

[CaseyCarter]

In D41048#950424, @lebedev.ri wrote:

Test?

Four of the existing variant tests fail without this change (Using the clang-6 dailies from apt.llvm.org and -std=c++2a):

Failing Tests (4):
    libc++ :: std/utilities/variant/variant.get/get_if_type.pass.cpp
    libc++ :: std/utilities/variant/variant.get/get_type.pass.cpp
    libc++ :: std/utilities/variant/variant.get/holds_alternative.pass.cpp
    libc++ :: std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp

  Expected Passes    : 36
  Unexpected Failures: 4

I think that's plenty of test coverage.

[mclow.lists]

These tests don't fail for me. (using a clang I built two days ago)

[mclow.lists]

include/tuple

1014–1015

We try not to have naked tests like this in the code.

If we have to, maybe:

#if defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER > 500 && _LIBCPP_STD_VER > 14

but I would rather define a new macro in <__config> and give it a meaningful name.

[CaseyCarter]

In D41048#950601, @mclow.lists wrote:

These tests don't fail for me. (using a clang I built two days ago)

What about the repro for #35578? That repro and these four tests trigger the bug for me (no matching function for call to '__find_idx'...could not match "const bool" against "const bool") with clang version 6.0.0-svn320282-1~exp1 (trunk) freshly updated from apt.llvm.org on Ubuntu trusty running atop Windows Subsystem for Linux. (We've been shipping a workaround for 28385 in MSFT <variant> forever, and in range-v3 for even longer.) Both Godbolt and Wandbox agree.

[lebedev.ri]

In D41048#950606, @CaseyCarter wrote:

In D41048#950601, @mclow.lists wrote:

These tests don't fail for me. (using a clang I built two days ago)

What about the repro for #35578? That repro and these four tests trigger the bug for me (no matching function for call to '__find_idx'...could not match "const bool" against "const bool") with clang version 6.0.0-svn320282-1~exp1 (trunk) freshly updated from apt.llvm.org on Ubuntu trusty running atop Windows Subsystem for Linux. (We've been shipping a workaround for 28385 in MSFT <variant> forever, and in range-v3 for even longer.) Both Godbolt and Wandbox agree.

What we've been trying to convey is, none of the current tests with current default params trigger the problem.
The difference being -std=c++1z. So some new tests are needed.

[mclow.lists]

Ah - that was the factor I was missing.
The tests pass for me with -std=c++2a, but fail for std=c++17

Casey's original post said they fail with 2a, and I'm *still* not seeing that.

[CaseyCarter]

In D41048#950635, @mclow.lists wrote:

Ah - that was the factor I was missing.
The tests pass for me with -std=c++2a, but fail for std=c++17

Casey's original post said they fail with 2a, and I'm *still* not seeing that.

They fail for me both with -std=c++2a and -std=c++17. (The wandbox & godbolt links above show the bug with the repro from 35578 and -std=c++2a.)

[CaseyCarter]

Hide the ugly version test in <__config>, define a slightly-more-meaningful macro _LIBCPP_WORKAROUND_CLANG_28385.

include/tuple

1014–1015

Better?

[CaseyCarter]

Make the workaround unconditional.

[CaseyCarter]

This unconditional workaround addresses Marshall's concerns about the naked version test.

[lichray]

Reproduced with

lit -sv --param=cxx_under_test="$HOME/bin/clang++" test/std/utilities/tuple/
lit: [...] note: Using available_features: ['libc++', 'verify-support', 'clang-6', 'modules-support', 'locale.en_US.UTF-8', 'diagnose-if-support', 'long_tests', 'fdelayed-template-parsing', '-faligned-allocation', 'locale.zh_CN.UTF-8', 'c++2a', 'locale.fr_CA.ISO8859-1', 'c++filesystem', 'c++experimental', 'clang', 'locale.fr_FR.UTF-8', 'locale.ru_RU.UTF-8', 'fsized-deallocation', 'freebsd11', 'fcoroutines-ts', 'locale.cs_CZ.ISO8859-2', 'clang-6.0', 'thread-safety']

[CaseyCarter]

Better/simpler workaround from Zhihao.

[CaseyCarter]

This new-and-improved workaround - thanks to @lichray - is unintrusive enough that I can't imagine it being unnacceptable to anyone. I'm going to go ahead and check this in.

include/tuple

 template <size_t _Nx>
 inline _LIBCPP_INLINE_VISIBILITY
 constexpr size_t __find_idx(size_t __i, const bool (&__matches)[_Nx]) {
   return __i == _Nx ? __not_found :
       __find_idx_return(__i, __find_idx(__i + 1, __matches), __matches[__i]);
 }
 
 template <class _T1, class ..._Args>
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
-    static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-    static_assert (value != __not_found, "type not found in type list" );
-    static_assert(value != __ambiguous,"type occurs more than once in type list");
+    inline _LIBCPP_INLINE_VISIBILITY
+    static constexpr size_t __index()
+    {
+        constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+        return __find_detail::__find_idx(0, __matches);
+    }
+    static constexpr size_t value = __index();
+    static_assert(value != __not_found, "type not found in type list" );
+    static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template <class _T1>
 struct __find_exactly_one_checked<_T1> {
     static_assert(!is_same<_T1, _T1>::value, "type not in empty type list");
 };
 
 } // namespace __find_detail;