ned14 / llfio

P1031 low level file i/o and filesystem library for the C++ standard
https://ned14.github.io/llfio/
Other
880 stars 45 forks source link

Enable MSVC STL workaround until next major toolset version #99

Closed BurningEnlightenment closed 2 years ago

BurningEnlightenment commented 2 years ago

See #98 for further rationale.

BurningEnlightenment commented 2 years ago

May I suggest that we test _CPPLIB_VER instead, but perhaps compare to a version some way off from current so this workaround doesn't drop out for a while? Next time I'm talking to a MSVC person I may drop heavy hints they ought to go improve the story here.

I opted for _MSVC_STL_VERSION which seems to be kept in sync with the toolset version (see microsoft/STL#2090). I chose 150, because (as outlined in the issue) they can't fix it without breaking binary compatibility. Which in turn won't happen earlier than the next major toolset version release.

BurningEnlightenment commented 2 years ago

Surely 150 is a bit aggressive though? It's currently 143 which I would assume will remain until VS2024 is released. It seems reasonable to me that this workaround get retested on VS2024's release.

Maybe I'm wrong, but my understanding is that the number is composed of a major and a minor part (i.e. 14.3). Whereas toolsets with the same major version retain binary compatibility. This would imply that if VS2024 breaks bincompat it will release with _MSVC_STL_VERSION=150. And from reading Stephan's reddit post I gathered that this cannot be fixed without breaking bincompat.

Therefore 150 should be the earliest version which can contain a fix.

EDIT:

other standard libraries compile this code just fine, and I think his STL needs to try harder here than it currently does.

The underlying issue is that MSVC's std::list<T> move constructor is always noexcept(false) due to the fact that the empty list state is implemented with a dynamically allocated sentinel (which is allowed by the standard). This forces std::vector<std::list<T>> to use the std::list copy constructor in order to provide the required strong EH guarantees.

github-actions[bot] commented 2 years ago

Unit Test Results

2 files   -     4  0 suites   - 192   0s :stopwatch: - 35m 55s 0 tests  -   57  0 :heavy_check_mark:  -   57  0 :zzz: ±0  0 :x: ±0  0 runs   - 360  0 :heavy_check_mark:  - 360  0 :zzz: ±0  0 :x: ±0 

Results for commit fa0940ef. ± Comparison against base commit 7986b922.

github-actions[bot] commented 2 years ago

Unit Test Results

    4 files   -     2  124 suites   - 68   33m 10s :stopwatch: - 2m 45s   58 tests +    1    58 :heavy_check_mark: +    1  0 :zzz: ±0  0 :x: ±0  240 runs   - 120  240 :heavy_check_mark:  - 120  0 :zzz: ±0  0 :x: ±0 

Results for commit fa0940ef. ± Comparison against base commit 7986b922.

github-actions[bot] commented 2 years ago

Unit Test Results

    6 files  ±0  192 suites  ±0   28m 20s :stopwatch: - 7m 35s   57 tests ±0    57 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0  360 runs  ±0  360 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0 

Results for commit fa0940ef. ± Comparison against base commit 7986b922.