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(
