From 3a182f3d0cd769322c6f690da1d8f8033389c4c5 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 21:15:26 +0100 Subject: [PATCH 01/25] Bootstrap project --- .clang-format | 23 ++++ .envrc | 5 + .gitignore | 7 ++ .pre-commit-config.yaml | 1 + .woodpecker/check.yml | 31 +++++ flake.lock | 140 +++++++++++++++++++++++ flake.nix | 112 ++++++++++++++++++ meson.build | 13 +++ src/include/interval-map/interval-map.hh | 1 + src/meson.build | 1 + tests/meson.build | 1 + tests/unit/meson.build | 16 +++ tests/unit/unit_test.cc | 7 ++ 13 files changed, 358 insertions(+) create mode 100644 .clang-format create mode 100644 .envrc create mode 100644 .gitignore create mode 120000 .pre-commit-config.yaml create mode 100644 .woodpecker/check.yml create mode 100644 flake.lock create mode 100644 flake.nix create mode 100644 meson.build create mode 100644 src/include/interval-map/interval-map.hh create mode 100644 src/meson.build create mode 100644 tests/meson.build create mode 100644 tests/unit/meson.build create mode 100644 tests/unit/unit_test.cc diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..19c58aa --- /dev/null +++ b/.clang-format @@ -0,0 +1,23 @@ +# vim: ft=yaml +--- +BasedOnStyle: LLVM +IndentWidth: 4 +--- +Language: Cpp +# Force pointers to the type for C++. +DerivePointerAlignment: false +PointerAlignment: Left + +# Short functions should not be on a single line, unless empty +AllowShortFunctionsOnASingleLine: Empty + +# Make them level +AccessModifierOffset: -4 + +# It makes more sense this way +BreakBeforeBinaryOperators: All +BreakBeforeTernaryOperators: true + +# Aesthetic +AlignOperands: AlignAfterOperator +--- diff --git a/.envrc b/.envrc new file mode 100644 index 0000000..de77fcb --- /dev/null +++ b/.envrc @@ -0,0 +1,5 @@ +if ! has nix_direnv_version || ! nix_direnv_version 3.0.0; then + source_url "https://raw.githubusercontent.com/nix-community/nix-direnv/3.0.0/direnvrc" "sha256-21TMnI2xWX7HkSTjFFri2UaohXVj854mgvWapWrxRXg=" +fi + +use flake diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..09ba440 --- /dev/null +++ b/.gitignore @@ -0,0 +1,7 @@ +# CMake build directories +/build +/build-* + +# Nix generated files +/.pre-commit-config.yaml +/result diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 120000 index 0000000..d433189 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1 @@ +/nix/store/jn4b7bg3pb1ixnpmr8g4q6qbbciqqfhp-pre-commit-config.json \ No newline at end of file diff --git a/.woodpecker/check.yml b/.woodpecker/check.yml new file mode 100644 index 0000000..272c0e4 --- /dev/null +++ b/.woodpecker/check.yml @@ -0,0 +1,31 @@ +labels: + backend: local + +steps: +- name: pre-commit check + image: bash + commands: + - nix develop --command pre-commit run --all + +- name: nix flake check + image: bash + commands: + - nix flake check + +- name: notify + image: bash + environment: + ADDRESS: + from_secret: matrix_homeserver + ROOM: + from_secret: matrix_roomid + USER: + from_secret: matrix_username + PASS: + from_secret: matrix_password + commands: + - nix run github:ambroisie/matrix-notifier + when: + status: + - failure + - success diff --git a/flake.lock b/flake.lock new file mode 100644 index 0000000..a4cfc13 --- /dev/null +++ b/flake.lock @@ -0,0 +1,140 @@ +{ + "nodes": { + "flake-compat": { + "flake": false, + "locked": { + "lastModified": 1696426674, + "narHash": "sha256-kvjfFW7WAETZlt09AgDn1MrtKzP7t90Vf7vypd3OL1U=", + "owner": "edolstra", + "repo": "flake-compat", + "rev": "0f9255e01c2351cc7d116c072cb317785dd33b33", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, + "futils": { + "inputs": { + "systems": "systems" + }, + "locked": { + "lastModified": 1710146030, + "narHash": "sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "b1d9ab70662946ef0850d488da1c9019f3a9752a", + "type": "github" + }, + "original": { + "owner": "numtide", + "ref": "main", + "repo": "flake-utils", + "type": "github" + } + }, + "gitignore": { + "inputs": { + "nixpkgs": [ + "pre-commit-hooks", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1709087332, + "narHash": "sha256-HG2cCnktfHsKV0s4XW83gU3F57gaTljL9KNSuG6bnQs=", + "owner": "hercules-ci", + "repo": "gitignore.nix", + "rev": "637db329424fd7e46cf4185293b9cc8c88c95394", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "gitignore.nix", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1724224976, + "narHash": "sha256-Z/ELQhrSd7bMzTO8r7NZgi9g5emh+aRKoCdaAv5fiO0=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "c374d94f1536013ca8e92341b540eba4c22f9c62", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "nixpkgs-stable": { + "locked": { + "lastModified": 1720386169, + "narHash": "sha256-NGKVY4PjzwAa4upkGtAMz1npHGoRzWotlSnVlqI40mo=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "194846768975b7ad2c4988bdb82572c00222c0d7", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-24.05", + "repo": "nixpkgs", + "type": "github" + } + }, + "pre-commit-hooks": { + "inputs": { + "flake-compat": "flake-compat", + "gitignore": "gitignore", + "nixpkgs": [ + "nixpkgs" + ], + "nixpkgs-stable": "nixpkgs-stable" + }, + "locked": { + "lastModified": 1724440431, + "narHash": "sha256-9etXEOUtzeMgqg1u0wp+EdwG7RpmrAZ2yX516bMj2aE=", + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "rev": "c8a54057aae480c56e28ef3e14e4960628ac495b", + "type": "github" + }, + "original": { + "owner": "cachix", + "ref": "master", + "repo": "pre-commit-hooks.nix", + "type": "github" + } + }, + "root": { + "inputs": { + "futils": "futils", + "nixpkgs": "nixpkgs", + "pre-commit-hooks": "pre-commit-hooks" + } + }, + "systems": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 0000000..9cd1afa --- /dev/null +++ b/flake.nix @@ -0,0 +1,112 @@ +{ + description = "Interval Map coding exercise"; + + inputs = { + futils = { + type = "github"; + owner = "numtide"; + repo = "flake-utils"; + ref = "main"; + }; + + nixpkgs = { + type = "github"; + owner = "NixOS"; + repo = "nixpkgs"; + ref = "nixos-unstable"; + }; + + pre-commit-hooks = { + type = "github"; + owner = "cachix"; + repo = "pre-commit-hooks.nix"; + ref = "master"; + inputs = { + flake-utils.follows = "futils"; + nixpkgs.follows = "nixpkgs"; + }; + }; + }; + + outputs = { self, futils, nixpkgs, pre-commit-hooks }: + { + overlays = { + default = final: _prev: { + interval-map = with final; stdenv.mkDerivation { + pname = "interval-map"; + version = "0.0.0"; + + src = self; + + nativeBuildInputs = with pkgs; [ + meson + ninja + pkg-config + ]; + + checkInputs = with pkgs; [ + gtest + ]; + + doCheck = true; + + meta = with lib; { + description = "An implementation of interval_map"; + homepage = "https://git.belanyi.fr/ambroisie/interval-map"; + license = licenses.mit; + maintainers = with maintainers; [ ambroisie ]; + platforms = platforms.unix; + }; + }; + }; + }; + } // futils.lib.eachDefaultSystem (system: + let + pkgs = import nixpkgs { + inherit system; + overlays = [ + self.overlays.default + ]; + }; + + pre-commit = pre-commit-hooks.lib.${system}.run { + src = self; + + hooks = { + nixpkgs-fmt = { + enable = true; + }; + + clang-format = { + enable = true; + }; + }; + }; + in + { + checks = { + inherit (self.packages.${system}) interval-map; + + inherit pre-commit; + }; + + devShells = { + default = pkgs.mkShell { + inputsFrom = with self.packages.${system}; [ + interval-map + ]; + + packages = with pkgs; [ + clang-tools + ]; + + inherit (pre-commit) shellHook; + }; + }; + + packages = futils.lib.flattenTree { + default = pkgs.interval-map; + inherit (pkgs) interval-map; + }; + }); +} diff --git a/meson.build b/meson.build new file mode 100644 index 0000000..067c27e --- /dev/null +++ b/meson.build @@ -0,0 +1,13 @@ +project( + 'interval-map', + 'cpp', + version : '0.0.0', + license: 'MIT', + default_options : [ + 'cpp_std=c++20', + 'warning_level=3', + ], +) + +subdir('src') +subdir('tests') diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh new file mode 100644 index 0000000..6f70f09 --- /dev/null +++ b/src/include/interval-map/interval-map.hh @@ -0,0 +1 @@ +#pragma once diff --git a/src/meson.build b/src/meson.build new file mode 100644 index 0000000..0e8c25d --- /dev/null +++ b/src/meson.build @@ -0,0 +1 @@ +interval_map_dep = declare_dependency(include_directories: 'include') diff --git a/tests/meson.build b/tests/meson.build new file mode 100644 index 0000000..082b746 --- /dev/null +++ b/tests/meson.build @@ -0,0 +1 @@ +subdir('unit') diff --git a/tests/unit/meson.build b/tests/unit/meson.build new file mode 100644 index 0000000..3e243c2 --- /dev/null +++ b/tests/unit/meson.build @@ -0,0 +1,16 @@ +gtest_dep = dependency( + 'gtest', + main : true, + required : false, +) + +dummy_test = executable( + 'unit_test', + 'unit_test.cc', + dependencies : [ + gtest_dep, + interval_map_dep + ], +) + +test('unit test', dummy_test) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc new file mode 100644 index 0000000..e488c79 --- /dev/null +++ b/tests/unit/unit_test.cc @@ -0,0 +1,7 @@ +#include + +#include + +TEST(misc, passing) { + ASSERT_EQ(1, 1); +} From e16ecf060b064768a7188804c97024221f2965a7 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 21:40:22 +0100 Subject: [PATCH 02/25] Add given code --- src/include/interval-map/interval-map.hh | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 6f70f09..ce1d9dd 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -1 +1,27 @@ #pragma once + +#include + +namespace amby { + +template class interval_map { +public: + interval_map(V const& init) : init_(init) {} + + void assign(K const& begin, K const& end, V const& val) { + // TODO: implement + } + + V const& operator[](K const& key) const { + auto it = underlying_.upper_bound(key); + if (it == underlying_.begin()) + return init_; + return std::prev(it)->second; + } + +private: + V init_; + std::map underlying_{}; +}; + +} // namespace amby From 9d2e062c386b7e010c6e19c4f34a08fb5e39ab71 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 21:55:33 +0100 Subject: [PATCH 03/25] Add test for minimal 'interval_map' type interface Add `KeyInterface` (respectively `ValueInterface`). Those classes provide the minimum documented interface for `K` (respectively `V`) in `interval_map`. --- tests/unit/unit_test.cc | 65 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index e488c79..288c811 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -2,6 +2,67 @@ #include -TEST(misc, passing) { - ASSERT_EQ(1, 1); +#include + +template class KeyInterface { +public: + explicit KeyInterface(T val) : underlying_(val) {} + + KeyInterface(KeyInterface const&) = default; + KeyInterface& operator=(KeyInterface const&) = default; + + bool operator<(KeyInterface const& other) const { + return underlying_ < other.underlying_; + } + +private: + T underlying_; +}; + +template struct std::numeric_limits> { + static KeyInterface lowest() { + return KeyInterface(std::numeric_limits::lowest()); + } +}; + +static_assert(std::is_copy_constructible_v>); +static_assert(std::is_copy_assignable_v>); +static_assert( + std::is_same_v, + decltype(std::numeric_limits>::lowest())>); + +template class ValueInterface { +public: + explicit ValueInterface(T val) : underlying_(val) {} + + ValueInterface(ValueInterface const&) = default; + ValueInterface& operator=(ValueInterface const&) = default; + + bool operator==(ValueInterface const& other) const { + return underlying_ == other.underlying_; + } + +private: + T underlying_; +}; + +template struct std::numeric_limits> { + static ValueInterface lowest() { + return ValueInterface(std::numeric_limits::lowest()); + } +}; + +static_assert(std::is_copy_constructible_v>); +static_assert(std::is_copy_assignable_v>); +static_assert(std::is_same_v< + ValueInterface, + decltype(std::numeric_limits>::lowest())>); + +TEST(interval_map, minimal_interface) { + using Key = KeyInterface; + using Value = ValueInterface; + + auto map = amby::interval_map{Value(0)}; + ASSERT_EQ(map[Key(0)], Value(0)); + map.assign(Key(0), Key(1), Value(1)); } From b23af215e8ab0a59a444a521c2030b1905604158 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 22:13:34 +0100 Subject: [PATCH 04/25] Handle empty range insertion --- src/include/interval-map/interval-map.hh | 2 ++ tests/unit/unit_test.cc | 26 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index ce1d9dd..8f806c3 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -9,6 +9,8 @@ public: interval_map(V const& init) : init_(init) {} void assign(K const& begin, K const& end, V const& val) { + if (!(begin < end)) + return; // TODO: implement } diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index 288c811..b2b990e 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -66,3 +66,29 @@ TEST(interval_map, minimal_interface) { ASSERT_EQ(map[Key(0)], Value(0)); map.assign(Key(0), Key(1), Value(1)); } + +TEST(interval_map, no_insertion) { + auto map = amby::interval_map{0}; + for (int i = std::numeric_limits::min(); + i <= std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], 0); + } +} + +TEST(interval_map, insert_begin_equal_end) { + auto map = amby::interval_map{0}; + map.assign(0, 0, 1); + for (int i = std::numeric_limits::min(); + i <= std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], 0); + } +} + +TEST(interval_map, insert_begin_bigger_than_end) { + auto map = amby::interval_map{0}; + map.assign(1, 0, 1); + for (int i = std::numeric_limits::min(); + i <= std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], 0); + } +} From 5868b5d36cd358b7914efb48fb4398dba85178e9 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:15:12 +0100 Subject: [PATCH 05/25] Introduce 'IntervalMapTest' fixture --- tests/unit/unit_test.cc | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index b2b990e..3cdf79e 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -58,7 +58,20 @@ static_assert(std::is_same_v< ValueInterface, decltype(std::numeric_limits>::lowest())>); -TEST(interval_map, minimal_interface) { +class IntervalMapTest : public testing::Test { +protected: + using key_type = char; + using value_type = int; + using map_type = amby::interval_map; + + map_type map{0}; + + void SetUp() override { + map = map_type{0}; + } +}; + +TEST_F(IntervalMapTest, minimal_interface) { using Key = KeyInterface; using Value = ValueInterface; @@ -67,16 +80,14 @@ TEST(interval_map, minimal_interface) { map.assign(Key(0), Key(1), Value(1)); } -TEST(interval_map, no_insertion) { - auto map = amby::interval_map{0}; +TEST_F(IntervalMapTest, no_insertion) { for (int i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) { ASSERT_EQ(map[i], 0); } } -TEST(interval_map, insert_begin_equal_end) { - auto map = amby::interval_map{0}; +TEST_F(IntervalMapTest, insert_begin_equal_end) { map.assign(0, 0, 1); for (int i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) { @@ -84,8 +95,7 @@ TEST(interval_map, insert_begin_equal_end) { } } -TEST(interval_map, insert_begin_bigger_than_end) { - auto map = amby::interval_map{0}; +TEST_F(IntervalMapTest, insert_begin_bigger_than_end) { map.assign(1, 0, 1); for (int i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) { From ce7cc4492ce867e8632f55a930d123e937d53af1 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:16:39 +0100 Subject: [PATCH 06/25] Explicitly test for canonicity This will come in handy later, with more complex test cases. --- src/include/interval-map/interval-map.hh | 6 ++++++ tests/unit/unit_test.cc | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 8f806c3..4dc2335 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -2,6 +2,9 @@ #include +// Testing class forward declaration +class IntervalMapTest; + namespace amby { template class interval_map { @@ -21,6 +24,9 @@ public: return std::prev(it)->second; } + // Used in testing + friend class ::IntervalMapTest; + private: V init_; std::map underlying_{}; diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index 3cdf79e..d51db5a 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -69,6 +69,27 @@ protected: void SetUp() override { map = map_type{0}; } + + void TearDown() override { + check_canonicity(); + } + + void check_canonicity() const { + // Consecutive map entries must not contain the same value + for (auto it = map.underlying_.begin(); it != map.underlying_.end(); + ++it) { + const auto next = std::next(it, 1); + if (next == map.underlying_.end()) + break; + EXPECT_NE(it->second, next->second); + } + + // The first entry must not contain the initial value + if (const auto it = map.underlying_.begin(); + it != map.underlying_.end()) { + EXPECT_NE(it->second, map.init_); + } + } }; TEST_F(IntervalMapTest, minimal_interface) { From 1126ee30d8e762a79b4575988d2e1ee3552bba6c Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:38:05 +0100 Subject: [PATCH 07/25] Test against fake 'Model' implementation This removes most of the redundant `for` loops that would appear in hand-written unit tests, and could potentially open the door to fuzz-testing/property-test the implementation for a more in-depth test-suite. --- tests/unit/model.hh | 35 ++++++++++++++++++++++++++++++++ tests/unit/unit_test.cc | 44 ++++++++++++++++++++++++++--------------- 2 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 tests/unit/model.hh diff --git a/tests/unit/model.hh b/tests/unit/model.hh new file mode 100644 index 0000000..9acb19f --- /dev/null +++ b/tests/unit/model.hh @@ -0,0 +1,35 @@ +#pragma once + +#include + +template struct Range { + K begin; + K end; + V val; +}; + +// Same behaviour as interval_map, but implementation is trivally correct +template class Model { +public: + Model(V const& init) : init_(init) {} + + void assign(K const& begin, K const& end, V const& val) { + if (!(begin < end)) + return; + ranges_.emplace_back(begin, end, val); + } + + V const& operator[](K const& key) const { + for (auto it = ranges_.rbegin(); it != ranges_.rend(); ++it) { + if (key < it->begin) + continue; + if (it->end <= key) + continue; + return it->val; + } + return init_; + } + + V init_; + std::vector> ranges_{}; +}; diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index d51db5a..7b9e4ce 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -4,6 +4,8 @@ #include +#include "model.hh" + template class KeyInterface { public: explicit KeyInterface(T val) : underlying_(val) {} @@ -63,17 +65,40 @@ protected: using key_type = char; using value_type = int; using map_type = amby::interval_map; + using model_type = Model; map_type map{0}; + model_type model{0}; void SetUp() override { map = map_type{0}; + model = model_type{0}; } void TearDown() override { + check(); + } + + void assign(key_type const& begin, key_type const& end, + value_type const& val) { + map.assign(begin, end, val); + model.assign(begin, end, val); + } + + void check() const { + check_ranges(); check_canonicity(); } + // Compare against the fake 'Model' implementation + void check_ranges() const { + auto i = std::numeric_limits::min(); + for (; i < std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], model[i]) << "(i: " << +i << ")"; + } + ASSERT_EQ(map[i], model[i]) << "(i: " << +i << ")"; + }; + void check_canonicity() const { // Consecutive map entries must not contain the same value for (auto it = map.underlying_.begin(); it != map.underlying_.end(); @@ -101,25 +126,12 @@ TEST_F(IntervalMapTest, minimal_interface) { map.assign(Key(0), Key(1), Value(1)); } -TEST_F(IntervalMapTest, no_insertion) { - for (int i = std::numeric_limits::min(); - i <= std::numeric_limits::max(); ++i) { - ASSERT_EQ(map[i], 0); - } -} +TEST_F(IntervalMapTest, no_insertion) {} TEST_F(IntervalMapTest, insert_begin_equal_end) { - map.assign(0, 0, 1); - for (int i = std::numeric_limits::min(); - i <= std::numeric_limits::max(); ++i) { - ASSERT_EQ(map[i], 0); - } + assign(0, 0, 1); } TEST_F(IntervalMapTest, insert_begin_bigger_than_end) { - map.assign(1, 0, 1); - for (int i = std::numeric_limits::min(); - i <= std::numeric_limits::max(); ++i) { - ASSERT_EQ(map[i], 0); - } + assign(1, 0, 1); } From 9d9091630712bb360d788dbb5449c002bb1e7025 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:49:52 +0100 Subject: [PATCH 08/25] Add implementation --- src/include/interval-map/interval-map.hh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 4dc2335..1352b79 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -14,7 +14,16 @@ public: void assign(K const& begin, K const& end, V const& val) { if (!(begin < end)) return; - // TODO: implement + + auto const end_val = (*this)[end]; + underlying_.erase(underlying_.lower_bound(begin), + underlying_.upper_bound(end)); + if (!((*this)[begin] == val)) { + underlying_.insert({begin, val}); + } + if (!((*this)[end] == end_val)) { + underlying_.insert({end, end_val}); + } } V const& operator[](K const& key) const { From 301945884ab80083cab1090a9ece4b167020dcdf Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 16:48:30 +0100 Subject: [PATCH 09/25] Add initial tests Those were the ones I used for the initial implementation. --- tests/unit/unit_test.cc | 67 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index 7b9e4ce..a99c643 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -135,3 +135,70 @@ TEST_F(IntervalMapTest, insert_begin_equal_end) { TEST_F(IntervalMapTest, insert_begin_bigger_than_end) { assign(1, 0, 1); } + +TEST_F(IntervalMapTest, insert_one_range) { + assign(std::numeric_limits::min(), 0, 1); +} + +TEST_F(IntervalMapTest, insert_non_overlapping_ranges) { + assign(std::numeric_limits::min(), 0, 1); + assign(10, std::numeric_limits::max(), 2); +} + +TEST_F(IntervalMapTest, insert_up_to_max) { + assign(std::numeric_limits::min(), + std::numeric_limits::max(), 1); +} + +TEST_F(IntervalMapTest, insert_range_right_after) { + assign(0, 10, 1); + assign(10, 20, 1); +} + +TEST_F(IntervalMapTest, insert_range_right_before) { + assign(10, 20, 1); + assign(0, 10, 1); +} + +TEST_F(IntervalMapTest, insert_range_middle) { + assign(0, 10, 1); + assign(20, 30, 1); + assign(10, 20, 1); +} + +TEST_F(IntervalMapTest, insert_range_inside_another) { + assign(0, 20, 1); + assign(5, 15, 2); +} + +TEST_F(IntervalMapTest, insert_range_around_another) { + assign(5, 15, 2); + assign(0, 20, 1); +} + +TEST_F(IntervalMapTest, insert_range_overlaps_many) { + assign(0, 10, 1); + assign(10, 20, 2); + assign(20, 30, 3); + assign(30, 40, 4); + assign(40, 50, 5); + assign(0, 50, -1); +} + +TEST_F(IntervalMapTest, insert_range_overlaps_many_init_value) { + assign(0, 10, 1); + assign(10, 20, 2); + assign(20, 30, 3); + assign(30, 40, 4); + assign(40, 50, 5); + assign(0, 50, 0); +} + +TEST_F(IntervalMapTest, insert_range_overlaps_many_oversize) { + assign(0, 10, 1); + assign(10, 20, 2); + assign(20, 30, 3); + assign(30, 40, 4); + assign(40, 50, 5); + assign(-10, 60, -1); +} From 67ed4d4d68e466b2fae59c1d0c362d463f0579e2 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 19:19:27 +0100 Subject: [PATCH 10/25] Make test failure more verbose Outputting the state of the map and this history of operations makes debugging easier. --- tests/unit/unit_test.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index a99c643..a5d2c5c 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -2,6 +2,7 @@ #include +#include #include #include "model.hh" @@ -86,10 +87,28 @@ protected: } void check() const { + SCOPED_TRACE(stringify_map()); + SCOPED_TRACE(stringify_operations()); check_ranges(); check_canonicity(); } + std::string stringify_map() const { + std::ostringstream out; + out << "map: "; + for (const auto& [key, val] : map.underlying_) + out << "[" << +key << ": " << +val << "]"; + return out.str(); + } + + std::string stringify_operations() const { + std::ostringstream out; + out << "ops: "; + for (const auto& [start, end, val] : model.ranges_) + out << "[" << +start << ":" << +end << " => " << +val << "]"; + return out.str(); + } + // Compare against the fake 'Model' implementation void check_ranges() const { auto i = std::numeric_limits::min(); From 8165fc6f99deef2f905ec70b8cc48c06e87aeed0 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 16:48:30 +0100 Subject: [PATCH 11/25] Add randomized test This was especially helpful for my previous attempt at a solution, which did was more complicated. The original rules for this assignment are quite silly, I don't think they really optimize anything, and make understanding the actual algorithm more difficult than it should be. --- tests/unit/unit_test.cc | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index a5d2c5c..c429d29 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -2,6 +2,7 @@ #include +#include #include #include @@ -221,3 +222,44 @@ TEST_F(IntervalMapTest, insert_range_overlaps_many_oversize) { assign(40, 50, 5); assign(-10, 60, -1); } + +TEST_F(IntervalMapTest, fuzzing_001) { + assign(-50, 20, 1); + assign(40, 80, 2); + assign(-100, -10, 3); +} + +TEST_F(IntervalMapTest, fuzzing_002) { + assign(-100, 90, 1); + assign(0, 120, 2); + assign(-60, 60, 3); +} + +TEST_F(IntervalMapTest, fuzzing_003) { + assign(-80, 70, 1); + assign(-50, 40, 2); + assign(-40, 20, 3); + assign(-110, -10, 4); +} + +TEST_F(IntervalMapTest, randomized_test) { + auto const seed = []() { + std::random_device r; + return r(); + }(); + SCOPED_TRACE(seed); + auto random = std::mt19937_64(seed); + + auto keys = std::uniform_int_distribution( + std::numeric_limits::min(), + std::numeric_limits::max()); + auto values = std::uniform_int_distribution(0, 10); + + for (auto i = 0; i < 1000; ++i) { + auto const start = keys(random); + auto const end = keys(random); + auto const value = values(random); + assign(start, end, value); + check(); + } +} From bdccd17936479d4939738d1d9373e3cddede9c40 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 19:29:09 +0100 Subject: [PATCH 12/25] fixup! Add implementation --- src/include/interval-map/interval-map.hh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 1352b79..a514daf 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -18,12 +18,10 @@ public: auto const end_val = (*this)[end]; underlying_.erase(underlying_.lower_bound(begin), underlying_.upper_bound(end)); - if (!((*this)[begin] == val)) { + if (!((*this)[begin] == val)) underlying_.insert({begin, val}); - } - if (!((*this)[end] == end_val)) { + if (!((*this)[end] == end_val)) underlying_.insert({end, end_val}); - } } V const& operator[](K const& key) const { From dc3408524b0bf7bf4adf0300c6067126dc9e1048 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 20:19:32 +0100 Subject: [PATCH 13/25] Make implementation follow assignment rules The assignment wants us to use exactly *one* O(log(N)) call. Hence the single usage of `upper_bound`. It also wants us to use a minimal amount of operations on `K` and `V` values in the map. Finally it asks us to make the answer simple. --- src/include/interval-map/interval-map.hh | 41 +++++++++++++++++------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index a514daf..ab3fb44 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -15,26 +15,45 @@ public: if (!(begin < end)) return; - auto const end_val = (*this)[end]; - underlying_.erase(underlying_.lower_bound(begin), - underlying_.upper_bound(end)); - if (!((*this)[begin] == val)) - underlying_.insert({begin, val}); - if (!((*this)[end] == end_val)) - underlying_.insert({end, end_val}); + auto it = underlying_.upper_bound(end); + auto const end_val = at_upper_bound(it); + + bool insert_begin = !(val == init_); + + while (it != underlying_.begin()) { + it = std::prev(it); + auto begin_found = it->first < begin; + if (begin_found) { + insert_begin = !(val == it->second); + break; + } + if (it != underlying_.end()) + // Account for up-coming `std::prev` with `++` + underlying_.erase(it++); + } + + if (insert_begin) + it = underlying_.insert(it, {begin, val}); + // Get the proper upper-bound for `end` + it = (it == underlying_.end()) ? it : std::next(it); + if (!(at_upper_bound(it) == end_val)) + underlying_.insert(it, {end, end_val}); } V const& operator[](K const& key) const { - auto it = underlying_.upper_bound(key); - if (it == underlying_.begin()) - return init_; - return std::prev(it)->second; + return at_upper_bound(underlying_.upper_bound(key)); } // Used in testing friend class ::IntervalMapTest; private: + V const& at_upper_bound(std::map::const_iterator it) const { + if (it == underlying_.begin()) + return init_; + return std::prev(it)->second; + } + V init_; std::map underlying_{}; }; From c4d81c4e81c8b3fcb937f083cdc7ed2228c0bca0 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 21:15:26 +0100 Subject: [PATCH 14/25] Bootstrap project --- .clang-format | 23 ++++ .envrc | 5 + .gitignore | 7 ++ .woodpecker/check.yml | 31 +++++ flake.lock | 140 +++++++++++++++++++++++ flake.nix | 112 ++++++++++++++++++ meson.build | 13 +++ src/include/interval-map/interval-map.hh | 1 + src/meson.build | 1 + tests/meson.build | 1 + tests/unit/meson.build | 16 +++ tests/unit/unit_test.cc | 7 ++ 12 files changed, 357 insertions(+) create mode 100644 .clang-format create mode 100644 .envrc create mode 100644 .gitignore create mode 100644 .woodpecker/check.yml create mode 100644 flake.lock create mode 100644 flake.nix create mode 100644 meson.build create mode 100644 src/include/interval-map/interval-map.hh create mode 100644 src/meson.build create mode 100644 tests/meson.build create mode 100644 tests/unit/meson.build create mode 100644 tests/unit/unit_test.cc diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..19c58aa --- /dev/null +++ b/.clang-format @@ -0,0 +1,23 @@ +# vim: ft=yaml +--- +BasedOnStyle: LLVM +IndentWidth: 4 +--- +Language: Cpp +# Force pointers to the type for C++. +DerivePointerAlignment: false +PointerAlignment: Left + +# Short functions should not be on a single line, unless empty +AllowShortFunctionsOnASingleLine: Empty + +# Make them level +AccessModifierOffset: -4 + +# It makes more sense this way +BreakBeforeBinaryOperators: All +BreakBeforeTernaryOperators: true + +# Aesthetic +AlignOperands: AlignAfterOperator +--- diff --git a/.envrc b/.envrc new file mode 100644 index 0000000..de77fcb --- /dev/null +++ b/.envrc @@ -0,0 +1,5 @@ +if ! has nix_direnv_version || ! nix_direnv_version 3.0.0; then + source_url "https://raw.githubusercontent.com/nix-community/nix-direnv/3.0.0/direnvrc" "sha256-21TMnI2xWX7HkSTjFFri2UaohXVj854mgvWapWrxRXg=" +fi + +use flake diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..09ba440 --- /dev/null +++ b/.gitignore @@ -0,0 +1,7 @@ +# CMake build directories +/build +/build-* + +# Nix generated files +/.pre-commit-config.yaml +/result diff --git a/.woodpecker/check.yml b/.woodpecker/check.yml new file mode 100644 index 0000000..272c0e4 --- /dev/null +++ b/.woodpecker/check.yml @@ -0,0 +1,31 @@ +labels: + backend: local + +steps: +- name: pre-commit check + image: bash + commands: + - nix develop --command pre-commit run --all + +- name: nix flake check + image: bash + commands: + - nix flake check + +- name: notify + image: bash + environment: + ADDRESS: + from_secret: matrix_homeserver + ROOM: + from_secret: matrix_roomid + USER: + from_secret: matrix_username + PASS: + from_secret: matrix_password + commands: + - nix run github:ambroisie/matrix-notifier + when: + status: + - failure + - success diff --git a/flake.lock b/flake.lock new file mode 100644 index 0000000..a4cfc13 --- /dev/null +++ b/flake.lock @@ -0,0 +1,140 @@ +{ + "nodes": { + "flake-compat": { + "flake": false, + "locked": { + "lastModified": 1696426674, + "narHash": "sha256-kvjfFW7WAETZlt09AgDn1MrtKzP7t90Vf7vypd3OL1U=", + "owner": "edolstra", + "repo": "flake-compat", + "rev": "0f9255e01c2351cc7d116c072cb317785dd33b33", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, + "futils": { + "inputs": { + "systems": "systems" + }, + "locked": { + "lastModified": 1710146030, + "narHash": "sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "b1d9ab70662946ef0850d488da1c9019f3a9752a", + "type": "github" + }, + "original": { + "owner": "numtide", + "ref": "main", + "repo": "flake-utils", + "type": "github" + } + }, + "gitignore": { + "inputs": { + "nixpkgs": [ + "pre-commit-hooks", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1709087332, + "narHash": "sha256-HG2cCnktfHsKV0s4XW83gU3F57gaTljL9KNSuG6bnQs=", + "owner": "hercules-ci", + "repo": "gitignore.nix", + "rev": "637db329424fd7e46cf4185293b9cc8c88c95394", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "gitignore.nix", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1724224976, + "narHash": "sha256-Z/ELQhrSd7bMzTO8r7NZgi9g5emh+aRKoCdaAv5fiO0=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "c374d94f1536013ca8e92341b540eba4c22f9c62", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "nixpkgs-stable": { + "locked": { + "lastModified": 1720386169, + "narHash": "sha256-NGKVY4PjzwAa4upkGtAMz1npHGoRzWotlSnVlqI40mo=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "194846768975b7ad2c4988bdb82572c00222c0d7", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-24.05", + "repo": "nixpkgs", + "type": "github" + } + }, + "pre-commit-hooks": { + "inputs": { + "flake-compat": "flake-compat", + "gitignore": "gitignore", + "nixpkgs": [ + "nixpkgs" + ], + "nixpkgs-stable": "nixpkgs-stable" + }, + "locked": { + "lastModified": 1724440431, + "narHash": "sha256-9etXEOUtzeMgqg1u0wp+EdwG7RpmrAZ2yX516bMj2aE=", + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "rev": "c8a54057aae480c56e28ef3e14e4960628ac495b", + "type": "github" + }, + "original": { + "owner": "cachix", + "ref": "master", + "repo": "pre-commit-hooks.nix", + "type": "github" + } + }, + "root": { + "inputs": { + "futils": "futils", + "nixpkgs": "nixpkgs", + "pre-commit-hooks": "pre-commit-hooks" + } + }, + "systems": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 0000000..9cd1afa --- /dev/null +++ b/flake.nix @@ -0,0 +1,112 @@ +{ + description = "Interval Map coding exercise"; + + inputs = { + futils = { + type = "github"; + owner = "numtide"; + repo = "flake-utils"; + ref = "main"; + }; + + nixpkgs = { + type = "github"; + owner = "NixOS"; + repo = "nixpkgs"; + ref = "nixos-unstable"; + }; + + pre-commit-hooks = { + type = "github"; + owner = "cachix"; + repo = "pre-commit-hooks.nix"; + ref = "master"; + inputs = { + flake-utils.follows = "futils"; + nixpkgs.follows = "nixpkgs"; + }; + }; + }; + + outputs = { self, futils, nixpkgs, pre-commit-hooks }: + { + overlays = { + default = final: _prev: { + interval-map = with final; stdenv.mkDerivation { + pname = "interval-map"; + version = "0.0.0"; + + src = self; + + nativeBuildInputs = with pkgs; [ + meson + ninja + pkg-config + ]; + + checkInputs = with pkgs; [ + gtest + ]; + + doCheck = true; + + meta = with lib; { + description = "An implementation of interval_map"; + homepage = "https://git.belanyi.fr/ambroisie/interval-map"; + license = licenses.mit; + maintainers = with maintainers; [ ambroisie ]; + platforms = platforms.unix; + }; + }; + }; + }; + } // futils.lib.eachDefaultSystem (system: + let + pkgs = import nixpkgs { + inherit system; + overlays = [ + self.overlays.default + ]; + }; + + pre-commit = pre-commit-hooks.lib.${system}.run { + src = self; + + hooks = { + nixpkgs-fmt = { + enable = true; + }; + + clang-format = { + enable = true; + }; + }; + }; + in + { + checks = { + inherit (self.packages.${system}) interval-map; + + inherit pre-commit; + }; + + devShells = { + default = pkgs.mkShell { + inputsFrom = with self.packages.${system}; [ + interval-map + ]; + + packages = with pkgs; [ + clang-tools + ]; + + inherit (pre-commit) shellHook; + }; + }; + + packages = futils.lib.flattenTree { + default = pkgs.interval-map; + inherit (pkgs) interval-map; + }; + }); +} diff --git a/meson.build b/meson.build new file mode 100644 index 0000000..067c27e --- /dev/null +++ b/meson.build @@ -0,0 +1,13 @@ +project( + 'interval-map', + 'cpp', + version : '0.0.0', + license: 'MIT', + default_options : [ + 'cpp_std=c++20', + 'warning_level=3', + ], +) + +subdir('src') +subdir('tests') diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh new file mode 100644 index 0000000..6f70f09 --- /dev/null +++ b/src/include/interval-map/interval-map.hh @@ -0,0 +1 @@ +#pragma once diff --git a/src/meson.build b/src/meson.build new file mode 100644 index 0000000..0e8c25d --- /dev/null +++ b/src/meson.build @@ -0,0 +1 @@ +interval_map_dep = declare_dependency(include_directories: 'include') diff --git a/tests/meson.build b/tests/meson.build new file mode 100644 index 0000000..082b746 --- /dev/null +++ b/tests/meson.build @@ -0,0 +1 @@ +subdir('unit') diff --git a/tests/unit/meson.build b/tests/unit/meson.build new file mode 100644 index 0000000..3e243c2 --- /dev/null +++ b/tests/unit/meson.build @@ -0,0 +1,16 @@ +gtest_dep = dependency( + 'gtest', + main : true, + required : false, +) + +dummy_test = executable( + 'unit_test', + 'unit_test.cc', + dependencies : [ + gtest_dep, + interval_map_dep + ], +) + +test('unit test', dummy_test) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc new file mode 100644 index 0000000..e488c79 --- /dev/null +++ b/tests/unit/unit_test.cc @@ -0,0 +1,7 @@ +#include + +#include + +TEST(misc, passing) { + ASSERT_EQ(1, 1); +} From 3c0ddf3021bad052cc61169245521232183f9e5a Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 21:40:22 +0100 Subject: [PATCH 15/25] Add given code --- src/include/interval-map/interval-map.hh | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 6f70f09..ce1d9dd 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -1 +1,27 @@ #pragma once + +#include + +namespace amby { + +template class interval_map { +public: + interval_map(V const& init) : init_(init) {} + + void assign(K const& begin, K const& end, V const& val) { + // TODO: implement + } + + V const& operator[](K const& key) const { + auto it = underlying_.upper_bound(key); + if (it == underlying_.begin()) + return init_; + return std::prev(it)->second; + } + +private: + V init_; + std::map underlying_{}; +}; + +} // namespace amby From 5292c22e2c3fc8d8d2a2ef0a1115fe5e9a16ea4c Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 21:55:33 +0100 Subject: [PATCH 16/25] Add test for minimal 'interval_map' type interface Add `KeyInterface` (respectively `ValueInterface`). Those classes provide the minimum documented interface for `K` (respectively `V`) in `interval_map`. --- tests/unit/unit_test.cc | 65 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index e488c79..288c811 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -2,6 +2,67 @@ #include -TEST(misc, passing) { - ASSERT_EQ(1, 1); +#include + +template class KeyInterface { +public: + explicit KeyInterface(T val) : underlying_(val) {} + + KeyInterface(KeyInterface const&) = default; + KeyInterface& operator=(KeyInterface const&) = default; + + bool operator<(KeyInterface const& other) const { + return underlying_ < other.underlying_; + } + +private: + T underlying_; +}; + +template struct std::numeric_limits> { + static KeyInterface lowest() { + return KeyInterface(std::numeric_limits::lowest()); + } +}; + +static_assert(std::is_copy_constructible_v>); +static_assert(std::is_copy_assignable_v>); +static_assert( + std::is_same_v, + decltype(std::numeric_limits>::lowest())>); + +template class ValueInterface { +public: + explicit ValueInterface(T val) : underlying_(val) {} + + ValueInterface(ValueInterface const&) = default; + ValueInterface& operator=(ValueInterface const&) = default; + + bool operator==(ValueInterface const& other) const { + return underlying_ == other.underlying_; + } + +private: + T underlying_; +}; + +template struct std::numeric_limits> { + static ValueInterface lowest() { + return ValueInterface(std::numeric_limits::lowest()); + } +}; + +static_assert(std::is_copy_constructible_v>); +static_assert(std::is_copy_assignable_v>); +static_assert(std::is_same_v< + ValueInterface, + decltype(std::numeric_limits>::lowest())>); + +TEST(interval_map, minimal_interface) { + using Key = KeyInterface; + using Value = ValueInterface; + + auto map = amby::interval_map{Value(0)}; + ASSERT_EQ(map[Key(0)], Value(0)); + map.assign(Key(0), Key(1), Value(1)); } From c8183d99a6af9c1256a177ef8a5fa4b11cefcd15 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 22:13:34 +0100 Subject: [PATCH 17/25] Handle empty range insertion --- src/include/interval-map/interval-map.hh | 2 ++ tests/unit/unit_test.cc | 26 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index ce1d9dd..8f806c3 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -9,6 +9,8 @@ public: interval_map(V const& init) : init_(init) {} void assign(K const& begin, K const& end, V const& val) { + if (!(begin < end)) + return; // TODO: implement } diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index 288c811..b2b990e 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -66,3 +66,29 @@ TEST(interval_map, minimal_interface) { ASSERT_EQ(map[Key(0)], Value(0)); map.assign(Key(0), Key(1), Value(1)); } + +TEST(interval_map, no_insertion) { + auto map = amby::interval_map{0}; + for (int i = std::numeric_limits::min(); + i <= std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], 0); + } +} + +TEST(interval_map, insert_begin_equal_end) { + auto map = amby::interval_map{0}; + map.assign(0, 0, 1); + for (int i = std::numeric_limits::min(); + i <= std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], 0); + } +} + +TEST(interval_map, insert_begin_bigger_than_end) { + auto map = amby::interval_map{0}; + map.assign(1, 0, 1); + for (int i = std::numeric_limits::min(); + i <= std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], 0); + } +} From 6bc72fdc253419902af64a90898e876aac8776df Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:15:12 +0100 Subject: [PATCH 18/25] Introduce 'IntervalMapTest' fixture --- tests/unit/unit_test.cc | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index b2b990e..3cdf79e 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -58,7 +58,20 @@ static_assert(std::is_same_v< ValueInterface, decltype(std::numeric_limits>::lowest())>); -TEST(interval_map, minimal_interface) { +class IntervalMapTest : public testing::Test { +protected: + using key_type = char; + using value_type = int; + using map_type = amby::interval_map; + + map_type map{0}; + + void SetUp() override { + map = map_type{0}; + } +}; + +TEST_F(IntervalMapTest, minimal_interface) { using Key = KeyInterface; using Value = ValueInterface; @@ -67,16 +80,14 @@ TEST(interval_map, minimal_interface) { map.assign(Key(0), Key(1), Value(1)); } -TEST(interval_map, no_insertion) { - auto map = amby::interval_map{0}; +TEST_F(IntervalMapTest, no_insertion) { for (int i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) { ASSERT_EQ(map[i], 0); } } -TEST(interval_map, insert_begin_equal_end) { - auto map = amby::interval_map{0}; +TEST_F(IntervalMapTest, insert_begin_equal_end) { map.assign(0, 0, 1); for (int i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) { @@ -84,8 +95,7 @@ TEST(interval_map, insert_begin_equal_end) { } } -TEST(interval_map, insert_begin_bigger_than_end) { - auto map = amby::interval_map{0}; +TEST_F(IntervalMapTest, insert_begin_bigger_than_end) { map.assign(1, 0, 1); for (int i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) { From 73c2b21b94cf5d237e635691e7100ca1d235cbff Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:16:39 +0100 Subject: [PATCH 19/25] Explicitly test for canonicity This will come in handy later, with more complex test cases. --- src/include/interval-map/interval-map.hh | 6 ++++++ tests/unit/unit_test.cc | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 8f806c3..4dc2335 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -2,6 +2,9 @@ #include +// Testing class forward declaration +class IntervalMapTest; + namespace amby { template class interval_map { @@ -21,6 +24,9 @@ public: return std::prev(it)->second; } + // Used in testing + friend class ::IntervalMapTest; + private: V init_; std::map underlying_{}; diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index 3cdf79e..d51db5a 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -69,6 +69,27 @@ protected: void SetUp() override { map = map_type{0}; } + + void TearDown() override { + check_canonicity(); + } + + void check_canonicity() const { + // Consecutive map entries must not contain the same value + for (auto it = map.underlying_.begin(); it != map.underlying_.end(); + ++it) { + const auto next = std::next(it, 1); + if (next == map.underlying_.end()) + break; + EXPECT_NE(it->second, next->second); + } + + // The first entry must not contain the initial value + if (const auto it = map.underlying_.begin(); + it != map.underlying_.end()) { + EXPECT_NE(it->second, map.init_); + } + } }; TEST_F(IntervalMapTest, minimal_interface) { From 65e0d4991fc7679fcf61bfd0f253e9d8589a3ec5 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:38:05 +0100 Subject: [PATCH 20/25] Test against fake 'Model' implementation This removes most of the redundant `for` loops that would appear in hand-written unit tests, and could potentially open the door to fuzz-testing/property-test the implementation for a more in-depth test-suite. --- tests/unit/model.hh | 35 ++++++++++++++++++++++++++++++++ tests/unit/unit_test.cc | 44 ++++++++++++++++++++++++++--------------- 2 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 tests/unit/model.hh diff --git a/tests/unit/model.hh b/tests/unit/model.hh new file mode 100644 index 0000000..9acb19f --- /dev/null +++ b/tests/unit/model.hh @@ -0,0 +1,35 @@ +#pragma once + +#include + +template struct Range { + K begin; + K end; + V val; +}; + +// Same behaviour as interval_map, but implementation is trivally correct +template class Model { +public: + Model(V const& init) : init_(init) {} + + void assign(K const& begin, K const& end, V const& val) { + if (!(begin < end)) + return; + ranges_.emplace_back(begin, end, val); + } + + V const& operator[](K const& key) const { + for (auto it = ranges_.rbegin(); it != ranges_.rend(); ++it) { + if (key < it->begin) + continue; + if (it->end <= key) + continue; + return it->val; + } + return init_; + } + + V init_; + std::vector> ranges_{}; +}; diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index d51db5a..7b9e4ce 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -4,6 +4,8 @@ #include +#include "model.hh" + template class KeyInterface { public: explicit KeyInterface(T val) : underlying_(val) {} @@ -63,17 +65,40 @@ protected: using key_type = char; using value_type = int; using map_type = amby::interval_map; + using model_type = Model; map_type map{0}; + model_type model{0}; void SetUp() override { map = map_type{0}; + model = model_type{0}; } void TearDown() override { + check(); + } + + void assign(key_type const& begin, key_type const& end, + value_type const& val) { + map.assign(begin, end, val); + model.assign(begin, end, val); + } + + void check() const { + check_ranges(); check_canonicity(); } + // Compare against the fake 'Model' implementation + void check_ranges() const { + auto i = std::numeric_limits::min(); + for (; i < std::numeric_limits::max(); ++i) { + ASSERT_EQ(map[i], model[i]) << "(i: " << +i << ")"; + } + ASSERT_EQ(map[i], model[i]) << "(i: " << +i << ")"; + }; + void check_canonicity() const { // Consecutive map entries must not contain the same value for (auto it = map.underlying_.begin(); it != map.underlying_.end(); @@ -101,25 +126,12 @@ TEST_F(IntervalMapTest, minimal_interface) { map.assign(Key(0), Key(1), Value(1)); } -TEST_F(IntervalMapTest, no_insertion) { - for (int i = std::numeric_limits::min(); - i <= std::numeric_limits::max(); ++i) { - ASSERT_EQ(map[i], 0); - } -} +TEST_F(IntervalMapTest, no_insertion) {} TEST_F(IntervalMapTest, insert_begin_equal_end) { - map.assign(0, 0, 1); - for (int i = std::numeric_limits::min(); - i <= std::numeric_limits::max(); ++i) { - ASSERT_EQ(map[i], 0); - } + assign(0, 0, 1); } TEST_F(IntervalMapTest, insert_begin_bigger_than_end) { - map.assign(1, 0, 1); - for (int i = std::numeric_limits::min(); - i <= std::numeric_limits::max(); ++i) { - ASSERT_EQ(map[i], 0); - } + assign(1, 0, 1); } From 4ab8dd37ff55dfa30f8897668c813b0221533b93 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Fri, 23 Aug 2024 23:49:52 +0100 Subject: [PATCH 21/25] Add implementation --- src/include/interval-map/interval-map.hh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index 4dc2335..a514daf 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -14,7 +14,14 @@ public: void assign(K const& begin, K const& end, V const& val) { if (!(begin < end)) return; - // TODO: implement + + auto const end_val = (*this)[end]; + underlying_.erase(underlying_.lower_bound(begin), + underlying_.upper_bound(end)); + if (!((*this)[begin] == val)) + underlying_.insert({begin, val}); + if (!((*this)[end] == end_val)) + underlying_.insert({end, end_val}); } V const& operator[](K const& key) const { From 4d59126f10aaa5c7b363939dbc76369226fa0e69 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 16:48:30 +0100 Subject: [PATCH 22/25] Add initial tests Those were the ones I used for the initial implementation. --- tests/unit/unit_test.cc | 67 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index 7b9e4ce..a99c643 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -135,3 +135,70 @@ TEST_F(IntervalMapTest, insert_begin_equal_end) { TEST_F(IntervalMapTest, insert_begin_bigger_than_end) { assign(1, 0, 1); } + +TEST_F(IntervalMapTest, insert_one_range) { + assign(std::numeric_limits::min(), 0, 1); +} + +TEST_F(IntervalMapTest, insert_non_overlapping_ranges) { + assign(std::numeric_limits::min(), 0, 1); + assign(10, std::numeric_limits::max(), 2); +} + +TEST_F(IntervalMapTest, insert_up_to_max) { + assign(std::numeric_limits::min(), + std::numeric_limits::max(), 1); +} + +TEST_F(IntervalMapTest, insert_range_right_after) { + assign(0, 10, 1); + assign(10, 20, 1); +} + +TEST_F(IntervalMapTest, insert_range_right_before) { + assign(10, 20, 1); + assign(0, 10, 1); +} + +TEST_F(IntervalMapTest, insert_range_middle) { + assign(0, 10, 1); + assign(20, 30, 1); + assign(10, 20, 1); +} + +TEST_F(IntervalMapTest, insert_range_inside_another) { + assign(0, 20, 1); + assign(5, 15, 2); +} + +TEST_F(IntervalMapTest, insert_range_around_another) { + assign(5, 15, 2); + assign(0, 20, 1); +} + +TEST_F(IntervalMapTest, insert_range_overlaps_many) { + assign(0, 10, 1); + assign(10, 20, 2); + assign(20, 30, 3); + assign(30, 40, 4); + assign(40, 50, 5); + assign(0, 50, -1); +} + +TEST_F(IntervalMapTest, insert_range_overlaps_many_init_value) { + assign(0, 10, 1); + assign(10, 20, 2); + assign(20, 30, 3); + assign(30, 40, 4); + assign(40, 50, 5); + assign(0, 50, 0); +} + +TEST_F(IntervalMapTest, insert_range_overlaps_many_oversize) { + assign(0, 10, 1); + assign(10, 20, 2); + assign(20, 30, 3); + assign(30, 40, 4); + assign(40, 50, 5); + assign(-10, 60, -1); +} From 749cb335e3394ae94ac655eac1ca6ccd4d090901 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 19:19:27 +0100 Subject: [PATCH 23/25] Make test failure more verbose Outputting the state of the map and this history of operations makes debugging easier. --- tests/unit/unit_test.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index a99c643..a5d2c5c 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -2,6 +2,7 @@ #include +#include #include #include "model.hh" @@ -86,10 +87,28 @@ protected: } void check() const { + SCOPED_TRACE(stringify_map()); + SCOPED_TRACE(stringify_operations()); check_ranges(); check_canonicity(); } + std::string stringify_map() const { + std::ostringstream out; + out << "map: "; + for (const auto& [key, val] : map.underlying_) + out << "[" << +key << ": " << +val << "]"; + return out.str(); + } + + std::string stringify_operations() const { + std::ostringstream out; + out << "ops: "; + for (const auto& [start, end, val] : model.ranges_) + out << "[" << +start << ":" << +end << " => " << +val << "]"; + return out.str(); + } + // Compare against the fake 'Model' implementation void check_ranges() const { auto i = std::numeric_limits::min(); From c41d1b292283e5fedb34f091dbe857729464a973 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 16:48:30 +0100 Subject: [PATCH 24/25] Add randomized test This was especially helpful for my previous attempt at a solution, which did was more complicated. The original rules for this assignment are quite silly, I don't think they really optimize anything, and make understanding the actual algorithm more difficult than it should be. --- tests/unit/unit_test.cc | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/unit/unit_test.cc b/tests/unit/unit_test.cc index a5d2c5c..c429d29 100644 --- a/tests/unit/unit_test.cc +++ b/tests/unit/unit_test.cc @@ -2,6 +2,7 @@ #include +#include #include #include @@ -221,3 +222,44 @@ TEST_F(IntervalMapTest, insert_range_overlaps_many_oversize) { assign(40, 50, 5); assign(-10, 60, -1); } + +TEST_F(IntervalMapTest, fuzzing_001) { + assign(-50, 20, 1); + assign(40, 80, 2); + assign(-100, -10, 3); +} + +TEST_F(IntervalMapTest, fuzzing_002) { + assign(-100, 90, 1); + assign(0, 120, 2); + assign(-60, 60, 3); +} + +TEST_F(IntervalMapTest, fuzzing_003) { + assign(-80, 70, 1); + assign(-50, 40, 2); + assign(-40, 20, 3); + assign(-110, -10, 4); +} + +TEST_F(IntervalMapTest, randomized_test) { + auto const seed = []() { + std::random_device r; + return r(); + }(); + SCOPED_TRACE(seed); + auto random = std::mt19937_64(seed); + + auto keys = std::uniform_int_distribution( + std::numeric_limits::min(), + std::numeric_limits::max()); + auto values = std::uniform_int_distribution(0, 10); + + for (auto i = 0; i < 1000; ++i) { + auto const start = keys(random); + auto const end = keys(random); + auto const value = values(random); + assign(start, end, value); + check(); + } +} From 2a6b89e420765a761b5485487b33128e03e53c00 Mon Sep 17 00:00:00 2001 From: Bruno BELANYI Date: Sat, 24 Aug 2024 20:19:32 +0100 Subject: [PATCH 25/25] Make implementation follow assignment rules From what I could find, the rules of the assignment seem to be: 1. Restrict oneself to at most *one* O(log(N)) call, and otherwise use constant time operations on the map. 2. Don't use more operations than strictly necessary on `K` and `V`. 3. Prefer simplicity to performance. I think my solution would fair well under those constraints. --- src/include/interval-map/interval-map.hh | 41 +++++++++++++++++------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/include/interval-map/interval-map.hh b/src/include/interval-map/interval-map.hh index a514daf..ab3fb44 100644 --- a/src/include/interval-map/interval-map.hh +++ b/src/include/interval-map/interval-map.hh @@ -15,26 +15,45 @@ public: if (!(begin < end)) return; - auto const end_val = (*this)[end]; - underlying_.erase(underlying_.lower_bound(begin), - underlying_.upper_bound(end)); - if (!((*this)[begin] == val)) - underlying_.insert({begin, val}); - if (!((*this)[end] == end_val)) - underlying_.insert({end, end_val}); + auto it = underlying_.upper_bound(end); + auto const end_val = at_upper_bound(it); + + bool insert_begin = !(val == init_); + + while (it != underlying_.begin()) { + it = std::prev(it); + auto begin_found = it->first < begin; + if (begin_found) { + insert_begin = !(val == it->second); + break; + } + if (it != underlying_.end()) + // Account for up-coming `std::prev` with `++` + underlying_.erase(it++); + } + + if (insert_begin) + it = underlying_.insert(it, {begin, val}); + // Get the proper upper-bound for `end` + it = (it == underlying_.end()) ? it : std::next(it); + if (!(at_upper_bound(it) == end_val)) + underlying_.insert(it, {end, end_val}); } V const& operator[](K const& key) const { - auto it = underlying_.upper_bound(key); - if (it == underlying_.begin()) - return init_; - return std::prev(it)->second; + return at_upper_bound(underlying_.upper_bound(key)); } // Used in testing friend class ::IntervalMapTest; private: + V const& at_upper_bound(std::map::const_iterator it) const { + if (it == underlying_.begin()) + return init_; + return std::prev(it)->second; + } + V init_; std::map underlying_{}; };