When you call a map field setter, we currently make an unnecessary extra copy,
so this CL fixes that problem.
I followed the example of how we already handle this for repeated field
setters. This required adding a new move setter thunk for map fields with the
C++ kernel. Originally I tried instead to add an FFI endpoint that could swap
two `RawMap` pointers, but it turned out to be difficult to implement this in a
way that worked correctly when the two maps are not on the same arena.
PiperOrigin-RevId: 687334655
I realized that as long as we implement `UpbTypeConversions` for enums, we can
easily get the blanket implementation for messages to work for enums as well.
Luckily the blanket implementation also happens to work for non-generated
types, so this gets us down to just one ProxiedInMapValue implementation for
upb.
PiperOrigin-RevId: 673927343
This turned out to be much easier for upb than for C++. This CL pretty much
just does a cut-and-paste of the `ProxiedInMapValue` implementation from the
upb code generator to the runtime. This should help a bit with code size since
it removes the need to generate six `ProxiedInMapValue` implementations per
message.
PiperOrigin-RevId: 671438289
These types are effectively two ways to spell the same type, but MapView/MapMut is more terse, especially where the unnamed lifetime was used which can now be implied (`View<'_, Map<K, V>>` can just be `MapView<K,V>`)
PiperOrigin-RevId: 667636945
We were really inconsistent on where we put Private or not and this tries to make a sensible consistent state of:
- For types that are exposed to application code, any pub methods which are only pub so they can be used by gencode (which is mostly anything that has any internal/runtime type anywhere on the parameters or return type list), have those methods have both a `Private` arg and doc(hidden)
- For structs that are only inside __runtime / __internal to begin with, put doc(hidden) on the types, and don't put Private on any of their methods since callers can't reach those types regardless.
Note that for exposed functions which also _accept_ another internal/runtime type in a parameter, the additional `Private` arg is superfluous since application code shouldn't ever be able to reach one of those internal types to be able to pass one in, but this keeps the pattern of keeping Private on it in those cases as well (the `Private` would still be the only guard on methods which only _return_ an internal type).
PiperOrigin-RevId: 667547566
The purpose of this trait is that it is declared as a supertrait of the traits that we don't want application code to implement (like "Proxied" and "MessageView"); application code can see those traits but not the supertrait, which enables them to use them but not implement them.
PiperOrigin-RevId: 657555025
Adds a new 'bzl' feature that is used to adjust import paths that need to change
in the Cargo build vs Blaze build.
This protobuf rust crate is a single crate that merges all of our current crates (protobuf, rust/upb, and utf8).
PiperOrigin-RevId: 656405153
When you have a `T: SomeTrait + ?Sized` and `trait SomeTrait:Sized`, the ?Sized has no effect (its not able to "remove" the requirement).
PiperOrigin-RevId: 654836513
Rather than two traits (MutProxy subtrait of ViewProxy), instead have three traits (MutProxy subtrait of Proxy, and ViewProxy subtrait of Proxy).
This makes things more consistent, including that (MutProxied subtraits Proxied) is now more parallel to (MutProxy subtraits Proxy).
ViewProxy is largely a marker trait here but leaves a spot for methods that should be on ViewProxies but not MutProxies if those do show up later.
PiperOrigin-RevId: 653661953
Our ASAN test runs have not had the heap checker enabled, so this has allowed a
few memory leaks to slip in. This CL fixes all of them so that we can turn on
the heap checker.
The first one takes place whenever we add an entry into a string-valued map
using the C++ kernel. The problem is that `InnerProtoString::into_raw()` gives
up ownership of the raw `std::string` pointer it holds, but then we never
delete that pointer. This CL fixes the problem by deleting the pointer in C++
right after we perform the map insertion. To simplify things, I created a
`MakeCleanup()` helper function that we always call in our map insertion
thunks, but it's a no-op in the cases where we don't need to free anything.
There were a couple similar memory leaks related to repeated field accessors in
the C++ kernel, and those were simple to fix just by adding the necessary
`delete` call.
Finally, there were two benign memory leaks in the upb kernel involving global
variables used for empty repeated fields and maps. It turned out that we did
not need to use `Box` at all here, so removing that simplified things and fixed
the leaks.
PiperOrigin-RevId: 652947042
Calling into_proxied() already does a copy and before this change we were doing a second one.
I am not using set_allocated_<field(std::string* s) because the method is not generated when [features.(pb.cpp).string_type = VIEW] is specified.
PiperOrigin-RevId: 650612909
* The public Repeated::{push, set} and Map::insert methods now accept any value that implements IntoProxied<T>, allowing us to move owned values instead of copying them.
* This change also updates the FFI layer for strings/bytes in the repeated and maps thunks to accept a std::string* that can be moved rather than a PtrAndLen type that needs to be copied.
* Tests are updated to no longer .as_view() when setting a message / string on a repeated / map field. The IntoProxied trait makes calling .as_view() obsolete.
PiperOrigin-RevId: 650580788
The test I am removing fails on 32-bit ARM, as pointers are 4-byte aligned. `upb_MessageValue` is a union. The alignment of a union is the max. alignment of any of its fields. Among the fields are 8-byte types like u64 and thus `upb_MessageValue` is 8-byte aligned even on 32-bit ARM. 4 != 8 and it fails.
I am split on the usefulness of the test, as I find it somewhat unlikely to catch any divergence between the FFI and C types in the future. I can't imagine a practical scenario where upb_MessageValue would change in C code and this test would fail. IMO our actual unit & integration tests plus the IFTT lints are much better guards.
We could fix the test by testing the alignment of u64 instead of *const c_void, which is always 8 bytes on the hw platforms we care about.
PiperOrigin-RevId: 638976482
We modify set_<repeated_field> to accept the IntoProxied type as the value and move the value (avoid copying) whenever possible.
For UPB:
- We fuse the arena of Repeated<T> with the parent message arena.
- We use upb_Message_SetBaseField to set the upb_Array contained in the Repeated<T>.
For C++:
- We generate an additional setter thunk that moves the value.
- The move assignment operator of RepeatedField/RepeatedPtrField is specialized. In order to adhere to the layering check we need to add '#include' statements for all .proto imports to the generated thunks.pb.cc.
PiperOrigin-RevId: 631010333
The last callside that used PrimitiveMut was in our enums code. This change makes it so that enums nolonger implement MutProxied and thus no longer need the PrimitiveMut type.
PiperOrigin-RevId: 629017282
This change removes the only remaining instance of SettableValue in a generated accessor. The principled fix is to implement IntoProxied for ProtoStr/[u8], but this will have to wait until we have agreed on & implemented the 1.0 string types. So we'll use AsRef in the meantime in order to not break any user code while allowing us to make progress on IntoProxied and Proxied changes.
PiperOrigin-RevId: 627735404
The intent of this directory would be for a layer of Rust bindings that directly map to upb semantics; Rust Protobuf runtime would be layer on top of that Rust, instead of directly on the upb C api.
PiperOrigin-RevId: 624282429
The actual construction of the zeroed block is now entirely safe.
This will be accessed in nearly every program using protobuf.
Using a static in .bss has much less overhead than an atomically-constructed
dynamic allocation and is far more predictable for space-constrained systems.
In the future, if dynamic allocation is kept, it should use std::sync::OnceLock
instead of the much less safe Once combined with `static mut`.
PiperOrigin-RevId: 609635555
This memory management should be handled by Rust.
I've confirmed this works by running the new included tests with msan.
The sanitizer is necessary to detect an incorrect copy_from impl
that uses-after-free from the upb arena.
PiperOrigin-RevId: 604689154
A public "raw" field in a safe wrapper is guaranteed unsound!
This makes repeated and map inner access consistent and
avoids exposing raw internals.
It also provides the accessors necessary for implementing map
access for external types.
PiperOrigin-RevId: 604405543
Part 3 of 4 (added a stage).
The getter and mut_getter bifurcate based on the kernel.
upb returns Option<RawMessage>, while cpp returns the RawMessage.
This means that we can't have a unified MessageVTable in vtable.rs, so we've split these out in {upb.rs and cpp.rs}.
$field$_entry is now prepped and populated for the $field$_mut swappage in the following CL.
PiperOrigin-RevId: 601230880
We want to return $pb$::FieldEntry<'_, $msg_type$> for msg_mut accessors as opposed to the current state (returning $Msg$Mut directly).
In this CL, we pave the way to implementing field entry returns.
We introduce { MessagePresentMutData, MessageAbsentMutData } and impl { ProxiedWithRawVTable, ProxiedWithRawOptionalVTable }. I initially tried a blanket impl approach, but it collided with the already existing PrimitiveVTable constructs; perhaps worth revisiting post 0.6.
In a followup, we'll flesh out the bodies. Lastly, we'll perform the swapover by
replacing $field$_mut with $field$_entry, updating all related tests.
PiperOrigin-RevId: 599282850
We now support fields with bytes as map values e.g. map<i32, bytes>. The implementation for the C++ runtime was straightforward. The majority of the changes in this CL are about the UPB runtime. In UPB, when we insert Rust bytes/string into the map we need to first copy the bytes onto the maps arena. To support this I have rewritten the macro that implements the ProxiedInMapValue types. I refactored the functionality to convert between UPB and Rust types into the 'UpbTypeConversions' trait. This trait has a function 'to_message_value_if_required' which does the copying for bytes and strings.
PiperOrigin-RevId: 599118416
This adds private methods of:
-- .raw_msg() to Msg+MsgMut+MsgView
-- .raw_arena() to Msg+MsgMut [upb kernel only]
And updates the accessors to use the self.raw_msg() / self.raw_arena().
A couple more things will need to be changed before the accessors can be verbatim reused in Msg/MsgView/MsgMut which will be mailed separately.
PiperOrigin-RevId: 598869392
- ProxiedInMapValue is defined in maps.rs, and no longer in the runtime files {upb, cpp}.rs.
- ProxiedInMapValue's methods accept and return Proxied types.
- InnerMapMut no longer has any generic type parameters.
- Through this refactoring the Map type is no longer a ZST. Creating a new map is now as simple as `Map::new()`.
PiperOrigin-RevId: 597765165
This change is a pure refactoring and simplification of the code. We replace all MapsWith<TYPE>KeyOps traits through a single generic ProxiedInMapValue<K> trait. Through connecting the runtime maps implementation with Proxied the code gets a lot simpler e.g. we can use View<T> instead of hardcoding the concrete type behind it.
I also expect this change to be beneficial for the gencode. In a subsequent CL we'll implement message values for maps. After this change we'll only have to implement a single trait, while before we had to implement num(key types) many traits.
PiperOrigin-RevId: 596562909
Adam Cozzette
Protobuf Team Bot
Derek Benson
Jakob Buchgraber
Marcel Hlopko
Alyssa Haroldsen
Hong Shin
Kevin King