From 87f9d768866548b5b86e72be66c60c5abd4d9b37 Mon Sep 17 00:00:00 2001 From: Carson Radtke Date: Mon, 21 Oct 2024 10:19:37 -0500 Subject: [PATCH 1/7] GSL v4.1.0 (#1163) Fixes: #1161 Update version strings in README and CMakeLists.txt. --- CMakeLists.txt | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 71bb9975..b1d24e5f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.14...3.16) -project(GSL VERSION 4.0.0 LANGUAGES CXX) +project(GSL VERSION 4.1.0 LANGUAGES CXX) add_library(GSL INTERFACE) add_library(Microsoft.GSL::GSL ALIAS GSL) diff --git a/README.md b/README.md index 0e266b46..dd61e3fd 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ include(FetchContent) FetchContent_Declare(GSL GIT_REPOSITORY "https://github.com/microsoft/GSL" - GIT_TAG "v4.0.0" + GIT_TAG "v4.1.0" GIT_SHALLOW ON ) From f8ec3091183e44b53a36b8f3857f234767f8ed8a Mon Sep 17 00:00:00 2001 From: Carson Radtke Date: Tue, 12 Nov 2024 15:41:21 -0600 Subject: [PATCH 2/7] improve performance of span_iterator w/ clang (#1166) * improve performance of span_iterator w/ clang Issue: #1165 Before this PR, the range-for loop was ~3300x slower. After this PR, it is ~1.005x slower The clang optimizer is very good at optimizing `current != end`, so we changed to this idiom. This moves the Expects assertion into the constructor instead of on the hot-path which is called whenever either operator++ or operator* is called. Note: The codegen for the assertion is still a missed optimization, but less worrisome as it only happens once per iterator. Note: benchmarks on M1 Macbook Pro w/ Apple Clang 16.0.0 --- include/gsl/span | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/include/gsl/span b/include/gsl/span index d2ef9f7d..a01b6877 100644 --- a/include/gsl/span +++ b/include/gsl/span @@ -140,7 +140,9 @@ namespace details constexpr span_iterator(pointer begin, pointer end, pointer current) : begin_(begin), end_(end), current_(current) - {} + { + Expects(begin_ <= current_ && current <= end_); + } constexpr operator span_iterator() const noexcept { @@ -149,21 +151,18 @@ namespace details constexpr reference operator*() const noexcept { - Expects(begin_ && end_); - Expects(begin_ <= current_ && current_ < end_); + Expects(current_ != end_); return *current_; } constexpr pointer operator->() const noexcept { - Expects(begin_ && end_); - Expects(begin_ <= current_ && current_ < end_); + Expects(current_ != end_); return current_; } constexpr span_iterator& operator++() noexcept { - Expects(begin_ && current_ && end_); - Expects(current_ < end_); + Expects(current_ != end_); // clang-format off GSL_SUPPRESS(bounds.1) // NO-FORMAT: attribute // clang-format on @@ -180,8 +179,7 @@ namespace details constexpr span_iterator& operator--() noexcept { - Expects(begin_ && end_); - Expects(begin_ < current_); + Expects(begin_ != current_); --current_; return *this; } From 74d2bb79d48e0f5027eb950b4ca808cb3c507116 Mon Sep 17 00:00:00 2001 From: Carson Radtke Date: Mon, 25 Nov 2024 12:58:45 -0600 Subject: [PATCH 3/7] fix various pipeline failures (#1172) * fix failing pipeline tests * upgrade to googletest v1.14.0 so it works with newer cmake versions * fix android pipeline to permit new cmake versions (short-term fix) --- .github/workflows/android.yml | 2 +- .github/workflows/compilers.yml | 9 ++++++++- tests/CMakeLists.txt | 1 + tests/CMakeLists.txt.in | 4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index a2c88fc9..97259a83 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -38,7 +38,7 @@ jobs: echo "Emulator starting in background" - name: Configure - run: cmake -Werror=dev -DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_LATEST_HOME/build/cmake/android.toolchain.cmake -DANDROID_PLATFORM=16 -DANDROID_ABI=x86_64 -DCMAKE_BUILD_TYPE=Debug .. + run: cmake -DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_LATEST_HOME/build/cmake/android.toolchain.cmake -DANDROID_PLATFORM=16 -DANDROID_ABI=x86_64 -DCMAKE_BUILD_TYPE=Debug .. - name: Build run: cmake --build . --parallel diff --git a/.github/workflows/compilers.yml b/.github/workflows/compilers.yml index 23866a63..14daeb6c 100644 --- a/.github/workflows/compilers.yml +++ b/.github/workflows/compilers.yml @@ -23,6 +23,13 @@ jobs: exclude: - gcc_version: 10 cxx_version: 23 + # https://github.com/google/googletest/issues/4232 + # Looks like GoogleTest is not interested in making version 1.14 + # work with gcc-12. + - gcc_version: 12 + cxx_version: 20 + - gcc_version: 12 + cxx_version: 23 runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -54,7 +61,7 @@ jobs: xcode: strategy: matrix: - xcode_version: [ '14.3.1', '15.4' ] + xcode_version: [ '15.4' ] build_type: [ Debug, Release ] cxx_version: [ 14, 17, 20, 23 ] runs-on: macos-latest diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9de2519b..928cdba1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -110,6 +110,7 @@ if(MSVC) # MSVC or simulating MSVC -Wno-shift-sign-overflow # GTest gtest-port.h -Wno-undef # GTest -Wno-used-but-marked-unused # GTest EXPECT_DEATH + -Wno-switch-default # GTest EXPECT_DEATH $<$: # no support for [[maybe_unused]] -Wno-unused-member-function -Wno-unused-variable diff --git a/tests/CMakeLists.txt.in b/tests/CMakeLists.txt.in index 2535f8f4..4f919f67 100644 --- a/tests/CMakeLists.txt.in +++ b/tests/CMakeLists.txt.in @@ -1,10 +1,10 @@ -cmake_minimum_required(VERSION 3.0.2) +cmake_minimum_required(VERSION 3.13) project(googletest-download NONE) include(ExternalProject) ExternalProject_Add(googletest GIT_REPOSITORY https://github.com/google/googletest.git - GIT_TAG 1b18723e874b256c1e39378c6774a90701d70f7a + GIT_TAG v1.14.0 SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}/googletest-src" BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/googletest-build" CONFIGURE_COMMAND "" From ddae9d72b6bd5fe58bc5c54e8c513fca67c833f6 Mon Sep 17 00:00:00 2001 From: Carson Radtke Date: Mon, 25 Nov 2024 13:06:02 -0600 Subject: [PATCH 4/7] fix stale badge links in README (#1170) * update pipeline badge links * add link to compilers pipeline * add badge to vcpkg version --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index dd61e3fd..f1278da4 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ # GSL: Guidelines Support Library -[![Build Status](https://dev.azure.com/cppstat/GSL/_apis/build/status/microsoft.GSL?branchName=main)](https://dev.azure.com/cppstat/GSL/_build/latest?definitionId=1&branchName=main) +[![CI](https://github.com/Microsoft/GSL/actions/workflows/compilers.yml/badge.svg)](https://github.com/microsoft/GSL/actions/workflows/compilers.yml?query=branch%3Amain) +[![vcpkg](https://img.shields.io/vcpkg/v/ms-gsl)](https://vcpkg.io/en/package/ms-gsl) The Guidelines Support Library (GSL) contains functions and types that are suggested for use by the [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines) maintained by the [Standard C++ Foundation](https://isocpp.org). @@ -106,8 +107,8 @@ If you successfully port GSL to another platform, we would love to hear from you Target | CI/CD Status :------- | -----------: -iOS | ![CI_iOS](https://github.com/microsoft/GSL/workflows/CI_iOS/badge.svg) -Android | ![CI_Android](https://github.com/microsoft/GSL/workflows/CI_Android/badge.svg) +iOS | [![CI_iOS](https://github.com/microsoft/GSL/workflows/CI_iOS/badge.svg?branch=main)](https://github.com/microsoft/GSL/actions/workflows/ios.yml?query=branch%3Amain) +Android | [![CI_Android](https://github.com/microsoft/GSL/workflows/CI_Android/badge.svg?branch=main)](https://github.com/microsoft/GSL/actions/workflows/android.yml?query=branch%3Amain) Note: These CI/CD steps are run with each pull request, however failures in them are non-blocking. From 4b190d2e2a2c9ddbff06d33afe260a5b3505e69a Mon Sep 17 00:00:00 2001 From: Carson Radtke Date: Mon, 2 Dec 2024 14:34:38 -0600 Subject: [PATCH 5/7] Create issue templates (#1173) --- .github/ISSUE_TEMPLATE/bug_report.md | 29 ++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..be72e493 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,29 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: 'Status: Open, Type: Bug' +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**To Reproduce** +```c++ +#include + +// your repro here: ... +``` + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Spec (please complete the following information):** + - OS: [e.g. Windows] + - Compiler: [e.g. MSVC] + - C++ Version: [e.g. C++20] + +**Additional context** +Add any other context about the problem here. From aed09c41b6ea3d58ddb27e65fcfd5b781b8b3dcd Mon Sep 17 00:00:00 2001 From: Alberto La Rocca Date: Fri, 13 Dec 2024 16:56:28 +0100 Subject: [PATCH 6/7] Fix SFINAE on `gsl::owner`. (#1174) `std::enable_if_t` must not be used as a default template argument, otherwise the instantiator will be able to override it freely with something that doesn't fail substitution. Instead, `std::enable_if_t` itself must be the type of the template argument. More information in the examples here: https://en.cppreference.com/w/cpp/types/enable_if --- include/gsl/pointers | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/gsl/pointers b/include/gsl/pointers index 63e193e9..dd0f0682 100644 --- a/include/gsl/pointers +++ b/include/gsl/pointers @@ -75,7 +75,7 @@ using std::unique_ptr; // T must be a pointer type // - disallow construction from any type other than pointer type // -template ::value>> +template ::value, bool> = true> using owner = T; // From 8a0e3d8a9be432ead5d07095252c5c070b8d8dc8 Mon Sep 17 00:00:00 2001 From: Carson Radtke Date: Fri, 13 Dec 2024 10:22:16 -0600 Subject: [PATCH 7/7] fix: direct-init const ref instead of list-init (#1175) fixes: https://github.com/microsoft/GSL/issues/1162 gcc has a bug that generates a call to the copy constructor when list initializing a const reference. This PR offers a workaround which is to direct initialize the value. see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117900 --- include/gsl/pointers | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/gsl/pointers b/include/gsl/pointers index dd0f0682..594ef818 100644 --- a/include/gsl/pointers +++ b/include/gsl/pointers @@ -117,7 +117,7 @@ public: not_null(const not_null& other) = default; not_null& operator=(const not_null& other) = default; constexpr details::value_or_reference_return_t get() const - noexcept(noexcept(details::value_or_reference_return_t{std::declval()})) + noexcept(noexcept(details::value_or_reference_return_t(std::declval()))) { return ptr_; }