diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13d76ea..e5005f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,44 +11,35 @@ on: workflow_dispatch: jobs: - test_debug: + test: runs-on: ubuntu-latest permissions: contents: read - container: - image: tschijnmo/drudge:base - - steps: - - name: Checkout repository with submodules - uses: actions/checkout@v3 - with: - submodules: recursive - - - name: Build and test (debug) - run: | - mkdir cmake-test-debug - cd cmake-test-debug - cmake -DCMAKE_BUILD_TYPE=debug .. - make - ./test/testmain + strategy: + matrix: + build_type: [Debug, Release] - test_release: - runs-on: ubuntu-latest - permissions: - contents: read - container: - image: tschijnmo/drudge:base - steps: - - name: Checkout repository with submodules - uses: actions/checkout@v3 - with: - submodules: recursive - - - name: Build and test (release) - run: | - mkdir cmake-test-release - cd cmake-test-release - cmake -DCMAKE_BUILD_TYPE=release .. - make - ./test/testmain + - name: Checkout code + uses: actions/checkout@v6 + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y cmake g++ python3-dev + + - name: Configure CMake + run: | + mkdir build + cd build + cmake -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} .. + + - name: Build + run: | + cd build + make + + - name: Run tests + run: | + cd build + ./test/testmain diff --git a/CMakeLists.txt b/CMakeLists.txt index 255432b..989df01 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,20 +1,21 @@ -cmake_minimum_required(VERSION 3.6) +cmake_minimum_required(VERSION 3.18) project(cpypp VERSION 0.1.0 LANGUAGES CXX) # OPTIONS option(BUILD_TESTS "Build unit tests" ON) # Set the building options. -set(CMAKE_CXX_STANDARD 14) +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) # Find the Python runtime. -include(FindPythonLibs) +find_package(Python3 REQUIRED COMPONENTS Development.Embed) message(STATUS - "Find Python header at ${PYTHON_INCLUDE_DIRS}\nand library at ${PYTHON_LIBRARIES}" + "Find Python header at ${Python3_INCLUDE_DIRS}\nand library at ${Python3_LIBRARIES}" ) if (BUILD_TESTS) enable_testing() add_subdirectory(test) endif () - diff --git a/include/cpypp.hpp b/include/cpypp.hpp index 05b028d..25f59f4 100644 --- a/include/cpypp.hpp +++ b/include/cpypp.hpp @@ -636,13 +636,17 @@ class Handle { } } - /** Increments the reference count only for owning reference. + /** Increments the reference count for an owning reference, if not null. + * + * An owning handle can hold a null pointer, for instance the value of an + * exhausted `Iter_handle`, so null is tolerated here just as it is in + * `decr_ref`. */ void incr_ref() noexcept { if (!if_borrow_) { - Py_INCREF(ref_); + Py_XINCREF(ref_); } } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index a228902..8114d5d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,10 +1,14 @@ -# Download the Catch2 single header. +# Fetch Catch2 v3.11.0 +include(FetchContent) -file(DOWNLOAD - "https://github.com/catchorg/Catch2/releases/download/v2.0.1/catch.hpp" - "${CMAKE_CURRENT_BINARY_DIR}/catch.hpp" +FetchContent_Declare( + Catch2 + GIT_REPOSITORY https://github.com/catchorg/Catch2.git + GIT_TAG v3.11.0 ) +FetchContent_MakeAvailable(Catch2) + # The main test driver. add_executable(testmain testmain.cpp @@ -21,11 +25,9 @@ add_executable(testmain ) target_include_directories(testmain - PRIVATE ${CMAKE_CURRENT_BINARY_DIR} PRIVATE "${PROJECT_SOURCE_DIR}/include" - PRIVATE ${PYTHON_INCLUDE_DIRS} ) target_link_libraries(testmain - PRIVATE ${PYTHON_LIBRARIES} + PRIVATE Catch2::Catch2 + PRIVATE Python3::Python ) - diff --git a/test/exc.cpp b/test/exc.cpp index 337d6c1..7465bcb 100644 --- a/test/exc.cpp +++ b/test/exc.cpp @@ -1,7 +1,7 @@ /** Tests for the utilities related to Python exception handling. */ -#include +#include #include diff --git a/test/fundamentalobjects.cpp b/test/fundamentalobjects.cpp index 6fc14fb..18ccd08 100644 --- a/test/fundamentalobjects.cpp +++ b/test/fundamentalobjects.cpp @@ -1,7 +1,7 @@ /** Tests for the utility for fundamental objects. */ -#include +#include #include @@ -77,7 +77,11 @@ TEST_CASE("Trivial static type has basic features", "[Handle][Static_type]") auto handle = noddy_type.get_handle(true); CHECK(handle.get() == noddy_ptr); CHECK(!handle.if_borrow()); - CHECK(Py_REFCNT(noddy_ptr) == init_count + 1); + // Static types are immortal from Python 3.12 on, and then their + // reference count does not move. + if (init_count < (static_cast(1) << 30)) { + CHECK(Py_REFCNT(noddy_ptr) == init_count + 1); + } } CHECK(Py_REFCNT(noddy_ptr) == init_count); } diff --git a/test/handle.cpp b/test/handle.cpp index a3333ec..fc62d5a 100644 --- a/test/handle.cpp +++ b/test/handle.cpp @@ -11,7 +11,7 @@ #include -#include +#include #include @@ -21,17 +21,18 @@ using namespace cpypp; TEST_CASE("Owning handles correctly manages reference counts", "[Handle]") { - PyObject* one = Py_BuildValue("i", 1); + // Large integers are used, since small integers are cached by CPython + // and immortal from Python 3.12 on, where their reference count never + // moves. These two are fresh objects whose count we fully control. + PyObject* one = Py_BuildValue("L", static_cast(1) << 40); // Py_BuildValue gives a new reference to be possibly stolen by // Handles. Here we increment the reference count again to make sure // that even after that reference is destroyed, we still have at least - // one reference. Practically this is unnecessary in that the integer - // unity normally has hundreds of references to prevent it from being - // collected. + // one reference. Py_INCREF(one); Py_ssize_t init_count = Py_REFCNT(one); - PyObject* two = Py_BuildValue("i", 2); + PyObject* two = Py_BuildValue("L", static_cast(1) << 41); Py_INCREF(two); // For the same rational as one. Py_ssize_t init_count2 = Py_REFCNT(two); @@ -351,8 +352,8 @@ TEST_CASE("Borrowing handles correctly treats reference counts", "[Handle]") { // Testing of borrowing handles is relatively easy, no matter what // happens, the reference count should never be touched. - PyObject* one = Py_BuildValue("i", 1); - PyObject* two = Py_BuildValue("i", 2); + PyObject* one = Py_BuildValue("L", static_cast(1) << 40); + PyObject* two = Py_BuildValue("L", static_cast(1) << 41); Py_ssize_t init_count = Py_REFCNT(one); Py_ssize_t init_count2 = Py_REFCNT(two); auto check_ref = [&]() { diff --git a/test/iteratorprotocol.cpp b/test/iteratorprotocol.cpp index 28f092c..7f8ceb6 100644 --- a/test/iteratorprotocol.cpp +++ b/test/iteratorprotocol.cpp @@ -5,7 +5,7 @@ #include #include -#include +#include #include @@ -54,6 +54,37 @@ TEST_CASE("Handles has basic iterator protocol", "[Handle][Iter_handle]") } } +TEST_CASE("Iterators over empty iterables can be copied", "[Handle][Iter_handle]") +{ + // An exhausted iterator holds an owning null handle as its value. Copying + // it used to increment the reference count of the null pointer. + Handle empty("[]"); + + SECTION("an exhausted iterator can be copied") + { + auto it = empty.begin(); + check_exc(); + REQUIRE_FALSE(it.has_val()); + Iter_handle copy(it); + CHECK_FALSE(copy.has_val()); + CHECK(copy == empty.end()); + } + + SECTION("a container can be built from an empty iterable") + { + std::vector res(empty.begin(), empty.end()); + check_exc(); + CHECK(res.empty()); + } + + SECTION("an owning null handle can be copied") + { + Handle null(nullptr, STEAL, true); + Handle copy(null); + CHECK_FALSE(copy); + } +} + TEST_CASE("Iterator utilities report wrong types", "[Handle][Iter_handle]") { // A non-iterable and non-iterator integer. diff --git a/test/numberprotocol.cpp b/test/numberprotocol.cpp index 3b384a0..cb64d0b 100644 --- a/test/numberprotocol.cpp +++ b/test/numberprotocol.cpp @@ -3,7 +3,7 @@ #include -#include +#include #include diff --git a/test/numericobjects.cpp b/test/numericobjects.cpp index 1c2097a..47017e4 100644 --- a/test/numericobjects.cpp +++ b/test/numericobjects.cpp @@ -1,7 +1,7 @@ /** Tests for the utilities to related numeric objects. */ -#include +#include #include @@ -9,6 +9,13 @@ using namespace cpypp; +// Small integers are immortal from Python 3.12 on, and the reference count of +// an immortal object does not move. Their count is only checked when it can. +static bool count_moves(PyObject* obj) +{ + return Py_REFCNT(obj) < (static_cast(1) << 30); +} + TEST_CASE("Integer can be built and parsed", "[Handle]") { @@ -22,7 +29,9 @@ TEST_CASE("Integer can be built and parsed", "[Handle]") { Handle from_gen(v); CHECK(from_gen.get() == one); - CHECK(Py_REFCNT(one) == curr_count + 1); + if (count_moves(one)) { + CHECK(Py_REFCNT(one) == curr_count + 1); + } long val; from_gen.as(val); diff --git a/test/objectprotocol.cpp b/test/objectprotocol.cpp index ff25d76..3a97281 100644 --- a/test/objectprotocol.cpp +++ b/test/objectprotocol.cpp @@ -3,7 +3,7 @@ #include -#include +#include #include diff --git a/test/otherobjects.cpp b/test/otherobjects.cpp index 0088a99..16a6325 100644 --- a/test/otherobjects.cpp +++ b/test/otherobjects.cpp @@ -1,7 +1,7 @@ /** Tests for utility for uncategorized concrete object. */ -#include +#include #include diff --git a/test/parsebuild.cpp b/test/parsebuild.cpp index c51c0ae..df4c160 100644 --- a/test/parsebuild.cpp +++ b/test/parsebuild.cpp @@ -5,7 +5,7 @@ * objects. */ -#include +#include #include @@ -13,6 +13,13 @@ using namespace cpypp; +// Small integers are immortal from Python 3.12 on, and the reference count of +// an immortal object does not move. Their count is only checked when it can. +static bool count_moves(PyObject* obj) +{ + return Py_REFCNT(obj) < (static_cast(1) << 30); +} + TEST_CASE("General utility can make and parse simple integers", "[Handle]") { @@ -26,7 +33,9 @@ TEST_CASE("General utility can make and parse simple integers", "[Handle]") { Handle from_gen("i", 1); CHECK(from_gen.get() == one); - CHECK(Py_REFCNT(one) == curr_count + 1); + if (count_moves(one)) { + CHECK(Py_REFCNT(one) == curr_count + 1); + } long val; from_gen.as(val); diff --git a/test/sequenceobjects.cpp b/test/sequenceobjects.cpp index 2fdc5c5..40de1d4 100644 --- a/test/sequenceobjects.cpp +++ b/test/sequenceobjects.cpp @@ -1,7 +1,7 @@ /** Tests for the utility for sequence objects. */ -#include +#include #include diff --git a/test/testmain.cpp b/test/testmain.cpp index 0dd0df5..ad221bc 100644 --- a/test/testmain.cpp +++ b/test/testmain.cpp @@ -1,5 +1,4 @@ -#define CATCH_CONFIG_RUNNER -#include +#include #include