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

<utility>: pair::swap(const pair&) interacts badly with __declspec(dllexport) #3013

Closed
StephanTLavavej opened this issue Aug 9, 2022 · 3 comments · Fixed by #3045
Closed
Labels
bug Something isn't working fixed Something works now, yay! high priority Important!

Comments

@StephanTLavavej
Copy link
Member

This is a regression caused by merging #2687 in VS 2022 17.3 Preview 2:

C:\Temp>type meow.cpp
#include <utility>
using namespace std;

struct __declspec(dllexport) MyPair : pair<int, int> {};
C:\Temp>cl /EHsc /nologo /W4 /MD /c /std:c++20 meow.cpp
meow.cpp

C:\Temp>cl /EHsc /nologo /W4 /MD /c /std:c++latest meow.cpp
meow.cpp
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\utility(107): error C2672: 'swap': no matching overloaded function found
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\type_traits(1965): note: could be 'void std::swap(_Ty (&)[_Size],_Ty (&)[_Size]) noexcept(<expr>)'
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\utility(107): note: 'void std::swap(_Ty (&)[_Size],_Ty (&)[_Size]) noexcept(<expr>)': could not deduce template argument for '_Ty (&)[_Size]' from '_Ty'
        with
        [
            _Ty=int
        ]
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\type_traits(1965): note: see declaration of 'std::swap'
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\type_traits(1962): note: or       'void std::swap(_Ty &,_Ty &) noexcept(<expr>)'
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\utility(106): note: 'void std::swap(_Ty &,_Ty &) noexcept(<expr>)': could not deduce template argument for '_Enabled'
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\type_traits(1962): note: see declaration of 'std::swap'
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\utility(408): note: see reference to function template instantiation 'void std::_Swap_adl<const _Ty1>(_Ty &,_Ty &) noexcept(false)' being compiled
        with
        [
            _Ty1=int,
            _Ty=int
        ]
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31629\include\utility(406): note: while compiling class template member function 'void std::pair<int,int>::swap(const std::pair<int,int> &) noexcept(false) const'
meow.cpp(4): note: see reference to class template instantiation 'std::pair<int,int>' being compiled

Relevant code:

STL/stl/inc/utility

Lines 331 to 339 in 2263d93

#if _HAS_CXX23
constexpr void swap(const pair& _Right) const
noexcept(is_nothrow_swappable_v<const _Ty1>&& is_nothrow_swappable_v<const _Ty2>) {
if (this != _STD addressof(_Right)) {
_Swap_adl(first, _Right.first);
_Swap_adl(second, _Right.second);
}
}
#endif // _HAS_CXX23

WG21-N4910 [pairs.pair]/45.2 says:

Mandates: For the second overload, is_swappable_v<const T1> is true and is_swappable_v<const T2> is true.

So we aren't supposed to constrain this, but that interacts badly with dllexport which tries to instantiate everything.

I suspect that we need the template <int = 0> workaround, also applied to tuple.

Originally reported as DevCom-10109884 and internal VSO-1586226 / AB#1586226 .

@StephanTLavavej StephanTLavavej added the bug Something isn't working label Aug 9, 2022
@CaseyCarter
Copy link
Contributor

CaseyCarter commented Aug 9, 2022

Mandates: For the second overload, is_swappable_v<const T1> is true and is_swappable_v<const T2> is true.

So we aren't supposed to constrain this, but that interacts badly with dllexport which tries to instantiate everything.

It's permissible but not required to implement a "Mandates" element via constraints as long as an expression that would call this overload won't inadvertently result in a call of some other overload. (So yes we can do this and it's conforming.) The Standard only cares that calls produce a diagnostic when the requirements are not met, it doesn't care if the diagnostic is pretty.

@StephanTLavavej StephanTLavavej added the high priority Important! label Aug 19, 2022
@StephanTLavavej
Copy link
Member Author

I'll fix this with template <int = 0> because adding a requires is more verbose and ran into other issues.

@jwakely
Copy link

jwakely commented Aug 25, 2022

For libstdc++ we've added requires is_swappable_v<T...> because our tests that explicitly instantiate pair and tuple were failing.

Since you need something similar, maybe it should be an LWG issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay! high priority Important!
Projects
None yet
3 participants