From bf31a309e8cdb3155a9aaa550b2e8dcde767abcc Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Mon, 6 Oct 2025 09:43:26 -0600 Subject: [PATCH 1/2] :children_crossing: Add fail tests for field not being in message Problem: - When a field is not in a message, trying to access it can give an help message from inside `stdx::tuple` instead of a nicer message. Solution: - Give a nicer help message when trying to access a named field that isn't in a message. --- include/msg/message.hpp | 11 +++++++---- test/msg/fail/CMakeLists.txt | 4 ++++ test/msg/fail/message_get_nonexistent_field.cpp | 17 +++++++++++++++++ test/msg/fail/message_set_nonexistent_field.cpp | 17 +++++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 test/msg/fail/message_get_nonexistent_field.cpp create mode 100644 test/msg/fail/message_set_nonexistent_field.cpp diff --git a/include/msg/message.hpp b/include/msg/message.hpp index 1bdfa22c..481c90c0 100644 --- a/include/msg/message.hpp +++ b/include/msg/message.hpp @@ -198,7 +198,10 @@ template class msg_access { using FieldsTuple = decltype(stdx::make_indexed_tuple(Fields{}...)); - template constexpr static auto check() { + template constexpr static auto check() { + static_assert((std::is_same_v> or ...), + "Field does not belong to this message!"); + using Field = field_t; constexpr auto belongs = (std::is_same_v or ...); @@ -209,20 +212,20 @@ template class msg_access { template constexpr static auto set1(R &&r, V v) -> void { + check, std::remove_cvref_t>(); using Field = field_t>; - check>(); Field::insert(std::forward(r), static_cast(v.value)); } template constexpr static auto set_default(R &&r) -> void { - check, std::remove_cvref_t>(); + check>(); field_t::insert_default(std::forward(r)); } template constexpr static auto get(R &&r) { - check, std::remove_cvref_t>(); + check>(); return field_t::extract(std::forward(r)); } diff --git a/test/msg/fail/CMakeLists.txt b/test/msg/fail/CMakeLists.txt index 5c44f143..0af5cc91 100644 --- a/test/msg/fail/CMakeLists.txt +++ b/test/msg/fail/CMakeLists.txt @@ -13,8 +13,12 @@ add_compile_fail_test(message_cmp_view.cpp LIBRARIES warnings cib_msg) add_compile_fail_test(message_const_field_write.cpp LIBRARIES warnings cib_msg) add_compile_fail_test(message_dangling_view.cpp LIBRARIES warnings cib_msg) add_compile_fail_test(message_dup_fieldnames.cpp LIBRARIES warnings cib_msg) +add_compile_fail_test(message_get_nonexistent_field.cpp LIBRARIES warnings + cib_msg) add_compile_fail_test(message_incompatible_matcher.cpp LIBRARIES warnings cib_msg) +add_compile_fail_test(message_set_nonexistent_field.cpp LIBRARIES warnings + cib_msg) add_compile_fail_test(message_uninitialized_field.cpp LIBRARIES warnings cib_msg) add_compile_fail_test(view_upsize.cpp LIBRARIES warnings cib_msg) diff --git a/test/msg/fail/message_get_nonexistent_field.cpp b/test/msg/fail/message_get_nonexistent_field.cpp new file mode 100644 index 00000000..692204da --- /dev/null +++ b/test/msg/fail/message_get_nonexistent_field.cpp @@ -0,0 +1,17 @@ +#include +#include + +// EXPECT: Field does not belong to this message +namespace { +using namespace msg; + +using test_field1 = + field<"test_field", std::uint32_t>::located; + +using msg_defn = message<"test_msg", test_field1>; +} // namespace + +auto main() -> int { + owning m{}; + [[maybe_unused]] auto f = m.get("no"_field); +} diff --git a/test/msg/fail/message_set_nonexistent_field.cpp b/test/msg/fail/message_set_nonexistent_field.cpp new file mode 100644 index 00000000..889dd949 --- /dev/null +++ b/test/msg/fail/message_set_nonexistent_field.cpp @@ -0,0 +1,17 @@ +#include +#include + +// EXPECT: Field does not belong to this message +namespace { +using namespace msg; + +using test_field1 = + field<"test_field", std::uint32_t>::located; + +using msg_defn = message<"test_msg", test_field1>; +} // namespace + +auto main() -> int { + owning m{}; + m.set("no"_field = 1); +} From 52fa25437795d6f831999529bef465ed4e76921e Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Mon, 6 Oct 2025 09:44:45 -0600 Subject: [PATCH 2/2] :children_crossing: Add `operator[]` to messages Problem: - Messages support `get` and `set` but not a natural indexing syntax. Solution: - Add `operator[]` to message, so that the follow work: ```c+++ msg["a"_field] = 42; auto x = msg["a"_field]; ``` --- include/msg/message.hpp | 25 +++++++++++++++++++++++++ test/msg/message.cpp | 15 +++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/include/msg/message.hpp b/include/msg/message.hpp index 481c90c0..0859b588 100644 --- a/include/msg/message.hpp +++ b/include/msg/message.hpp @@ -344,11 +344,36 @@ template struct msg_base { [[nodiscard]] constexpr auto get(auto f) const { return Access::get(as_derived().data(), f); } + constexpr auto set(auto... fs) -> void { Access::set(as_derived().data(), fs...); } constexpr auto set() -> void {} + template struct proxy { + // NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members) + msg_base &b; + + // NOLINTNEXTLINE(misc-unconventional-assign-operator) + constexpr auto operator=(auto val) const && -> void { + b.set(field_name{} = val); + } + + using V = decltype(b.get(std::declval>())); + + // NOLINTNEXTLINE(google-explicit-constructor) + constexpr operator V() const { return b.get(field_name{}); } + }; + + template + [[nodiscard]] constexpr auto operator[](field_name f) const { + return get(f); + } + template + [[nodiscard]] constexpr auto operator[](field_name) LIFETIMEBOUND { + return proxy{*this}; + } + [[nodiscard]] constexpr auto describe() const { return Access::describe(as_derived().data()); } diff --git a/test/msg/message.cpp b/test/msg/message.cpp index 1f4a1e90..6d43d724 100644 --- a/test/msg/message.cpp +++ b/test/msg/message.cpp @@ -792,3 +792,18 @@ TEST_CASE("pack appends environments", "[message]") { using defn = pack<"defn", std::uint8_t, m1, m2>; STATIC_REQUIRE(custom(defn::env_t{}) == 18); } + +TEST_CASE("read indexing operator on message", "[message]") { + test_msg const msg{}; + CHECK(0x80 == msg["id"_field]); +} + +// This test causes clang-14 with libc++ to crash... +#if not __clang__ or __clang_major__ > 14 +TEST_CASE("write indexing operator on message", "[message]") { + test_msg msg{}; + CHECK((0 == msg["f1"_field])); + msg["f1"_field] = 0xba11; + CHECK((0xba11 == msg["f1"_field])); +} +#endif