Simplify Protobuf Rust runtime build setup PiperOrigin-RevId: 537300231
diff --git a/.github/workflows/test_rust.yml b/.github/workflows/test_rust.yml index 3edec3e..b90e5d6 100644 --- a/.github/workflows/test_rust.yml +++ b/.github/workflows/test_rust.yml
@@ -24,7 +24,6 @@ credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: rust_linux bazel: | - test //rust:protobuf_upb_test //rust:protobuf_cpp_test \ - //rust/upb_kernel:upb_test //rust/cpp_kernel:cpp_test \ + 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/... \ No newline at end of file
diff --git a/rust/BUILD b/rust/BUILD index e38f564..af69c57 100644 --- a/rust/BUILD +++ b/rust/BUILD
@@ -1,6 +1,6 @@ # Protobuf Rust runtime packages. -load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") +load("@rules_rust//rust:defs.bzl", "rust_library") load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("@rules_proto//proto:defs.bzl", "proto_lang_toolchain") @@ -30,68 +30,22 @@ "//conditions:default": ["--cfg=cpp_kernel"], }), deps = select({ - ":use_upb_kernel": [":protobuf_upb"], - "//conditions:default": [":protobuf_cpp"], + ":use_upb_kernel": ["//rust/upb_kernel:upb"], + "//conditions:default": ["//rust/cpp_kernel: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 = "protobuf_upb", - srcs = ["shared.rs"], - 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", - ], + name = "common", + srcs = ["common.rs"], + visibility = ["//rust:__subpackages__"], ) 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 = ":protobuf_upb", + runtime = "//rust/upb_kernel:upb", visibility = ["//visibility:public"], ) @@ -99,7 +53,7 @@ name = "proto_rust_cpp_toolchain", command_line = "--rust_out=experimental-codegen=enabled,kernel=cpp:$(OUT)", progress_message = "Generating Rust proto_library %{label}", - runtime = ":protobuf_cpp", + runtime = "//rust/cpp_kernel:cpp", visibility = ["//visibility:public"], )
diff --git a/rust/shared.rs b/rust/common.rs similarity index 88% rename from rust/shared.rs rename to rust/common.rs index b576ca6..d6f3215 100644 --- a/rust/shared.rs +++ b/rust/common.rs
@@ -28,17 +28,7 @@ // (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 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; +//! Kernel-agnostic logic for Rust Protobuf Runtimes. use std::fmt; use std::slice; @@ -67,4 +57,4 @@ pub unsafe fn as_ref<'a>(self) -> &'a [u8] { slice::from_raw_parts(self.ptr, self.len) } -} \ No newline at end of file +}
diff --git a/rust/cpp_kernel/BUILD b/rust/cpp_kernel/BUILD index 3750498..98b59eb 100644 --- a/rust/cpp_kernel/BUILD +++ b/rust/cpp_kernel/BUILD
@@ -2,6 +2,11 @@ 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"], @@ -9,6 +14,7 @@ "//src/google/protobuf:__subpackages__", "//rust:__subpackages__", ], + deps = ["//rust:common"], ) rust_test(
diff --git a/rust/cpp_kernel/cpp.rs b/rust/cpp_kernel/cpp.rs index 64e202b..bcda756 100644 --- a/rust/cpp_kernel/cpp.rs +++ b/rust/cpp_kernel/cpp.rs
@@ -30,79 +30,24 @@ // Rust Protobuf runtime using the C++ kernel. -use std::alloc; -use std::alloc::Layout; -use std::boxed::Box; -use std::cell::UnsafeCell; +pub use common::ParseError; +pub use common::PtrAndLen; use std::fmt; -use std::marker::PhantomData; -use std::mem::MaybeUninit; use std::ops::Deref; use std::ptr::NonNull; use std::slice; -/// A wrapper over a `proto2::Arena`. +use std::alloc; +use std::alloc::Layout; +use std::cell::UnsafeCell; +use std::marker::PhantomData; +use std::mem::MaybeUninit; + +/// Represents serialized Protobuf wire format data. It's typically produced +/// by `<Message>.serialize()`. /// -/// This is not a safe wrapper per se, because the allocation functions still -/// 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>, - _not_sync: PhantomData<UnsafeCell<()>>, -} - -impl Arena { - /// Allocates a fresh arena. - #[inline] - pub fn new() -> Self { - Self { ptr: NonNull::dangling(), _not_sync: PhantomData } - } - - /// Returns the raw, C++-managed pointer to the arena. - #[inline] - pub fn raw(&self) -> ! { - unimplemented!() - } - - /// Allocates some memory on the arena. - /// - /// # Safety - /// - /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`. - #[inline] - pub unsafe fn alloc(&self, layout: Layout) -> &mut [MaybeUninit<u8>] { - unimplemented!() - } - - /// Resizes some memory on the arena. - /// - /// # Safety - /// - /// After calling this function, `ptr` is essentially zapped. `old` must - /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. `new`'s - /// alignment must be less than `UPB_MALLOC_ALIGN`. - #[inline] - pub unsafe fn resize(&self, ptr: *mut u8, old: Layout, new: Layout) -> &[MaybeUninit<u8>] { - unimplemented!() - } -} - -impl Drop for Arena { - #[inline] - fn drop(&mut self) { - // unimplemented - } -} - -/// 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. +/// 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 @@ -143,9 +88,76 @@ } } +pub mod __runtime { + use super::*; + + /// A wrapper over a `proto2::Arena`. + /// + /// This is not a safe wrapper per se, because the allocation functions + /// still 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>, + _not_sync: PhantomData<UnsafeCell<()>>, + } + + impl Arena { + /// Allocates a fresh arena. + #[inline] + pub fn new() -> Self { + Self { _ptr: NonNull::dangling(), _not_sync: PhantomData } + } + + /// Returns the raw, C++-managed pointer to the arena. + #[inline] + pub fn raw(&self) -> ! { + unimplemented!() + } + + /// Allocates some memory on the arena. + /// + /// # Safety + /// + /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn alloc(&self, _layout: Layout) -> &mut [MaybeUninit<u8>] { + unimplemented!() + } + + /// Resizes some memory on the arena. + /// + /// # Safety + /// + /// After calling this function, `ptr` is essentially zapped. `old` must + /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. + /// `new`'s alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn resize( + &self, + _ptr: *mut u8, + _old: Layout, + _new: Layout, + ) -> &[MaybeUninit<u8>] { + unimplemented!() + } + } + + impl Drop for Arena { + #[inline] + fn drop(&mut self) { + // unimplemented + } + } +} // mod __runtime + #[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
diff --git a/rust/protobuf.rs b/rust/protobuf.rs index 3569b1c..657cee4 100644 --- a/rust/protobuf.rs +++ b/rust/protobuf.rs
@@ -36,6 +36,6 @@ //! cpp and upb kernels from user code. #[cfg(cpp_kernel)] -pub use protobuf_cpp::*; +pub use cpp::*; #[cfg(upb_kernel)] -pub use protobuf_upb::*; +pub use upb::*;
diff --git a/rust/test/cpp/interop/BUILD b/rust/test/cpp/interop/BUILD index ac08b42..93b7f85 100644 --- a/rust/test/cpp/interop/BUILD +++ b/rust/test/cpp/interop/BUILD
@@ -21,7 +21,7 @@ ], deps = [ ":test_utils", - "//rust:protobuf_cpp", + "//rust/cpp_kernel:cpp", "//rust/test:unittest_cc_rust_proto", ], )
diff --git a/rust/test/cpp/interop/main.rs b/rust/test/cpp/interop/main.rs index ef84d99..acc7b4f 100644 --- a/rust/test/cpp/interop/main.rs +++ b/rust/test/cpp/interop/main.rs
@@ -48,10 +48,10 @@ extern "C" { fn DeserializeTestAllTypes(data: *const u8, len: usize) -> NonNull<u8>; fn MutateTestAllTypes(msg: NonNull<u8>); - fn SerializeTestAllTypes(msg: NonNull<u8>) -> protobuf_cpp::SerializedData; + fn SerializeTestAllTypes(msg: NonNull<u8>) -> cpp::SerializedData; fn NewWithExtension() -> NonNull<u8>; - fn GetBytesExtension(msg: NonNull<u8>) -> protobuf_cpp::PtrAndLen; + fn GetBytesExtension(msg: NonNull<u8>) -> cpp::PtrAndLen; } #[test] @@ -110,8 +110,7 @@ let mut msg2 = TestAllExtensions::new(); msg2.deserialize(&data).unwrap(); - let bytes = unsafe { - GetBytesExtension(msg2.__unstable_cpp_repr_grant_permission_to_break()).as_ref() - }; + let bytes = + unsafe { GetBytesExtension(msg2.__unstable_cpp_repr_grant_permission_to_break()).as_ref() }; assert_eq!(&*bytes, b"smuggled"); }
diff --git a/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl b/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl index 3faf76b..84c3c96 100644 --- a/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl +++ b/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl
@@ -57,7 +57,7 @@ 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, "protobuf") + _find_rust_lib_input(rustc_action.inputs, "upb") # 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 @@ 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, "protobuf") + _find_rust_lib_input(rustc_action.inputs, "cpp") # 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"))
diff --git a/rust/upb_kernel/BUILD b/rust/upb_kernel/BUILD index 233c260..345804e 100644 --- a/rust/upb_kernel/BUILD +++ b/rust/upb_kernel/BUILD
@@ -2,6 +2,11 @@ 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"], @@ -9,7 +14,10 @@ "//src/google/protobuf:__subpackages__", "//rust:__subpackages__", ], - deps = [":upb_c_api"], + deps = [ + ":upb_c_api", + "//rust:common", + ], ) rust_test(
diff --git a/rust/upb_kernel/upb.rs b/rust/upb_kernel/upb.rs index f19e4dd..0e74283 100644 --- a/rust/upb_kernel/upb.rs +++ b/rust/upb_kernel/upb.rs
@@ -30,105 +30,18 @@ //! UPB FFI wrapper code for use by Rust Protobuf. -use std::alloc; -use std::alloc::Layout; -use std::cell::UnsafeCell; +pub use common::ParseError; +pub use common::PtrAndLen; use std::fmt; -use std::marker::PhantomData; -use std::mem::MaybeUninit; use std::ops::Deref; use std::ptr::NonNull; use std::slice; -/// See `upb/port/def.inc`. -const UPB_MALLOC_ALIGN: usize = 8; - -/// A UPB-managed pointer to a raw arena. -pub type RawArena = NonNull<RawArenaData>; - -/// The data behind a [`RawArena`]. Do not use this type. -#[repr(C)] -pub struct RawArenaData { - _data: [u8; 0], -} - -/// A wrapper over a `upb_Arena`. -/// -/// This is not a safe wrapper per se, because the allocation functions still -/// 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 { - raw: RawArena, - _not_sync: PhantomData<UnsafeCell<()>>, -} - -extern "C" { - fn upb_Arena_New() -> RawArena; - fn upb_Arena_Free(arena: RawArena); - fn upb_Arena_Malloc(arena: RawArena, size: usize) -> *mut u8; - fn upb_Arena_Realloc(arena: RawArena, ptr: *mut u8, old: usize, new: usize) -> *mut u8; -} - -impl Arena { - /// Allocates a fresh arena. - #[inline] - pub fn new() -> Self { - Self { raw: unsafe { upb_Arena_New() }, _not_sync: PhantomData } - } - - /// Returns the raw, UPB-managed pointer to the arena. - #[inline] - pub fn raw(&self) -> RawArena { - self.raw - } - - /// Allocates some memory on the arena. - /// - /// # Safety - /// - /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`. - #[inline] - pub unsafe fn alloc(&self, layout: Layout) -> &mut [MaybeUninit<u8>] { - debug_assert!(layout.align() <= UPB_MALLOC_ALIGN); - let ptr = upb_Arena_Malloc(self.raw, layout.size()); - if ptr.is_null() { - alloc::handle_alloc_error(layout); - } - - slice::from_raw_parts_mut(ptr.cast(), layout.size()) - } - - /// Resizes some memory on the arena. - /// - /// # Safety - /// - /// After calling this function, `ptr` is essentially zapped. `old` must - /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. `new`'s - /// 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); - let ptr = upb_Arena_Realloc(self.raw, ptr, old.size(), new.size()); - if ptr.is_null() { - alloc::handle_alloc_error(new); - } - - slice::from_raw_parts_mut(ptr.cast(), new.size()) - } -} - -impl Drop for Arena { - #[inline] - fn drop(&mut self) { - unsafe { - upb_Arena_Free(self.raw); - } - } -} +use std::alloc; +use std::alloc::Layout; +use std::cell::UnsafeCell; +use std::marker::PhantomData; +use std::mem::MaybeUninit; /// Represents serialized Protobuf wire format data. /// @@ -138,11 +51,11 @@ len: usize, // The arena that owns `data`. - _arena: Arena, + _arena: __runtime::Arena, } impl SerializedData { - pub unsafe fn from_raw_parts(arena: Arena, data: NonNull<u8>, len: usize) -> Self { + pub unsafe fn from_raw_parts(arena: __runtime::Arena, data: NonNull<u8>, len: usize) -> Self { SerializedData { _arena: arena, data, len } } } @@ -160,8 +73,104 @@ } } +pub mod __runtime { + + use super::*; + + /// See `upb/port/def.inc`. + const UPB_MALLOC_ALIGN: usize = 8; + + /// A UPB-managed pointer to a raw arena. + pub type RawArena = NonNull<RawArenaData>; + + /// The data behind a [`RawArena`]. Do not use this type. + #[repr(C)] + pub struct RawArenaData { + _data: [u8; 0], + } + + /// A wrapper over a `upb_Arena`. + /// + /// This is not a safe wrapper per se, because the allocation functions + /// still 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 { + raw: RawArena, + _not_sync: PhantomData<UnsafeCell<()>>, + } + + extern "C" { + fn upb_Arena_New() -> RawArena; + fn upb_Arena_Free(arena: RawArena); + fn upb_Arena_Malloc(arena: RawArena, size: usize) -> *mut u8; + fn upb_Arena_Realloc(arena: RawArena, ptr: *mut u8, old: usize, new: usize) -> *mut u8; + } + + impl Arena { + /// Allocates a fresh arena. + #[inline] + pub fn new() -> Self { + Self { raw: unsafe { upb_Arena_New() }, _not_sync: PhantomData } + } + + /// Returns the raw, UPB-managed pointer to the arena. + #[inline] + pub fn raw(&self) -> RawArena { + self.raw + } + + /// Allocates some memory on the arena. + /// + /// # Safety + /// + /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn alloc(&self, layout: Layout) -> &mut [MaybeUninit<u8>] { + debug_assert!(layout.align() <= UPB_MALLOC_ALIGN); + let ptr = upb_Arena_Malloc(self.raw, layout.size()); + if ptr.is_null() { + alloc::handle_alloc_error(layout); + } + + slice::from_raw_parts_mut(ptr.cast(), layout.size()) + } + + /// Resizes some memory on the arena. + /// + /// # Safety + /// + /// After calling this function, `ptr` is essentially zapped. `old` must + /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. + /// `new`'s 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); + let ptr = upb_Arena_Realloc(self.raw, ptr, old.size(), new.size()); + if ptr.is_null() { + alloc::handle_alloc_error(new); + } + + slice::from_raw_parts_mut(ptr.cast(), new.size()) + } + } + + impl Drop for Arena { + #[inline] + fn drop(&mut self) { + unsafe { + upb_Arena_Free(self.raw); + } + } + } +} // mod __runtime + #[cfg(test)] mod tests { + use super::__runtime::*; use super::*; #[test]
diff --git a/src/google/protobuf/compiler/rust/generator.cc b/src/google/protobuf/compiler/rust/generator.cc index 6ec0e6d..0de62ee 100644 --- a/src/google/protobuf/compiler/rust/generator.cc +++ b/src/google/protobuf/compiler/rust/generator.cc
@@ -102,7 +102,7 @@ }); file.Emit({{"kernel", KernelRsName(file.opts().kernel)}}, R"rs( - extern crate protobuf_$kernel$ as __pb; + extern crate $kernel$ as __pb; extern crate std as __std; )rs");