From a7c4da5dea34f96dcd35e29c16c31e46c0e4f217 Mon Sep 17 00:00:00 2001 From: Andrei Avram <6795248+andreiavrammsd@users.noreply.github.com> Date: Sun, 1 Sep 2024 10:54:29 +0300 Subject: [PATCH] Dev (#4) Set devcontainer. Add test scenarios. Set exception safety. --- .clang-format | 2 +- .devcontainer/Dockerfile | 51 ++++++++++++++++++++++++++++++++ .devcontainer/devcontainer.json | 31 ++++++++++++++++++++ .vscode/c_cpp_properties.json | 2 +- .vscode/extensions.json | 9 +----- .vscode/settings.json | 5 +++- .vscode/tasks.json | 16 ++++++++-- README.md | 23 ++++++++++++--- include/msd/zip.hpp | 52 +++++++++++++++++++++++---------- tests/zip_integration_test.cpp | 32 ++++++++++++++++++-- tests/zip_iterator_test.cpp | 30 ++++++++++++++++++- tests/zip_test.cpp | 11 +++++-- 12 files changed, 225 insertions(+), 39 deletions(-) create mode 100644 .devcontainer/Dockerfile create mode 100644 .devcontainer/devcontainer.json diff --git a/.clang-format b/.clang-format index 1e7f834..b76a090 100644 --- a/.clang-format +++ b/.clang-format @@ -3,4 +3,4 @@ IndentWidth: 4 Language: Cpp PointerAlignment: Left BreakBeforeBraces: Stroustrup -ColumnLimit: 120 \ No newline at end of file +ColumnLimit: 120 diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile new file mode 100644 index 0000000..efa2504 --- /dev/null +++ b/.devcontainer/Dockerfile @@ -0,0 +1,51 @@ +# Use the official Ubuntu base image +FROM ubuntu:20.04 + +# Avoid interactive package installation +ENV DEBIAN_FRONTEND=noninteractive + +# Install required packages +RUN apt update && apt install -y \ + build-essential \ + cmake \ + clang-tidy \ + clang-format \ + lcov \ + git \ + curl \ + wget \ + unzip \ + python3 \ + python3-pip \ + gdb \ + lldb \ + clang \ + llvm \ + nano \ + software-properties-common \ + && rm -rf /var/lib/apt/lists/* + +# Create a non-root user +ARG USERNAME=vscode +ARG USER_UID=1000 +ARG USER_GID=$USER_UID + +RUN groupadd -g $USER_GID $USERNAME || true \ + && useradd --uid $USER_UID --gid $USER_GID -m $USERNAME -s /bin/bash \ + && apt update \ + && apt install -y sudo \ + && echo $USERNAME ALL=\(ALL\) NOPASSWD:ALL > /etc/sudoers.d/$USERNAME \ + && chmod 0440 /etc/sudoers.d/$USERNAME + +# History +RUN SNIPPET="export PROMPT_COMMAND='history -a' && export HISTFILE=/commandhistory/.bash_history" \ + && mkdir /commandhistory \ + && touch /commandhistory/.bash_history \ + && chown -R $USERNAME /commandhistory \ + && echo "$SNIPPET" >> "/home/$USERNAME/.bashrc" + + # Set the user to vscode +USER $USERNAME + +# Set the workspace directory +WORKDIR /workspace diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..ffd28b0 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,31 @@ +{ + "name": "C++ Development with CMake and Bazel", + "dockerFile": "Dockerfile", + "customizations": { + "vscode": { + "extensions": [ + "ms-vscode.cmake-tools", + "ms-vscode.cpptools", + "ms-vscode.cpptools-extension-pack", + "xaver.clang-format", + "twxs.cmake", + "fredericbonnet.cmake-test-adapter", + "ms-vscode.cmake-tools", + "notskm.clang-tidy", + "ryanluker.vscode-coverage-gutters", + ] + } + }, + "mounts": [ + "source=${localWorkspaceFolder},target=/workspace,type=bind,consistency=cached", + "source=${env:HOME}/.ssh,target=/home/vscode/.ssh,type=bind,consistency=cached", + "source=history,target=/commandhistory,type=volume" + ], + "workspaceFolder": "/workspace", + "runArgs": [ + "--cap-add=SYS_PTRACE", + "--security-opt", + "seccomp=unconfined" + ], + "remoteUser": "vscode" +} diff --git a/.vscode/c_cpp_properties.json b/.vscode/c_cpp_properties.json index a371949..25425b8 100644 --- a/.vscode/c_cpp_properties.json +++ b/.vscode/c_cpp_properties.json @@ -17,7 +17,7 @@ "${workspaceFolder}/**" ], "defines": [], - "compilerPath": "/usr/bin/gcc-11", + "compilerPath": "/usr/bin/gcc-9", "cStandard": "c17", "intelliSenseMode": "linux-clang-x64", "configurationProvider": "ms-vscode.cmake-tools" diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 624c3e4..933c580 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,12 +1,5 @@ { "recommendations": [ - "ms-vscode.cpptools", - "ms-vscode.cpptools-extension-pack", - "xaver.clang-format", - "twxs.cmake", - "fredericbonnet.cmake-test-adapter", - "ms-vscode.cmake-tools", - "notskm.clang-tidy", - "ryanluker.vscode-coverage-gutters", + "ms-vscode-remote.remote-containers", ] } diff --git a/.vscode/settings.json b/.vscode/settings.json index 077d174..ef3f263 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -27,8 +27,11 @@ "coverage.cobertura.xml" ], "C_Cpp.default.configurationProvider": "ms-vscode.cmake-tools", + "C_Cpp.codeAnalysis.clangTidy.enabled": true, "C_Cpp.codeAnalysis.clangTidy.useBuildPath": true, - "clang-tidy.buildPath": "build", + "C_Cpp.codeAnalysis.clangTidy.args": [ + "-p=build" + ], "files.associations": { "array": "cpp", "*.tcc": "cpp", diff --git a/.vscode/tasks.json b/.vscode/tasks.json index b08bf6d..475ae25 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -7,7 +7,7 @@ "command": "mkdir -p build", "group": { "kind": "build", - "isDefault": true + "isDefault": false }, "options": { "cwd": "${workspaceFolder}" @@ -21,7 +21,7 @@ "dependsOn": "Create build directory", "group": { "kind": "build", - "isDefault": true + "isDefault": false }, "options": { "cwd": "${workspaceFolder}/build" @@ -41,6 +41,16 @@ "cwd": "${workspaceFolder}/build" }, "problemMatcher": [] - } + }, + { + "label": "Run clang-tidy on current file", + "type": "shell", + "command": "clang-tidy", + "args": [ + "${file}", + "-p=build" + ], + "problemMatcher": ["$gcc"] + } ] } diff --git a/README.md b/README.md index 0f85eb2..0f83a2a 100644 --- a/README.md +++ b/README.md @@ -47,20 +47,35 @@ int main() { ``` -See [tests](tests/zip_test.cpp). +See [tests](tests/). + +## Development + +### Tools +* [SSH keys for GitHub](https://help.ubuntu.com/community/SSH/OpenSSH/Keys) (~/.ssh) +* [VS Code](https://code.visualstudio.com/) (see [.vscode/extensions.json](.vscode/extensions.json)) +* [Docker](https://docs.docker.com/engine/install/ubuntu/) + +### Actions +* Debug: F5 +* Coverage: Ctrl + Shift + P -> Run Task -> Generate Coverage Report +* Show coverage inline: Ctrl + Shift + 7 OR Ctrl + Shift + P -> Coverage Gutters: Display Coverage +* Coverage as HTML: See build/coverage_html/index.html ## TODO -* Write size() as end() - begin() * Exception guarantees * constexpr * const correctness * Write benchmarks * ContainersAndAlgorithms test fails at `EXPECT_EQ(it, std::prev(const_zip.end()));` for std::list + * Can the zip iterator really be bidirectional? + * Document or conditionaly set the iterator tag by the containers types * Consider checked access that returns an optional reference -* List tools required for dev or create devcontainer +* Do not allow to mix begin/end with cbegin/cend +* Run clang-tidy, clang-format in CI/file save * Test - * 100% coverage + * Analyze if LCOV_EXCL_LINE is needed * Finish tests * With std algorithms * Entire API in non-const and const context diff --git a/include/msd/zip.hpp b/include/msd/zip.hpp index 9a84f27..fa0b91f 100644 --- a/include/msd/zip.hpp +++ b/include/msd/zip.hpp @@ -19,13 +19,19 @@ class zip_iterator { using pointer = std::tuple::pointer...>; using reference = std::tuple::reference...>; - explicit zip_iterator(Iterators... iterators) : iterators_{iterators...} {} + explicit zip_iterator(Iterators... iterators) noexcept : iterators_{iterators...} {} - value_type operator*() const { return dereference(std::index_sequence_for{}); } + value_type operator*() const noexcept { return dereference(std::index_sequence_for{}); } - bool operator==(const zip_iterator& other) const { return equal(std::index_sequence_for{}, other); } + 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); } + bool operator!=(const zip_iterator& other) const noexcept + { + return !equal(std::index_sequence_for{}, other); + } zip_iterator& operator++() noexcept { @@ -33,41 +39,57 @@ class zip_iterator { return *this; } + zip_iterator operator+(const std::size_t offset) const noexcept + { + auto iterator = *this; + iterator.advance(std::index_sequence_for{}, offset); + return iterator; + } + + zip_iterator operator+(const zip_iterator& other) const noexcept + { + auto iterator = *this; + const auto distance = std::distance(iterator, other); + iterator.advance(std::index_sequence_for{}, distance); + return iterator; + } + zip_iterator& operator--() noexcept { advance(std::index_sequence_for{}, -1); return *this; } - zip_iterator operator+(const std::size_t offset) const noexcept + zip_iterator operator-(const int offset) const noexcept { auto iterator = *this; - iterator.advance(std::index_sequence_for{}, offset); + iterator.advance(std::index_sequence_for{}, -offset); return iterator; } - zip_iterator operator-(const int offset) const noexcept + zip_iterator operator-(const zip_iterator& other) const noexcept { auto iterator = *this; - iterator.advance(std::index_sequence_for{}, -offset); + const auto distance = std::distance(other, iterator); + iterator.advance(std::index_sequence_for{}, -distance); return iterator; } private: template - value_type dereference(std::index_sequence) const + value_type dereference(std::index_sequence) const noexcept { return std::tie(*std::get(iterators_)...); } template - bool equal(std::index_sequence, const zip_iterator& other) const + bool equal(std::index_sequence, const zip_iterator& other) const noexcept { return ((std::get(iterators_) == std::get(other.iterators_)) || ...); } template - void advance(std::index_sequence, int offset) + void advance(std::index_sequence, const int offset) noexcept { ((std::advance(std::get(iterators_), offset)), ...); } @@ -86,7 +108,7 @@ class zip { using value_type = typename iterator::value_type; - explicit zip(Containers&... containers) : containers_{containers...} {} + explicit zip(Containers&... containers) noexcept : containers_{containers...} {} iterator begin() const { return begin_impl(std::index_sequence_for{}); } iterator end() const { return end_impl(std::index_sequence_for{}); } @@ -108,7 +130,7 @@ class zip { value_type front() const { - assert(!empty()); + assert(!empty()); // LCOV_EXCL_LINE return *begin(); } @@ -132,13 +154,13 @@ class zip { private: template - iterator begin_impl(std::index_sequence) const + iterator begin_impl(std::index_sequence) const noexcept { return iterator{std::get(containers_).begin()...}; } template - iterator end_impl(std::index_sequence) const + iterator end_impl(std::index_sequence) const noexcept { return std::next(iterator{std::get(containers_).begin()...}, size()); } diff --git a/tests/zip_integration_test.cpp b/tests/zip_integration_test.cpp index 7d6c68e..f7d02c5 100644 --- a/tests/zip_integration_test.cpp +++ b/tests/zip_integration_test.cpp @@ -6,7 +6,10 @@ #include #include #include +#include #include +#include +#include #include #include #include @@ -19,7 +22,7 @@ TEST(ZipIntegrationTest, ContainersAndAlgorithms) std::deque deque{1, 2}; std::list list{1, 2, 3}; std::forward_list forward_list{1, 2, 3, 4}; - std::array array{1, 2, 3, 4, 5}; + const std::array array{1, 2, 3, 4, 5}; std::string string{"123456"}; std::set set{1, 2, 3, 4, 5, 6}; std::multiset multiset{1, 2, 3, 4, 5, 6}; @@ -57,9 +60,32 @@ TEST(ZipIntegrationTest, ContainersAndAlgorithms) auto&& [current_l, current_f] = current; auto&& [next_l, next_f] = next; - std::cout << current_l << ", " << current_f << " vs " << next_l << ", " << next_f << "\n\n"; - return current_l + current_f + next_l + next_f == 6; }); EXPECT_EQ(adjacent_iterator, adjacent_zip.begin()); + + msd::zip zip2(list, set); + auto iter = zip2.begin(); + + std::size_t iterations = 0; + while (iter != zip2.end()) { // NOLINT(altera-id-dependent-backward-branch) + auto [a, b] = *iter; + EXPECT_EQ(a, b); + ++iterations; + ++iter; + } + EXPECT_EQ(iterations, 3); + + iterations = 0; + for (auto it = zip2.cbegin(); it != zip2.cend(); ++it) { // NOLINT(altera-id-dependent-backward-branch) + ++iterations; + } + EXPECT_EQ(iterations, 3); + + iterations = 0; + // NOLINTNEXTLINE(altera-id-dependent-backward-branch) + for (auto it = std::next(zip2.begin()); it != std::prev(zip2.end()); ++it) { + ++iterations; + } + EXPECT_EQ(iterations, 1); } diff --git a/tests/zip_iterator_test.cpp b/tests/zip_iterator_test.cpp index ab52325..772b47d 100644 --- a/tests/zip_iterator_test.cpp +++ b/tests/zip_iterator_test.cpp @@ -65,9 +65,14 @@ TEST_F(ZipIteratorTest, OperatorEqual) { const auto copy = begin_iterator_; EXPECT_EQ(copy, begin_iterator_); + EXPECT_EQ(std::prev(end_iterator_), std::next(begin_iterator_)); } -TEST_F(ZipIteratorTest, OperatorNotEqual) { EXPECT_NE(end_iterator_, begin_iterator_); } +TEST_F(ZipIteratorTest, OperatorNotEqual) +{ + EXPECT_NE(end_iterator_, begin_iterator_); + EXPECT_NE(std::prev(end_iterator_), begin_iterator_); +} TEST_F(ZipIteratorTest, OperatorPreIncrement) { @@ -109,6 +114,11 @@ TEST_F(ZipIteratorTest, OperatorPlus) EXPECT_EQ(end, end_iterator_); } +TEST_F(ZipIteratorTest, OperatorPlusIterator) +{ + EXPECT_EQ(begin_iterator_ + std::next(begin_iterator_, 2), end_iterator_); +} + TEST_F(ZipIteratorTest, OperatorMinus) { const auto iterator = end_iterator_ - 1; @@ -121,3 +131,21 @@ TEST_F(ZipIteratorTest, OperatorMinus) const auto begin = end_iterator_ - 2; EXPECT_EQ(begin, begin_iterator_); } + +TEST_F(ZipIteratorTest, OperatorMinusIterator) +{ + EXPECT_EQ(end_iterator_ - std::prev(end_iterator_, 2), begin_iterator_); +} + +TEST_F(ZipIteratorTest, Prev) { EXPECT_EQ(std::prev(end_iterator_), end_iterator_ - 1); } + +TEST_F(ZipIteratorTest, Next) { EXPECT_EQ(std::next(begin_iterator_), begin_iterator_ + 1); } + +TEST_F(ZipIteratorTest, Advance) +{ + std::advance(begin_iterator_, 2); + std::advance(end_iterator_, -1); + EXPECT_EQ(begin_iterator_, end_iterator_); +} + +TEST_F(ZipIteratorTest, Distance) { EXPECT_EQ(std::distance(begin_iterator_, end_iterator_), 2); } diff --git a/tests/zip_test.cpp b/tests/zip_test.cpp index 35948de..b39178f 100644 --- a/tests/zip_test.cpp +++ b/tests/zip_test.cpp @@ -169,9 +169,12 @@ TEST_F(ZipTest, FrontWhenZipIsEmpty) { const std::array non_empty{1, 2, 3}; std::vector empty{}; - msd::zip zip(non_empty, empty); + msd::zip zip(non_empty, empty); EXPECT_DEBUG_DEATH(zip.front(), ""); + + const msd::zip const_zip = zip; + EXPECT_DEBUG_DEATH(const_zip.front(), ""); } TEST_F(ZipTest, Back) @@ -215,7 +218,11 @@ TEST_F(ZipTest, OperatorSubscript) EXPECT_EQ(std::get<2>(value), 6); } -TEST_F(ZipTest, OperatorSubscriptWithIndexOutOfRange) { EXPECT_DEBUG_DEATH(zip_[99], ""); } +TEST_F(ZipTest, OperatorSubscriptWithIndexOutOfRange) +{ + EXPECT_DEBUG_DEATH(zip_[99], ""); + EXPECT_DEBUG_DEATH(const_zip_[99], ""); +} TEST_F(ZipTest, NoCopiesAndMovesOfContainersWhileIterating) {