From e1faf09604d26cc6803970815f91225b220175d4 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 8 Jun 2023 09:53:00 -0700 Subject: [PATCH] Internal changes PiperOrigin-RevId: 538814351 --- src/google/protobuf/descriptor.cc | 28 +++++++++++++++++++++------- src/google/protobuf/test_textproto.h | 9 +++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 0d9c3851f0..4460e8fd4c 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -4087,9 +4087,6 @@ class DescriptorBuilder { absl::string_view declared_type_name, bool is_repeated); - // Must be run only after cross-linking. - void InterpretOptions(); - // A helper class for interpreting options. class OptionInterpreter { public: @@ -4114,15 +4111,21 @@ class DescriptorBuilder { class AggregateOptionFinder; private: + bool InterpretOptionsImpl(OptionsToInterpret* options_to_interpret, + bool features); + // Interprets uninterpreted_option_ on the specified message, which // must be the mutable copy of the original options message to which // uninterpreted_option_ belongs. The given src_path is the source // location path to the uninterpreted option, and options_path is the // source location path to the options message. The location paths are // recorded and then used in UpdateSourceCodeInfo. + // The features boolean controls whether or not we should only interpret + // feature options or skip them entirely. bool InterpretSingleOption(Message* options, const std::vector& src_path, - const std::vector& options_path); + const std::vector& options_path, + bool features); // Adds the uninterpreted_option to the given options message verbatim. // Used when AllowUnknownDependencies() is in effect and we can't find @@ -5505,6 +5508,7 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( SuggestFieldNumbers(result, proto); } + // Interpret any remaining uninterpreted options gathered into // options_to_interpret_ during descriptor building. Cross-linking has made // extension options known, so all interpretations should now succeed. @@ -5521,7 +5525,6 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( } } - // Validate options. See comments at InternalSetLazilyBuildDependencies about // error checking and lazy import building. if (!had_errors_ && !pool_->lazily_build_dependencies_) { @@ -7766,6 +7769,10 @@ DescriptorBuilder::OptionInterpreter::~OptionInterpreter() {} bool DescriptorBuilder::OptionInterpreter::InterpretOptions( OptionsToInterpret* options_to_interpret) { + return InterpretOptionsImpl(options_to_interpret, /*features=*/false); +} +bool DescriptorBuilder::OptionInterpreter::InterpretOptionsImpl( + OptionsToInterpret* options_to_interpret, bool features) { // Note that these may be in different pools, so we can't use the same // descriptor and reflection objects on both. Message* options = options_to_interpret->options; @@ -7801,7 +7808,7 @@ bool DescriptorBuilder::OptionInterpreter::InterpretOptions( &original_options->GetReflection()->GetRepeatedMessage( *original_options, original_uninterpreted_options_field, i)); if (!InterpretSingleOption(options, src_path, - options_to_interpret->element_path)) { + options_to_interpret->element_path, features)) { // Error already added by InterpretSingleOption(). failed = true; break; @@ -7851,7 +7858,7 @@ bool DescriptorBuilder::OptionInterpreter::InterpretOptions( bool DescriptorBuilder::OptionInterpreter::InterpretSingleOption( Message* options, const std::vector& src_path, - const std::vector& options_path) { + const std::vector& options_path, bool features) { // First do some basic validation. if (uninterpreted_option_->name_size() == 0) { // This should never happen unless the parser has gone seriously awry or @@ -7864,6 +7871,13 @@ bool DescriptorBuilder::OptionInterpreter::InterpretSingleOption( return "Option must not use reserved name \"uninterpreted_option\"."; }); } + if (features != (uninterpreted_option_->name(0).name_part() == "features")) { + // Allow feature and option interpretation to occur in two phases. This is + // necessary because features *are* options and need to be interpreted + // before resolving them. However, options can also *have* features + // attached to them. + return true; + } const Descriptor* options_descriptor = nullptr; // Get the options message's descriptor from the builder's pool, so that we diff --git a/src/google/protobuf/test_textproto.h b/src/google/protobuf/test_textproto.h index c6d24cf728..c798c1a25e 100644 --- a/src/google/protobuf/test_textproto.h +++ b/src/google/protobuf/test_textproto.h @@ -34,6 +34,7 @@ #include #include "absl/log/absl_check.h" #include "absl/memory/memory.h" +#include "google/protobuf/dynamic_message.h" #include "google/protobuf/text_format.h" // This file contains private helpers for dealing with textprotos in our @@ -48,6 +49,14 @@ MATCHER_P(EqualsProto, textproto, "") { msg->DebugString() == arg.DebugString(); } +MATCHER_P3(EqualsProtoSerialized, pool, type, textproto, "") { + const Descriptor* desc = pool->FindMessageTypeByName(type); + DynamicMessageFactory factory(pool); + auto msg = absl::WrapUnique(factory.GetPrototype(desc)->New()); + return TextFormat::ParseFromString(textproto, msg.get()) && + arg.SerializeAsString() == msg->SerializeAsString(); +} + class ParseTextOrDie { public: explicit ParseTextOrDie(absl::string_view text) : text_(text) {}