[P0980R1] constexpr std::string

[miscco]

I could not resist.

This implements constexpr string for clang only. Currently we are still blocked for MSVC due to the bug with the constexpr destructor.

There are some notable wrinkles:

1. Capacity: I have no Idea how this works

2. There is some subtle bug regarding the move + allocator constructor and some other constructors (those that are currently commented out), where we seem to have a double delete.

3. There is undefined behavior in string.insert(). We are checking for self inserts, where comparisons of pointers of different arrays are done. Here we need a fix similar to what we did in char_traits::copy where we linearly walk the buffer.

4. Testing is hell. I am through with char but need to find a clean way to test wchar_t

Fixes #43.

[miscco]

Note that there is also a plethora of tests for other functionality missing like find and friends and substr etc, that I need to implement, but this should be a good start

[miscco]

Urgh, I think I broke something ...

[miscco]

urgh, @zygoloid i am sorry, that is going to be a hell of a bug report :(

Build setup steps:
Build steps:
Build step failed unexpectedly.
Command: "C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\Llvm\bin\clang-cl.EXE" "C:\a\1\s\llvm-project\libcxx\test\std\input.output\filesystems\class.filesystem_error\filesystem_error.members.pass.cpp" "/ID:\build\x86\out\inc" "/IC:\a\1\s\llvm-project\libcxx\test\support" "/nologo" "/Od" "/W4" "/w14061" "/w14242" "/w14265" "/w14582" "/w14583" "/w14587" "/w14588" "/w14749" "/w14841" "/w14842" "/w15038" "/w15214" "/w15215" "/w15216" "/w15217" "/sdl" "/WX" "/Zc:strictStrings" "/D_ENABLE_STL_INTERNAL_CHECK" "/bigobj" "/EHsc" "/MTd" "/std:c++latest" "/permissive-" "/FImsvc_stdlib_force_include.h" "/wd4643" "-fno-ms-compatibility" "-fno-delayed-template-parsing" "-m32" "/FeD:\build\x86\tests\libcxx\std\input.output\filesystems\class.filesystem_error\Output\filesystem_error.members.pass.cpp.dir\1\filesystem_error.members.pass.exe" "/link" "/LIBPATH:D:\build\x86\out\lib\i386" "/LIBPATH:C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\MSVC\14.28.29515\lib\x86" "/MANIFEST:EMBED"
Exit Code: 1
Standard Error:
--
clang-cl: error: clang frontend command failed due to signal (use -v to see invocation)

clang version 10.0.0 

Target: i686-pc-windows-msvc

Thread model: posix

InstalledDir: C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\Llvm\bin

clang-cl: note: diagnostic msg: PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.

clang-cl: note: diagnostic msg: 

********************

[zygoloid]

urgh, @zygoloid i am sorry, that is going to be a hell of a bug report :(

Fun :)

clang version 10.0.0 

Would it be feasible to give this a try with Clang trunk or the 11.0 release? I'm confident there are things we've fixed in the area of constexpr allocation since 10.0.

[miscco]

I will try, once I find out which configuration is crashing. That said, the last time I tried to pull in llvm-project as a submodule my trusty workstation surrendered

[mnatsuhara]

Unfortunately, it looks like the ICE in the filesystem libcxx tests persists even with Clang 11 😿

FAIL: libc++ :: std/input.output/filesystems/class.filesystem_error/filesystem_error.members.pass.cpp:1 (1 of 2)
******************** TEST 'libc++ :: std/input.output/filesystems/class.filesystem_error/filesystem_error.members.pass.cpp:1' FAILED ********************
Build setup steps:
Build steps:
Build step failed unexpectedly.
Command: "C:\Program Files\LLVM\bin\clang-cl.EXE" "C:\Users\minatsuh\development\STL\llvm-project\libcxx\test\std\input.output\filesystems\class.filesystem_error\filesystem_error.members.pass.cpp" "/IC:\Users\minatsuh\development\STL\out\build\x64\out\inc" "/IC:\Users\minatsuh\development\STL\llvm-project\libcxx\test\support" "/nologo" "/Od" "/W4" "/w14061" "/w14242" "/w14265" "/w14582" "/w14583" "/w14587" "/w14588" "/w14749" "/w14841" "/w14842" "/w15038" "/w15214" "/w15215" "/w15216" "/w15217" "/sdl" "/WX" "/Zc:strictStrings" "/D_ENABLE_STL_INTERNAL_CHECK" "/bigobj" "/EHsc" "/MTd" "/std:c++latest" "/permissive-" "/FImsvc_stdlib_force_include.h" "/wd4643" "-fno-ms-compatibility" "-fno-delayed-template-parsing" "-m64" "/FeC:\Users\minatsuh\development\STL\out\build\x64\tests\libcxx\std\input.output\filesystems\class.filesystem_error\Output\filesystem_error.members.pass.cpp.dir\1\filesystem_error.members.pass.exe" "/link" "/LIBPATH:C:\Users\minatsuh\development\STL\out\build\x64\out\lib\amd64" "/LIBPATH:C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\MSVC\14.28.29515\lib\x64" "/MANIFEST:EMBED"
Exit Code: 1
Standard Error:
--
clang-cl: error: clang frontend command failed due to signal (use -v to see invocation)
clang version 11.0.0
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\LLVM\bin
clang-cl: note: diagnostic msg:
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-cl: note: diagnostic msg: C:\Users\minatsuh\development\STL\out\build\x64\tests\libcxx\std\input.output\filesystems\class.filesystem_error\Output\filesystem_error.members.pass.cpp.dir\1\filesystem_error-767c08.cpp
clang-cl: note: diagnostic msg: C:\Users\minatsuh\development\STL\out\build\x64\tests\libcxx\std\input.output\filesystems\class.filesystem_error\Output\filesystem_error.members.pass.cpp.dir\1\filesystem_error-767c08.sh
clang-cl: note: diagnostic msg:

********************
--


********************
PASS: libc++ :: std/input.output/filesystems/class.filesystem_error/filesystem_error.members.pass.cpp:0 (2 of 2)
********************
Failed Tests (1):
  libc++ :: std/input.output/filesystems/class.filesystem_error/filesystem_error.members.pass.cpp:1


Testing Time: 4.62s
  Passed: 1
  Failed: 1

[miscco]

With the new changes clang flags the following code in deat_test.hpp (

STL/tests/std/include/test_death.hpp

Lines 76 to 86 in 19c683d

	const DWORD result_size = ::GetModuleFileNameW(nullptr, &result[0], static_cast<DWORD>(result.size()));
	const size_t str_size = result.size();
	if (result_size == str_size) {
	// buffer was not big enough
	const size_t str_max_size = result.max_size();
	const size_t result_max_size = str_max_size - str_max_size / 2;
	if (result_size >= result_max_size) {
	api_unexpected("GetModuleFileNameW");
	}
	result.resize(result_size + result_size / 2);

):

                const DWORD result_size = ::GetModuleFileNameW(nullptr, &result[0], static_cast<DWORD>(result.size()));
                const size_t str_size   = result.size();
                if (result_size == str_size) {
                    // buffer was not big enough
                    const size_t str_max_size    = result.max_size();
                    const size_t result_max_size = str_max_size - str_max_size / 2;
                    if (result_size >= result_max_size) {
                        api_unexpected("GetModuleFileNameW");
                    }

                    result.resize(result_size + result_size / 2);

Importantly, max_size() falls back to allocator_traits::max_size(allocator) which is given as:

    _NODISCARD static constexpr size_type max_size(const _Alloc&) noexcept {
        return static_cast<size_t>(-1) / sizeof(value_type);
    }

With resut_size being of type DWORD (unsigned long int) the comparison result_size >= result_max_size would indeed be tautological for 64bit architetures. Should we guard this with an appropriate #ifdef?

[miscco]

@zygoloid we had a host of crashes with this PR (See https://github.com/microsoft/STL/runs/1486997695)

I found that with clang-10 the bug was with the default constructor of the _String_const_iterator (e346257)

However, it seems that we still get the ICE from libcxx\test\std\input.output\filesystems\class.filesystem_error\filesystem_error.members.pass.cpp

@mnatsuhara was investigating this on clang-11 and the crash is not solved with clang 11. She also experienced other failures that were not remedied with the smal workaround

[mnatsuhara]

I switched back to Clang 10 to verify that the issue of the _String_const_iterator workaround being unsuccessful on my local runs was due to Clang 11 specifically, and not some other configuration issue. This is confirmed - using Clang 10, the regex tests pass fine with the _String_const_iterator workaround. Using Clang 11, the Access Violation (return code 3221225477) errors persist in those tests even with the workaround.

The filesystem_error.members.pass.cpp ICE persists in both Clang 10 and Clang 11.

[zygoloid]

@zygoloid we had a host of crashes with this PR (See https://github.com/microsoft/STL/runs/1486997695)

I don't have an environment set up to build using the MS STL, but if folks here can provide me with preprocessed sources that trigger a crash, I can take a look.

[miscco]

@zygoloid we had a host of crashes with this PR (See https://github.com/microsoft/STL/runs/1486997695)

I don't have an environment set up to build using the MS STL, but if folks here can provide me with preprocessed sources that trigger a crash, I can take a look.

Yeah sorry, I wanted to do a full clang 11 run with maybe clang trunk, but am a bit tight with time right now to set everything up

[StephanTLavavej]

I've pushed a perma-workaround for Microsoft-internal VSO-1285539 "REPORTED: EDG ICE with constexpr std::string in do_constexpr_std_construct_at" caused by:

STL/tests/std/tests/VSO_0102478_moving_allocators/test.cpp

Lines 67 to 90 in 663f0dd

	struct test_info {
	string type_name;
	string test_name;
	int count;
	};
	using expected_table_t = vector<test_info>;
	#pragma warning(push)
	#pragma warning(disable : 4640) // 'variable': construction of local static object is not thread-safe
	#if _ITERATOR_DEBUG_LEVEL == 0
	const expected_table_t& get_expected_moves_table() {
	static const expected_table_t s_expected_moves{
	{"*", "*", 1},
	{"unordered_set", "*",
	2}, // unordered_* moves two allocators, one for list, and one for vector of list iterators.
	{"unordered_multiset", "*", 2},
	{"unordered_map", "*", 2},
	{"unordered_multimap", "*", 2},
	};
	return s_expected_moves;
	}

These function-static vectors containing strings were causing trouble for EDG now, and MSVC earlier (with the bugs that @mnatsuhara reported, fixed in 16.10 Preview 1). We can perma-workaround all of this, removing all guards for __EDG__ and MSVC_INTERNAL_TESTING, plus the pre-existing warning C4640: construction of local static object is not thread-safe suppression, by using constexpr arrays containing const char*. (string_view isn't available in C++14 mode.)

We need to adjust query_expected_table(), templating it on the ExpectedTable type (because the various sizes differ). It takes const string& type_name, const string& test_name which can be compared against the const char* in the tables. However, comparing "*" to the const char* in the tables won't work, so we need to add and capture const string star{"*"};.

Changes requested in Dec 2020 have been made

[StephanTLavavej]

static_assert("Thanks for implementing this major C++20 feature!"s
  == "Thanks for implementing "s + "this major C++20 feature!"s);

Amazing work, @miscco and @mnatsuhara! This will ship in VS 2019 16.10 Preview 2! 🚀 😺 🥇