From 3de2facc379653a3feac4210ceb8ccd7b95ad968 Mon Sep 17 00:00:00 2001 From: Andrei Avram <6795248+andreiavrammsd@users.noreply.github.com> Date: Tue, 3 Sep 2024 16:59:02 +0000 Subject: [PATCH 1/3] Exception safety --- include/msd/zip.hpp | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/include/msd/zip.hpp b/include/msd/zip.hpp index f094060..35569a5 100644 --- a/include/msd/zip.hpp +++ b/include/msd/zip.hpp @@ -50,14 +50,14 @@ class zip_iterator { * * @param iterators The iterators to be zipped together. */ - explicit zip_iterator(Iterators... iterators) noexcept : iterators_{iterators...} {} + explicit zip_iterator(Iterators... iterators) : iterators_{iterators...} {} /** * @brief Dereferences the `zip_iterator` to obtain a tuple of references from each iterator. * * @return A tuple containing the values pointed to by each iterator. */ - value_type operator*() const noexcept { return dereference(std::index_sequence_for{}); } + value_type operator*() const { return dereference(std::index_sequence_for{}); } /** * @brief Checks if two `zip_iterator` instances are equal. @@ -65,10 +65,7 @@ class zip_iterator { * @param other The other `zip_iterator` to compare with. * @return `true` if the iterators are equal, `false` otherwise. */ - bool operator==(const zip_iterator& other) const noexcept - { - return equal(std::index_sequence_for{}, other); - } + bool operator==(const zip_iterator& other) const { return equal(std::index_sequence_for{}, other); } /** * @brief Checks if two `zip_iterator` instances are not equal. @@ -76,17 +73,14 @@ class zip_iterator { * @param other The other `zip_iterator` to compare with. * @return `true` if the iterators are not equal, `false` otherwise. */ - bool operator!=(const zip_iterator& other) const noexcept - { - return !equal(std::index_sequence_for{}, other); - } + bool operator!=(const zip_iterator& other) const { return !equal(std::index_sequence_for{}, other); } /** * @brief Advances the `zip_iterator` by one position. * * @return A reference to the updated `zip_iterator`. */ - zip_iterator& operator++() noexcept + zip_iterator& operator++() { advance(std::index_sequence_for{}, 1); return *this; @@ -98,7 +92,7 @@ class zip_iterator { * @param offset The number of positions to advance. * @return A new `zip_iterator` advanced by the specified offset. */ - zip_iterator operator+(const std::size_t offset) const noexcept + zip_iterator operator+(const std::size_t offset) const { auto iterator = *this; iterator.advance(std::index_sequence_for{}, offset); @@ -111,7 +105,7 @@ class zip_iterator { * @param other The `zip_iterator` to measure the distance to. * @return A new `zip_iterator` advanced by the distance to the specified iterator. */ - zip_iterator operator+(const zip_iterator& other) const noexcept + zip_iterator operator+(const zip_iterator& other) const { auto iterator = *this; const auto distance = std::distance(iterator, other); @@ -124,7 +118,7 @@ class zip_iterator { * * @return A reference to the updated `zip_iterator`. */ - zip_iterator& operator--() noexcept + zip_iterator& operator--() { advance(std::index_sequence_for{}, -1); return *this; @@ -136,7 +130,7 @@ class zip_iterator { * @param offset The number of positions to move back. * @return A new `zip_iterator` moved back by the specified offset. */ - zip_iterator operator-(const int offset) const noexcept + zip_iterator operator-(const int offset) const { auto iterator = *this; iterator.advance(std::index_sequence_for{}, -offset); @@ -149,7 +143,7 @@ class zip_iterator { * @param other The `zip_iterator` to measure the distance from. * @return A new `zip_iterator` moved back by the distance to the specified iterator. */ - zip_iterator operator-(const zip_iterator& other) const noexcept + zip_iterator operator-(const zip_iterator& other) const { auto iterator = *this; const auto distance = std::distance(other, iterator); @@ -166,7 +160,7 @@ class zip_iterator { * @return A tuple containing the values pointed to by each iterator. */ template - value_type dereference(std::index_sequence) const noexcept + value_type dereference(std::index_sequence) const { return std::tie(*std::get(iterators_)...); } @@ -180,7 +174,7 @@ class zip_iterator { * @return `true` if all iterators are equal, `false` otherwise. */ template - bool equal(std::index_sequence, const zip_iterator& other) const noexcept + bool equal(std::index_sequence, const zip_iterator& other) const { return ((std::get(iterators_) == std::get(other.iterators_)) || ...); } @@ -193,7 +187,7 @@ class zip_iterator { * @param offset The number of positions to advance. */ template - void advance(std::index_sequence, const int offset) noexcept + void advance(std::index_sequence, const int offset) { ((std::advance(std::get(iterators_), offset)), ...); } @@ -208,6 +202,8 @@ class zip_iterator { * @brief A view over multiple containers simultaneously. * It allows iterating through multiple containers at once, stopping at the shortest container. * + * @note The zip never throws explicitly any exception. It all depends on what the given containers throw. + * * @tparam Containers The types of the containers to be zipped. * Each container must support standard iteration * (must have `begin()`, `end()`, `cbegin()`, and `cend()` methods). @@ -245,7 +241,7 @@ class zip { * @param containers The containers to be zipped together. * @pre At least two containers must be provided. */ - explicit zip(Containers&... containers) noexcept : containers_{containers...} {} + explicit zip(Containers&... containers) : containers_{containers...} {} /** * @brief Returns an iterator pointing to the beginning of the zipped containers. @@ -287,7 +283,7 @@ class zip { * * @return `true` if the zipped sequence is empty, `false` otherwise. */ - [[nodiscard]] bool empty() const noexcept { return begin() == end(); } + [[nodiscard]] bool empty() const { return begin() == end(); } /** * @brief Allows the `zip` object to be used in a boolean context, indicating whether the zipped sequence is @@ -368,7 +364,7 @@ class zip { * @return An iterator to the beginning of the zipped sequence. */ template - Iterator begin_impl(std::index_sequence) const noexcept + Iterator begin_impl(std::index_sequence) const { return Iterator{std::get(containers_).begin()...}; } @@ -382,7 +378,7 @@ class zip { * @return An iterator to the end of the zipped sequence. */ template - Iterator end_impl(std::index_sequence) const noexcept + Iterator end_impl(std::index_sequence) const { return std::next(Iterator{std::get(containers_).begin()...}, size()); } From 82a6873ec44a76090ba076c06ebae74d114cb0d7 Mon Sep 17 00:00:00 2001 From: Andrei Avram <6795248+andreiavrammsd@users.noreply.github.com> Date: Tue, 3 Sep 2024 17:30:14 +0000 Subject: [PATCH 2/3] Convert qa tools to python --- .devcontainer/devcontainer.json | 2 +- .github/workflows/test.yml | 8 +- .vscode/tasks.json | 20 ++-- tools/code-quality.sh | 55 --------- tools/generate-compilation-database.sh | 23 ---- tools/qa | 150 +++++++++++++++++++++++++ 6 files changed, 166 insertions(+), 92 deletions(-) delete mode 100755 tools/code-quality.sh delete mode 100755 tools/generate-compilation-database.sh create mode 100755 tools/qa diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index db29994..c38cd53 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -30,5 +30,5 @@ "seccomp=unconfined" ], "remoteUser": "vscode", - "postStartCommand": "./tools/generate-compilation-database.sh $PWD build-code-quality Debug tests" + "postStartCommand": "./tools/qa compilationdb --build-path=build-code-quality" } diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c126083..8b1d0df 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -69,8 +69,12 @@ jobs: - name: Code Quality run: | - ./tools/code-quality.sh ${{ github.event.repository.default_branch }} ${{ github.event_name }} ${{github.workspace}} \ - ${{github.workspace}}/build-code-quality ${{ matrix.config.build_type }} tests + ./tools/qa quality --src=${{github.workspace}}/include \ + --default-branch=${{ github.event.repository.default_branch }} \ + --event=${{ github.event_name }} \ + --workspace=${{github.workspace}} \ + --build-path=${{github.workspace}}/build-code-quality \ + --build-type=${{ matrix.config.build_type }} if: matrix.config.code_quality == true - name: Create Build Environment diff --git a/.vscode/tasks.json b/.vscode/tasks.json index e492e91..4f66a91 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -45,31 +45,29 @@ { "label": "Generate Compilation Database", "type": "shell", - "command": "./tools/generate-compilation-database.sh", + "command": "./tools/qa", "args": [ - "${workspaceFolder}", - "${workspaceFolder}/build-code-quality", - "Debug", - "tests" + "compilationdb", + "--build-path=${workspaceFolder}/build-code-quality" ], "problemMatcher": [] - }, + }, { "label": "Run clang-tidy on current file", "type": "shell", "command": "clang-tidy", "args": [ - "${file}", - "-p=${workspaceFolder}/build-code-quality" + "${file}", + "-p=${workspaceFolder}/build-code-quality" ], "dependsOn": "Generate Compilation Database", "problemMatcher": [] - }, - { + }, + { "label": "Generate Documentation", "type": "shell", "command": "doxygen", "problemMatcher": [] - } + } ] } diff --git a/tools/code-quality.sh b/tools/code-quality.sh deleted file mode 100755 index d5c5068..0000000 --- a/tools/code-quality.sh +++ /dev/null @@ -1,55 +0,0 @@ -#!/bin/bash - -set -e - -default_branch=$1 # master -event=$2 # pull_request|push -workspace=$3 # .. -build_path=$4 # build-dir -build_type=$5 # Debug -build_target=$6 # tests - -echo default_branch=$default_branch -echo event=$event -echo workspace=$workspace -echo build_path=$build_path -echo build_type=$build_type -echo build_target=$build_target - -if [ -z ${default_branch} ]; then - echo "Default branch is missing" - exit 1 -fi - -./tools/generate-compilation-database.sh ${workspace} ${build_path} ${build_type} ${build_target} - -# Build -cwd=$PWD -cd ${build_path} -cmake --build . --config ${build_type} --target ${build_target} -cd ${cwd} - -# Find files -files=$(if [ $event == "pull_request" ]; then - git diff --name-only origin/"${default_branch}"...HEAD include -else - git ls-files --directory include -fi | grep '\.*pp$' || true) - -# Run tools -echo - -if [ ! -z "${files}" ]; then - file_count=$(echo "${files}" | wc -w) - - echo "Format ${file_count} file(s)" - clang-format -i $files - - git diff --exit-code - echo - - echo "Tidy ${file_count} file(s)" - clang-tidy -p ${build_path} $files -else - echo "No files for code quality" -fi diff --git a/tools/generate-compilation-database.sh b/tools/generate-compilation-database.sh deleted file mode 100755 index 0de27fd..0000000 --- a/tools/generate-compilation-database.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/bin/bash -set -e - -workspace=$1 # .. -build_path=$2 # build-dir -build_type=$3 # Debug -build_target=$4 - -cwd=$PWD - -mkdir -p ${build_path} -cd ${build_path} - -# Build with clang for compilation database used by clang-tidy -cmake ${workspace} \ - -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ - -DCMAKE_C_COMPILER=clang \ - -DCMAKE_CXX_COMPILER=clang++ \ - -DCMAKE_BUILD_TYPE=${build_type} \ - -DENABLE_TESTS=ON \ - -DENABLE_BENCHMARKS=ON - -cd ${cwd} diff --git a/tools/qa b/tools/qa new file mode 100755 index 0000000..1983e8e --- /dev/null +++ b/tools/qa @@ -0,0 +1,150 @@ +#!/usr/bin/env python3 + +import subprocess +import argparse + +files_pattern = ".*pp" + + +def generate_clang_compilation_database(workspace, build_path, build_type): + subprocess.run( + [ + "cmake", + workspace, + "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON", + "-DCMAKE_C_COMPILER=clang", + "-DCMAKE_CXX_COMPILER=clang++", + "-DCMAKE_BUILD_TYPE=" + build_type, + "-DENABLE_TESTS=ON", + "-B", + build_path, + ], + check=True, + ) + + +def code_quality(src, default_branch, event, workspace, build_path, build_type): + generate_clang_compilation_database(workspace, build_path, build_type) + + # Find files + if event == "pull_request": + diff_command = f"git diff --name-only origin/{default_branch}...HEAD {src} | grep '\{files_pattern}$'" + else: + diff_command = f"git ls-files --directory {src} | grep '\{files_pattern}$'" + + files = subprocess.run( + diff_command, shell=True, capture_output=True, text=True + ).stdout.strip() + files = [f for f in files.split("\n")] + + # Run tools + if files: + file_count = len(files) + print(f"Format {file_count} file(s)") + subprocess.run(["clang-format", "-i"] + files, check=True) + + subprocess.run(["git", "diff", "--exit-code"], check=True) + print() + + print(f"Tidy {file_count} file(s)") + subprocess.run(["clang-tidy", "-p", build_path] + files, check=True) + else: + print("No files for code quality") + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description="Code quality") + + # Add subcommands + subparsers = parser.add_subparsers( + dest="command", help="Subcommands: generate or quality" + ) + + # Subcommand: generate + parser_generate = subparsers.add_parser( + "compilationdb", help="Generate the compilation database" + ) + parser_generate.add_argument( + "--workspace", + required=False, + help="Path to the workspace, e.g., ..", + default=".", + ) + parser_generate.add_argument( + "--build-path", + required=True, + help="Path to the build directory, e.g., build-dir", + ) + parser_generate.add_argument( + "--build-type", + choices=["Debug", "Release"], + required=False, + help="Build type", + default="Debug", + ) + + # Subcommand: quality + parser_quality = subparsers.add_parser("quality", help="Run code quality checks") + parser_quality.add_argument( + "--src", + required=True, + help="Source code directory", + ) + parser_quality.add_argument( + "--default-branch", + required=False, + help="Default branch name, e.g., master", + default="master", + ) + parser_quality.add_argument( + "--event", + choices=["pull_request", "push"], + required=False, + help="Event type", + default="push", + ) + parser_quality.add_argument( + "--workspace", + required=False, + help="Path to the workspace, e.g., ..", + default=".", + ) + parser_quality.add_argument( + "--build-path", + required=True, + help="Path to the build directory, e.g., build-dir", + ) + parser_quality.add_argument( + "--build-type", + choices=["Debug", "Release"], + required=False, + help="Build type", + default="Debug", + ) + + args = parser.parse_args() + + # Dispatch to the appropriate function based on the command + if args.command == "compilationdb": + generate_clang_compilation_database( + workspace=args.workspace, + build_path=args.build_path, + build_type=args.build_type, + ) + elif args.command == "quality": + print(f"default_branch={args.default_branch}") + print(f"event={args.event}") + print(f"workspace={args.workspace}") + print(f"build_path={args.build_path}") + print(f"build_type={args.build_type}") + + code_quality( + src=args.src, + default_branch=args.default_branch, + event=args.event, + workspace=args.workspace, + build_path=args.build_path, + build_type=args.build_type, + ) + else: + parser.print_help() From 58113752723e351ca33b0f582f64aa7fb3c3ecb0 Mon Sep 17 00:00:00 2001 From: Andrei Avram <6795248+andreiavrammsd@users.noreply.github.com> Date: Tue, 3 Sep 2024 17:33:17 +0000 Subject: [PATCH 3/3] Add issue --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 65bc739..0c902a7 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,7 @@ for (auto it = zip.begin(); it != zip.end(); it++) {} * Consider checked access that returns an optional reference * Run clang-tidy on file save * Cancel running jobs on new commit +* Investigate possible documentation issue: [cp: no such file or directory](https://github.com/andreiavrammsd/cpp-zip/actions/runs/10673792787/job/29583324820) * Test * Analyze if LCOV_EXCL_LINE is needed * Finish tests