Skip to content

Commit

Permalink
Remove options from some functions we want to use externally.
Browse files Browse the repository at this point in the history
`Options` is a fairly heavy class and not easy to initialize, and not required for the common functions we change here allowing a lower bar for re-use.

PiperOrigin-RevId: 573920014
  • Loading branch information
martijnvels authored and copybara-github committed Oct 16, 2023
1 parent 90e1b49 commit 4354846
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ void SingularString::GenerateConstructorCode(io::Printer* p) const {
$field_$.InitDefault();
)cc");

if (IsString(field_, *opts_) && EmptyDefault()) {
if (IsString(field_) && EmptyDefault()) {
p->Emit(R"cc(
#ifdef PROTOBUF_FORCE_COPY_DEFAULT_STRING
$field_$.Set("", GetArena());
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/compiler/cpp/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ void FileGenerator::GenerateSourceForMessage(int idx, io::Printer* p) {
GenerateSourceIncludes(p);
GenerateSourcePrelude(p);

if (IsAnyMessage(file_, options_)) {
if (IsAnyMessage(file_)) {
MuteWuninitialized(p);
}

Expand Down Expand Up @@ -943,7 +943,7 @@ void FileGenerator::GenerateSourceForMessage(int idx, io::Printer* p) {
message_generators_[idx]->GenerateSourceInProto2Namespace(p);
}

if (IsAnyMessage(file_, options_)) {
if (IsAnyMessage(file_)) {
UnmuteWuninitialized(p);
}

Expand Down Expand Up @@ -989,7 +989,7 @@ void FileGenerator::GenerateSource(io::Printer* p) {
GetCrossFileReferencesForFile(file_, &refs);
GenerateInternalForwardDeclarations(refs, p);

if (IsAnyMessage(file_, options_)) {
if (IsAnyMessage(file_)) {
MuteWuninitialized(p);
}

Expand Down Expand Up @@ -1058,7 +1058,7 @@ void FileGenerator::GenerateSource(io::Printer* p) {
// @@protoc_insertion_point(global_scope)
)cc");

if (IsAnyMessage(file_, options_)) {
if (IsAnyMessage(file_)) {
UnmuteWuninitialized(p);
}

Expand Down
23 changes: 20 additions & 3 deletions src/google/protobuf/compiler/cpp/helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,23 @@ bool IsStringInliningEnabled(const Options& options) {
return options.force_inline_string || IsProfileDriven(options);
}

bool CanStringBeInlined(const FieldDescriptor* field) {
// TODO: Handle inlining for any.proto.
if (IsAnyMessage(field->containing_type())) return false;
if (field->containing_type()->options().map_entry()) return false;
if (field->is_repeated()) return false;

// We rely on has bits to distinguish field presence for release_$name$. When
// there is no hasbit, we cannot use the address of the string instance when
// the field has been inlined.
if (!internal::cpp::HasHasbit(field)) return false;

if (!IsString(field)) return false;
if (!field->default_value_string().empty()) return false;

return true;
}

bool IsStringInlined(const FieldDescriptor* field, const Options& options) {
(void)field;
(void)options;
Expand Down Expand Up @@ -1148,13 +1165,13 @@ bool IsStringOrMessage(const FieldDescriptor* field) {
return false;
}

bool IsAnyMessage(const FileDescriptor* descriptor, const Options& options) {
bool IsAnyMessage(const FileDescriptor* descriptor) {
return descriptor->name() == kAnyProtoFile;
}

bool IsAnyMessage(const Descriptor* descriptor, const Options& options) {
bool IsAnyMessage(const Descriptor* descriptor) {
return descriptor->name() == kAnyMessageName &&
IsAnyMessage(descriptor->file(), options);
IsAnyMessage(descriptor->file());
}

bool IsWellKnownMessage(const FileDescriptor* file) {
Expand Down
9 changes: 6 additions & 3 deletions src/google/protobuf/compiler/cpp/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ inline bool IsCord(const FieldDescriptor* field) {
internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD;
}

inline bool IsString(const FieldDescriptor* field, const Options& options) {
inline bool IsString(const FieldDescriptor* field) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::EffectiveStringCType(field) == FieldOptions::STRING;
}
Expand All @@ -355,6 +355,9 @@ float GetPresenceProbability(const FieldDescriptor* field,

bool IsStringInliningEnabled(const Options& options);

// Returns true if the provided field is a singular string and can be inlined.
bool CanStringBeInlined(const FieldDescriptor* field);

// Returns true if `field` should be inlined based on PDProto profile.
bool IsStringInlined(const FieldDescriptor* field, const Options& options);

Expand Down Expand Up @@ -529,8 +532,8 @@ inline std::string MakeVarintCachedSizeFieldName(const FieldDescriptor* field,
// while the two functions below use FileDescriptor::name(). In a sane world the
// two approaches should be equivalent. But if you are dealing with descriptors
// from untrusted sources, you might need to match semantics across libraries.
bool IsAnyMessage(const FileDescriptor* descriptor, const Options& options);
bool IsAnyMessage(const Descriptor* descriptor, const Options& options);
bool IsAnyMessage(const FileDescriptor* descriptor);
bool IsAnyMessage(const Descriptor* descriptor);

bool IsWellKnownMessage(const FileDescriptor* descriptor);

Expand Down
12 changes: 6 additions & 6 deletions src/google/protobuf/compiler/cpp/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ bool IsCrossFileMaybeMap(const FieldDescriptor* field) {

bool HasNonSplitOptionalString(const Descriptor* desc, const Options& options) {
for (const auto* field : FieldRange(desc)) {
if (IsString(field, options) && !field->is_repeated() &&
if (IsString(field) && !field->is_repeated() &&
!field->real_containing_oneof() && !ShouldSplit(field, options)) {
return true;
}
Expand Down Expand Up @@ -1384,7 +1384,7 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* p) {
"\n",
index_in_file_messages_);

if (IsAnyMessage(descriptor_, options_)) {
if (IsAnyMessage(descriptor_)) {
format(
"// implements Any -----------------------------------------------\n"
"\n");
Expand Down Expand Up @@ -1870,7 +1870,7 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* p) {
format("::$proto_ns$::internal::WeakFieldMap _weak_field_map_;\n");
}
// Generate _any_metadata_ for the Any type.
if (IsAnyMessage(descriptor_, options_)) {
if (IsAnyMessage(descriptor_)) {
format("::$proto_ns$::internal::AnyMetadata _any_metadata_;\n");
}

Expand Down Expand Up @@ -1983,7 +1983,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* p) {
return;
}

if (IsAnyMessage(descriptor_, options_)) {
if (IsAnyMessage(descriptor_)) {
if (HasDescriptorMethods(descriptor_->file(), options_)) {
format(
"bool $classname$::GetAnyFieldDescriptors(\n"
Expand Down Expand Up @@ -2622,7 +2622,7 @@ void MessageGenerator::GenerateImplMemberInit(io::Printer* p,
};

auto init_any_metadata = [&] {
if (IsAnyMessage(descriptor_, options_)) {
if (IsAnyMessage(descriptor_)) {
separator();
p->Emit("_any_metadata_{&type_url_, &value_}");
}
Expand Down Expand Up @@ -3124,7 +3124,7 @@ void MessageGenerator::GenerateCopyConstructorBodyImpl(io::Printer* p) const {
decltype($weak_field_map$){from.$weak_field_map$},
)cc");
}
if (IsAnyMessage(descriptor_, options_)) {
if (IsAnyMessage(descriptor_)) {
p->Emit(R"cc(
/*decltype($any_metadata$)*/ {
&_impl_.type_url_,
Expand Down

0 comments on commit 4354846

Please sign in to comment.