Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: split implementation files and add more inline / constexpr / noexcept qualifiers #159

Merged
merged 8 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,6 @@ SeparateDefinitionBlocks: Leave
SkipMacroDefinitionBody: false
SortIncludes: CaseSensitive
SpaceAroundPointerQualifiers: Both
StatementAttributeLikeMacros:
- Py_ALWAYS_INLINE
- Py_NO_INLINE
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Checks: |
misc-*,
modernize-*,
-modernize-use-trailing-return-type,
-modernize-use-transparent-functors,
performance-*,
readability-*,
-readability-redundant-inline-specifier,
Expand Down
101 changes: 63 additions & 38 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ concurrency:
jobs:
build-sdist:
name: Build sdist
runs-on: ubuntu-latest
if: github.repository == 'metaopt/optree' && (github.event_name != 'push' || startsWith(github.ref, 'refs/tags/'))
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- name: Checkout
Expand Down Expand Up @@ -80,15 +80,15 @@ jobs:
if-no-files-found: error

build-wheels:
name: Build wheels for Python ${{ matrix.python-version }} on ${{ matrix.os }} (${{ matrix.archs }})
runs-on: ${{ matrix.os }}
needs: [build-sdist]
name: Build wheels for Python ${{ matrix.python-version }} on ${{ matrix.runner }} (${{ matrix.archs }})
if: github.repository == 'metaopt/optree' && (github.event_name != 'push' || startsWith(github.ref, 'refs/tags/'))
runs-on: ${{ matrix.runner }}
needs: [build-sdist]
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
runner: [ubuntu-latest, windows-latest, macos-latest]
python-version:
["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "pypy-3.9", "pypy-3.10"]
["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.13t", "pypy-3.9", "pypy-3.10"]
archs: [
# Generic
"auto",
Expand All @@ -100,62 +100,72 @@ jobs:
"ARM64",
]
include:
- os: macos-13
- runner: macos-13
python-version: "3.7"
archs: "auto"
exclude:
- os: ubuntu-latest
archs: "ARM64"
- os: windows-latest
archs: "aarch64"
- os: windows-latest
archs: "ppc64le"
- os: windows-latest
archs: "s390x"
- os: macos-latest
archs: "aarch64"
- os: macos-latest
archs: "ppc64le"
- os: macos-latest
archs: "s390x"
- os: macos-latest
archs: "ARM64"
- os: ubuntu-latest
- runner: ubuntu-latest
python-version: "pypy-3.9"
archs: "ppc64le"
- os: ubuntu-latest
- runner: ubuntu-latest
python-version: "pypy-3.10"
archs: "ppc64le"
- os: ubuntu-latest
- runner: ubuntu-latest
python-version: "pypy-3.9"
archs: "s390x"
- os: ubuntu-latest
- runner: ubuntu-latest
python-version: "pypy-3.10"
archs: "s390x"
- os: windows-latest
- runner: windows-latest
python-version: "3.7"
archs: "ARM64"
- os: windows-latest
- runner: windows-latest
python-version: "3.8"
archs: "ARM64"
- os: windows-latest
- runner: windows-latest
python-version: "pypy-3.9"
archs: "ARM64"
- os: windows-latest
- runner: windows-latest
python-version: "pypy-3.10"
archs: "ARM64"
- os: macos-latest
python-version: "3.7"
- runner: macos-latest
python-version: "3.7" # Python 3.7 does not support macOS ARM64

# Exclude archs of other platforms
- runner: ubuntu-latest
archs: "ARM64"
- runner: windows-latest
archs: "aarch64"
- runner: windows-latest
archs: "ppc64le"
- runner: windows-latest
archs: "s390x"
- runner: macos-latest
archs: "aarch64"
- runner: macos-latest
archs: "ppc64le"
- runner: macos-latest
archs: "s390x"
- runner: macos-latest
archs: "ARM64"
fail-fast: false
timeout-minutes: 180
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Determine Python version
shell: bash
run: |
PYTHON_VERSION="${{ matrix.python-version }}"
PYTHON_VERSION="${PYTHON_VERSION%t}"
echo "Using Python version: ${PYTHON_VERSION}"
echo "PYTHON_VERSION=${PYTHON_VERSION}" >> "${GITHUB_ENV}"

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
python-version: ${{ env.PYTHON_VERSION }}
update-environment: true
allow-prereleases: true

Expand All @@ -173,7 +183,22 @@ jobs:
run: python setup.py --version

- name: Set CIBW_BUILD
run: python .github/workflows/set_cibw_build.py
shell: bash
run: |
PYTHON_TAG="$(
python -c \
"import sys; print(
'{0.name[0]}p{1.major}{1.minor}'.format(
sys.implementation,
sys.version_info,
).lower(),
)"
)"
if [[ "${{ matrix.python-version }}" == *"t" ]]; then
PYTHON_TAG="${PYTHON_TAG}t"
fi
echo "PYTHON_TAG=${PYTHON_TAG}" | tee -a "${GITHUB_ENV}"
echo "CIBW_BUILD=${PYTHON_TAG}-*" | tee -a "${GITHUB_ENV}"

- name: Set up QEMU
if: runner.os == 'Linux'
Expand All @@ -194,15 +219,15 @@ jobs:

- uses: actions/upload-artifact@v4
with:
name: wheels-${{ matrix.python-version }}-${{ matrix.os }}-${{ matrix.archs }}
name: wheels-${{ env.PYTHON_TAG }}-${{ runner.os }}-${{ matrix.archs }}
path: wheelhouse/*.whl
if-no-files-found: error

list-artifacts:
name: List artifacts
if: github.repository == 'metaopt/optree' && (github.event_name != 'push' || startsWith(github.ref, 'refs/tags/'))
runs-on: ubuntu-latest
needs: [build-sdist, build-wheels]
if: github.repository == 'metaopt/optree' && (github.event_name != 'push' || startsWith(github.ref, 'refs/tags/'))
timeout-minutes: 15
steps:
- name: Download built sdist
Expand Down Expand Up @@ -243,8 +268,8 @@ jobs:
uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
if: startsWith(github.ref, 'refs/tags/')
uses: actions/setup-python@v5
with:
python-version: "3.7 - 3.13"
update-environment: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:

- name: clang-tidy
run: |
make clang-tidy
make clang-tidy CMAKE_CXX_STANDARD=17

- name: addlicense
run: |
Expand Down
15 changes: 0 additions & 15 deletions .github/workflows/set_cibw_build.py

This file was deleted.

8 changes: 4 additions & 4 deletions .github/workflows/tests-with-pydebug.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ env:

jobs:
test:
name: Test for Python ${{ matrix.python-version }}${{ matrix.python-abiflags }} on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
name: Test for Python ${{ matrix.python-version }}${{ matrix.python-abiflags }} on ${{ matrix.runner }}
runs-on: ${{ matrix.runner }}
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
runner: [ubuntu-latest, windows-latest, macos-latest]
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
python-abiflags: ["d", "td"]
exclude:
Expand All @@ -51,7 +51,7 @@ jobs:
python-abiflags: "td"
- python-version: "3.12"
python-abiflags: "td"
- os: windows-latest # pyenv-win does not support Python 3.13t yet
- runner: windows-latest # pyenv-win does not support Python 3.13t yet
python-version: "3.13"
python-abiflags: "td"
fail-fast: false
Expand Down
19 changes: 13 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ env:

jobs:
test:
name: Test for Python ${{ matrix.python-version }} on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
name: Test for Python ${{ matrix.python-version }} on ${{ matrix.runner }}
runs-on: ${{ matrix.runner }}
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
runner: [ubuntu-latest, windows-latest, macos-latest]
python-version:
["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "pypy-3.9", "pypy-3.10"]
include:
- os: macos-13
- runner: macos-13
python-version: "3.7"
exclude:
- os: macos-latest
- runner: macos-latest
python-version: "3.7" # Python 3.7 does not support macOS ARM64
fail-fast: false
timeout-minutes: 90
Expand Down Expand Up @@ -74,6 +74,13 @@ jobs:
python -m pip install -r tests/requirements.txt
fi

- name: Test installable with C++17
if: runner.os != 'Windows'
run: |
OPTREE_CXX_WERROR=OFF CMAKE_CXX_STANDARD=17 python -m pip install -vvv --editable .
python -X dev -W 'always' -W 'error' -c 'import optree'
python -m pip uninstall -y optree

- name: Install OpTree
run: |
python -m pip install -vvv --editable '.[test]'
Expand All @@ -83,8 +90,8 @@ jobs:
make test PYTESTOPTS="--exitfirst"

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4
if: runner.os == 'Linux'
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: ./tests/coverage.xml
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ repos:
- id: debug-statements
- id: double-quote-string-fixer
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v19.1.0
rev: v19.1.1
hooks:
- id: clang-format
- repo: https://github.com/astral-sh/ruff-pre-commit
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Split implementation files and add more `inline` / `constexpr` / `noexcept` qualifiers by [@XuehaiPan](https://github.com/XuehaiPan) in [#159](https://github.com/metaopt/optree/pull/159).
- Use `cmake`'s `FindPython` module by [@XuehaiPan](https://github.com/XuehaiPan) in [#151](https://github.com/metaopt/optree/pull/151).

### Fixed
Expand Down
15 changes: 13 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,17 @@ if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE Release)
endif()

set(CMAKE_CXX_STANDARD 20) # for likely/unlikely attributes
if(NOT DEFINED CMAKE_CXX_STANDARD AND NOT "$ENV{CMAKE_CXX_STANDARD}" STREQUAL "")
set(CMAKE_CXX_STANDARD "$ENV{CMAKE_CXX_STANDARD}")
endif()
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 20) # for likely/unlikely attributes
endif()
if (CMAKE_CXX_STANDARD VERSION_LESS 17)
message(FATAL_ERROR "C++17 or higher is required")
endif()
set(CMAKE_CXX_STANDARD_REQUIRED ON)
message(STATUS "Use C++ standard: C++${CMAKE_CXX_STANDARD}")

set(CMAKE_POSITION_INDEPENDENT_CODE ON) # -fPIC
set(CMAKE_CXX_VISIBILITY_PRESET hidden) # -fvisibility=hidden
Expand All @@ -51,6 +60,7 @@ endif()
if(MSVC)
string(
APPEND CMAKE_CXX_FLAGS
" /EHsc /bigobj"
" /Zc:preprocessor"
" /experimental:external /external:anglebrackets /external:W0"
" /Wall /Wv:19.40" # Visual Studio 2022 version 17.10
Expand All @@ -59,6 +69,7 @@ if(MSVC)
" /wd4514" # unreferenced inline function has been removed
" /wd4710" # function not inlined
" /wd4711" # function selected for inline expansion
" /wd4714" # function marked as forceinline not inlined
" /wd4820" # bytes padding added after construct 'member_name'
" /wd4868" # compiler may not enforce left-to-right evaluation order in braced initializer list
" /wd5045" # compiler will insert Spectre mitigation for memory load if /Qspectre switch specified
Expand Down Expand Up @@ -88,7 +99,7 @@ if(OPTREE_CXX_WERROR)
endif()

string(LENGTH "${CMAKE_SOURCE_DIR}/" SOURCE_PATH_PREFIX_SIZE)
add_definitions("-DSOURCE_PATH_PREFIX_SIZE=(${SOURCE_PATH_PREFIX_SIZE})")
add_definitions("-DSOURCE_PATH_PREFIX_SIZE=${SOURCE_PATH_PREFIX_SIZE}")

function(system)
set(options STRIP)
Expand Down
Loading
Loading