From 1094eec28129209b137aa8f0ecaf228ed49d7301 Mon Sep 17 00:00:00 2001 From: Marcus Boerger Date: Wed, 13 Mar 2024 18:32:01 +0000 Subject: [PATCH] Costmetics. (#17) * Costmetics. Avoid `snprintf` return value related issues. * Add missing `__clang__` checks. --- .clangd | 2 -- mbo/container/any_scan.h | 4 +-- mbo/types/extend_test.cc | 68 +++++++++++++++++++++++++--------------- mbo/types/extender.h | 2 +- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/.clangd b/.clangd index 8152c7a..fdac5bf 100644 --- a/.clangd +++ b/.clangd @@ -1,5 +1,3 @@ -CompileFlags: - Compiler: /usr/bin/clang Diagnostics: UnusedIncludes: Strict diff --git a/mbo/container/any_scan.h b/mbo/container/any_scan.h index 84cc55c..5aba58a 100644 --- a/mbo/container/any_scan.h +++ b/mbo/container/any_scan.h @@ -418,14 +418,14 @@ class AnyScanImpl { // That means we bypass any protection an iterator may have, but we can make this function // `noexcept` assuming the iterator is noexcept for access. On the other hand we expect that // out of bounds access may actually raise. So we effectively side step such exceptions. - ABSL_CHECK(funcs_.curr != nullptr && funcs_.more()); + ABSL_CHECK(funcs_.curr != nullptr && funcs_.more()); // NOLINT(*-missing-default-case) return funcs_.curr(); } value_type operator*() const noexcept requires(!kAccessByRef) { - ABSL_CHECK(funcs_.curr != nullptr && funcs_.more()); + ABSL_CHECK(funcs_.curr != nullptr && funcs_.more()); // NOLINT(*-missing-default-case) return value_type(funcs_.curr()); } diff --git a/mbo/types/extend_test.cc b/mbo/types/extend_test.cc index a4b40b7..a73b909 100644 --- a/mbo/types/extend_test.cc +++ b/mbo/types/extend_test.cc @@ -177,10 +177,13 @@ TEST_F(ExtendTest, Streamable) { Conditional(kStructNameSupport, R"({.a: 25, .b: 42, .c: "", .ptr: })", R"({25, 42, "", })")); } +#if defined(__clang__) + namespace debug { +// NOLINTBEGIN(bugprone-easily-swappable-parameters,cert-dcl50-cpp) int DumpStructVisitor( - std::size_t field_index, + std::size_t /*field_index*/, std::string_view format, std::string_view indent = {}, std::string_view type = {}, @@ -190,55 +193,64 @@ int DumpStructVisitor( std::cout << ", Indent: '" << indent << "'"; std::cout << ", Type: '" << type << "'"; std::cout << ", Name: '" << name << "'"; - std::cout << std::endl; + std::cout << '\n'; return 0; } int PrintStructVisitor( - std::size_t field_index, + std::size_t /*field_index*/, std::vector>& fields, std::string_view format, std::string_view indent = {}, std::string_view type = {}, std::string_view name = {}, ...) { - char buffer[1'024]; - int l = 0; - if (!format.starts_with('%')) { - l = snprintf(buffer, 1'023, "%s", format.data()); - } else if (format == "%s" || format == "%s}\n") { - l = snprintf(buffer, 1'023, format.data(), indent.data()); - } else if (format == "%s%s") { - l = snprintf(buffer, 1'023, format.data(), indent.data(), type.data()); - } else if (format == "%s%s %s =") { - l = snprintf(buffer, 1'023, "%s%s %s =", indent.data(), type.data(), name.data()); - } else if (format.starts_with("%s%s %s =")) { - l = snprintf(buffer, 1'023, "%s%s %s =\n", indent.data(), type.data(), name.data()); - } + const std::string line = [&]() -> std::string { + if (!format.starts_with('%')) { + return std::string(format); + } else if (format == "%s" || format == "%s}\n") { + return absl::StrCat(indent, format.substr(2)); + } else if (format == "%s%s") { + return absl::StrFormat("%s%s", indent.data(), type.data()); + } else if (format == "%s%s %s =") { + return absl::StrFormat("%s%s %s =", indent.data(), type.data(), name.data()); + } else if (format.starts_with("%s%s %s =")) { + return absl::StrFormat("%s%s %s =\n", indent.data(), type.data(), name.data()); + } else { + return absl::StrFormat("Unknown format: '%s'", format); + } + }(); + std::cout << line; if (format.starts_with("%s%s %s =") && indent == " ") { fields.emplace_back(format, indent, name); } - buffer[l] = 0; - std::cout << buffer; return 0; } +// NOLINTEND(bugprone-easily-swappable-parameters,cert-dcl50-cpp) + +// NOLINTBEGIN(cppcoreguidelines-pro-type-vararg,hicpp-vararg) template void Print(const T* ptr) { std::size_t field_index = 0; std::vector> fields; __builtin_dump_struct(ptr, &PrintStructVisitor, field_index, fields); - std::cout << std::endl; + std::cout << '\n'; for (const auto& [format, indent, name] : fields) { - std::cout << format << " -> " << name << std::endl; + std::cout << format << " -> " << name << '\n'; } field_index = 0; __builtin_dump_struct(ptr, &DumpStructVisitor, field_index); } + +// NOLINTEND(cppcoreguidelines-pro-type-vararg,hicpp-vararg) + } // namespace debug +#endif // __clang__ + struct PersonData : Extend { - int index; + int index = 0; Person person; const std::set* data = nullptr; }; @@ -246,7 +258,7 @@ struct PersonData : Extend { TEST_F(ExtendTest, StreamableComplexFields) { const std::set data{"foo", "bar"}; PersonData person{ - .index = 25, + .index = 25, // NOLINT(*-magic-numbers) .person = { .name = @@ -254,7 +266,7 @@ TEST_F(ExtendTest, StreamableComplexFields) { .first = "Hugo", .last = "Meyer", }, - .age = 42, + .age = 42, // NOLINT(*-magic-numbers) }, .data = &data, }; @@ -266,8 +278,14 @@ TEST_F(ExtendTest, StreamableComplexFields) { Conditional( kStructNameSupport, R"({25, {.name: {.first: "Hugo", .last: "Meyer"}, .age: 42}, *{{"bar", "foo"}}})", R"({25, {{"Hugo", "Meyer"}, 42}, *{{"bar", "foo"}}})")); - // debug::Print(&person); - // debug::Print(&person.person.name); +#ifdef __clang__ + if (HasFailure()) { + std::cout << "Person:\n"; + debug::Print(&person); + std::cout << "Person::person.name:\n"; + debug::Print(&person.person.name); + } +#endif // __clang__ } TEST_F(ExtendTest, Comparable) { diff --git a/mbo/types/extender.h b/mbo/types/extender.h index e3f13ef..3a824b2 100644 --- a/mbo/types/extender.h +++ b/mbo/types/extender.h @@ -60,7 +60,7 @@ #include "absl/strings/str_format.h" #include "mbo/types/internal/extender.h" // IWYU pragma: export #include "mbo/types/internal/struct_names.h" // IWYU pragma: keep -#include "mbo/types/traits.h" +#include "mbo/types/traits.h" // IWYU pragma: keep #include "mbo/types/tstring.h" namespace mbo::types::extender {