Move internal-only but kernel-agnostic paths into an `__internal` module

Add some documentation along the way.

PiperOrigin-RevId: 540311409

rust/BUILD

@@ -49,6 +49,7 @@ rust_library(
 rust_library(
     name = "protobuf_upb",
     srcs = [
+        "internal.rs",
         "proxied.rs",
         "shared.rs",
         "upb.rs",
@@ -83,6 +84,7 @@ rust_library(
     name = "protobuf_cpp",
     srcs = [
         "cpp.rs",
+        "internal.rs",
         "proxied.rs",
         "shared.rs",
     ],

rust/internal.rs

@@ -0,0 +1,71 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2023 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (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 that should not be
+//! exposed to through the `protobuf` path but must be public for use by
+//! generated code.
+
+use std::slice;
+
+/// Represents an ABI-stable version of `NonNull<[u8]>`/`string_view` (a
+/// borrowed slice of bytes) for FFI use only.
+///
+/// Has semantics similar to `std::string_view` in C++ and `&[u8]` in Rust,
+/// but is not ABI-compatible with either.
+///
+/// If `len` is 0, then `ptr` can be null or dangling. C++ considers a dangling
+/// 0-len `std::string_view` to be invalid, and Rust considers a `&[u8]` with a
+/// null data pointer to be invalid.
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub struct PtrAndLen {
+    /// Pointer to the first byte.
+    /// Borrows the memory.
+    pub ptr: *const u8,
+
+    /// Length of the `[u8]` pointed to by `ptr`.
+    pub len: usize,
+}
+
+impl PtrAndLen {
+    /// Unsafely dereference this slice.
+    ///
+    /// # Safety
+    /// - `ptr` must be valid for `len` bytes. It can be null or dangling if
+    ///   `self.len == 0`.
+    pub unsafe fn as_ref<'a>(self) -> &'a [u8] {
+        if self.ptr.is_null() {
+            assert_eq!(self.len, 0, "Non-empty slice with null data pointer");
+            &[]
+        } else {
+            slice::from_raw_parts(self.ptr, self.len)
+        }
+    }
+}

rust/protobuf.rs

@@ -30,12 +30,15 @@
 
 //! Rust Protobuf Runtime
 //!
-//! This file forwards to the kernel specific implementation. Rust Protobuf
-//! gencode actually depends directly on kernel specific crates. The only reason
-//! this crate exists is to be able to use `protobuf` as a crate name for both
-//! cpp and upb kernels from user code.
+//! This file forwards to `shared.rs`, which exports a kernel-specific
+//! `__runtime`. Rust Protobuf gencode actually depends directly on kernel
+//! specific crates. The only reason this crate exists is to be able to use
+//! `protobuf` as a crate name for both cpp and upb kernels from user code.
 
 #[cfg(cpp_kernel)]
-pub use protobuf_cpp::*;
+use protobuf_cpp as kernel;
+
 #[cfg(upb_kernel)]
-pub use protobuf_upb::*;
+use protobuf_upb as kernel;
+
+pub use kernel::{Mut, MutProxy, ParseError, Proxied, View, ViewProxy};

rust/shared.rs

@@ -30,9 +30,12 @@
 
 //! Kernel-agnostic logic for the Rust Protobuf Runtime.
 //!
-//! For kernel-specific logic this crate delegates to the respective __runtime
+//! For kernel-specific logic this crate delegates to the respective `__runtime`
 //! crate.
 
+/// Everything in `__runtime` is allowed to change without it being considered
+/// a breaking change for the protobuf library. Nothing in here should be
+/// exported in `protobuf.rs`.
 #[cfg(cpp_kernel)]
 #[path = "cpp.rs"]
 pub mod __runtime;
@@ -42,11 +45,15 @@ pub mod __runtime;
 
 mod proxied;
 
-pub use __runtime::SerializedData;
+pub use proxied::{Mut, MutProxy, Proxied, View, ViewProxy};
+
+/// Everything in `__internal` is allowed to change without it being considered
+/// a breaking change for the protobuf library. Nothing in here should be
+/// exported in `protobuf.rs`.
+#[path = "internal.rs"]
+pub mod __internal;
 
 use std::fmt;
-use std::slice;
-use std::ptr::NonNull;
 
 /// An error that happened during deserialization.
 #[derive(Debug, Clone)]
@@ -57,28 +64,3 @@ impl fmt::Display for ParseError {
         write!(f, "Couldn't deserialize given bytes into a proto")
     }
 }
-
-/// An ABI-stable, FFI-safe borrowed slice of bytes.
-///
-/// Has semantics equivalent to `&[u8]` in Rust and `std::string_view` in C++,
-/// but is not ABI-compatible with them.
-///
-/// TODO: Is `ptr` allowed to be null or dangling when `len` is 0?
-#[repr(C)]
-#[derive(Copy, Clone)]
-pub struct PtrAndLen {
-    /// Borrows the memory.
-    pub ptr: *const u8,
-    pub len: usize,
-}
-
-impl PtrAndLen {
-    pub unsafe fn as_ref<'a>(self) -> &'a [u8] {
-        assert!(self.len == 0 || !self.ptr.is_null());
-        if self.len == 0 {
-            slice::from_raw_parts(NonNull::dangling().as_ptr(), 0)
-        } else {
-            slice::from_raw_parts(self.ptr, self.len)
-        }
-    }
-}

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>) -> protobuf_cpp::SerializedData;
+    fn SerializeTestAllTypes(msg: NonNull<u8>) -> protobuf_cpp::__runtime::SerializedData;
 
     fn NewWithExtension() -> NonNull<u8>;
-    fn GetBytesExtension(msg: NonNull<u8>) -> protobuf_cpp::PtrAndLen;
+    fn GetBytesExtension(msg: NonNull<u8>) -> protobuf_cpp::__internal::PtrAndLen;
 }
 
 #[test]
@@ -91,7 +91,7 @@ fn deserialize_in_cpp() {
 
     let msg2 = unsafe {
         TestAllTypes::__unstable_wrap_cpp_grant_permission_to_break(DeserializeTestAllTypes(
-            data.as_ptr(),
+            (*data).as_ptr(),
             data.len(),
         ))
     };
@@ -110,8 +110,7 @@ fn smuggle_extension() {
 
     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");
 }

src/google/protobuf/compiler/rust/accessors/singular_bytes.cc

@@ -84,7 +84,7 @@ class SingularBytes final : public AccessorGenerator {
         },
         R"rs(
           fn $hazzer_thunk$(raw_msg: $NonNull$<u8>) -> bool;
-          fn $getter_thunk$(raw_msg: $NonNull$<u8>) -> $pb$::PtrAndLen;
+          fn $getter_thunk$(raw_msg: $NonNull$<u8>) -> $pbi$::PtrAndLen;
           fn $setter_thunk$(raw_msg: $NonNull$<u8>, val: *const u8, len: usize);
           fn $clearer_thunk$(raw_msg: $NonNull$<u8>);
         )rs");

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

@@ -97,7 +97,8 @@ bool RustGenerator::Generate(const FileDescriptor* file_desc,
   auto v = file.printer().WithVars({
       {"std", "::__std"},
       {"pb", "::__pb"},
-      {"pbi", "::__pb::__runtime"},
+      {"pbi", "::__pb::__internal"},
+      {"pbr", "::__pb::__runtime"},
       {"NonNull", "::__std::ptr::NonNull"},
   });
 

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

@@ -59,7 +59,7 @@ void MessageStructFields(Context<Descriptor> msg) {
         //~ rustc incorrectly thinks this field is never read, even though
         //~ it has a destructor!
         #[allow(dead_code)]
-        arena: $pbi$::Arena,
+        arena: $pbr$::Arena,
       )rs");
       return;
   }
@@ -77,7 +77,7 @@ void MessageNew(Context<Descriptor> msg) {
 
     case Kernel::kUpb:
       msg.Emit({{"new_thunk", Thunk(msg, "new")}}, R"rs(
-        let arena = $pbi$::Arena::new();
+        let arena = $pbr$::Arena::new();
         Self {
           msg: unsafe { $new_thunk$(arena.raw()) },
           arena,
@@ -99,11 +99,11 @@ void MessageSerialize(Context<Descriptor> msg) {
 
     case Kernel::kUpb:
       msg.Emit({{"serialize_thunk", Thunk(msg, "serialize")}}, R"rs(
-        let arena = $pbi$::Arena::new();
+        let arena = $pbr$::Arena::new();
         let mut len = 0;
         unsafe {
           let data = $serialize_thunk$(self.msg, arena.raw(), &mut len);
-          $pb$::SerializedData::from_raw_parts(arena, data, len)
+          $pbr$::SerializedData::from_raw_parts(arena, data, len)
         }
       )rs");
       return;
@@ -121,7 +121,7 @@ void MessageDeserialize(Context<Descriptor> msg) {
           },
           R"rs(
           let success = unsafe {
-            let data = $pb$::SerializedData::from_raw_parts(
+            let data = $pbr$::SerializedData::from_raw_parts(
               $NonNull$::new(data.as_ptr() as *mut _).unwrap(),
               data.len(),
             );
@@ -134,7 +134,7 @@ void MessageDeserialize(Context<Descriptor> msg) {
 
     case Kernel::kUpb:
       msg.Emit({{"deserialize_thunk", Thunk(msg, "parse")}}, R"rs(
-        let arena = $pbi$::Arena::new();
+        let arena = $pbr$::Arena::new();
         let msg = unsafe {
           $NonNull$::<u8>::new(
             $deserialize_thunk$(data.as_ptr(), data.len(), arena.raw())
@@ -171,8 +171,8 @@ void MessageExterns(Context<Descriptor> msg) {
           R"rs(
           fn $new_thunk$() -> $NonNull$<u8>;
           fn $delete_thunk$(raw_msg: $NonNull$<u8>);
-          fn $serialize_thunk$(raw_msg: $NonNull$<u8>) -> $pb$::SerializedData;
-          fn $deserialize_thunk$(raw_msg: $NonNull$<u8>, data: $pb$::SerializedData) -> bool;
+          fn $serialize_thunk$(raw_msg: $NonNull$<u8>) -> $pbr$::SerializedData;
+          fn $deserialize_thunk$(raw_msg: $NonNull$<u8>, data: $pbr$::SerializedData) -> bool;
         )rs");
       return;
 
@@ -184,9 +184,9 @@ void MessageExterns(Context<Descriptor> msg) {
               {"deserialize_thunk", Thunk(msg, "parse")},
           },
           R"rs(
-          fn $new_thunk$(arena: $pbi$::RawArena) -> $NonNull$<u8>;
-          fn $serialize_thunk$(msg: $NonNull$<u8>, arena: $pbi$::RawArena, len: &mut usize) -> $NonNull$<u8>;
-          fn $deserialize_thunk$(data: *const u8, size: usize, arena: $pbi$::RawArena) -> *mut u8;
+          fn $new_thunk$(arena: $pbr$::RawArena) -> $NonNull$<u8>;
+          fn $serialize_thunk$(msg: $NonNull$<u8>, arena: $pbr$::RawArena, len: &mut usize) -> $NonNull$<u8>;
+          fn $deserialize_thunk$(data: *const u8, size: usize, arena: $pbr$::RawArena) -> *mut u8;
       )rs");
       return;
   }
@@ -293,7 +293,7 @@ void MessageGenerator::GenerateRs(Context<Descriptor> msg) {
             $Msg::new$
           }
 
-          pub fn serialize(&self) -> $pb$::SerializedData {
+          pub fn serialize(&self) -> $pbr$::SerializedData {
             $Msg::serialize$
           }
           pub fn deserialize(&mut self, data: &[u8]) -> Result<(), $pb$::ParseError> {