From 442f1acc715ca87391713c3528104fca05de0ff8 Mon Sep 17 00:00:00 2001 From: "A. Cody Schuffelen" Date: Tue, 13 Jan 2026 09:19:55 -0800 Subject: [PATCH] Create a generic `cuttlefish::Pretty` function overloaded for multiple types. The main hard edge here is the use of ADL to make `cuttlefish::Pretty` overloads accessible for invocations from `PrettyContainer` and `PrettyStruct` template functions, which may or may not otherwise see them depending on include order. A longer explanation of the ADL usage is in `pretty.h`. Bug: b/474678754 --- base/cvd/cuttlefish/pretty/BUILD.bazel | 53 +++++++++ base/cvd/cuttlefish/pretty/container.h | 27 ++--- base/cvd/cuttlefish/pretty/container_test.cc | 12 +- base/cvd/cuttlefish/pretty/pretty.h | 56 +++++++++ base/cvd/cuttlefish/pretty/pretty_test.cc | 118 +++++++++++++++++++ base/cvd/cuttlefish/pretty/string.cc | 39 ++++++ base/cvd/cuttlefish/pretty/string.h | 37 ++++++ base/cvd/cuttlefish/pretty/struct.cc | 34 ------ base/cvd/cuttlefish/pretty/struct.h | 14 +-- base/cvd/cuttlefish/pretty/struct_test.cc | 3 +- base/cvd/cuttlefish/pretty/unique_ptr.h | 34 ++++++ base/cvd/cuttlefish/pretty/vector.h | 32 +++++ 12 files changed, 389 insertions(+), 70 deletions(-) create mode 100644 base/cvd/cuttlefish/pretty/pretty.h create mode 100644 base/cvd/cuttlefish/pretty/pretty_test.cc create mode 100644 base/cvd/cuttlefish/pretty/string.cc create mode 100644 base/cvd/cuttlefish/pretty/string.h create mode 100644 base/cvd/cuttlefish/pretty/unique_ptr.h create mode 100644 base/cvd/cuttlefish/pretty/vector.h diff --git a/base/cvd/cuttlefish/pretty/BUILD.bazel b/base/cvd/cuttlefish/pretty/BUILD.bazel index e5cf8f33d59..1be0a2bdc51 100644 --- a/base/cvd/cuttlefish/pretty/BUILD.bazel +++ b/base/cvd/cuttlefish/pretty/BUILD.bazel @@ -9,6 +9,8 @@ cf_cc_library( srcs = ["container.cc"], hdrs = ["container.h"], deps = [ + "//cuttlefish/pretty", + "//cuttlefish/pretty:string", "@abseil-cpp//absl/strings", "@abseil-cpp//absl/strings:str_format", ], @@ -26,11 +28,44 @@ cf_cc_test( ], ) +cf_cc_library( + name = "pretty", + hdrs = ["pretty.h"], +) + +cf_cc_test( + name = "pretty_test", + srcs = ["pretty_test.cc"], + deps = [ + "//cuttlefish/pretty", + "//cuttlefish/pretty:string", + "//cuttlefish/pretty:struct", + "//cuttlefish/pretty:unique_ptr", + "//cuttlefish/pretty:vector", + "@abseil-cpp//absl/strings", + "@fmt", + "@googletest//:gtest", + "@googletest//:gtest_main", + ], +) + +cf_cc_library( + name = "string", + srcs = ["string.cc"], + hdrs = ["string.h"], + deps = [ + "//cuttlefish/pretty", + "@abseil-cpp//absl/strings", + ], +) + cf_cc_library( name = "struct", srcs = ["struct.cc"], hdrs = ["struct.h"], deps = [ + "//cuttlefish/pretty", + "//cuttlefish/pretty:string", "@abseil-cpp//absl/strings", "@abseil-cpp//absl/strings:str_format", ], @@ -40,6 +75,7 @@ cf_cc_test( name = "struct_test", srcs = ["struct_test.cc"], deps = [ + "//cuttlefish/pretty:string", "//cuttlefish/pretty:struct", "@abseil-cpp//absl/strings", "@fmt", @@ -47,3 +83,20 @@ cf_cc_test( "@googletest//:gtest_main", ], ) + +cf_cc_library( + name = "unique_ptr", + hdrs = ["unique_ptr.h"], + deps = [ + "@abseil-cpp//absl/strings", + ], +) + +cf_cc_library( + name = "vector", + hdrs = ["vector.h"], + deps = [ + "//cuttlefish/pretty:container", + "@abseil-cpp//absl/strings", + ], +) diff --git a/base/cvd/cuttlefish/pretty/container.h b/base/cvd/cuttlefish/pretty/container.h index d93febf2ef1..20c25e8dfb0 100644 --- a/base/cvd/cuttlefish/pretty/container.h +++ b/base/cvd/cuttlefish/pretty/container.h @@ -19,26 +19,16 @@ #include #include #include -#include #include #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" -namespace cuttlefish { -namespace { +#include "cuttlefish/pretty/pretty.h" +#include "cuttlefish/pretty/string.h" // IWYU pragma: export -template -constexpr bool ShouldQuote() { - using Simplified = std::decay_t; - bool is_string = std::is_same_v; - bool is_string_view = std::is_same_v; - bool is_char_ptr = std::is_same_v; - return is_string || is_string_view || is_char_ptr; -}; - -} // namespace +namespace cuttlefish { /** * Pretty-prints a container. Invoke this using the `PrettyContainer` overloads. @@ -87,19 +77,16 @@ PrettyContainerType PrettyContainer(const T& container, FmtMemberFn format_member_fn) { PrettyContainerType pretty; for (const auto& member : container) { - std::string formatted = - ShouldQuote() - ? absl::StrCat("\"", format_member_fn(member), "\"") - : absl::StrCat(format_member_fn(member)); - pretty.MemberInternal(formatted); + pretty.MemberInternal(absl::StrCat(format_member_fn(member))); } return pretty; } template PrettyContainerType PrettyContainer(const T& container) { - return PrettyContainer(container, - [](auto&& v) { return std::forward(v); }); + return PrettyContainer(container, [](const auto& member) { + return Pretty(member, PrettyAdlPlaceholder()); + }); } std::ostream& operator<<(std::ostream& out, const PrettyContainerType&); diff --git a/base/cvd/cuttlefish/pretty/container_test.cc b/base/cvd/cuttlefish/pretty/container_test.cc index 6e6b9e49ebb..b791ab0e190 100644 --- a/base/cvd/cuttlefish/pretty/container_test.cc +++ b/base/cvd/cuttlefish/pretty/container_test.cc @@ -40,11 +40,11 @@ void ExpectFormatsTo(const PrettyContainerType& ps, } // namespace -TEST(PrettyStruct, Empty) { +TEST(PrettyContainer, Empty) { ExpectFormatsTo(PrettyContainer(std::vector{}), "{}"); } -TEST(PrettyStruct, OneMember) { +TEST(PrettyContainer, OneMember) { ExpectFormatsTo(PrettyContainer(std::vector{1}), R"( { 1 @@ -52,7 +52,7 @@ TEST(PrettyStruct, OneMember) { )"); } -TEST(PrettyStruct, StringMember) { +TEST(PrettyContainer, StringMember) { ExpectFormatsTo(PrettyContainer(std::vector{"abc"}), R"( { "abc" @@ -60,7 +60,7 @@ TEST(PrettyStruct, StringMember) { )"); } -TEST(PrettyStruct, TwoMembers) { +TEST(PrettyContainer, TwoMembers) { ExpectFormatsTo(PrettyContainer(std::vector{1, 2}), R"( { 1, @@ -69,7 +69,7 @@ TEST(PrettyStruct, TwoMembers) { )"); } -TEST(PrettyStruct, MembersWithNewlines) { +TEST(PrettyContainer, MembersWithNewlines) { ExpectFormatsTo(PrettyContainer(std::vector{"abc\ndef"}), R"( { @@ -79,7 +79,7 @@ TEST(PrettyStruct, MembersWithNewlines) { )"); } -TEST(PrettyStruct, NestedMember) { +TEST(PrettyContainer, NestedMember) { std::vector> container = {{1, 2}, {3, 4}}; ExpectFormatsTo(PrettyContainer(container, PrettyContainer>), R"( diff --git a/base/cvd/cuttlefish/pretty/pretty.h b/base/cvd/cuttlefish/pretty/pretty.h new file mode 100644 index 00000000000..b039525db6c --- /dev/null +++ b/base/cvd/cuttlefish/pretty/pretty.h @@ -0,0 +1,56 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// 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. + +#pragma once + +namespace cuttlefish { + +struct PrettyAdlPlaceholder {}; + +/** + * This is the fallback pretty-printing case: if abseil can print the value, + * have abseil do it. + * + * This function is intended to be overloaded. Overloads for common types are in + * this directory, but other types can also define their own overloads. + * + * Not all overloads will return `std::string`. The returned type should be + * formattable by abseil, libfmt, and ostreams. This allows deferring formatting + * to avoid allocating fewer intermediate strings. + * + * If a type defines both a cuttlefish::Pretty overload and an AbslStringify + * hook, the cuttlefish::Pretty implementation is preferred by overload + * resolution as it is a more specific type. However, if the cuttlefish::Pretty + * overload is defined in a different header, the presence of the header affects + * which overloads are in scope, so leaving the header include out could still + * compile but unintentionally refer to this default fallback. Therefore, + * cuttlefish::Pretty overloads should be declared together with the header. + * + * Worse than that, the `PrettyAdlPlaceHolder()` hack is there to trigger + * argument-dependent lookup within `namespace cuttlefish`: otherwise the + * `Pretty` overloads for non-`namespace cuttlefish` types will only be detected + * based on declaration order through standard name lookup rules. Having an + * argument type declared within `namespace cuttlefish` forces it to search the + * entire namespace rather than only declarations defined earlier in the + * translation unit. This is relevant for `Pretty` invocations within + * `PrettyContainer` and `PrettyStruct`. + */ +template +const T& Pretty(const T& value, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()) { + return value; +} + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/pretty/pretty_test.cc b/base/cvd/cuttlefish/pretty/pretty_test.cc new file mode 100644 index 00000000000..74c5cf9adb6 --- /dev/null +++ b/base/cvd/cuttlefish/pretty/pretty_test.cc @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * 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. + */ + +#include "cuttlefish/pretty/pretty.h" + +#include +#include + +#include "absl/strings/ascii.h" +#include "absl/strings/str_cat.h" +#include "gtest/gtest.h" + +#include "cuttlefish/pretty/string.h" +#include "cuttlefish/pretty/struct.h" +#include "cuttlefish/pretty/unique_ptr.h" +#include "cuttlefish/pretty/vector.h" + +namespace cuttlefish { +namespace { + +struct InnerStruct { + std::string inner_string; + int inner_number; +}; + +PrettyStruct Pretty(const InnerStruct& inner, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()) { + return PrettyStruct("InnerStruct") + .Member("inner_string", inner.inner_string) + .Member("inner_number", inner.inner_number); +} + +struct OuterStruct { + std::vector number_vector; + InnerStruct nested_member; + std::vector nested_vector; + std::unique_ptr int_ptr_set; + std::unique_ptr int_ptr_unset; +}; + +PrettyStruct Pretty(const OuterStruct& outer, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()) { + return PrettyStruct("OuterStruct") + .Member("number_vector", outer.number_vector) + .Member("nested_member", outer.nested_member) + .Member("nested_vector", outer.nested_vector) + .Member("int_ptr_set", outer.int_ptr_set) + .Member("int_ptr_unset", outer.int_ptr_unset); +} + +} // namespace + +TEST(Pretty, OuterInnerStruct) { + OuterStruct outer{ + .number_vector = {1, 2, 3}, + .nested_member = + { + .inner_string = "a", + .inner_number = 1, + }, + .nested_vector = + { + { + .inner_string = "b", + .inner_number = 2, + }, + { + .inner_string = "c", + .inner_number = 3, + }, + }, + .int_ptr_set = std::make_unique(5), + .int_ptr_unset = std::unique_ptr(), + }; + + std::string expected(absl::StripAsciiWhitespace(R"( +OuterStruct { + number_vector: { + 1, + 2, + 3 + }, + nested_member: InnerStruct { + inner_string: "a", + inner_number: 1 + }, + nested_vector: { + InnerStruct { + inner_string: "b", + inner_number: 2 + }, + InnerStruct { + inner_string: "c", + inner_number: 3 + } + }, + int_ptr_set: 5, + int_ptr_unset: (nullptr) +} + )")); + + EXPECT_EQ(absl::StrCat(Pretty(outer)), expected); +} + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/pretty/string.cc b/base/cvd/cuttlefish/pretty/string.cc new file mode 100644 index 00000000000..10ed313267f --- /dev/null +++ b/base/cvd/cuttlefish/pretty/string.cc @@ -0,0 +1,39 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// 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. + +#include "cuttlefish/pretty/string.h" + +#include +#include + +#include "absl/strings/str_cat.h" +#include "cuttlefish/pretty/pretty.h" + +namespace cuttlefish { + +// TODO: schuffelen - escape inner special characters +std::string Pretty(const std::string_view& value, PrettyAdlPlaceholder) { + return absl::StrCat("\"", value, "\""); +} + +std::string Pretty(const char* const& value, PrettyAdlPlaceholder) { + return Pretty(std::string_view(value)); +} + +std::string Pretty(const std::string& value, PrettyAdlPlaceholder) { + return Pretty(std::string_view(value)); +} + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/pretty/string.h b/base/cvd/cuttlefish/pretty/string.h new file mode 100644 index 00000000000..784530037d0 --- /dev/null +++ b/base/cvd/cuttlefish/pretty/string.h @@ -0,0 +1,37 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// 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. + +#pragma once + +#include +#include + +#include "cuttlefish/pretty/pretty.h" + +namespace cuttlefish { + +/** Strings are quoted to distinguish them from other text. */ + +// Necessary for operator overloading precedence on Pretty<...>() +std::string Pretty(const std::string_view& value, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()); + +std::string Pretty(const char* const& value, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()); + +std::string Pretty(const std::string& value, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()); + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/pretty/struct.cc b/base/cvd/cuttlefish/pretty/struct.cc index ed1c02fa345..e91861001ab 100644 --- a/base/cvd/cuttlefish/pretty/struct.cc +++ b/base/cvd/cuttlefish/pretty/struct.cc @@ -25,40 +25,6 @@ namespace cuttlefish { PrettyStruct::PrettyStruct(std::string_view name) : name_(name) {} -PrettyStruct& PrettyStruct::Member(std::string_view name, - std::string_view value) & { - MemberInternal(absl::StrCat(name, ": \"", value, "\"")); - return *this; -} - -PrettyStruct PrettyStruct::Member(std::string_view name, - std::string_view value) && { - Member(name, value); - return *this; -} - -PrettyStruct& PrettyStruct::Member(std::string_view name, const char* value) & { - Member(name, std::string_view(value)); - return *this; -} - -PrettyStruct PrettyStruct::Member(std::string_view name, const char* value) && { - Member(name, std::string_view(value)); - return *this; -} - -PrettyStruct& PrettyStruct::Member(std::string_view name, - const std::string& value) & { - Member(name, std::string_view(value)); - return *this; -} - -PrettyStruct PrettyStruct::Member(std::string_view name, - const std::string& value) && { - Member(name, std::string_view(value)); - return *this; -} - void PrettyStruct::MemberInternal(std::string_view line) { members_.emplace_back(absl::StrReplaceAll(line, {{"\n", "\n "}})); } diff --git a/base/cvd/cuttlefish/pretty/struct.h b/base/cvd/cuttlefish/pretty/struct.h index 50e363ef707..b5af682f855 100644 --- a/base/cvd/cuttlefish/pretty/struct.h +++ b/base/cvd/cuttlefish/pretty/struct.h @@ -25,6 +25,9 @@ #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" +#include "cuttlefish/pretty/pretty.h" +#include "cuttlefish/pretty/string.h" // IWYU pragma: export + namespace cuttlefish { /** @@ -59,7 +62,8 @@ class PrettyStruct { template PrettyStruct& Member(std::string_view name, const T& value) & { - MemberInternal(absl::StrCat(name, ": ", value)); + MemberInternal( + absl::StrCat(name, ": ", Pretty(value, PrettyAdlPlaceholder()))); return *this; } @@ -69,14 +73,6 @@ class PrettyStruct { return std::move(*this); } - // String members are quoted - PrettyStruct& Member(std::string_view name, std::string_view value) &; - PrettyStruct Member(std::string_view name, std::string_view value) &&; - PrettyStruct& Member(std::string_view name, const char* value) &; - PrettyStruct Member(std::string_view name, const char* value) &&; - PrettyStruct& Member(std::string_view name, const std::string& value) &; - PrettyStruct Member(std::string_view name, const std::string& value) &&; - template friend void AbslStringify(Sink& sink, const PrettyStruct& ps) { if (ps.members_.empty()) { diff --git a/base/cvd/cuttlefish/pretty/struct_test.cc b/base/cvd/cuttlefish/pretty/struct_test.cc index 8f476f9bc27..4be0d789d71 100644 --- a/base/cvd/cuttlefish/pretty/struct_test.cc +++ b/base/cvd/cuttlefish/pretty/struct_test.cc @@ -17,11 +17,12 @@ #include "cuttlefish/pretty/struct.h" #include - #include "absl/strings/ascii.h" #include "fmt/format.h" #include "gtest/gtest.h" +#include "cuttlefish/pretty/string.h" + namespace cuttlefish { namespace { diff --git a/base/cvd/cuttlefish/pretty/unique_ptr.h b/base/cvd/cuttlefish/pretty/unique_ptr.h new file mode 100644 index 00000000000..050aa6a6835 --- /dev/null +++ b/base/cvd/cuttlefish/pretty/unique_ptr.h @@ -0,0 +1,34 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// 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. + +#pragma once + +#include +#include + +#include "absl/strings/str_cat.h" + +#include "cuttlefish/pretty/pretty.h" + +namespace cuttlefish { + +template +std::string Pretty(const std::unique_ptr& value, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()) { + return value.get() ? absl::StrCat(Pretty(*value, PrettyAdlPlaceholder())) + : "(nullptr)"; +} + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/pretty/vector.h b/base/cvd/cuttlefish/pretty/vector.h new file mode 100644 index 00000000000..f066ec904d1 --- /dev/null +++ b/base/cvd/cuttlefish/pretty/vector.h @@ -0,0 +1,32 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// 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. + +#pragma once + +#include + +#include "cuttlefish/pretty/container.h" +#include "cuttlefish/pretty/pretty.h" + +namespace cuttlefish { + +template +PrettyContainerType Pretty( + const std::vector& vec, + PrettyAdlPlaceholder unused = PrettyAdlPlaceholder()) { + return PrettyContainer(vec); +} + +} // namespace cuttlefish