Down-integrate internal changes to github. (#5527)
diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index db19ff5..0638393 100644 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc
@@ -2848,44 +2848,50 @@ // fields with presence. if (field->is_map()) { printer->Print( + "/** Clears values from the map. The map will be non-null. */\n" "$class$.prototype.$clearername$ = function() {\n" " this.$gettername$().clear();$returnvalue$\n" "};\n" "\n" "\n", "class", GetMessagePath(options, field->containing_type()), - "clearername", "clear" + JSGetterName(options, field), - "gettername", "get" + JSGetterName(options, field), - "returnvalue", JSReturnClause(field)); + "clearername", "clear" + JSGetterName(options, field), "gettername", + "get" + JSGetterName(options, field), "returnvalue", + JSReturnClause(field)); printer->Annotate("clearername", field); } else if (field->is_repeated() || (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE && !field->is_required())) { // Fields where we can delegate to the regular setter. printer->Print( + "/** $jsdoc$ */\n" "$class$.prototype.$clearername$ = function() {\n" " this.$settername$($clearedvalue$);$returnvalue$\n" "};\n" "\n" "\n", + "jsdoc", + field->is_repeated() ? "Clears the list making it empty but non-null." + : "Clears the message field making it undefined.", "class", GetMessagePath(options, field->containing_type()), - "clearername", "clear" + JSGetterName(options, field), - "settername", "set" + JSGetterName(options, field), - "clearedvalue", (field->is_repeated() ? "[]" : "undefined"), - "returnvalue", JSReturnClause(field)); + "clearername", "clear" + JSGetterName(options, field), "settername", + "set" + JSGetterName(options, field), "clearedvalue", + (field->is_repeated() ? "[]" : "undefined"), "returnvalue", + JSReturnClause(field)); printer->Annotate("clearername", field); } else if (HasFieldPresence(options, field)) { // Fields where we can't delegate to the regular setter because it doesn't // accept "undefined" as an argument. printer->Print( + "/** Clears the field making it undefined. */\n" "$class$.prototype.$clearername$ = function() {\n" " jspb.Message.set$maybeoneof$Field(this, " "$index$$maybeoneofgroup$, ", "class", GetMessagePath(options, field->containing_type()), - "clearername", "clear" + JSGetterName(options, field), - "maybeoneof", (field->containing_oneof() ? "Oneof" : ""), - "maybeoneofgroup", (field->containing_oneof() ? - (", " + JSOneofArray(options, field)) : ""), + "clearername", "clear" + JSGetterName(options, field), "maybeoneof", + (field->containing_oneof() ? "Oneof" : ""), "maybeoneofgroup", + (field->containing_oneof() ? (", " + JSOneofArray(options, field)) + : ""), "index", JSFieldIndex(field)); printer->Annotate("clearername", field); printer->Print( @@ -2963,14 +2969,15 @@ " * @param {number=} opt_index\n" " * @return {!$optionaltype$}\n" " */\n" - "$class$.prototype.add$name$ = function(opt_value, opt_index) {\n" + "$class$.prototype.$addername$ = function(opt_value, opt_index) {\n" " return jspb.Message.addTo$repeatedtag$WrapperField(", - "optionaltype", JSTypeName(options, field, BYTES_DEFAULT), - "class", GetMessagePath(options, field->containing_type()), - "name", JSGetterName(options, field, BYTES_DEFAULT, - /* drop_list = */ true), + "optionaltype", JSTypeName(options, field, BYTES_DEFAULT), "class", + GetMessagePath(options, field->containing_type()), "addername", + "add" + JSGetterName(options, field, BYTES_DEFAULT, + /* drop_list = */ true), "repeatedtag", (field->is_repeated() ? "Repeated" : "")); + printer->Annotate("addername", field); printer->Print( "this, $index$$oneofgroup$, opt_value, $ctor$, opt_index);\n" "};\n"
diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 96852ea..e77026c 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc
@@ -867,9 +867,7 @@ case FieldDescriptor::CPPTYPE_MESSAGE: { if (IsMapFieldInApi(field)) { - MutableRaw<MapFieldBase>(message, field) - ->MutableRepeatedField() - ->Clear<GenericTypeHandler<Message> >(); + MutableRaw<MapFieldBase>(message, field)->Clear(); } else { // We don't know which subclass of RepeatedPtrFieldBase the type is, // so we use RepeatedPtrFieldBase directly.
diff --git a/src/google/protobuf/map_field.cc b/src/google/protobuf/map_field.cc index 7c10d67..9b27a32 100644 --- a/src/google/protobuf/map_field.cc +++ b/src/google/protobuf/map_field.cc
@@ -91,21 +91,47 @@ state_.store(STATE_MODIFIED_REPEATED, std::memory_order_relaxed); } +void MapFieldBase::SetClean() { + // These are called by (non-const) mutator functions. So by our API it's the + // callers responsibility to have these calls properly ordered. + state_.store(CLEAN, std::memory_order_relaxed); +} + void* MapFieldBase::MutableRepeatedPtrField() const { return repeated_field_; } void MapFieldBase::SyncRepeatedFieldWithMap() const { // acquire here matches with release below to ensure that we can only see a // value of CLEAN after all previous changes have been synced. - if (state_.load(std::memory_order_acquire) == STATE_MODIFIED_MAP) { - mutex_.Lock(); - // Double check state, because another thread may have seen the same state - // and done the synchronization before the current thread. - if (state_.load(std::memory_order_relaxed) == STATE_MODIFIED_MAP) { - SyncRepeatedFieldWithMapNoLock(); - state_.store(CLEAN, std::memory_order_release); + switch (state_.load(std::memory_order_acquire)) { + case STATE_MODIFIED_MAP: + mutex_.Lock(); + // Double check state, because another thread may have seen the same + // state and done the synchronization before the current thread. + if (state_.load(std::memory_order_relaxed) == STATE_MODIFIED_MAP) { + SyncRepeatedFieldWithMapNoLock(); + state_.store(CLEAN, std::memory_order_release); + } + mutex_.Unlock(); + break; + case CLEAN: + mutex_.Lock(); + // Double check state + if (state_.load(std::memory_order_relaxed) == CLEAN) { + if (repeated_field_ == nullptr) { + if (arena_ == nullptr) { + repeated_field_ = new RepeatedPtrField<Message>(); + } else { + repeated_field_ = + Arena::CreateMessage<RepeatedPtrField<Message> >(arena_); + } + } + state_.store(CLEAN, std::memory_order_release); + } + mutex_.Unlock(); + break; + default: + break; } - mutex_.Unlock(); - } } void MapFieldBase::SyncRepeatedFieldWithMapNoLock() const { @@ -155,6 +181,20 @@ return GetMap().size(); } +void DynamicMapField::Clear() { + Map<MapKey, MapValueRef>* map = &const_cast<DynamicMapField*>(this)->map_; + for (Map<MapKey, MapValueRef>::iterator iter = map->begin(); + iter != map->end(); ++iter) { + iter->second.DeleteData(); + } + map->clear(); + + if (MapFieldBase::repeated_field_ != nullptr) { + MapFieldBase::repeated_field_->Clear(); + } + MapFieldBase::SetClean(); +} + bool DynamicMapField::ContainsMapKey( const MapKey& map_key) const { const Map<MapKey, MapValueRef>& map = GetMap();
diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index 5e598b8..0bb5598 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h
@@ -107,6 +107,7 @@ virtual void Swap(MapFieldBase* other) = 0; // Sync Map with repeated field and returns the size of map. virtual int size() const = 0; + virtual void Clear() = 0; // Returns the number of bytes used by the repeated field, excluding // sizeof(*this) @@ -136,6 +137,9 @@ // Tells MapFieldBase that there is new change to RepeatedPTrField. void SetRepeatedDirty(); + // Tells MapFieldBase that map and repeated are the same. + void SetClean(); + // Provides derived class the access to repeated field. void* MutableRepeatedPtrField() const; @@ -268,9 +272,8 @@ return result; } - // Convenient methods for generated message implementation. int size() const override; - void Clear(); + void Clear() override; void MergeFrom(const MapFieldBase& other) override; void Swap(MapFieldBase* other) override; @@ -334,6 +337,7 @@ Map<MapKey, MapValueRef>* MutableMap() override; int size() const override; + void Clear() override; private: Map<MapKey, MapValueRef> map_;
diff --git a/src/google/protobuf/map_field_inl.h b/src/google/protobuf/map_field_inl.h index 73c16ed..40ceb50 100644 --- a/src/google/protobuf/map_field_inl.h +++ b/src/google/protobuf/map_field_inl.h
@@ -178,9 +178,15 @@ WireFormatLite::FieldType kValueFieldType, int default_enum_value> void MapField<Derived, Key, T, kKeyFieldType, kValueFieldType, default_enum_value>::Clear() { - MapFieldBase::SyncMapWithRepeatedField(); + if (this->MapFieldBase::repeated_field_ != nullptr) { + RepeatedPtrField<EntryType>* repeated_field = + reinterpret_cast<RepeatedPtrField<EntryType>*>( + this->MapFieldBase::repeated_field_); + repeated_field->Clear(); + } + impl_.MutableMap()->clear(); - MapFieldBase::SetMapDirty(); + MapFieldBase::SetClean(); } template <typename Derived, typename Key, typename T,
diff --git a/src/google/protobuf/map_field_test.cc b/src/google/protobuf/map_field_test.cc index ae7bd58..9e6604b 100644 --- a/src/google/protobuf/map_field_test.cc +++ b/src/google/protobuf/map_field_test.cc
@@ -94,6 +94,7 @@ return false; } int size() const { return 0; } + void Clear() override {} void MapBegin(MapIterator* map_iter) const {} void MapEnd(MapIterator* map_iter) const {} void MergeFrom(const MapFieldBase& other) override {} @@ -295,7 +296,11 @@ if (is_repeated_null) { EXPECT_TRUE(repeated_field == NULL); } else { - EXPECT_EQ(repeated_size, repeated_field->size()); + if (repeated_field == nullptr) { + EXPECT_EQ(repeated_size, 0); + } else { + EXPECT_EQ(repeated_size, repeated_field->size()); + } } } @@ -442,11 +447,7 @@ TEST_P(MapFieldStateTest, Clear) { map_field_->Clear(); - if (state_ != MAP_DIRTY) { - Expect(map_field_.get(), MAP_DIRTY, 0, 1, false); - } else { - Expect(map_field_.get(), MAP_DIRTY, 0, 0, true); - } + Expect(map_field_.get(), CLEAN, 0, 0, false); } TEST_P(MapFieldStateTest, SpaceUsedExcludingSelf) {
diff --git a/src/google/protobuf/map_test_util.cc b/src/google/protobuf/map_test_util.cc index 31ac173..25f027f 100644 --- a/src/google/protobuf/map_test_util.cc +++ b/src/google/protobuf/map_test_util.cc
@@ -1582,6 +1582,10 @@ EXPECT_EQ(0, reflection->FieldSize(message, F("map_int32_bytes"))); EXPECT_EQ(0, reflection->FieldSize(message, F("map_int32_enum"))); EXPECT_EQ(0, reflection->FieldSize(message, F("map_int32_foreign_message"))); + EXPECT_TRUE(reflection->GetMapData( + message, F("map_int32_foreign_message"))->IsMapValid()); + EXPECT_TRUE(reflection->GetMapData( + message, F("map_int32_foreign_message"))->IsRepeatedFieldValid()); } void MapReflectionTester::ExpectClearViaReflectionIterator(
diff --git a/src/google/protobuf/util/internal/protostream_objectsource.cc b/src/google/protobuf/util/internal/protostream_objectsource.cc index a11d983..2c1a739 100644 --- a/src/google/protobuf/util/internal/protostream_objectsource.cc +++ b/src/google/protobuf/util/internal/protostream_objectsource.cc
@@ -188,10 +188,10 @@ bool include_start_and_end, ObjectWriter* ow) const { - const TypeRenderer* type_renderer = FindTypeRenderer(type.name()); - if (type_renderer != nullptr) { - return (*type_renderer)(this, type, name, ow); - } + const TypeRenderer* type_renderer = FindTypeRenderer(type.name()); + if (type_renderer != nullptr) { + return (*type_renderer)(this, type, name, ow); + } const google::protobuf::Field* field = nullptr; string field_name; @@ -229,9 +229,7 @@ if (field->cardinality() == google::protobuf::Field_Cardinality_CARDINALITY_REPEATED) { - bool check_maps = true; - - if (check_maps && IsMap(*field)) { + if (IsMap(*field)) { ow->StartObject(field_name); ASSIGN_OR_RETURN(tag, RenderMap(field, field_name, tag, ow)); ow->EndObject(); @@ -332,6 +330,7 @@ return util::Status(); } + Status ProtoStreamObjectSource::RenderTimestamp( const ProtoStreamObjectSource* os, const google::protobuf::Type& type, StringPiece field_name, ObjectWriter* ow) { @@ -708,6 +707,7 @@ ProtoStreamObjectSource::renderers_ = NULL; PROTOBUF_NAMESPACE_ID::internal::once_flag source_renderers_init_; + void ProtoStreamObjectSource::InitRendererMap() { renderers_ = new std::unordered_map<string, ProtoStreamObjectSource::TypeRenderer>(); @@ -745,6 +745,7 @@ ::google::protobuf::internal::OnShutdown(&DeleteRendererMap); } + void ProtoStreamObjectSource::DeleteRendererMap() { delete ProtoStreamObjectSource::renderers_; renderers_ = NULL; @@ -781,10 +782,8 @@ // Short-circuit any special type rendering to save call-stack space. const TypeRenderer* type_renderer = FindTypeRenderer(type->name()); - bool use_type_renderer = type_renderer != nullptr; - RETURN_IF_ERROR(IncrementRecursionDepth(type->name(), field_name)); - if (use_type_renderer) { + if (type_renderer != nullptr) { RETURN_IF_ERROR((*type_renderer)(this, *type, field_name, ow)); } else { RETURN_IF_ERROR(WriteMessage(*type, field_name, 0, true, ow));
diff --git a/src/google/protobuf/util/internal/protostream_objectsource.h b/src/google/protobuf/util/internal/protostream_objectsource.h index 124a36a..ac94b03 100644 --- a/src/google/protobuf/util/internal/protostream_objectsource.h +++ b/src/google/protobuf/util/internal/protostream_objectsource.h
@@ -46,6 +46,7 @@ #include <google/protobuf/stubs/status.h> #include <google/protobuf/stubs/statusor.h> + #include <google/protobuf/port_def.inc> namespace google { @@ -181,9 +182,10 @@ // Renders a NWP map. // Returns the next tag after reading all map entries. The caller should use // this tag before reading more tags from the stream. - util::StatusOr<uint32> RenderMap(const google::protobuf::Field* field, - StringPiece name, uint32 list_tag, - ObjectWriter* ow) const; + util::StatusOr<uint32> + RenderMap(const google::protobuf::Field* field, + StringPiece name, uint32 list_tag, + ObjectWriter* ow) const; // Renders a packed repeating field. A packed field is stored as: // {tag length item1 item2 item3} instead of the less efficient @@ -191,6 +193,7 @@ util::Status RenderPacked(const google::protobuf::Field* field, ObjectWriter* ow) const; + // Renders a google.protobuf.Timestamp value to ObjectWriter static util::Status RenderTimestamp(const ProtoStreamObjectSource* os, const google::protobuf::Type& type,
diff --git a/src/google/protobuf/util/type_resolver_util.cc b/src/google/protobuf/util/type_resolver_util.cc index f40ce94..4d7a3c0 100644 --- a/src/google/protobuf/util/type_resolver_util.cc +++ b/src/google/protobuf/util/type_resolver_util.cc
@@ -118,6 +118,27 @@ void ConvertMessageOptions(const MessageOptions& options, RepeatedPtrField<Option>* output) { + return ConvertOptionsInternal(options, output); + } + + void ConvertFieldOptions(const FieldOptions& options, + RepeatedPtrField<Option>* output) { + return ConvertOptionsInternal(options, output); + } + + void ConvertEnumOptions(const EnumOptions& options, + RepeatedPtrField<Option>* output) { + return ConvertOptionsInternal(options, output); + } + + void ConvertEnumValueOptions(const EnumValueOptions& options, + RepeatedPtrField<Option>* output) { + return ConvertOptionsInternal(options, output); + } + + // Implementation details for Convert*Options. + void ConvertOptionsInternal(const Message& options, + RepeatedPtrField<Option>* output) { const Reflection* reflection = options.GetReflection(); std::vector<const FieldDescriptor*> fields; reflection->ListFields(options, &fields); @@ -125,18 +146,18 @@ if (field->is_repeated()) { const int size = reflection->FieldSize(options, field); for (int i = 0; i < size; i++) { - ConvertMessageOption(reflection, options, field, i, output->Add()); + ConvertOptionField(reflection, options, field, i, output->Add()); } } else { - ConvertMessageOption(reflection, options, field, -1, output->Add()); + ConvertOptionField(reflection, options, field, -1, output->Add()); } } } - static void ConvertMessageOption(const Reflection* reflection, - const MessageOptions& options, - const FieldDescriptor* field, int index, - Option* out) { + static void ConvertOptionField(const Reflection* reflection, + const Message& options, + const FieldDescriptor* field, int index, + Option* out) { out->set_name(field->is_extension() ? field->full_name() : field->name()); Any* value = out->mutable_value(); switch (field->cpp_type()) { @@ -250,7 +271,7 @@ field->set_packed(true); } - // TODO(xiaofeng): Set other field "options"? + ConvertFieldOptions(descriptor->options(), field->mutable_options()); } void ConvertEnumDescriptor(const EnumDescriptor* descriptor, @@ -265,9 +286,11 @@ value->set_name(value_descriptor->name()); value->set_number(value_descriptor->number()); - // TODO(xiaofeng): Set EnumValue options. + ConvertEnumValueOptions(value_descriptor->options(), + value->mutable_options()); } - // TODO(xiaofeng): Set Enum "options". + + ConvertEnumOptions(descriptor->options(), enum_type->mutable_options()); } string GetTypeUrl(const Descriptor* descriptor) {
diff --git a/src/google/protobuf/util/type_resolver_util_test.cc b/src/google/protobuf/util/type_resolver_util_test.cc index fa06037..a564686 100644 --- a/src/google/protobuf/util/type_resolver_util_test.cc +++ b/src/google/protobuf/util/type_resolver_util_test.cc
@@ -52,10 +52,12 @@ namespace { using google::protobuf::BoolValue; using google::protobuf::Enum; +using google::protobuf::EnumValue; using google::protobuf::Field; using google::protobuf::Int32Value; using google::protobuf::Option; using google::protobuf::Type; +using google::protobuf::UInt64Value; static const char kUrlPrefix[] = "type.googleapis.com"; @@ -117,14 +119,18 @@ return field->packed(); } - bool EnumHasValue(const Enum& type, const string& name, int number) { - for (int i = 0; i < type.enumvalue_size(); ++i) { - if (type.enumvalue(i).name() == name && - type.enumvalue(i).number() == number) { - return true; + const EnumValue* FindEnumValue(const Enum& type, const string& name) { + for (const EnumValue& value : type.enumvalue()) { + if (value.name() == name) { + return &value; } } - return false; + return nullptr; + } + + bool EnumHasValue(const Enum& type, const string& name, int number) { + const EnumValue* value = FindEnumValue(type, name); + return value != nullptr && value->number() == number; } bool HasBoolOption(const RepeatedPtrField<Option>& options, @@ -137,6 +143,11 @@ return HasOption<Int32Value>(options, name, value); } + bool HasUInt64Option(const RepeatedPtrField<Option>& options, + const string& name, uint64 value) { + return HasOption<UInt64Value>(options, name, value); + } + template <typename WrapperT, typename T> bool HasOption(const RepeatedPtrField<Option>& options, const string& name, T value) { @@ -338,7 +349,7 @@ EXPECT_TRUE(HasBoolOption(type.options(), "map_entry", true)); } -TEST_F(DescriptorPoolTypeResolverTest, TestCustomOptions) { +TEST_F(DescriptorPoolTypeResolverTest, TestCustomMessageOptions) { Type type; ASSERT_TRUE( resolver_ @@ -350,6 +361,20 @@ HasInt32Option(type.options(), "protobuf_unittest.message_opt1", -56)); } +TEST_F(DescriptorPoolTypeResolverTest, TestCustomFieldOptions) { + Type type; + ASSERT_TRUE( + resolver_ + ->ResolveMessageType( + GetTypeUrl<protobuf_unittest::TestMessageWithCustomOptions>(), + &type) + .ok()); + const Field* field = FindField(type, "field1"); + ASSERT_TRUE(field != nullptr); + EXPECT_TRUE(HasUInt64Option(field->options(), "protobuf_unittest.field_opt1", + 8765432109)); +} + TEST_F(DescriptorPoolTypeResolverTest, TestEnum) { Enum type; ASSERT_TRUE(resolver_->ResolveEnumType( @@ -360,6 +385,32 @@ EnumHasValue(type, "NEG", -1); } +TEST_F(DescriptorPoolTypeResolverTest, TestCustomEnumOptions) { + Enum type; + ASSERT_TRUE( + resolver_ + ->ResolveEnumType( + GetTypeUrl("protobuf_unittest.TestMessageWithCustomOptions.AnEnum"), + &type) + .ok()); + ASSERT_TRUE( + HasInt32Option(type.options(), "protobuf_unittest.enum_opt1", -789)); +} + +TEST_F(DescriptorPoolTypeResolverTest, TestCustomValueOptions) { + Enum type; + ASSERT_TRUE( + resolver_ + ->ResolveEnumType( + GetTypeUrl("protobuf_unittest.TestMessageWithCustomOptions.AnEnum"), + &type) + .ok()); + const EnumValue* value = FindEnumValue(type, "ANENUM_VAL2"); + ASSERT_TRUE(value != nullptr); + ASSERT_TRUE( + HasInt32Option(value->options(), "protobuf_unittest.enum_value_opt1", 123)); +} + TEST_F(DescriptorPoolTypeResolverTest, TestJsonName) { Type type; ASSERT_TRUE(resolver_->ResolveMessageType(