Automated rollback of commit fe7c4f9422.

PiperOrigin-RevId: 538128679

.github/workflows/test_rust.yml

@@ -27,6 +27,7 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: rust_linux
           bazel: |
-            test //rust/upb_kernel:upb_test //rust/cpp_kernel:cpp_test \
+            test //rust:protobuf_upb_test //rust:protobuf_cpp_test \
+              //rust/upb_kernel:upb_test //rust/cpp_kernel:cpp_test \
               //rust/test/rust_proto_library_unit_test:rust_upb_aspect_test \
               //rust/upb_kernel:upb_test //src/google/protobuf/compiler/rust/...

rust/BUILD

@@ -1,6 +1,6 @@
 # Protobuf Rust runtime packages.
 
-load("@rules_rust//rust:defs.bzl", "rust_library")
+load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test")
 load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
 
 load("@rules_proto//proto:defs.bzl", "proto_lang_toolchain")
@@ -30,22 +30,68 @@ rust_library(
         "//conditions:default": ["--cfg=cpp_kernel"],
     }),
     deps = select({
-        ":use_upb_kernel": ["//rust/upb_kernel:upb"],
+        ":use_upb_kernel": [":protobuf_upb"],
-        "//conditions:default": ["//rust/cpp_kernel:cpp"],
+        "//conditions:default": [":protobuf_cpp"],
     }),
 )
 
+# Represents Rust Protobuf runtime using the upb kernel.
+#
+# `rust_upb_proto_library` implicitly depends on this target. This target cannot depend on
+# `:rust_proto_library_kernel` build setting; it has to be fully functional under any value of that
+# setting.
+#
+# `shared.rs` contains kernel-agnostic logic and simple kernel-specific logic controlled by
+# `#[cfg(...)]` attributes. That forces us to compile this file twice, once for each kernel. As a
+# result this file is declared in both `:protobuf_upb` and `:protobuf_cpp`. This is in principle
+# identical to how we compile regular Rust source files twice (once for production, and once for
+# unittesting).
 rust_library(
-    name = "common",
+    name = "protobuf_upb",
-    srcs = ["common.rs"],
+    srcs = ["shared.rs"],
-    visibility = ["//rust:__subpackages__"],
+    rustc_flags = ["--cfg=upb_kernel"],
+    deps = ["//rust/upb_kernel:upb"],
+)
+
+rust_test(
+    name = "protobuf_upb_test",
+    crate = ":protobuf_upb",
+    rustc_flags = ["--cfg=upb_kernel"],
+    tags = [
+        # TODO(b/270274576): Enable testing on arm once we have a Rust Arm toolchain.
+        "not_build:arm",
+    ],
+)
+
+# Represents Rust Protobuf runtime using the cpp kernel.
+#
+# `rust_cpp_proto_library` implicitly depends on this target. This target cannot depend on
+# `:rust_proto_library_kernel` build setting; it has to be fully functional under any value of that
+# setting.
+#
+# See the comment for `:protobuf` for discussion of `shared.rs` file.
+rust_library(
+    name = "protobuf_cpp",
+    srcs = ["shared.rs"],
+    rustc_flags = ["--cfg=cpp_kernel"],
+    deps = ["//rust/cpp_kernel:cpp"],
+)
+
+rust_test(
+    name = "protobuf_cpp_test",
+    crate = ":protobuf_cpp",
+    rustc_flags = ["--cfg=cpp_kernel"],
+    tags = [
+        # TODO(b/270274576): Enable testing on arm once we have a Rust Arm toolchain.
+        "not_build:arm",
+    ],
 )
 
 proto_lang_toolchain(
     name = "proto_rust_upb_toolchain",
     command_line = "--rust_out=experimental-codegen=enabled,kernel=upb:$(OUT)",
     progress_message = "Generating Rust proto_library %{label}",
-    runtime = "//rust/upb_kernel:upb",
+    runtime = ":protobuf_upb",
     visibility = ["//visibility:public"],
 )
 
@@ -53,7 +99,7 @@ proto_lang_toolchain(
     name = "proto_rust_cpp_toolchain",
     command_line = "--rust_out=experimental-codegen=enabled,kernel=cpp:$(OUT)",
     progress_message = "Generating Rust proto_library %{label}",
-    runtime = "//rust/cpp_kernel:cpp",
+    runtime = ":protobuf_cpp",
     visibility = ["//visibility:public"],
 )
 

rust/cpp_kernel/BUILD

@@ -2,11 +2,6 @@
 
 load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test")
 
-# Represents Rust Protobuf runtime using the cpp kernel.
-#
-# `rust_cpp_proto_library` implicitly depends on this target. This target cannot depend on
-# `:rust_proto_library_kernel` build setting; it has to be fully functional under any value of that
-# setting.
 rust_library(
     name = "cpp",
     srcs = ["cpp.rs"],
@@ -14,7 +9,6 @@ rust_library(
         "//src/google/protobuf:__subpackages__",
         "//rust:__subpackages__",
     ],
-    deps = ["//rust:common"],
 )
 
 rust_test(

rust/cpp_kernel/cpp.rs

@@ -30,78 +30,28 @@
 
 // Rust Protobuf runtime using the C++ kernel.
 
-pub use common::ParseError;
-pub use common::PtrAndLen;
-use std::fmt;
-use std::ops::Deref;
-use std::ptr::NonNull;
-use std::slice;
-
 use std::alloc;
 use std::alloc::Layout;
+use std::boxed::Box;
 use std::cell::UnsafeCell;
+use std::fmt;
 use std::marker::PhantomData;
 use std::mem::MaybeUninit;
-
+use std::ops::Deref;
-/// Represents serialized Protobuf wire format data. It's typically produced
+use std::ptr::NonNull;
-/// by `<Message>.serialize()`.
+use std::slice;
-///
-/// This struct is ABI compatible with the equivalent struct on the C++
-/// side. It owns (and drops) its data.
-// copybara:strip_begin
-// LINT.IfChange
-// copybara:strip_end
-#[repr(C)]
-pub struct SerializedData {
-    /// Owns the memory.
-    data: NonNull<u8>,
-    len: usize,
-}
-// copybara:strip_begin
-// LINT.ThenChange(//depot/google3/third_party/protobuf/rust/cpp_kernel/cpp_api.
-// h) copybara:strip_end
-
-impl SerializedData {
-    pub unsafe fn from_raw_parts(data: NonNull<u8>, len: usize) -> Self {
-        Self { data, len }
-    }
-}
-
-impl Deref for SerializedData {
-    type Target = [u8];
-    fn deref(&self) -> &Self::Target {
-        unsafe { slice::from_raw_parts(self.data.as_ptr(), self.len) }
-    }
-}
-
-impl Drop for SerializedData {
-    fn drop(&mut self) {
-        unsafe {
-            alloc::dealloc(self.data.as_ptr(), Layout::array::<u8>(self.len).unwrap());
-        };
-    }
-}
-
-impl fmt::Debug for SerializedData {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Debug::fmt(self.deref(), f)
-    }
-}
-
-pub mod __runtime {
-    use super::*;
 
 /// A wrapper over a `proto2::Arena`.
 ///
-    /// This is not a safe wrapper per se, because the allocation functions
+/// This is not a safe wrapper per se, because the allocation functions still
-    /// still have sharp edges (see their safety docs for more info).
+/// have sharp edges (see their safety docs for more info).
 ///
 /// This is an owning type and will automatically free the arena when
 /// dropped.
 ///
 /// Note that this type is neither `Sync` nor `Send`.
 pub struct Arena {
-        _ptr: NonNull<u8>,
+    ptr: NonNull<u8>,
     _not_sync: PhantomData<UnsafeCell<()>>,
 }
 
@@ -109,7 +59,7 @@ pub mod __runtime {
     /// Allocates a fresh arena.
     #[inline]
     pub fn new() -> Self {
-            Self { _ptr: NonNull::dangling(), _not_sync: PhantomData }
+        Self { ptr: NonNull::dangling(), _not_sync: PhantomData }
     }
 
     /// Returns the raw, C++-managed pointer to the arena.
@@ -124,7 +74,7 @@ pub mod __runtime {
     ///
     /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`.
     #[inline]
-        pub unsafe fn alloc(&self, _layout: Layout) -> &mut [MaybeUninit<u8>] {
+    pub unsafe fn alloc(&self, layout: Layout) -> &mut [MaybeUninit<u8>] {
         unimplemented!()
     }
 
@@ -133,15 +83,10 @@ pub mod __runtime {
     /// # Safety
     ///
     /// After calling this function, `ptr` is essentially zapped. `old` must
-        /// be the layout `ptr` was allocated with via [`Arena::alloc()`].
+    /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. `new`'s
-        /// `new`'s alignment must be less than `UPB_MALLOC_ALIGN`.
+    /// alignment must be less than `UPB_MALLOC_ALIGN`.
     #[inline]
-        pub unsafe fn resize(
+    pub unsafe fn resize(&self, ptr: *mut u8, old: Layout, new: Layout) -> &[MaybeUninit<u8>] {
-            &self,
-            _ptr: *mut u8,
-            _old: Layout,
-            _new: Layout,
-        ) -> &[MaybeUninit<u8>] {
         unimplemented!()
     }
 }
@@ -152,12 +97,55 @@ pub mod __runtime {
         // unimplemented
     }
 }
-} // mod __runtime
+
+/// Represents serialized Protobuf wire format data. It's typically produced by
+/// `<Message>.serialize()`.
+///
+/// This struct is ABI compatible with the equivalent struct on the C++ side. It
+/// owns (and drops) its data.
+// copybara:strip_begin
+// LINT.IfChange
+// copybara:strip_end
+#[repr(C)]
+pub struct SerializedData {
+    /// Owns the memory.
+    data: NonNull<u8>,
+    len: usize,
+}
+// copybara:strip_begin
+// LINT.ThenChange(//depot/google3/third_party/protobuf/rust/cpp_kernel/cpp_api.
+// h) copybara:strip_end
+
+impl SerializedData {
+    pub unsafe fn from_raw_parts(data: NonNull<u8>, len: usize) -> Self {
+        Self { data, len }
+    }
+}
+
+impl Deref for SerializedData {
+    type Target = [u8];
+    fn deref(&self) -> &Self::Target {
+        unsafe { slice::from_raw_parts(self.data.as_ptr(), self.len) }
+    }
+}
+
+impl Drop for SerializedData {
+    fn drop(&mut self) {
+        unsafe {
+            alloc::dealloc(self.data.as_ptr(), Layout::array::<u8>(self.len).unwrap());
+        };
+    }
+}
+
+impl fmt::Debug for SerializedData {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self.deref(), f)
+    }
+}
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::boxed::Box;
 
     // We need to allocate the byte array so SerializedData can own it and
     // deallocate it in its drop. This function makes it easier to do so for our

rust/protobuf.rs

@@ -36,6 +36,6 @@
 //! cpp and upb kernels from user code.
 
 #[cfg(cpp_kernel)]
-pub use cpp::*;
+pub use protobuf_cpp::*;
 #[cfg(upb_kernel)]
-pub use upb::*;
+pub use protobuf_upb::*;

rust/shared.rs

@@ -28,7 +28,17 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-//! Kernel-agnostic logic for Rust Protobuf Runtimes.
+//! Kernel-agnostic logic for the Rust Protobuf Runtime.
+//!
+//! For kernel-specific logic this crate delegates to the respective __runtime
+//! crate.
+
+#[cfg(cpp_kernel)]
+pub extern crate cpp as __runtime;
+#[cfg(upb_kernel)]
+pub extern crate upb as __runtime;
+
+pub use __runtime::SerializedData;
 
 use std::fmt;
 use std::slice;

rust/test/cpp/interop/BUILD

@@ -21,7 +21,7 @@ rust_test(
     ],
     deps = [
         ":test_utils",
-        "//rust/cpp_kernel:cpp",
+        "//rust:protobuf_cpp",
         "//rust/test:unittest_cc_rust_proto",
     ],
 )

rust/test/cpp/interop/main.rs

@@ -48,10 +48,10 @@ macro_rules! proto_assert_eq {
 extern "C" {
     fn DeserializeTestAllTypes(data: *const u8, len: usize) -> NonNull<u8>;
     fn MutateTestAllTypes(msg: NonNull<u8>);
-    fn SerializeTestAllTypes(msg: NonNull<u8>) -> cpp::SerializedData;
+    fn SerializeTestAllTypes(msg: NonNull<u8>) -> protobuf_cpp::SerializedData;
 
     fn NewWithExtension() -> NonNull<u8>;
-    fn GetBytesExtension(msg: NonNull<u8>) -> cpp::PtrAndLen;
+    fn GetBytesExtension(msg: NonNull<u8>) -> protobuf_cpp::PtrAndLen;
 }
 
 #[test]
@@ -110,7 +110,8 @@ fn smuggle_extension() {
 
     let mut msg2 = TestAllExtensions::new();
     msg2.deserialize(&data).unwrap();
-    let bytes =
+    let bytes = unsafe {
-        unsafe { GetBytesExtension(msg2.__unstable_cpp_repr_grant_permission_to_break()).as_ref() };
+        GetBytesExtension(msg2.__unstable_cpp_repr_grant_permission_to_break()).as_ref()
+    };
     assert_eq!(&*bytes, b"smuggled");
 }

rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl

@@ -57,7 +57,7 @@ def _rust_upb_aspect_test_impl(ctx):
     rustc_action = _find_action_with_mnemonic(actions, "Rustc")
 
     # The action needs to have the Rust runtime as an input
-    _find_rust_lib_input(rustc_action.inputs, "upb")
+    _find_rust_lib_input(rustc_action.inputs, "protobuf")
 
     # The action needs to produce a .rlib artifact (sometimes .rmeta as well, not tested here).
     asserts.true(env, rustc_action.outputs.to_list()[0].path.endswith(".rlib"))
@@ -88,7 +88,7 @@ def _rust_cc_aspect_test_impl(ctx):
     rustc_action = _find_action_with_mnemonic(actions, "Rustc")
 
     # The action needs to have the Rust runtime as an input
-    _find_rust_lib_input(rustc_action.inputs, "cpp")
+    _find_rust_lib_input(rustc_action.inputs, "protobuf")
 
     # The action needs to produce a .rlib artifact (sometimes .rmeta as well, not tested here).
     asserts.true(env, rustc_action.outputs.to_list()[0].path.endswith(".rlib"))

rust/upb_kernel/BUILD

@@ -2,11 +2,6 @@
 
 load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test")
 
-# Represents Rust Protobuf runtime using the upb kernel.
-#
-# `rust_upb_proto_library` implicitly depends on this target. This target cannot depend on
-# `:rust_proto_library_kernel` build setting; it has to be fully functional under any value of that
-# setting.
 rust_library(
     name = "upb",
     srcs = ["upb.rs"],
@@ -14,10 +9,7 @@ rust_library(
         "//src/google/protobuf:__subpackages__",
         "//rust:__subpackages__",
     ],
-    deps = [
+    deps = [":upb_c_api"],
-        ":upb_c_api",
-        "//rust:common",
-    ],
 )
 
 rust_test(

rust/upb_kernel/upb.rs

@@ -30,52 +30,15 @@
 
 //! UPB FFI wrapper code for use by Rust Protobuf.
 
-pub use common::ParseError;
-pub use common::PtrAndLen;
-use std::fmt;
-use std::ops::Deref;
-use std::ptr::NonNull;
-use std::slice;
-
 use std::alloc;
 use std::alloc::Layout;
 use std::cell::UnsafeCell;
+use std::fmt;
 use std::marker::PhantomData;
 use std::mem::MaybeUninit;
-
+use std::ops::Deref;
-/// Represents serialized Protobuf wire format data.
+use std::ptr::NonNull;
-///
+use std::slice;
-/// It's typically produced by `<Message>::serialize()`.
-pub struct SerializedData {
-    data: NonNull<u8>,
-    len: usize,
-
-    // The arena that owns `data`.
-    _arena: __runtime::Arena,
-}
-
-impl SerializedData {
-    pub unsafe fn from_raw_parts(arena: __runtime::Arena, data: NonNull<u8>, len: usize) -> Self {
-        SerializedData { _arena: arena, data, len }
-    }
-}
-
-impl Deref for SerializedData {
-    type Target = [u8];
-    fn deref(&self) -> &Self::Target {
-        unsafe { slice::from_raw_parts(self.data.as_ptr() as *const _, self.len) }
-    }
-}
-
-impl fmt::Debug for SerializedData {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Debug::fmt(self.deref(), f)
-    }
-}
-
-pub mod __runtime {
-
-    use super::*;
 
 /// See `upb/port/def.inc`.
 const UPB_MALLOC_ALIGN: usize = 8;
@@ -91,8 +54,8 @@ pub mod __runtime {
 
 /// A wrapper over a `upb_Arena`.
 ///
-    /// This is not a safe wrapper per se, because the allocation functions
+/// This is not a safe wrapper per se, because the allocation functions still
-    /// still have sharp edges (see their safety docs for more info).
+/// have sharp edges (see their safety docs for more info).
 ///
 /// This is an owning type and will automatically free the arena when
 /// dropped.
@@ -144,8 +107,8 @@ pub mod __runtime {
     /// # Safety
     ///
     /// After calling this function, `ptr` is essentially zapped. `old` must
-        /// be the layout `ptr` was allocated with via [`Arena::alloc()`].
+    /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. `new`'s
-        /// `new`'s alignment must be less than `UPB_MALLOC_ALIGN`.
+    /// alignment must be less than `UPB_MALLOC_ALIGN`.
     #[inline]
     pub unsafe fn resize(&self, ptr: *mut u8, old: Layout, new: Layout) -> &[MaybeUninit<u8>] {
         debug_assert!(new.align() <= UPB_MALLOC_ALIGN);
@@ -166,11 +129,39 @@ pub mod __runtime {
         }
     }
 }
-} // mod __runtime
+
+/// Represents serialized Protobuf wire format data.
+///
+/// It's typically produced by `<Message>::serialize()`.
+pub struct SerializedData {
+    data: NonNull<u8>,
+    len: usize,
+
+    // The arena that owns `data`.
+    _arena: Arena,
+}
+
+impl SerializedData {
+    pub unsafe fn from_raw_parts(arena: Arena, data: NonNull<u8>, len: usize) -> Self {
+        SerializedData { _arena: arena, data, len }
+    }
+}
+
+impl Deref for SerializedData {
+    type Target = [u8];
+    fn deref(&self) -> &Self::Target {
+        unsafe { slice::from_raw_parts(self.data.as_ptr() as *const _, self.len) }
+    }
+}
+
+impl fmt::Debug for SerializedData {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self.deref(), f)
+    }
+}
 
 #[cfg(test)]
 mod tests {
-    use super::__runtime::*;
     use super::*;
 
     #[test]

src/google/protobuf/compiler/rust/generator.cc

@@ -102,7 +102,7 @@ bool RustGenerator::Generate(const FileDescriptor* file_desc,
   });
 
   file.Emit({{"kernel", KernelRsName(file.opts().kernel)}}, R"rs(
-    extern crate $kernel$ as __pb;
+    extern crate protobuf_$kernel$ as __pb;
     extern crate std as __std;
 
   )rs");