This CL migrates messages, enums, and primitive types all onto the same blanket
implementation of the `ProxiedInMapValue` trait. This gets us to the point
where messages and enums no longer need to generate any significant amount of
extra code just in case they might be used as a map value.
There are a few big pieces to this:
- I generalized the message-specific FFI endpoints in `rust/cpp_kernel/map.cc`
to be able to additionally handle enums and primitive types as values. This
mostly consisted of replacing `MessageLite*` parameters with a new `MapValue`
tagged union.
- On the Rust side, I added a new blanket implementation of
`ProxiedInMapValue` in rust/cpp.rs. It relies on its value type to implement
a new `CppMapTypeConversions` trait so that it can convert to and from the
`MapValue` tagged union used for FFI.
- In the Rust generated code, I deleted the generated `ProxiedInMapValue`
implementations for messages and enums and replaced them with
implementations of the `CppMapTypeConversions` trait.
PiperOrigin-RevId: 687355817
There is an old convention that a `proto_library` target with empty `srcs`
should re-export its dependencies. This is sometimes useful as a way of
creating something like an alias target, except that it can point to multiple
dependencies rather than just one.
The `rust_proto_library` aspect currently cannot handle this pattern and will
raise an error if it encounters a `proto_library` without `srcs`. This CL fixes
that problem by updating the `RustProtoInfo` provider generated by the aspect
to include a list of `DepVariantInfo` instead of just one. In the typical case
the provider will have just one `DepVariantInfo`, but this gives us the ability
to return more than one in the case where we're exporting all the dependencies
of a `src`-less `proto_library`.
One thing this CL specifically does not attempt to do is add support for a
`rust_proto_library` directly wrapping a `proto_library` without `srcs`. The
reason for that is that it appears difficult to do in a way that would respect
the layering check. It's also not obvious that we would want to encourage this
pattern anyway. It's still valuable to support `src`-less `proto_library`
targets when they're somewhere else in the transitive dependencies. This way
you can freely create a `rust_proto_library` without having to fix up all your
transitive deps first.
PiperOrigin-RevId: 687344284
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
`ProtoStr` is a public type and it has a public
method `to_str`, however `to_str` returns a private
type `Utf8Error`, which triggers `private-interfaces` [1].
Exporting string::Utf8Error makes it possible for other
crates to wrap it in another error type and propagate
it with "?".
[1] https://doc.rust-lang.org/beta/rustc/lints/listing/warn-by-default.html#private-interfaces
PiperOrigin-RevId: 684477510
We generate these constants to enable map operations, but this is no longer
necessary now that we can get the relevant size and alignment information for
each message through its vtable.
PiperOrigin-RevId: 680712939
The immediate motivation for this is that it will facilitate writing a blanket
implementation of `ProxiedInMapValue` for C++-backed messages. The default
instance gives us access to the message vtable in cases where we don't already
have a message to work with.
However, it also seems generally useful just to have an implementation of
`Default`, so I implemented it for both C++ and upb-backed message views.
PiperOrigin-RevId: 677808048
We have been relying on a per-message generated `placement_new` function for
implementing map insertion, but this CL simplifies things by removing that.
Instead, we do a reflective swap if possible, or else fall back on a copy.
This will probably make insertions a bit slower, but I think it may be worth it
because it should make it much simpler to have a blanket implementation for
ProxedInMapValue that works for all map types.
It looks like it should be possible to make this faster in the future by
implementing a bitwise move that will work for any message.
PiperOrigin-RevId: 676495920
`UntypedMapIterator::next_unchecked` currently expects to be passed a pointer
to a C++ function that it can use to dereference an iterator. However, this is
awkward because it's not natural for this C++ function to have the same
signature for every map type. Maps with a message as value need a
`MapNodeSizeInfo`, but other map types do not. We are working around this by
sometimes passing an ignored placeholder constant, but this is messy.
This CL replaces the `extern "C"` functions with closures. This way, we can
capture the `MapNodeSizeInfo` in the closure in cases where we need it, but
otherwise we no longer need to pass around placeholder values.
(Note: `MapNodeSizeInfo` is going away soon, but this is still relevant because
it will likely need to be replaced by a message default instance pointer.)
PiperOrigin-RevId: 676438640
This CL doesn't unbreak the bazel tests, but is yak shaving in prep for changes that will. This makes it more straightforward how the tests are broken by having bazel test rules name rust proto library targets that don't exist in bazel repo, instead of naming targets that do exist but are bogus.
Also correct unittest_edition target to match the order of words in the .proto file name (edition_unittest)
PiperOrigin-RevId: 674295376
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
This version works where both sides are the same message, messageview or messagemut type, but not any mix of them (e.g. cannot compare a message against its corresponding view).
PiperOrigin-RevId: 671091099
This CL is mostly a no-op, except that now google3-only code is actually stripped from OSS, instead of being preserved in `# begin:google_only` blocks.
This follows the conventions of the greater Copybara ecosystem.
PiperOrigin-RevId: 669513564
This simplifies upb by removing differences between google3 and OSS.
This also points upb at the protobuf license, instead of keeping a separate copy around for upb.
PiperOrigin-RevId: 669447145
`unsafe trait` means that the _implementation_ has constraints they need to meet for the behavior to be safe (which is the intent here, namely that it should always return the same value and always be a safe to deref value), unsafe on the fn means that the _caller_ of that fn has some constraints they need to meet for it to be safe (of which there are none in this case).
PiperOrigin-RevId: 668028628
Adam Cozzette
Protobuf Team Bot
Tony Liao
Derek Benson
Nathan Baulch
Hong Shin
Joshua Haberman