Introduce MetadataBytes wrapper for compiled-in metadata accessors#4016
Merged
mandlil merged 2 commits intoJun 29, 2026
Conversation
Replace the C-style `int <type>_size()` / `const void* <type>_get()` pair
in metadata.h, short_metadata.h and alternate_format.h with a single
`MetadataBytes Get<Type>()` accessor that returns a small RAII wrapper.
The default implementations shipped in this repo wrap the existing
static `data[]` arrays in a non-owning MetadataBytes, so the on-disk
layout and runtime behaviour are unchanged for upstream consumers.
The motivating use case is downstream embedders (e.g. Chromium) that
produce the metadata at runtime, for example by decompressing a
brotli-compressed blob on first use to save binary size. With the old
API such embedders had to either leak the decompressed buffer or do
gymnastics to free it after PhoneNumberUtil parsed it. With
MetadataBytes the typical call site
MetadataBytes bytes = GetMetadata();
collection->ParseFromArray(bytes.data(), bytes.size());
keeps the buffer alive for exactly as long as ParseFromArray() needs it
and then frees it when the wrapper goes out of scope, simply by having
the downstream-supplied implementation return an owning instance built
from a std::unique_ptr<uint8_t[]>.
CppMetadataGenerator and its tests are updated to emit the new API.
kkeshava
previously approved these changes
Jun 26, 2026
Contributor
|
Hi @hjanuschka, |
The previous commit updated CppMetadataGenerator.java to emit the new
MetadataBytes API but did not rebuild the checked-in
cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar that CMake invokes to
regenerate metadata.{h,cc} at build time. As a result CI regenerated the
headers using the old API and phonenumberutil.cc failed with
'MetadataBytes was not declared in this scope'.
Also reorder the assertions in CppMetadataGeneratorTest.outputHeaderFile
to match the actual emit order (the metadata_bytes.h include is emitted
before the namespace openings, not after).
Contributor
Author
|
@mandlil tests pass now! |
mandlil
approved these changes
Jun 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces a small
MetadataBytesRAII wrapper for the compiled-in metadatabuffers returned by the internal
metadata.h,short_metadata.handalternate_format.haccessors.The C-style API
is replaced by
where
MetadataBytesis a move-only handle that holds either:const void*+int size, orstd::unique_ptr<uint8_t[]>+int size(owning).The default implementations shipped in this repo continue to wrap the
existing
static const unsigned char data[]arrays with a non-owninginstance, so on-disk layout and runtime behaviour are unchanged for
upstream consumers.
CppMetadataGenerator(and its tests) are updated toemit the new API.
Backwards compatibility
These headers are library-internal (not part of the public API).
Related
Chromium CL discussion that motivated this:
https://chromium-review.googlesource.com/c/chromium/src/+/7928952