microsoft / GSL

Guidelines Support Library
Other
6.19k stars 740 forks source link

Fix/implement C++2020 compilation, tests, and CI #1017

Closed dmitrykobets-msft closed 2 years ago

dmitrykobets-msft commented 2 years ago

The C++2020 features (std::span support) were not enabled in the CI. As a result, the code had compilation issues that were never caught. Furthermore, the compiler versions in the README did not match up to the compilers that were actually being tested. For instance, the minimum GCC version advertised is 8, but we are testing 7.5.0 instead. The precise versions we should be testing will be re-visited in a later PR. But for now, this PR simply updates the README to be accurate to what is currently being tested.

Compiler Advertised (before this PR) Actually Tested
XCode 11.4 & 10.3 12.4 & 11.3
GCC 9 & 8 9.3.0 & 7.5.0
Clang 11 & 10 11.0.0 & 9.0.0
Visual Studio with MSVC VS2017 (15.9) & VS2019 (16.4) VS2017 (15.9.21) & VS2019 (16.11.2)
Visual Studio with LLVM VS2017 (Clang 9) & VS2019 (Clang 10) none none

Previously, the guard for the span tests was __cplusplus >= 202002L. Unfortunately, this does not work for all compilers. For instance, in GCC 10, running with -std=c++20 will fail the above condition (__cplusplus will still be at 2017), and yet __cpp_lib_span >= 202002L will be true. But for GCC 9 and GCC 8, __cplusplus will still be at 2017 and __cpp_lib_span is undefined (neither have <span> support). In all cases above, the span tests will be silently skipped. To avoid this confusing / incorrect (in the case of GCC 10) behavior, make the span tests execute unconditionally if running in CI mode with C++20 enabled. If a compiler version does not support span, the C++20 version should be explicitly removed from its CI jobs.

SloppyJaconda commented 2 years ago

Out of curiosity, is there a specific issue that you're looking to fix with this?

dmitrykobets-msft commented 2 years ago

@SloppyJaconda It appears that the C++20 feature support actually had compilation / syntax errors. This was hidden because C++20 was never part of the CI pipeline, so the tests were never run. So this PR fixes those errors and attempts to enable C++20 in the CI