From 13fd045dbb2b4dacea32be162a41d5a4b0d1802f Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Tue, 12 Sep 2017 10:32:01 -0700 Subject: Integrated internal changes from Google --- src/google/protobuf/descriptor.cc | 337 +++++++++++++++++++++++++++++++------- 1 file changed, 277 insertions(+), 60 deletions(-) (limited to 'src/google/protobuf/descriptor.cc') diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 89b37ee3..58829560 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -593,7 +593,7 @@ class DescriptorPool::Tables { // These return NULL if not found. inline const FileDescriptor* FindFile(const string& key) const; inline const FieldDescriptor* FindExtension(const Descriptor* extendee, - int number); + int number) const; inline void FindAllExtensions(const Descriptor* extendee, std::vector* out) const; @@ -752,11 +752,27 @@ class FileDescriptorTables { const SourceCodeInfo_Location* GetSourceLocation( const std::vector& path, const SourceCodeInfo* info) const; + // Must be called after BuildFileImpl(), even if the build failed and + // we are going to roll back to the last checkpoint. + void FinalizeTables(); + private: - SymbolsByParentMap symbols_by_parent_; - FieldsByNameMap fields_by_lowercase_name_; - FieldsByNameMap fields_by_camelcase_name_; - FieldsByNumberMap fields_by_number_; // Not including extensions. + const void* FindParentForFieldsByMap(const FieldDescriptor* field) const; + static void FieldsByLowercaseNamesLazyInitStatic( + const FileDescriptorTables* tables); + void FieldsByLowercaseNamesLazyInitInternal() const; + static void FieldsByCamelcaseNamesLazyInitStatic( + const FileDescriptorTables* tables); + void FieldsByCamelcaseNamesLazyInitInternal() const; + + SymbolsByParentMap symbols_by_parent_; + mutable FieldsByNameMap fields_by_lowercase_name_; + mutable FieldsByNameMap* fields_by_lowercase_name_tmp_; + mutable GoogleOnceDynamic fields_by_lowercase_name_once_; + mutable FieldsByNameMap fields_by_camelcase_name_; + mutable FieldsByNameMap* fields_by_camelcase_name_tmp_; + mutable GoogleOnceDynamic fields_by_camelcase_name_once_; + FieldsByNumberMap fields_by_number_; // Not including extensions. EnumValuesByNumberMap enum_values_by_number_; mutable EnumValuesByNumberMap unknown_enum_values_by_number_ GOOGLE_GUARDED_BY(unknown_enum_values_mu_); @@ -793,14 +809,16 @@ DescriptorPool::Tables::~Tables() { } FileDescriptorTables::FileDescriptorTables() - // Initialize all the hash tables to start out with a small # of buckets + // Initialize all the hash tables to start out with a small # of buckets. : symbols_by_parent_(3), fields_by_lowercase_name_(3), + fields_by_lowercase_name_tmp_(new FieldsByNameMap()), fields_by_camelcase_name_(3), + fields_by_camelcase_name_tmp_(new FieldsByNameMap()), fields_by_number_(3), enum_values_by_number_(3), - unknown_enum_values_by_number_(3) { -} + unknown_enum_values_by_number_(3), + locations_by_path_(3) {} FileDescriptorTables::~FileDescriptorTables() {} @@ -960,14 +978,59 @@ inline const FieldDescriptor* FileDescriptorTables::FindFieldByNumber( return FindPtrOrNull(fields_by_number_, std::make_pair(parent, number)); } +const void* FileDescriptorTables::FindParentForFieldsByMap( + const FieldDescriptor* field) const { + if (field->is_extension()) { + if (field->extension_scope() == NULL) { + return field->file(); + } else { + return field->extension_scope(); + } + } else { + return field->containing_type(); + } +} + +void FileDescriptorTables::FieldsByLowercaseNamesLazyInitStatic( + const FileDescriptorTables* tables) { + tables->FieldsByLowercaseNamesLazyInitInternal(); +} + +void FileDescriptorTables::FieldsByLowercaseNamesLazyInitInternal() const { + for (FieldsByNumberMap::const_iterator it = fields_by_number_.begin(); + it != fields_by_number_.end(); it++) { + PointerStringPair lowercase_key(FindParentForFieldsByMap(it->second), + it->second->lowercase_name().c_str()); + InsertIfNotPresent(&fields_by_lowercase_name_, lowercase_key, it->second); + } +} + inline const FieldDescriptor* FileDescriptorTables::FindFieldByLowercaseName( const void* parent, const string& lowercase_name) const { + fields_by_lowercase_name_once_.Init( + &FileDescriptorTables::FieldsByLowercaseNamesLazyInitStatic, this); return FindPtrOrNull(fields_by_lowercase_name_, PointerStringPair(parent, lowercase_name.c_str())); } +void FileDescriptorTables::FieldsByCamelcaseNamesLazyInitStatic( + const FileDescriptorTables* tables) { + tables->FieldsByCamelcaseNamesLazyInitInternal(); +} + +void FileDescriptorTables::FieldsByCamelcaseNamesLazyInitInternal() const { + for (FieldsByNumberMap::const_iterator it = fields_by_number_.begin(); + it != fields_by_number_.end(); it++) { + PointerStringPair camelcase_key(FindParentForFieldsByMap(it->second), + it->second->camelcase_name().c_str()); + InsertIfNotPresent(&fields_by_camelcase_name_, camelcase_key, it->second); + } +} + inline const FieldDescriptor* FileDescriptorTables::FindFieldByCamelcaseName( const void* parent, const string& camelcase_name) const { + fields_by_camelcase_name_once_.Init( + &FileDescriptorTables::FieldsByCamelcaseNamesLazyInitStatic, this); return FindPtrOrNull(fields_by_camelcase_name_, PointerStringPair(parent, camelcase_name.c_str())); } @@ -1031,7 +1094,7 @@ FileDescriptorTables::FindEnumValueByNumberCreatingIfUnknown( inline const FieldDescriptor* DescriptorPool::Tables::FindExtension( - const Descriptor* extendee, int number) { + const Descriptor* extendee, int number) const { return FindPtrOrNull(extensions_, std::make_pair(extendee, number)); } @@ -1072,24 +1135,40 @@ bool DescriptorPool::Tables::AddFile(const FileDescriptor* file) { } } +void FileDescriptorTables::FinalizeTables() { + // Clean up the temporary maps used by AddFieldByStylizedNames(). + delete fields_by_lowercase_name_tmp_; + fields_by_lowercase_name_tmp_ = NULL; + delete fields_by_camelcase_name_tmp_; + fields_by_camelcase_name_tmp_ = NULL; +} + void FileDescriptorTables::AddFieldByStylizedNames( const FieldDescriptor* field) { - const void* parent; - if (field->is_extension()) { - if (field->extension_scope() == NULL) { - parent = field->file(); - } else { - parent = field->extension_scope(); - } - } else { - parent = field->containing_type(); - } + const void* parent = FindParentForFieldsByMap(field); + + // We want fields_by_{lower,camel}case_name_ to be lazily built, but + // cross-link order determines which entry will be present in the case of a + // conflict. So we use the temporary maps that get destroyed after + // BuildFileImpl() to detect the conflicts, and only store the conflicts in + // the map that will persist. We will then lazily populate the rest of the + // entries from fields_by_number_. PointerStringPair lowercase_key(parent, field->lowercase_name().c_str()); - InsertIfNotPresent(&fields_by_lowercase_name_, lowercase_key, field); + if (!InsertIfNotPresent(fields_by_lowercase_name_tmp_, lowercase_key, + field)) { + InsertIfNotPresent( + &fields_by_lowercase_name_, lowercase_key, + FindPtrOrNull(*fields_by_lowercase_name_tmp_, lowercase_key)); + } PointerStringPair camelcase_key(parent, field->camelcase_name().c_str()); - InsertIfNotPresent(&fields_by_camelcase_name_, camelcase_key, field); + if (!InsertIfNotPresent(fields_by_camelcase_name_tmp_, camelcase_key, + field)) { + InsertIfNotPresent( + &fields_by_camelcase_name_, camelcase_key, + FindPtrOrNull(*fields_by_camelcase_name_tmp_, camelcase_key)); + } } bool FileDescriptorTables::AddFieldByNumber(const FieldDescriptor* field) { @@ -1419,6 +1498,15 @@ const MethodDescriptor* DescriptorPool::FindMethodByName( const FieldDescriptor* DescriptorPool::FindExtensionByNumber( const Descriptor* extendee, int number) const { + // A faster path to reduce lock contention in finding extensions, assuming + // most extensions will be cache hit. + if (mutex_ != NULL) { + ReaderMutexLock lock(mutex_); + const FieldDescriptor* result = tables_->FindExtension(extendee, number); + if (result != NULL) { + return result; + } + } MutexLockMaybe lock(mutex_); tables_->known_bad_symbols_.clear(); tables_->known_bad_files_.clear(); @@ -1721,6 +1809,18 @@ Descriptor::FindReservedRangeContainingNumber(int number) const { return NULL; } +const EnumDescriptor::ReservedRange* +EnumDescriptor::FindReservedRangeContainingNumber(int number) const { + // TODO(chrisn): Consider a non-linear search. + for (int i = 0; i < reserved_range_count(); i++) { + if (number >= reserved_range(i)->start && + number <= reserved_range(i)->end) { + return reserved_range(i); + } + } + return NULL; +} + // ------------------------------------------------------------------- bool DescriptorPool::TryFindFileInFallbackDatabase(const string& name) const { @@ -1826,8 +1926,8 @@ bool DescriptorPool::TryFindExtensionInFallbackDatabase( // =================================================================== -bool FieldDescriptor::is_map() const { - return type() == TYPE_MESSAGE && message_type()->options().map_entry(); +bool FieldDescriptor::is_map_message_type() const { + return message_type_->options().map_entry(); } string FieldDescriptor::DefaultValueAsString(bool quote_string_type) const { @@ -2063,6 +2163,14 @@ void EnumDescriptor::CopyTo(EnumDescriptorProto* proto) const { for (int i = 0; i < value_count(); i++) { value(i)->CopyTo(proto->add_value()); } + for (int i = 0; i < reserved_range_count(); i++) { + EnumDescriptorProto::EnumReservedRange* range = proto->add_reserved_range(); + range->set_start(reserved_range(i)->start); + range->set_end(reserved_range(i)->end); + } + for (int i = 0; i < reserved_name_count(); i++) { + proto->add_reserved_name(reserved_name(i)); + } if (&options() != &EnumOptions::default_instance()) { proto->mutable_options()->CopyFrom(options()); @@ -2695,6 +2803,30 @@ void EnumDescriptor::DebugString(int depth, string *contents, for (int i = 0; i < value_count(); i++) { value(i)->DebugString(depth, contents, debug_string_options); } + + if (reserved_range_count() > 0) { + strings::SubstituteAndAppend(contents, "$0 reserved ", prefix); + for (int i = 0; i < reserved_range_count(); i++) { + const EnumDescriptor::ReservedRange* range = reserved_range(i); + if (range->end == range->start) { + strings::SubstituteAndAppend(contents, "$0, ", range->start); + } else { + strings::SubstituteAndAppend(contents, "$0 to $1, ", + range->start, range->end); + } + } + contents->replace(contents->size() - 2, 2, ";\n"); + } + + if (reserved_name_count() > 0) { + strings::SubstituteAndAppend(contents, "$0 reserved ", prefix); + for (int i = 0; i < reserved_name_count(); i++) { + strings::SubstituteAndAppend(contents, "\"$0\", ", + CEscape(reserved_name(i))); + } + contents->replace(contents->size() - 2, 2, ";\n"); + } + strings::SubstituteAndAppend(contents, "$0}\n", prefix); comment_printer.AddPostComment(contents); @@ -2994,7 +3126,7 @@ class DescriptorBuilder { friend class OptionInterpreter; // Non-recursive part of BuildFile functionality. - const FileDescriptor* BuildFileImpl(const FileDescriptorProto& proto); + FileDescriptor* BuildFileImpl(const FileDescriptorProto& proto); const DescriptorPool* pool_; DescriptorPool::Tables* tables_; // for convenience @@ -3178,6 +3310,9 @@ class DescriptorBuilder { void BuildReservedRange(const DescriptorProto::ReservedRange& proto, const Descriptor* parent, Descriptor::ReservedRange* result); + void BuildReservedRange(const EnumDescriptorProto::EnumReservedRange& proto, + const EnumDescriptor* parent, + EnumDescriptor::ReservedRange* result); void BuildOneof(const OneofDescriptorProto& proto, Descriptor* parent, OneofDescriptor* result); @@ -4089,14 +4224,25 @@ const FileDescriptor* DescriptorBuilder::BuildFile( tables_->pending_files_.pop_back(); } } - return BuildFileImpl(proto); -} -const FileDescriptor* DescriptorBuilder::BuildFileImpl( - const FileDescriptorProto& proto) { // Checkpoint the tables so that we can roll back if something goes wrong. tables_->AddCheckpoint(); + FileDescriptor* result = BuildFileImpl(proto); + + file_tables_->FinalizeTables(); + if (result) { + tables_->ClearLastCheckpoint(); + result->finished_building_ = true; + } else { + tables_->RollbackToLastCheckpoint(); + } + + return result; +} + +FileDescriptor* DescriptorBuilder::BuildFileImpl( + const FileDescriptorProto& proto) { FileDescriptor* result = tables_->Allocate(); file_ = result; @@ -4148,7 +4294,6 @@ const FileDescriptor* DescriptorBuilder::BuildFileImpl( "A file with this name is already in the pool."); // Bail out early so that if this is actually the exact same file, we // don't end up reporting that every single symbol is already defined. - tables_->RollbackToLastCheckpoint(); return NULL; } if (!result->package().empty()) { @@ -4189,17 +4334,16 @@ const FileDescriptor* DescriptorBuilder::BuildFileImpl( // Recursive import. dependency/result is not fully initialized, and it's // dangerous to try to do anything with it. The recursive import error // will be detected and reported in DescriptorBuilder::BuildFile(). - tables_->RollbackToLastCheckpoint(); return NULL; } if (dependency == NULL) { - if (pool_->allow_unknown_ || - (!pool_->enforce_weak_ && weak_deps.find(i) != weak_deps.end())) { - dependency = - pool_->NewPlaceholderFileWithMutexHeld(proto.dependency(i)); - } else { - if (!pool_->lazily_build_dependencies_) { + if (!pool_->lazily_build_dependencies_) { + if (pool_->allow_unknown_ || + (!pool_->enforce_weak_ && weak_deps.find(i) != weak_deps.end())) { + dependency = + pool_->NewPlaceholderFileWithMutexHeld(proto.dependency(i)); + } else { AddImportError(proto, i); } } @@ -4325,11 +4469,8 @@ const FileDescriptor* DescriptorBuilder::BuildFileImpl( } if (had_errors_) { - tables_->RollbackToLastCheckpoint(); return NULL; } else { - tables_->ClearLastCheckpoint(); - result->finished_building_ = true; return result; } } @@ -4807,6 +4948,19 @@ void DescriptorBuilder::BuildReservedRange( } } +void DescriptorBuilder::BuildReservedRange( + const EnumDescriptorProto::EnumReservedRange& proto, + const EnumDescriptor* parent, EnumDescriptor::ReservedRange* result) { + result->start = proto.start(); + result->end = proto.end(); + + if (result->start > result->end) { + AddError(parent->full_name(), proto, + DescriptorPool::ErrorCollector::NUMBER, + "Reserved range end number must be greater than start number."); + } +} + void DescriptorBuilder::BuildOneof(const OneofDescriptorProto& proto, Descriptor* parent, OneofDescriptor* result) { @@ -4928,6 +5082,17 @@ void DescriptorBuilder::BuildEnum(const EnumDescriptorProto& proto, } BUILD_ARRAY(proto, result, value, BuildEnumValue, result); + BUILD_ARRAY(proto, result, reserved_range, BuildReservedRange, result); + + // Copy reserved names. + int reserved_name_count = proto.reserved_name_size(); + result->reserved_name_count_ = reserved_name_count; + result->reserved_names_ = + tables_->AllocateArray(reserved_name_count); + for (int i = 0; i < reserved_name_count; ++i) { + result->reserved_names_[i] = + tables_->AllocateString(proto.reserved_name(i)); + } CheckEnumValueUniqueness(proto, result); @@ -4940,6 +5105,56 @@ void DescriptorBuilder::BuildEnum(const EnumDescriptorProto& proto, AddSymbol(result->full_name(), parent, result->name(), proto, Symbol(result)); + + for (int i = 0; i < proto.reserved_range_size(); i++) { + const EnumDescriptorProto_EnumReservedRange& range1 = + proto.reserved_range(i); + for (int j = i + 1; j < proto.reserved_range_size(); j++) { + const EnumDescriptorProto_EnumReservedRange& range2 = + proto.reserved_range(j); + if (range1.end() > range2.start() && range2.end() > range1.start()) { + AddError(result->full_name(), proto.reserved_range(i), + DescriptorPool::ErrorCollector::NUMBER, + strings::Substitute("Reserved range $0 to $1 overlaps with " + "already-defined range $2 to $3.", + range2.start(), range2.end() - 1, + range1.start(), range1.end() - 1)); + } + } + } + + hash_set reserved_name_set; + for (int i = 0; i < proto.reserved_name_size(); i++) { + const string& name = proto.reserved_name(i); + if (reserved_name_set.find(name) == reserved_name_set.end()) { + reserved_name_set.insert(name); + } else { + AddError(name, proto, DescriptorPool::ErrorCollector::NAME, + strings::Substitute( + "Enum value \"$0\" is reserved multiple times.", + name)); + } + } + + for (int i = 0; i < result->value_count(); i++) { + const EnumValueDescriptor* value = result->value(i); + for (int j = 0; j < result->reserved_range_count(); j++) { + const EnumDescriptor::ReservedRange* range = result->reserved_range(j); + if (range->start <= value->number() && value->number() <= range->end) { + AddError(value->full_name(), proto.reserved_range(j), + DescriptorPool::ErrorCollector::NUMBER, + strings::Substitute( + "Enum value \"$0\" uses reserved number $1.", + value->name(), value->number())); + } + } + if (reserved_name_set.find(value->name()) != reserved_name_set.end()) { + AddError(value->full_name(), proto.value(i), + DescriptorPool::ErrorCollector::NAME, + strings::Substitute( + "Enum value \"$0\" is reserved.", value->name())); + } + } } void DescriptorBuilder::BuildEnumValue(const EnumValueDescriptorProto& proto, @@ -5248,25 +5463,27 @@ void DescriptorBuilder::CrossLinkField( bool expecting_enum = (proto.type() == FieldDescriptorProto::TYPE_ENUM) || proto.has_default_value(); + // In case of weak fields we force building the dependency. We need to know + // if the type exist or not. If it doesnt exist we substitute Empty which + // should only be done if the type can't be found in the generated pool. + // TODO(gerbens) Ideally we should query the database directly to check + // if weak fields exist or not so that we don't need to force building + // weak dependencies. However the name lookup rules for symbols are + // somewhat complicated, so I defer it too another CL. + bool is_weak = !pool_->enforce_weak_ && proto.options().weak(); + bool is_lazy = pool_->lazily_build_dependencies_ && !is_weak; + Symbol type = LookupSymbol(proto.type_name(), field->full_name(), expecting_enum ? DescriptorPool::PLACEHOLDER_ENUM : DescriptorPool::PLACEHOLDER_MESSAGE, - LOOKUP_TYPES, !pool_->lazily_build_dependencies_); - - // If the type is a weak type, we change the type to a google.protobuf.Empty field. - if (type.IsNull() && !pool_->enforce_weak_ && proto.options().weak()) { - type = FindSymbol(kNonLinkedWeakMessageReplacementName); - } + LOOKUP_TYPES, !is_lazy); if (type.IsNull()) { - if (pool_->lazily_build_dependencies_) { + if (is_lazy) { // Save the symbol names for later for lookup, and allocate the once // object needed for the accessors. string name = proto.type_name(); - if (!pool_->enforce_weak_ && proto.options().weak()) { - name = kNonLinkedWeakMessageReplacementName; - } field->type_once_ = tables_->AllocateOnceDynamic(); field->type_name_ = tables_->AllocateString(name); if (proto.has_default_value()) { @@ -5284,10 +5501,17 @@ void DescriptorBuilder::CrossLinkField( } return; } else { - AddNotDefinedError(field->full_name(), proto, - DescriptorPool::ErrorCollector::TYPE, - proto.type_name()); - return; + // If the type is a weak type, we change the type to a google.protobuf.Empty + // field. + if (is_weak) { + type = FindSymbol(kNonLinkedWeakMessageReplacementName); + } + if (type.IsNull()) { + AddNotDefinedError(field->full_name(), proto, + DescriptorPool::ErrorCollector::TYPE, + proto.type_name()); + return; + } } } @@ -6835,13 +7059,6 @@ const EnumValueDescriptor* FieldDescriptor::default_value_enum() const { return default_value_enum_; } -FieldDescriptor::Type FieldDescriptor::type() const { - if (type_once_) { - type_once_->Init(&FieldDescriptor::TypeOnceInit, this); - } - return type_; -} - void FileDescriptor::InternalDependenciesOnceInit() const { GOOGLE_CHECK(finished_building_ == true); for (int i = 0; i < dependency_count(); i++) { -- cgit v1.2.3