Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<type_traits>: common_reference workaround still necessary? #2650

Closed
StephanTLavavej opened this issue Apr 13, 2022 · 2 comments
Closed

<type_traits>: common_reference workaround still necessary? #2650

StephanTLavavej opened this issue Apr 13, 2022 · 2 comments
Assignees
Labels
test Related to test code

Comments

@StephanTLavavej
Copy link
Member

VS 2022 17.2 Preview 3 is now available. It contains internal MSVC-PR-382810 that fixed DevCom-876860 and its internal mirror VSO-1049320. We have workarounds for both bug numbers:

STL/stl/inc/type_traits

Lines 1248 to 1274 in 52505b9

#if !defined(__EDG__) && !defined(__clang__) // TRANSITION, DevCom-876860
template <class _Ty1, class _Ty2>
using _Cond_res_if_right = // N4810 [meta.trans.other]/2.4
decltype(false ? _Returns_exactly<_Ty1>() : _Returns_exactly<_Ty2>());
template <class _Ty>
using _Is_scalar_or_array = disjunction<is_scalar<_Ty>, is_array<_Ty>>;
template <class _Ty1, class _Ty2, class = void>
struct _Cond_res_workaround {};
template <class _Ty1, class _Ty2>
struct _Cond_res_workaround<_Ty1, _Ty2, void_t<_Cond_res_if_right<_Ty1, _Ty2>>> {
using _Uty = remove_cvref_t<_Ty1>;
using type = conditional_t<conjunction_v<is_same<_Uty, remove_cvref_t<_Ty2>>, _Is_scalar_or_array<_Uty>,
disjunction<conjunction<is_lvalue_reference<_Ty1>, is_rvalue_reference<_Ty2>>,
conjunction<is_rvalue_reference<_Ty1>, is_lvalue_reference<_Ty2>>>>,
decay_t<_Copy_cv<remove_reference_t<_Ty1>, remove_reference_t<_Ty2>>>, _Cond_res_if_right<_Ty1, _Ty2>>;
};
template <class _Ty1, class _Ty2>
using _Cond_res = typename _Cond_res_workaround<_Ty1, _Ty2>::type;
#else // ^^^ workaround / no workaround vvv
template <class _Ty1, class _Ty2>
using _Cond_res = // N4810 [meta.trans.other]/2.4
decltype(false ? _Returns_exactly<_Ty1>() : _Returns_exactly<_Ty2>());
#endif // ^^^ no workaround ^^^

#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-1049320
ranges::destroy_at(reinterpret_cast<U(*)[N]>(ptr));
#else // ^^^ no workaround / workaround vvv
ranges::destroy_at(reinterpret_cast<T(*)[N]>(const_cast<T*>(ptr)));
#endif // TRANSITION, VSO-1049320

The P0784R7_library_support_for_more_constexpr_containers/test.cpp workaround can apparently be removed without issue. However, attempting to remove the <type_traits> workaround (which was added by GH-2592) causes a libcxx test to fail, llvm-project\libcxx\test\std\concepts\concepts.lang\concept.commonref\common_reference.compile.pass.cpp.

common_reference.compile.pass.cpp(38): error C2338: static_assert failed: 'std::common_reference_with<volatile T&, volatile U&&>'
common_reference.compile.pass.cpp(61): note: see reference to function template instantiation 'bool CheckCommonReferenceWith<int[5],int[5]>(void) noexcept' being compiled
common_reference.compile.pass.cpp(44): error C2607: static assertion failed

I don't know if this is:

  • Remaining compiler bugginess that needs to be reported
  • A library bug somewhere
  • A libcxx test bug
@StephanTLavavej StephanTLavavej added help wanted Extra attention is needed test Related to test code labels Apr 13, 2022
@StephanTLavavej
Copy link
Member Author

StephanTLavavej commented Apr 13, 2022

I suspect this may be remaining compiler bugginess - I experienced a bunch of compiler and test crashes with this workaround removal plus other changes, and I seem to be able to restore all my other changes without incident (I haven't tried running a full test pass with just the workaround removal yet).

Update: I found the root causes of the compiler crash (crash-after-error in an XFAILed ranges test) and test crashes (our assertion dialog avoidance machinery has bit-rotted), so the issue here is purely common_reference.compile.pass.cpp. 🎉

@StephanTLavavej StephanTLavavej removed the help wanted Extra attention is needed label Apr 13, 2022
@CaseyCarter
Copy link
Member

This is a duplicate of #2860.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Related to test code
Projects
None yet
Development

No branches or pull requests

2 participants