From bfa41cd185d7e01ecf098aa472b384544b876457 Mon Sep 17 00:00:00 2001 From: Marcus Boerger Date: Sat, 19 Oct 2024 14:14:17 +0200 Subject: [PATCH] * Added custom Bazel flags (#74) * * Added custom Bazel flag `--//mbo/config:require_throws` which controls whether `MBO_CONFIG_REQUIRE` throw exceptions or use crash logging (the default `False` or `0`). This mostly affects containers. * Added custom Bazel flag `--//mbo/config:limited_ordered_max_unroll_capacity`. This was undocumented as `--//mbo/container:limited_ordered_max_unroll_capacity` until now. It controls the maximum unroll size for LimitedOrdered/Map/Set. * Make test `LimitedOrderedTest.ConstexprRequireSortedInputThrows` work in all configs and all compilation modes for all supported compilers. --- CHANGELOG.md | 2 + README.md | 7 ++ RULES.md | 2 + mbo/config/BUILD.bazel | 62 ++++++++++++ .../internal/config.bzl} | 11 ++- .../internal/config.h.in} | 10 +- mbo/config/require.h | 50 ++++++++++ mbo/container/BUILD.bazel | 35 ++----- mbo/container/internal/limited_ordered.h | 59 ++++------- mbo/container/limited_map.h | 6 -- mbo/container/limited_ordered_test.cc | 37 ++++++- mbo/container/limited_set.h | 6 -- mbo/container/limited_vector.h | 98 +++++++++---------- mbo/log/log_timing_test.cc | 4 +- mbo/testing/matchers_test.cc | 1 - 15 files changed, 245 insertions(+), 145 deletions(-) create mode 100644 mbo/config/BUILD.bazel rename mbo/{container/internal/limited_ordered_config.bzl => config/internal/config.bzl} (78%) rename mbo/{container/internal/limited_ordered_config.h.in => config/internal/config.h.in} (81%) create mode 100644 mbo/config/require.h diff --git a/CHANGELOG.md b/CHANGELOG.md index a572522..94ee061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # 0.2.34 * Added `mbo::testing::WhenTransformedBy` which allows to compare containers after transforming them. +* Added custom Bazel flag `--//mbo/config:require_throws` which controls whether `MBO_CONFIG_REQUIRE` throw exceptions or use crash logging (the default `False` or `0`). This mostly affects containers. +* Added custom Bazel flag `--//mbo/config:limited_ordered_max_unroll_capacity`. This was undocumented as `--//mbo/container:limited_ordered_max_unroll_capacity` until now. It controls the maximum unroll size for LimitedOrdered/Map/Set. # 0.2.33 diff --git a/README.md b/README.md index a48ed3b..4e23c7b 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,13 @@ The library is tested with Clang and GCC using continuous integration: [![Test]( The C++ library is organized in functional groups each residing in their own directory. +* Config + * `namespace mbo::config` + * mbo/config:config_cc, mbo/config/config.h + * Custom Bazel flag `--//mbo/config:limited_ordered_max_unroll_capacity` which controls the maximum unroll size for `LimitedOrdered` and thus `LimitedMap` and `LimitedSet`. + * Custom Bazel flag `--//mbo/config:require_throws` which controls whether `MBO_CONFIG_REQUIRE` throw exceptions or use crash logging (the default `False` or `0`). This mostly affects containers. + * mbo/config:require_cc, mbo/config/require.h + * Marcos `MBO_CONFIG_REQUIRE(condition, message)` which allows to check a `condition` and either throw an exception or crash with Abseil FATAL logging. The behavior is controlled by `--//mbo/config:require_throws`. * Container * `namespace mbo::container` * mbo/container:any_scan_cc, mbo/container/any_scan.h diff --git a/RULES.md b/RULES.md index b9daa6c..8f93321 100644 --- a/RULES.md +++ b/RULES.md @@ -54,3 +54,5 @@ Some rules for the code layout and its development. * Flags in libraries should be prefixed with their path/namespace. E.g. the flag `--mbo_log_timing_min_duration` has the prefix `mbo_log` as it is defined in `mbo/log/log_timing.cc` (path `mbo/log`) and uses namespace `mbo::log`. +* API changes that are not backwards compatible should not occur in minor version changes. +* Undocumented and private/internal APIs may be changed in any way at any time. diff --git a/mbo/config/BUILD.bazel b/mbo/config/BUILD.bazel new file mode 100644 index 0000000..c2ac46e --- /dev/null +++ b/mbo/config/BUILD.bazel @@ -0,0 +1,62 @@ +# SPDX-FileCopyrightText: Copyright (c) The helly25/mbo authors (helly25.com) +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "int_flag") +load(":internal/config.bzl", "config_gen") + +# Create custom bazel flag `--//mbo/container:limited_ordered_max_unroll_capacity`. +int_flag( + name = "limited_ordered_max_unroll_capacity", + build_setting_default = 16, + visibility = ["//visibility:private"], +) + +# Create custom bazel flag `--//mbo/config:require_throws`. +bool_flag( + name = "require_throws", + build_setting_default = False, + visibility = ["//visibility:private"], +) + +bzl_library( + name = "config_bzl", + srcs = [":internal/config.bzl"], + visibility = ["//visibility:private"], + deps = ["@bazel_skylib//rules:common_settings"], +) + +config_gen( + name = "config_gen", + visibility = ["//visibility:private"], + output = "config.h", + template = "internal/config.h.in", +) + +cc_library( + name = "config_cc", + hdrs = ["config.h"], + visibility = ["//mbo:__subpackages__"], +) + +cc_library( + name = "require_cc", + hdrs = ["require.h"], + deps = [ + ":config_cc", + "@com_google_absl//absl/log:absl_log", + ], + visibility = ["//mbo:__subpackages__"], +) diff --git a/mbo/container/internal/limited_ordered_config.bzl b/mbo/config/internal/config.bzl similarity index 78% rename from mbo/container/internal/limited_ordered_config.bzl rename to mbo/config/internal/config.bzl index 41f3fe0..53e0333 100644 --- a/mbo/container/internal/limited_ordered_config.bzl +++ b/mbo/config/internal/config.bzl @@ -17,17 +17,19 @@ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") -def _limited_ordered_config_gen_impl(ctx): +def _config_gen_impl(ctx): limited_ordered_max_unroll_capacity = ctx.attr._limited_ordered_max_unroll_capacity[BuildSettingInfo].value + require_throws = ctx.attr._require_throws[BuildSettingInfo].value ctx.actions.expand_template( template = ctx.file.template, output = ctx.outputs.output, substitutions = { "kUnrollMaxCapacityDefault = 16;": "kUnrollMaxCapacityDefault = " + repr(limited_ordered_max_unroll_capacity) + ";", + "kRequireThrows = false;": "kRequireThrows = " + repr(require_throws).lower() + ";", }, ) -limited_ordered_config_gen = rule( +config_gen = rule( attrs = { "output": attr.output( mandatory = True, @@ -36,7 +38,8 @@ limited_ordered_config_gen = rule( allow_single_file = True, mandatory = True, ), - "_limited_ordered_max_unroll_capacity": attr.label(default = Label("//mbo/container:limited_ordered_max_unroll_capacity")), + "_limited_ordered_max_unroll_capacity": attr.label(default = Label("//mbo/config:limited_ordered_max_unroll_capacity")), + "_require_throws": attr.label(default = Label("//mbo/config:require_throws")), }, - implementation = _limited_ordered_config_gen_impl, + implementation = _config_gen_impl, ) diff --git a/mbo/container/internal/limited_ordered_config.h.in b/mbo/config/internal/config.h.in similarity index 81% rename from mbo/container/internal/limited_ordered_config.h.in rename to mbo/config/internal/config.h.in index 1e83aef..731b462 100644 --- a/mbo/container/internal/limited_ordered_config.h.in +++ b/mbo/config/internal/config.h.in @@ -18,7 +18,7 @@ #include -namespace mbo::container::container_internal { +namespace mbo::config { // The maximum unroll capacity for `LimitedOrdered` based containers: `LimitedSet` and `LimitedMap`. // Beyond unrolling 32 comparison steps, unrolling has deminishing returns for any architecture. @@ -33,6 +33,12 @@ namespace mbo::container::container_internal { // `--@com_helly_25//mbo/container:limited_ordered_max_unroll_capacity`. static constexpr std::size_t kUnrollMaxCapacityDefault = 16; -} // namespace mbo::container::container_internal +// Config macro `MBO_CONFIG_REQUIRE_THROWS` controls whether container requirement violations +// result in throwing a `std::runtime_error` or a `ABSL_LOG_IF` (the latter (0) being the default). +// +// When exceptions are used then the affected functions cannot be declared `noexcept`. +static constexpr bool kRequireThrows = false; + +} // namespace mbo::config #endif // MBO_CONTAINER_INTERNAL_LIMITED_ORDERED_CONFIG_H_ diff --git a/mbo/config/require.h b/mbo/config/require.h new file mode 100644 index 0000000..d71036c --- /dev/null +++ b/mbo/config/require.h @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: Copyright (c) The helly25/mbo authors (helly25.com) +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef MBO_CONTAINER_INTERNAL_REQUIRE_H_ +#define MBO_CONTAINER_INTERNAL_REQUIRE_H_ + +#include // IWYU pragma: keep + +#include "absl/log/absl_log.h" // IWYU pragma: keep + +// If `mbo/config/config.h` is available, then include that. In order to +// make indexers work, we also include the generator "...in" as a fallback. +#if __has_include("mbo/config/config.h") +# include "mbo/config/config.h" // IWYU pragma: keep +#else +# include "mbo/config/internal/config.h.in" // IWYU pragma: keep +#endif + +// NOLINTBEGIN(cppcoreguidelines-macro-usage) + +#define MBO_PRIVATE_CONFIG_CAT_CAT_(line) #line +#define MBO_PRIVATE_CONFIG_NUM2STR_(line) MBO_PRIVATE_CONFIG_CAT_CAT_(line) + +#ifdef MBO_CONFIG_REQUIRE +# undef MBO_CONFIG_REQUIRE +#endif + +#define MBO_CONFIG_REQUIRE(condition, message) \ + if constexpr (!::mbo::config::kRequireThrows) { \ + /* NOLINTNEXTLINE(bugprone-switch-missing-default-case) */ \ + ABSL_LOG_IF(FATAL, !(condition)) << message; \ + } else if ((condition)) { /* GOOD */ \ + } else \ + throw std::runtime_error(__FILE__ ":" MBO_PRIVATE_CONFIG_NUM2STR_(__LINE__) " : " #condition " : " message) + +// NOLINTEND(cppcoreguidelines-macro-usage) + +#endif // MBO_CONTAINER_INTERNAL_REQUIRE_H_ diff --git a/mbo/container/BUILD.bazel b/mbo/container/BUILD.bazel index e982a75..2c191fe 100644 --- a/mbo/container/BUILD.bazel +++ b/mbo/container/BUILD.bazel @@ -13,31 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@bazel_skylib//:bzl_library.bzl", "bzl_library") -load("@bazel_skylib//rules:common_settings.bzl", "int_flag") -load(":internal/limited_ordered_config.bzl", "limited_ordered_config_gen") - -# Create custom bazel flag `--//mbo/container:limited_ordered_max_unroll_capacity`. -int_flag( - name = "limited_ordered_max_unroll_capacity", - build_setting_default = 16, - visibility = ["//visibility:private"], -) - -bzl_library( - name = "limited_ordered_config_bzl", - srcs = [":internal/limited_ordered_config.bzl"], - visibility = ["//visibility:private"], - deps = ["@bazel_skylib//rules:common_settings"], -) - -limited_ordered_config_gen( - name = "limited_ordered_config_gen", - visibility = ["//visibility:private"], - output = "internal/limited_ordered_config.h", - template = "internal/limited_ordered_config.h.in" -) - cc_library( name = "any_scan_cc", hdrs = ["any_scan.h"], @@ -118,13 +93,12 @@ cc_library( cc_library( name = "limited_ordered_cc", - hdrs = [ - "internal/limited_ordered_config.h", - "internal/limited_ordered.h", - ], + hdrs = ["internal/limited_ordered.h"], visibility = ["//visibility:private"], deps = [ ":limited_options_cc", + "//mbo/config:config_cc", + "//mbo/config:require_cc", "//mbo/types:compare_cc", "//mbo/types:traits_cc", "@com_google_absl//absl/log:absl_log", @@ -137,6 +111,8 @@ cc_test( srcs = ["limited_ordered_test.cc"], deps = [ ":limited_ordered_cc", + "//mbo/config:config_cc", + "//mbo/config:require_cc", "//mbo/testing:matchers_cc", "@com_google_absl//absl/log:initialize", "@com_google_googletest//:gtest_main", @@ -189,6 +165,7 @@ cc_library( visibility = ["//visibility:public"], deps = [ ":limited_options_cc", + "//mbo/config:require_cc", "//mbo/types:traits_cc", "@com_google_absl//absl/log:absl_log", ], diff --git a/mbo/container/internal/limited_ordered.h b/mbo/container/internal/limited_ordered.h index b6c5dd4..170d35c 100644 --- a/mbo/container/internal/limited_ordered.h +++ b/mbo/container/internal/limited_ordered.h @@ -25,19 +25,12 @@ #include #include -#include "absl/log/absl_log.h" +#include "absl/log/absl_log.h" // IWYU pragma: keep +#include "mbo/config/require.h" #include "mbo/container/limited_options.h" // IWYU pragma: export #include "mbo/types/compare.h" // IWYU pragma: export #include "mbo/types/traits.h" -// If `mbo/container/internal/limited_ordered_config.h` is available, then include that. In order to -// make indexers work, we also include the generator "...in" as a fallback. -#if __has_include("mbo/container/internal/limited_ordered_config.h") -# include "mbo/container/internal/limited_ordered_config.h" -#else -# include "mbo/container/internal/limited_ordered_config.h.in" -#endif - #ifdef MBO_FORCE_INLINE # undef MBO_FORCE_INLINE #endif @@ -52,15 +45,6 @@ namespace mbo::container::container_internal { -#ifdef LV_REQUIRE -# undef LV_REQUIRE -#endif - -// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -#define LV_REQUIRE(severity, condition) \ - /* NOLINTNEXTLINE(bugprone-switch-missing-default-case) */ \ - ABSL_LOG_IF(severity, !(condition)) - // NOLINTBEGIN(readability-identifier-naming) template @@ -76,7 +60,7 @@ concept LimitedOrderedValid = // template> requires(LimitedOrderedValid) -class LimitedOrdered { +class [[nodiscard]] LimitedOrdered { protected: static constexpr bool kKeyOnly = std::same_as; // true = set, false = map (of pairs). @@ -89,11 +73,13 @@ class LimitedOrdered { static constexpr bool kOptimizeIndexOf = !Options::Has(LimitedOptionsFlag::kNoOptimizeIndexOf); static constexpr bool kCustomIndexOfBeyondUnroll = Options::Has(LimitedOptionsFlag::kCustomIndexOfBeyondUnroll); - static constexpr std::size_t kUnrollMaxCapacityLimit = 32; // The maximum supported in code. - static constexpr std::size_t kUnrollMaxCapacity = kUnrollMaxCapacityDefault; // MUST MATCH `index_of`. + static constexpr std::size_t kUnrollMaxCapacityLimit = 32; // The maximum supported in code. + static constexpr std::size_t kUnrollMaxCapacity = ::mbo::config::kUnrollMaxCapacityDefault; // MUST MATCH `index_of`. static_assert( kUnrollMaxCapacity >= 4 && kUnrollMaxCapacity <= kUnrollMaxCapacityLimit, - "Check documentation for `mbo::container::container_internal::kUnrollMaxCapacityDefault`."); + "Check documentation for `::mbo::config::kUnrollMaxCapacityDefault`."); + + static constexpr bool kRequireThrows = ::mbo::config::kRequireThrows; // Must declare each other as friends so that we can correctly move from other. template @@ -429,12 +415,11 @@ class LimitedOrdered { template requires std::constructible_from> - constexpr LimitedOrdered(It first, It last, const Compare& key_comp = Compare()) noexcept : key_comp_(key_comp) { -#ifndef NDEBUG + constexpr LimitedOrdered(It first, It last, const Compare& key_comp = Compare()) noexcept(!kRequireThrows) + : key_comp_(key_comp) { if constexpr (Options::Has(LimitedOptionsFlag::kRequireSortedInput)) { - LV_REQUIRE(FATAL, std::is_sorted(first, last, key_comp_)); + MBO_CONFIG_REQUIRE(std::is_sorted(first, last, key_comp_), "Flag `kRequireSortedInput` violated."); } -#endif // NDEBUG while (first < last) { if constexpr (Options::Has(LimitedOptionsFlag::kRequireSortedInput)) { std::construct_at(&values_[size_++].data, *first); @@ -782,13 +767,13 @@ class LimitedOrdered { } template - constexpr std::pair emplace(Args&&... args) noexcept { + constexpr std::pair emplace(Args&&... args) noexcept(!kRequireThrows) { Value new_val(std::forward(args)...); const iterator dst = lower_bound(GetKey(new_val)); if (dst != end() && !key_comp_(GetKey(*dst), GetKey(new_val)) && !key_comp_(GetKey(new_val), GetKey(*dst))) { return std::make_pair(dst, false); } - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `emplace` at capacity."; + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `emplace` at capacity."); for (iterator next = end(); next > dst; --next) { std::construct_at(&*next, std::move(*std::prev(next))); } @@ -799,8 +784,8 @@ class LimitedOrdered { template requires IsIterator::value - constexpr iterator erase(It pos) noexcept { - LV_REQUIRE(FATAL, begin() <= pos && pos < end()) << "Invalid `pos`"; + constexpr iterator erase(It pos) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(begin() <= pos && pos < end(), "Invalid `pos`."); auto dst = to_iterator(pos); --size_; std::destroy_at(&*dst); @@ -810,8 +795,8 @@ class LimitedOrdered { return pos > end() ? end() : to_iterator(pos); } - constexpr iterator erase(const_iterator first, const_iterator last) noexcept { - LV_REQUIRE(FATAL, begin() <= first && first <= last && last <= end()) << "Invalid `first` or `last`"; + constexpr iterator erase(const_iterator first, const_iterator last) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(begin() <= first && first <= last && last <= end(), "Invalid `first` or `last`."); std::size_t deleted = 0; for (const_iterator it = first; it < last; ++it) { std::destroy_at(it); @@ -876,7 +861,7 @@ class LimitedOrdered { dst->second = Mapped(args...); return std::make_pair(dst, false); } - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `try_emplace` at capacity."; + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `try_emplace` at capacity."); for (iterator next = end(); next > dst; --next) { std::construct_at(&*next, std::move(*std::prev(next))); } @@ -895,7 +880,7 @@ class LimitedOrdered { dst->second = Mapped(args...); return std::make_pair(dst, false); } - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `try_emplace` at capacity."; + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `try_emplace` at capacity."); for (iterator next = end(); next > dst; --next) { std::construct_at(&*next, std::move(*std::prev(next))); } @@ -914,7 +899,7 @@ class LimitedOrdered { dst->second = std::forward(value); return std::make_pair(dst, false); } - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `insert_or_assign` at capacity."; + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `insert_or_assign` at capacity."); for (iterator next = end(); next > dst; --next) { std::construct_at(&*next, std::move(*std::prev(next))); } @@ -931,7 +916,7 @@ class LimitedOrdered { dst->second = std::forward(value); return std::make_pair(dst, false); } - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `emplace` at capacity."; + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `emplace` at capacity."); for (iterator next = end(); next > dst; --next) { std::construct_at(&*next, std::move(*std::prev(next))); } @@ -1024,8 +1009,6 @@ class LimitedOrdered { // NOLINTEND(readability-identifier-naming) -#undef LV_REQUIRE - } // namespace mbo::container::container_internal #ifdef MBO_FORCE_INLINE diff --git a/mbo/container/limited_map.h b/mbo/container/limited_map.h index 010adb8..ee076cb 100644 --- a/mbo/container/limited_map.h +++ b/mbo/container/limited_map.h @@ -25,16 +25,12 @@ #include #include -#include "absl/log/absl_log.h" #include "mbo/container/internal/limited_ordered.h" #include "mbo/types/compare.h" #include "mbo/types/traits.h" namespace mbo::container { -// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -#define LV_REQUIRE(severity, condition) ABSL_LOG_IF(severity, !(condition)) - // NOLINTBEGIN(readability-identifier-naming) // Implements a `std::map` like container that only uses inlined memory. So if used as a local @@ -386,8 +382,6 @@ constexpr auto ToLimitedMap(std::pair (&&array)[N], const KComp& key_comp // NOLINTEND(readability-identifier-naming) -#undef LV_REQUIRE - } // namespace mbo::container #endif // MBO_CONTAINER_LIMITED_MAP_H_ diff --git a/mbo/container/limited_ordered_test.cc b/mbo/container/limited_ordered_test.cc index 322cf06..3b84676 100644 --- a/mbo/container/limited_ordered_test.cc +++ b/mbo/container/limited_ordered_test.cc @@ -23,6 +23,7 @@ #include "absl/log/initialize.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "mbo/config/require.h" // IWYU pragma: keep #include "mbo/testing/matchers.h" namespace mbo::container { @@ -30,6 +31,7 @@ namespace { // NOLINTBEGIN(*-magic-numbers) +using ::mbo::config::kRequireThrows; using ::mbo::container::container_internal::LimitedOrdered; using ::mbo::testing::CapacityIs; using ::testing::ElementsAre; @@ -86,11 +88,38 @@ TEST_F(LimitedOrderedTest, ConstexprNoDtor) { TEST_F(LimitedOrderedTest, ConstexprRequireSortedInput) { static constexpr auto kTest = - LimitedOrdered{}>{}; - EXPECT_THAT(kTest, IsEmpty()); - EXPECT_THAT(kTest, SizeIs(0)); + LimitedOrdered{}>{1, 2, 3}; + EXPECT_THAT(kTest, Not(IsEmpty())); + EXPECT_THAT(kTest, SizeIs(3)); EXPECT_THAT(kTest, CapacityIs(3)); - EXPECT_THAT(kTest, ElementsAre()); + EXPECT_THAT(kTest, ElementsAre(1, 2, 3)); +} + +void DoTestConstexprRequireSortedInputThrows() { + constexpr auto kOptions = LimitedOptions<3, LimitedOptionsFlag::kRequireSortedInput>{}; + const std::vector v{1, 3, 2}; + const auto container = LimitedOrdered{v.begin(), v.end()}; + EXPECT_THAT(container, SizeIs(3)); +} + +TEST_F(LimitedOrderedTest, ConstexprRequireSortedInputThrows) { + if constexpr (kRequireThrows) { + bool caught = false; + try { + // Passing the value list direvtly into the constructor of `LimitedOrdered` results in a compile time exception. + // That exception cannot be tested here, so the values are being passed at run-time using a vector. That allows + // to catch and examine the excption. + DoTestConstexprRequireSortedInputThrows(); + } catch (const std::runtime_error& error) { + caught = true; + EXPECT_THAT(error.what(), ::testing::HasSubstr("std::is_sorted(first, last, key_comp_)")); + } catch (...) { + ADD_FAILURE() << "Wrong exception type."; + } + ASSERT_TRUE(caught); + } else { + ASSERT_DEATH(DoTestConstexprRequireSortedInputThrows(), "Flag `kRequireSortedInput` violated."); + } } // NOLINTEND(*-magic-numbers) diff --git a/mbo/container/limited_set.h b/mbo/container/limited_set.h index a41f535..3ecb186 100644 --- a/mbo/container/limited_set.h +++ b/mbo/container/limited_set.h @@ -25,16 +25,12 @@ #include #include -#include "absl/log/absl_log.h" #include "mbo/container/internal/limited_ordered.h" // IWYU pragma: export #include "mbo/types/compare.h" // IWYU pragma: keep #include "mbo/types/traits.h" namespace mbo::container { -// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -#define LV_REQUIRE(severity, condition) ABSL_LOG_IF(severity, !(condition)) - // NOLINTBEGIN(readability-identifier-naming) // Implements a `std::set` like container that only uses inlined memory. So if used as a local @@ -298,8 +294,6 @@ constexpr LimitedSet, N, Compare> ToLimitedSet( // NOLINTEND(readability-identifier-naming) -#undef LV_REQUIRE - } // namespace mbo::container #endif // MBO_CONTAINER_LIMITED_SET_H_ diff --git a/mbo/container/limited_vector.h b/mbo/container/limited_vector.h index 6659d2a..8cef094 100644 --- a/mbo/container/limited_vector.h +++ b/mbo/container/limited_vector.h @@ -26,21 +26,13 @@ #include #include -#include "absl/log/absl_log.h" +#include "mbo/config/config.h" +#include "mbo/config/require.h" #include "mbo/container/limited_options.h" // IWYU pragma: export #include "mbo/types/traits.h" namespace mbo::container { -#ifdef LV_REQUIRE -# undef LV_REQUIRE -#endif - -// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -#define LV_REQUIRE(severity, condition) \ - /* NOLINTNEXTLINE(bugprone-switch-missing-default-case) */ \ - ABSL_LOG_IF(severity, !(condition)) - // NOLINTBEGIN(readability-identifier-naming) template @@ -77,6 +69,8 @@ class LimitedVector final { // static_assert(std::is_trivially_destructible_v || Options::Has(LimitedOptionsFlag::kEmptyDestructor)); static constexpr std::size_t Capacity = Options::kCapacity; + static constexpr bool kRequireThrows = ::mbo::config::kRequireThrows; + union Data { constexpr Data() noexcept : none{} {} @@ -252,8 +246,8 @@ class LimitedVector final { } } - constexpr void reserve(std::size_t size) noexcept { - LV_REQUIRE(FATAL, size <= Capacity) << "Cannot reserve beyond capacity."; + constexpr void reserve(std::size_t size) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(size <= Capacity, "Cannot reserve beyond capacity."); } constexpr void shrink_to_fit() noexcept { @@ -280,9 +274,9 @@ class LimitedVector final { } template - constexpr iterator emplace(const_iterator pos, Args&&... args) noexcept { - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `emplace` at capacity."; - LV_REQUIRE(FATAL, &values_[0].data <= pos && pos <= &values_[size_].data) << "Invalid `pos`"; + constexpr iterator emplace(const_iterator pos, Args&&... args) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `emplace` at capacity."); + MBO_CONFIG_REQUIRE(&values_[0].data <= pos && pos <= &values_[size_].data, "Invalid `pos`."); iterator dst = end(); for (; dst > pos; --dst) { *dst = std::move(*std::prev(dst)); @@ -292,9 +286,9 @@ class LimitedVector final { return dst; } - constexpr iterator erase(const_iterator pos) noexcept { + constexpr iterator erase(const_iterator pos) noexcept(!kRequireThrows) { // NOLINTBEGIN(cppcoreguidelines-pro-type-const-cast) - LV_REQUIRE(FATAL, &values_[0].data <= pos && pos < &values_[size_].data) << "Invalid `pos`"; + MBO_CONFIG_REQUIRE(&values_[0].data <= pos && pos < &values_[size_].data, "Invalid `pos`."); auto dst = const_cast(pos); --size_; std::destroy_at(dst); @@ -305,10 +299,10 @@ class LimitedVector final { // NOLINTEND(cppcoreguidelines-pro-type-const-cast) } - constexpr iterator erase(const_iterator first, const_iterator last) noexcept { + constexpr iterator erase(const_iterator first, const_iterator last) noexcept(!kRequireThrows) { // NOLINTBEGIN(cppcoreguidelines-pro-type-const-cast) - LV_REQUIRE(FATAL, &values_[0].data <= first && first <= last && last <= &values_[size_].data) - << "Invalid `first` or `last`"; + MBO_CONFIG_REQUIRE( + &values_[0].data <= first && first <= last && last <= &values_[size_].data, "Invalid `first` or `last`."); std::size_t deleted = 0; for (const_iterator it = first; it < last; ++it) { std::destroy_at(it); @@ -325,29 +319,29 @@ class LimitedVector final { } template - constexpr reference emplace_back(Args&&... args) noexcept { - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `emplace_back` at capacity."; + constexpr reference emplace_back(Args&&... args) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `emplace_back` at capacity."); auto& data_ref{values_[size_++]}; std::construct_at(&data_ref.data, std::forward(args)...); return data_ref.data; } - constexpr reference push_back(T&& val) noexcept { - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `push_back` at capacity."; + constexpr reference push_back(T&& val) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `push_back` at capacity."); auto& data_ref{values_[size_++]}; - std::construct_at(&data_ref.data, std::forward(val)); + std::construct_at(&data_ref.data, std::move(val)); return data_ref.data; } - constexpr reference push_back(const T& val) noexcept { - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `push_back` at capacity."; + constexpr reference push_back(const T& val) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `push_back` at capacity."); auto& data_ref{values_[size_++]}; std::construct_at(&data_ref.data, val); return data_ref.data; } - constexpr void pop_back() noexcept { - LV_REQUIRE(FATAL, size_ > 0) << "No element to pop."; + constexpr void pop_back() noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(size_ > 0, "No element to pop."); std::destroy_at(&values_[--size_].data); } @@ -366,8 +360,8 @@ class LimitedVector final { } } - constexpr void assign(const std::initializer_list& list) noexcept { - LV_REQUIRE(FATAL, list.size() <= Capacity) << "Called `assign` at capacity."; + constexpr void assign(const std::initializer_list& list) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(list.size() <= Capacity, "Called `assign` at capacity."); clear(); auto it = list.begin(); while (it < list.end()) { @@ -377,25 +371,25 @@ class LimitedVector final { template U> constexpr iterator insert(const_iterator pos, U&& value) { - LV_REQUIRE(FATAL, size_ < Capacity) << "Called `insert` at capacity."; - LV_REQUIRE(FATAL, begin() <= pos && pos <= end()) << "Invalid `pos`."; + MBO_CONFIG_REQUIRE(size_ < Capacity, "Called `insert` at capacity."); + MBO_CONFIG_REQUIRE(begin() <= pos && pos <= end(), "Invalid `pos`."); // Clang does not like `std::distance`. The issue is that the iterators point into the union. // That makes them technically not point into an array AND that is indeed not allowed by C++. - const iterator dst = const_cast(pos); + const auto dst = const_cast(pos); // NOLINT(cppcoreguidelines-pro-type-const-cast) move_backward(dst, 1); std::construct_at(&*dst, std::forward(value)); return dst; } constexpr iterator insert(const_iterator pos, size_type count, const T& value) { - LV_REQUIRE(FATAL, size_ + count <= Capacity) << "Called `insert` at capacity."; - LV_REQUIRE(FATAL, begin() <= pos && pos <= end()) << "Invalid `pos`."; - const iterator dst = const_cast(pos); + MBO_CONFIG_REQUIRE(size_ + count <= Capacity, "Called `insert` at capacity."); + MBO_CONFIG_REQUIRE(begin() <= pos && pos <= end(), "Invalid `pos`."); + const auto dst = const_cast(pos); // NOLINT(cppcoreguidelines-pro-type-const-cast) if (count == 0) { return dst; } std::size_t index = move_backward(dst, count); - while (count) { + while (count > 0) { std::construct_at(&values_[index].data, value); ++index; --count; @@ -408,11 +402,11 @@ class LimitedVector final { template requires(std::constructible_from())>) constexpr iterator insert(const_iterator pos, InputIt first, InputIt last) { - LV_REQUIRE(FATAL, begin() <= pos && pos <= end()) << "Invalid `pos`."; - LV_REQUIRE(FATAL, first <= last) << "First > Last."; + MBO_CONFIG_REQUIRE(begin() <= pos && pos <= end(), "Invalid `pos`."); + MBO_CONFIG_REQUIRE(first <= last, "First > Last."); std::size_t count = std::distance(first, last); - LV_REQUIRE(FATAL, size_ + count <= Capacity) << "Called `insert` at capacity."; - const iterator dst = const_cast(pos); + MBO_CONFIG_REQUIRE(size_ + count <= Capacity, "Called `insert` at capacity."); + const auto dst = const_cast(pos); // NOLINT(*-pro-type-const-cast) if (count == 0) { return dst; } @@ -473,23 +467,23 @@ class LimitedVector final { constexpr const_reverse_iterator crend() const noexcept { return std::make_reverse_iterator(cbegin()); } - constexpr reference operator[](std::size_t index) noexcept { - LV_REQUIRE(FATAL, index < size_) << "Access past size."; + constexpr reference operator[](std::size_t index) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(index < size_, "Access past size."); return values_[index].data; } - constexpr reference at(std::size_t index) noexcept { - LV_REQUIRE(FATAL, index < size_) << "Access past size."; + constexpr reference at(std::size_t index) noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(index < size_, "Access past size."); return values_[index].data; } - constexpr const_reference operator[](std::size_t index) const noexcept { - LV_REQUIRE(FATAL, index < size_) << "Access past size."; + constexpr const_reference operator[](std::size_t index) const noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(index < size_, "Access past size."); return values_[index].data; } - constexpr const_reference at(std::size_t index) const noexcept { - LV_REQUIRE(FATAL, index < size_) << "Access past size."; + constexpr const_reference at(std::size_t index) const noexcept(!kRequireThrows) { + MBO_CONFIG_REQUIRE(index < size_, "Access past size."); return values_[index].data; } @@ -571,7 +565,7 @@ inline constexpr auto MakeLimitedVector(It&& begin, It&& end) noexcept { template inline constexpr auto MakeLimitedVector(const std::initializer_list& data) { - LV_REQUIRE(FATAL, data.size() <= N) << "Too many initlizer values."; + MBO_CONFIG_REQUIRE(data.size() <= N, "Too many initlizer values."); return LimitedVector{}>(data); } @@ -631,8 +625,6 @@ constexpr LimitedVector, LimitedOptions{}> T // NOLINTEND(readability-identifier-naming) -#undef LV_REQUIRE - } // namespace mbo::container #endif // MBO_CONTAINER_LIMITED_VECTOR_H_ diff --git a/mbo/log/log_timing_test.cc b/mbo/log/log_timing_test.cc index fcbb475..97f828b 100644 --- a/mbo/log/log_timing_test.cc +++ b/mbo/log/log_timing_test.cc @@ -131,8 +131,8 @@ TEST_F(LogTimingTest, LogFormat) { {")", "\\)"}, }); Sequence sequence; - const std::string expected_log1 = absl::StrFormat(".*LogTiming\\([0-9:.]+[mnu]s @ (%s)*\\)$", function); - const std::string expected_log2 = absl::StrFormat(".*LogTiming\\([0-9:.]+[mnu]s @ (%s)*\\): Foo$", function); + const std::string expected_log1 = absl::StrFormat(".*LogTiming\\(([0-9:.]+[mnu]s|0) @ (%s)*\\)$", function); + const std::string expected_log2 = absl::StrFormat(".*LogTiming\\(([0-9:.]+[mnu]s|0) @ (%s)*\\): Foo$", function); ExpectLogConst(absl::LogSeverity::kInfo, ContainsRegex(expected_log1)).Times(1).InSequence(sequence); ExpectLogConst(absl::LogSeverity::kInfo, ContainsRegex(expected_log2)).Times(1).InSequence(sequence); (void)LogTiming(); // Manually discarding the result means, this one logs immediately. diff --git a/mbo/testing/matchers_test.cc b/mbo/testing/matchers_test.cc index 96e73b8..a41ad63 100644 --- a/mbo/testing/matchers_test.cc +++ b/mbo/testing/matchers_test.cc @@ -31,7 +31,6 @@ namespace { // NOLINTBEGIN(*-magic-numbers) -using ::testing::_; using ::testing::ElementsAre; using ::testing::Ge; using ::testing::IsEmpty;