From f810cc5b668d78cb982f44b34f188160d55c27d6 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 3 Jul 2024 09:35:29 -0700 Subject: [PATCH] Enable MessageLite::DebugString to use Message::DebugString where possible. This will mean that calling DebugString on a MessageLite* which is actually a full Message will get the debug info instead of the minimal output. PiperOrigin-RevId: 649103508 --- rust/cpp.rs | 5 ----- rust/cpp_kernel/debug.cc | 7 ------- rust/cpp_kernel/debug.h | 4 ---- rust/test/cpp/BUILD | 16 ++++++++++++++++ rust/test/cpp/debug_test.rs | 12 ++++++++++++ rust/test/cpp/optimize_for_lite.proto | 10 ++++++++++ src/google/protobuf/compiler/rust/generator.cc | 1 + src/google/protobuf/message.cc | 11 +++++++---- src/google/protobuf/message_lite.cc | 6 ++++++ src/google/protobuf/message_lite.h | 1 + src/google/protobuf/message_unittest.inc | 18 ++++++++++++++---- 11 files changed, 67 insertions(+), 24 deletions(-) create mode 100644 rust/test/cpp/optimize_for_lite.proto diff --git a/rust/cpp.rs b/rust/cpp.rs index 7037747dee..c2a362aedd 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -293,17 +293,12 @@ impl From for String { extern "C" { fn proto2_rust_utf8_debug_string(msg: RawMessage) -> RustStringRawParts; - fn proto2_rust_utf8_debug_string_lite(msg: RawMessage) -> RustStringRawParts; } pub fn debug_string(_private: Private, msg: RawMessage, f: &mut fmt::Formatter<'_>) -> fmt::Result { // SAFETY: // - `msg` is a valid protobuf message. - #[cfg(not(lite_runtime))] let dbg_str: String = unsafe { proto2_rust_utf8_debug_string(msg) }.into(); - #[cfg(lite_runtime)] - let dbg_str: String = unsafe { proto2_rust_utf8_debug_string_lite(msg) }.into(); - write!(f, "{dbg_str}") } diff --git a/rust/cpp_kernel/debug.cc b/rust/cpp_kernel/debug.cc index 6a396cacf5..28b33d5e6e 100644 --- a/rust/cpp_kernel/debug.cc +++ b/rust/cpp_kernel/debug.cc @@ -2,19 +2,12 @@ #include -#include "google/protobuf/message.h" #include "google/protobuf/message_lite.h" #include "rust/cpp_kernel/strings.h" extern "C" { google::protobuf::rust::RustStringRawParts proto2_rust_utf8_debug_string( - const google::protobuf::Message* msg) { - std::string text = google::protobuf::Utf8Format(*msg); - return google::protobuf::rust::RustStringRawParts(text); -} - -google::protobuf::rust::RustStringRawParts proto2_rust_utf8_debug_string_lite( const google::protobuf::MessageLite* msg) { std::string text = google::protobuf::Utf8Format(*msg); return google::protobuf::rust::RustStringRawParts(text); diff --git a/rust/cpp_kernel/debug.h b/rust/cpp_kernel/debug.h index cd39ef5a3b..6925d17bc7 100644 --- a/rust/cpp_kernel/debug.h +++ b/rust/cpp_kernel/debug.h @@ -1,16 +1,12 @@ #ifndef GOOGLE_PROTOBUF_RUST_CPP_KERNEL_DEBUG_H__ #define GOOGLE_PROTOBUF_RUST_CPP_KERNEL_DEBUG_H__ -#include "google/protobuf/message.h" #include "google/protobuf/message_lite.h" #include "rust/cpp_kernel/strings.h" extern "C" { google::protobuf::rust::RustStringRawParts proto2_rust_utf8_debug_string( - const google::protobuf::Message* msg); - -google::protobuf::rust::RustStringRawParts proto2_rust_utf8_debug_string_lite( const google::protobuf::MessageLite* msg); } // extern "C" diff --git a/rust/test/cpp/BUILD b/rust/test/cpp/BUILD index 80b676122d..3d6d68b05b 100644 --- a/rust/test/cpp/BUILD +++ b/rust/test/cpp/BUILD @@ -29,11 +29,27 @@ rust_cc_proto_library( deps = [":debug_proto"], ) +proto_library( + name = "optimize_for_lite_proto", + testonly = True, + srcs = ["optimize_for_lite.proto"], +) + +rust_cc_proto_library( + name = "optimize_for_lite_cpp_rust_proto", + testonly = True, + visibility = [ + "//rust/test/shared:__subpackages__", + ], + deps = [":optimize_for_lite_proto"], +) + rust_test( name = "debug_test", srcs = ["debug_test.rs"], deps = [ ":debug_cpp_rust_proto", + ":optimize_for_lite_cpp_rust_proto", "//rust:protobuf_cpp", "@crate_index//:googletest", ], diff --git a/rust/test/cpp/debug_test.rs b/rust/test/cpp/debug_test.rs index 9da6289345..82b2d1b773 100644 --- a/rust/test/cpp/debug_test.rs +++ b/rust/test/cpp/debug_test.rs @@ -1,5 +1,6 @@ use debug_rust_proto::DebugMsg; use googletest::prelude::*; +use optimize_for_lite_rust_proto::OptimizeForLiteTestMessage; #[cfg(not(lite_runtime))] #[test] @@ -22,3 +23,14 @@ fn test_debug_lite() { assert_that!(format!("{msg:?}"), contains_substring("MessageLite")); assert_that!(format!("{msg:?}"), not(contains_substring("password"))); } + +/// A message with the option set to optimize for lite will behave as a lite +/// message regardless of the `lite_runtime` feature. Run this test not guarded +/// by the cfg(lite_runtime) and ensure it functions as lite. +#[test] +fn test_optimize_for_lite_option() { + let mut msg = OptimizeForLiteTestMessage::new(); + msg.set_value("password"); + assert_that!(format!("{msg:?}"), contains_substring("MessageLite")); + assert_that!(format!("{msg:?}"), not(contains_substring("password"))); +} diff --git a/rust/test/cpp/optimize_for_lite.proto b/rust/test/cpp/optimize_for_lite.proto new file mode 100644 index 0000000000..d8b093942c --- /dev/null +++ b/rust/test/cpp/optimize_for_lite.proto @@ -0,0 +1,10 @@ +edition = "2023"; + +package optimize_for_lite_test; + +option java_multiple_files = true; +option optimize_for = LITE_RUNTIME; + +message OptimizeForLiteTestMessage { + string value = 1; +} diff --git a/src/google/protobuf/compiler/rust/generator.cc b/src/google/protobuf/compiler/rust/generator.cc index 84ef0f6ba1..4412748c2b 100644 --- a/src/google/protobuf/compiler/rust/generator.cc +++ b/src/google/protobuf/compiler/rust/generator.cc @@ -218,6 +218,7 @@ bool RustGenerator::Generate(const FileDescriptor* file, #include "$proto_h$" $proto_deps_h$ #include "google/protobuf/map.h" +#include "google/protobuf/repeated_field.h" #include "google/protobuf/repeated_ptr_field.h" #include "google/protobuf/rust/cpp_kernel/map.h" #include "google/protobuf/rust/cpp_kernel/serialized_data.h" diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index c84194a1f7..27c82f7869 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -224,12 +224,15 @@ size_t Message::SpaceUsedLongImpl(const MessageLite& msg_lite) { return msg.GetReflection()->SpaceUsedLong(msg); } +static std::string DebugStringImpl(const MessageLite& msg) { + return DownCastMessage(msg).DebugString(); +} + PROTOBUF_CONSTINIT const MessageLite::DescriptorMethods Message::kDescriptorMethods = { - GetTypeNameImpl, - InitializationErrorStringImpl, - GetTcParseTableImpl, - SpaceUsedLongImpl, + GetTypeNameImpl, InitializationErrorStringImpl, + GetTcParseTableImpl, SpaceUsedLongImpl, + DebugStringImpl, }; namespace internal { diff --git a/src/google/protobuf/message_lite.cc b/src/google/protobuf/message_lite.cc index b70fc3fba7..22fe7d9634 100644 --- a/src/google/protobuf/message_lite.cc +++ b/src/google/protobuf/message_lite.cc @@ -128,6 +128,12 @@ std::string MessageLite::InitializationErrorString() const { } std::string MessageLite::DebugString() const { + auto* data = GetClassData(); + ABSL_DCHECK(data != nullptr); + if (!data->is_lite) { + return data->full().descriptor_methods->debug_string(*this); + } + return absl::StrCat("MessageLite at 0x", absl::Hex(this)); } diff --git a/src/google/protobuf/message_lite.h b/src/google/protobuf/message_lite.h index bc366f9824..9492277322 100644 --- a/src/google/protobuf/message_lite.h +++ b/src/google/protobuf/message_lite.h @@ -618,6 +618,7 @@ class PROTOBUF_EXPORT MessageLite { std::string (*initialization_error_string)(const MessageLite&); const internal::TcParseTableBase* (*get_tc_table)(const MessageLite&); size_t (*space_used_long)(const MessageLite&); + std::string (*debug_string)(const MessageLite&); }; // Note: The order of arguments in the functions is chosen so that it has diff --git a/src/google/protobuf/message_unittest.inc b/src/google/protobuf/message_unittest.inc index 37a1242980..42c06ed820 100644 --- a/src/google/protobuf/message_unittest.inc +++ b/src/google/protobuf/message_unittest.inc @@ -18,9 +18,10 @@ #include #include -#include +#include +#include #include -#include +#include #ifndef _MSC_VER #include @@ -825,14 +826,23 @@ TEST(MESSAGE_TEST_NAME, DownCastMessageInvalidPointerType) { TEST(MESSAGE_TEST_NAME, DownCastMessageInvalidReferenceType) { UNITTEST::TestAllTypes test_all_types; - MessageLite& test_all_types_pointer = test_all_types; + MessageLite& test_all_types_ref = test_all_types; ASSERT_DEBUG_DEATH( - DownCastMessage(test_all_types_pointer), + DownCastMessage(test_all_types_ref), "Cannot downcast " + test_all_types.GetTypeName() + " to " + UNITTEST::TestRequired::default_instance().GetTypeName()); } +TEST(MESSAGE_TEST_NAME, MessageDebugStringMatchesBehindPointerAndLitePointer) { + UNITTEST::TestAllTypes test_all_types; + test_all_types.set_optional_string("foo"); + Message* msg_full_pointer = &test_all_types; + MessageLite* msg_lite_pointer = &test_all_types; + ASSERT_EQ(test_all_types.DebugString(), msg_full_pointer->DebugString()); + ASSERT_EQ(test_all_types.DebugString(), msg_lite_pointer->DebugString()); +} + #if GTEST_HAS_DEATH_TEST // death tests do not work on Windows yet. TEST(MESSAGE_TEST_NAME, SerializeFailsIfNotInitialized) {