Enable MessageLite::DebugString to use Message::DebugString where pos…

…sible. 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
protocolbuffers · Jul 3, 2024 · f810cc5 · f810cc5
1 parent 6750ed8
commit f810cc5
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 24 deletions.
diff --git a/rust/cpp.rs b/rust/cpp.rs
@@ -293,17 +293,12 @@ impl From<RustStringRawParts> 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
@@ -2,19 +2,12 @@
 
 #include <string>
 
-#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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Message>(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
@@ -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
@@ -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
@@ -18,9 +18,10 @@
 #include <sys/types.h>
 
 #include <cmath>
-#include <functional>
+#include <cstddef>
+#include <cstdint>
 #include <limits>
-#include <vector>
+#include <string>
 
 #ifndef _MSC_VER
 #include <unistd.h>
@@ -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<UNITTEST::TestRequired>(test_all_types_pointer),
+      DownCastMessage<UNITTEST::TestRequired>(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) {