Update is_string_constructible for C++23 P2166R1

[StephanTLavavej]

I work on MSVC's C++ Standard Library implementation, and we regularly build sol2 (along with many other open-source projects) to identify compiler/library bugs that would break your project, so we can fix them before shipping. This also allows us to provide advance notice of source-breaking changes - which is the case here.

The paper P2166R1 "Prohibiting basic_string And basic_string_view Construction From nullptr" has been accepted for the upcoming C++23 Standard, and we recently implemented it by merging microsoft/STL#1995 from our contributor @sam20908. Our open-source test pass then discovered the following code in sol2 that leads to a compiler error in C++23:

sol2/include/sol/traits.hpp

Lines 663 to 666 in bb5f60e

	template <typename T, typename CharT = char>
	using is_string_constructible = meta::boolean<
	is_string_literal_array_of_v<T,
	CharT> || std::is_same_v<T, const CharT*> || std::is_same_v<T, CharT> || is_string_of_v<T, CharT> || std::is_same_v<T, std::initializer_list<CharT>> || is_string_view_of_v<T, CharT>>;

According to my understanding, this type trait lists the various things that std::basic_string is constructible from, which allows other conversions in your library to be constrained in order to avoid ambiguity. This has been broken by C++23's addition of a std::basic_string constructor from std::nullptr_t (which is deleted, but that doesn't affect overload resolution).

This PR contains my proposed fix: use std::is_null_pointer_v to detect (cv-qualified) std::nullptr_t. I believe that this can be added unconditionally (i.e. not guarded by Standard mode). First, this type trait was present in C++17 along with std::is_same_v being used here. Second, before C++23, attempting to construct std::basic_string<CharT> from std::nullptr_t would consider the const CharT* constructor (and then lead to UB at runtime), while in C++23 it selects the new constructor. Because a constructor would be considered either way, it seems correct for both types to activate your is_string_constructible.

There may be other ways to fix this C++23 source-breaking change; there may also be additional occurrences of this throughout the sol2 codebase that we haven't encountered yet. Thanks to @cdacamar, MSVC compiler front-end dev, for analyzing and reducing this issue (which I originally mistakenly reported as an MSVC compiler bug).

Here are my instructions for how to reproduce the bug and verify the proposed fix, using our development build of the STL (as the C++23 change to std::basic_string is available on GitHub but will take some time to ship in a Visual Studio Preview):

Click to expand lengthy repro instructions:

:: Download and extract https://www.lua.org/ftp/lua-5.4.3.tar.gz .
:: For example, my working directory is C:\Temp\REPRO
:: so I extract it to C:\Temp\REPRO\lua-5.4.3 with:

7z x lua-5.4.3.tar.gz
7z x lua-5.4.3.tar

:: Use an x64 native command prompt, VS 2019 16.11 Preview 2:

"C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Auxiliary\Build\vcvars64.bat"

:: Build the STL:

git clone https://github.com/microsoft/STL
pushd STL
git submodule update --init --progress vcpkg
.\vcpkg\bootstrap-vcpkg.bat
.\vcpkg\vcpkg.exe install boost-math:x64-windows
cmake -G Ninja -S . -B out\build\x64
ninja -C out\build\x64
set INCLUDE=%CD%\out\build\x64\out\inc;%INCLUDE%
set LIB=%CD%\out\build\x64\out\lib\amd64;%LIB%
set PATH=%CD%\out\build\x64\out\bin\amd64;%PATH%
popd

:: Clone my fork of sol2:

git clone https://github.com/StephanTLavavej/sol2.git

:: Build the repro. It works in C++17 mode, but fails in C++23 mode with basic_string's new overloads:
:: Note: /std:c++latest /D_HAS_CXX23=1 is necessary for Clang until https://reviews.llvm.org/D103155 ships in Clang 13.

cl /std:c++17                          /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
cl /std:c++latest                      /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
clang-cl /std:c++17                    /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
clang-cl /std:c++latest /D_HAS_CXX23=1 /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp

:: Check out my proposed fix:

pushd sol2
git checkout cxx23
popd

:: With the proposed fix, verify that the repro successfully compiles in both C++17 and C++23 modes:

cl /std:c++17                          /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
cl /std:c++latest                      /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
clang-cl /std:c++17                    /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
clang-cl /std:c++latest /D_HAS_CXX23=1 /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp

Here is the full compiler output (from VS 2019 16.11 Preview 2 and Clang 11) from the "build" and "verify" steps at the end:

Click to expand lengthy compiler output:

C:\Temp\REPRO>cl /std:c++17                          /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
multi_results.cpp

C:\Temp\REPRO>cl /std:c++latest                      /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
multi_results.cpp
sol2\include\sol/tie.hpp(64): error C2593: 'operator =' is ambiguous
C:\Temp\REPRO\STL\out\build\x64\out\inc\xstring(3142): note: could be 'std::basic_string<char,std::char_traits<char>,std::allocator<char>> &std::basic_string<char,std::char_traits<char>,std::allocator<char>>::operator =(std::nullptr_t)'
C:\Temp\REPRO\STL\out\build\x64\out\inc\xstring(2894): note: or       'std::basic_string<char,std::char_traits<char>,std::allocator<char>> &std::basic_string<char,std::char_traits<char>,std::allocator<char>>::operator =(std::basic_string<char,std::char_traits<char>,std::allocator<char>> &&) noexcept'
sol2\include\sol/tie.hpp(62): note: while trying to match the argument list '(std::basic_string<char,std::char_traits<char>,std::allocator<char>>, sol::stack_proxy)'
sol2\include\sol/tie.hpp(53): note: see reference to function template instantiation 'void sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::set_extra<0,1,_Ty>(std::true_type,std::integer_sequence<size_t,0,1>,T &&)' being compiled
        with
        [
            _Ty=sol::protected_function_result,
            T=sol::protected_function_result
        ]
sol2\include\sol/tie.hpp(58): note: see reference to function template instantiation 'void sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::set_extra<0,1,_Ty>(std::true_type,std::integer_sequence<size_t,0,1>,T &&)' being compiled
        with
        [
            _Ty=sol::protected_function_result,
            T=sol::protected_function_result
        ]
sol2\include\sol/tie.hpp(77): note: see reference to function template instantiation 'void sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::set<_Ty>(std::true_type,T &&)' being compiled
        with
        [
            _Ty=sol::protected_function_result,
            T=sol::protected_function_result
        ]
sol2\include\sol/tie.hpp(79): note: see reference to function template instantiation 'void sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::set<_Ty>(std::true_type,T &&)' being compiled
        with
        [
            _Ty=sol::protected_function_result,
            T=sol::protected_function_result
        ]
sol2\examples\source\multi_results.cpp(28): note: see reference to function template instantiation 'sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>> &sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::operator =<sol::protected_function_result>(T &&)' being compiled
        with
        [
            T=sol::protected_function_result
        ]
sol2\examples\source\multi_results.cpp(28): note: see reference to function template instantiation 'sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>> &sol::tie_t<int,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::operator =<sol::protected_function_result>(T &&)' being compiled
        with
        [
            T=sol::protected_function_result
        ]

C:\Temp\REPRO>clang-cl /std:c++17                    /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp

C:\Temp\REPRO>clang-cl /std:c++latest /D_HAS_CXX23=1 /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
In file included from sol2\examples\source\multi_results.cpp:2:
In file included from sol2\include\sol/sol.hpp:52:
In file included from sol2\include\sol/stack.hpp:28:
In file included from sol2\include\sol/stack_core.hpp:34:
sol2\include\sol/tie.hpp(64,68): error: use of overloaded operator '=' is ambiguous (with operand types
      'std::basic_string<char, std::char_traits<char>, std::allocator<char>>' and 'sol::stack_proxy')
  ...(void)detail::swallow { 0, (get<I>(static_cast<base_t&>(*this)) = get<I>(types<Tn...>(), target), 0)..., 0 };
                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sol2\include\sol/tie.hpp(58,4): note: in instantiation of function template specialization 'sol::tie_t<int,
      std::basic_string<char, std::char_traits<char>, std::allocator<char>>>::set_extra<0, 1,
      sol::protected_function_result>' requested here
                        set_extra(detail::is_speshul<meta::unqualified_t<T>>(), indices(), std::forward<T>(target));
                        ^
sol2\include\sol/tie.hpp(79,4): note: in instantiation of function template specialization 'sol::tie_t<int,
      std::basic_string<char, std::char_traits<char>, std::allocator<char>>>::set<sol::protected_function_result>'
      requested here
                        set(tieable(), std::forward<T>(value));
                        ^
sol2\examples\source\multi_results.cpp(28,26): note: in instantiation of function template specialization
      'sol::tie_t<int, std::basic_string<char, std::char_traits<char>,
      std::allocator<char>>>::operator=<sol::protected_function_result>' requested here
        sol::tie(first, second) = multi();
                                ^
C:\Temp\REPRO\STL\out\build\x64\out\inc\xstring(2894,42): note: candidate function
    _CONSTEXPR20_CONTAINER basic_string& operator=(basic_string&& _Right) noexcept(
                                         ^
C:\Temp\REPRO\STL\out\build\x64\out\inc\xstring(3122,42): note: candidate function
    _CONSTEXPR20_CONTAINER basic_string& operator=(const basic_string& _Right) {
                                         ^
C:\Temp\REPRO\STL\out\build\x64\out\inc\xstring(3142,19): note: candidate function has been explicitly deleted
    basic_string& operator=(nullptr_t) = delete;
                  ^
1 error generated.

C:\Temp\REPRO>pushd sol2

C:\Temp\REPRO\sol2>git checkout cxx23
Switched to a new branch 'cxx23'
Branch 'cxx23' set up to track remote branch 'cxx23' from 'origin'.

C:\Temp\REPRO\sol2>popd

C:\Temp\REPRO>cl /std:c++17                          /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
multi_results.cpp

C:\Temp\REPRO>cl /std:c++latest                      /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp
multi_results.cpp

C:\Temp\REPRO>clang-cl /std:c++17                    /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp

C:\Temp\REPRO>clang-cl /std:c++latest /D_HAS_CXX23=1 /EHsc /nologo /W4 /MT /Od /Ilua-5.4.3\src /Isol2\include /c sol2\examples\source\multi_results.cpp

C:\Temp\REPRO>

[ThePhD]

Nice, I'm glad this paper made it. Thanks for the PR!