Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge two functions that calculate the effective string field type and migrate all callers to the new one. #16504

Closed
wants to merge 1 commit into from
Closed

Merge two functions that calculate the effective string field type and migrate all callers to the new one. #16504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 3 additions & 34 deletions src/google/protobuf/compiler/cpp/field.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ namespace protobuf {
namespace compiler {
namespace cpp {
using ::google::protobuf::internal::WireFormat;
using ::google::protobuf::internal::cpp::GetStringType;
using ::google::protobuf::internal::cpp::StringType;
using Sub = ::google::protobuf::io::Printer::Sub;

std::vector<Sub> FieldVars(const FieldDescriptor* field, const Options& opts) {
Expand Down Expand Up @@ -228,39 +230,6 @@ void FieldGeneratorBase::GenerateCopyConstructorCode(io::Printer* p) const {
}

namespace {
// Use internal types instead of ctype or string_type.
enum class StringType {
kView,
kString,
kCord,
kStringPiece,
};

StringType GetStringType(const FieldDescriptor& field) {
ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING);

if (field.options().has_ctype()) {
switch (field.options().ctype()) {
case FieldOptions::CORD:
return StringType::kCord;
case FieldOptions::STRING_PIECE:
return StringType::kStringPiece;
default:
return StringType::kString;
}
}

const pb::CppFeatures& cpp_features =
CppGenerator::GetResolvedSourceFeatures(field).GetExtension(::pb::cpp);
switch (cpp_features.string_type()) {
case pb::CppFeatures::CORD:
return StringType::kCord;
case pb::CppFeatures::VIEW:
return StringType::kView;
default:
return StringType::kString;
}
}

std::unique_ptr<FieldGeneratorBase> MakeGenerator(const FieldDescriptor* field,
const Options& options,
Expand All @@ -278,7 +247,7 @@ std::unique_ptr<FieldGeneratorBase> MakeGenerator(const FieldDescriptor* field,
case FieldDescriptor::CPPTYPE_MESSAGE:
return MakeRepeatedMessageGenerator(field, options, scc);
case FieldDescriptor::CPPTYPE_STRING: {
if (GetStringType(*field) == StringType::kView) {
if (internal::cpp::StringTypeIsStringView(*field)) {
return MakeRepeatedStringViewGenerator(field, options, scc);
} else {
return MakeRepeatedStringGenerator(field, options, scc);
Expand Down
12 changes: 3 additions & 9 deletions src/google/protobuf/compiler/cpp/field_generators/string_field.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,9 @@ void SingularString::GenerateAccessorDeclarations(io::Printer* p) const {
// files that applied the ctype. The field can still be accessed via the
// reflection interface since the reflection interface is independent of
// the string's underlying representation.
bool unknown_ctype =
field_->options().ctype() != internal::cpp::EffectiveStringCType(field_);

if (unknown_ctype) {
if (!internal::cpp::StringTypeIsSupported(*field_)) {
p->Emit(R"cc(
private: // Hidden due to unknown ctype option.
private: // Hidden due to unsupported option.
)cc");
}

Expand Down Expand Up @@ -822,10 +819,7 @@ class RepeatedString : public FieldGeneratorBase {
};

void RepeatedString::GenerateAccessorDeclarations(io::Printer* p) const {
bool unknown_ctype =
field_->options().ctype() != internal::cpp::EffectiveStringCType(field_);

if (unknown_ctype) {
if (!internal::cpp::StringTypeIsSupported(*field_)) {
p->Emit(R"cc(
private: // Hidden due to unknown ctype option.
)cc");
Expand Down
11 changes: 0 additions & 11 deletions src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,6 @@ void SingularStringView::GenerateStaticMembers(io::Printer* p) const {
}

void SingularStringView::GenerateAccessorDeclarations(io::Printer* p) const {
ABSL_CHECK(!field_->options().has_ctype());

auto vars = AnnotatedAccessors(field_, {"", "set_allocated_"});
vars.push_back(Sub{
"release_name",
Expand Down Expand Up @@ -648,15 +646,6 @@ class RepeatedStringView : public FieldGeneratorBase {
};

void RepeatedStringView::GenerateAccessorDeclarations(io::Printer* p) const {
bool unknown_ctype =
field_->options().ctype() != internal::cpp::EffectiveStringCType(field_);

if (unknown_ctype) {
p->Emit(R"cc(
private: // Hidden due to unknown ctype option.
)cc");
}

auto v1 = p->WithVars(AnnotatedAccessors(field_, {"", "_internal_"}));
auto v2 = p->WithVars(
AnnotatedAccessors(field_, {"set_", "add_"}, AnnotationCollector::kSet));
Expand Down
16 changes: 2 additions & 14 deletions src/google/protobuf/compiler/cpp/helpers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,17 +1071,10 @@ bool HasRepeatedFields(const FileDescriptor* file) {
return false;
}

static bool IsStringPieceField(const FieldDescriptor* field,
const Options& options) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::EffectiveStringCType(field) ==
FieldOptions::STRING_PIECE;
}

static bool HasStringPieceFields(const Descriptor* descriptor,
const Options& options) {
for (int i = 0; i < descriptor->field_count(); ++i) {
if (IsStringPieceField(descriptor->field(i), options)) return true;
if (IsStringPiece(descriptor->field(i))) return true;
}
for (int i = 0; i < descriptor->nested_type_count(); ++i) {
if (HasStringPieceFields(descriptor->nested_type(i), options)) return true;
Expand All @@ -1096,15 +1089,10 @@ bool HasStringPieceFields(const FileDescriptor* file, const Options& options) {
return false;
}

static bool IsCordField(const FieldDescriptor* field, const Options& options) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD;
}

static bool HasCordFields(const Descriptor* descriptor,
const Options& options) {
for (int i = 0; i < descriptor->field_count(); ++i) {
if (IsCordField(descriptor->field(i), options)) return true;
if (IsCord(descriptor->field(i))) return true;
}
for (int i = 0; i < descriptor->nested_type_count(); ++i) {
if (HasCordFields(descriptor->nested_type(i), options)) return true;
Expand Down
13 changes: 9 additions & 4 deletions src/google/protobuf/compiler/cpp/helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,18 +330,23 @@ inline bool IsWeak(const FieldDescriptor* field, const Options& options) {

inline bool IsCord(const FieldDescriptor* field) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD;
internal::cpp::StringTypeIsCord(*field);
}

inline bool IsString(const FieldDescriptor* field) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::EffectiveStringCType(field) == FieldOptions::STRING;
internal::cpp::StringTypeIsStdString(*field);
}

inline bool IsStringView(const FieldDescriptor* field) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::StringTypeIsStringView(*field);
}

inline bool IsStringPiece(const FieldDescriptor* field) {
return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
internal::cpp::EffectiveStringCType(field) ==
FieldOptions::STRING_PIECE;
internal::cpp::GetStringType(*field) ==
internal::cpp::StringType::kStringPiece;
}

bool IsProfileDriven(const Options& options);
Expand Down
7 changes: 5 additions & 2 deletions src/google/protobuf/compiler/cpp/message.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ namespace cpp {
namespace {
using ::google::protobuf::internal::WireFormat;
using ::google::protobuf::internal::WireFormatLite;
using ::google::protobuf::internal::cpp::GetStringType;
using ::google::protobuf::internal::cpp::HasHasbit;
using ::google::protobuf::internal::cpp::StringType;
using Semantic = ::google::protobuf::io::AnnotationCollector::Semantic;
using Sub = ::google::protobuf::io::Printer::Sub;

Expand Down Expand Up @@ -302,8 +304,9 @@ bool IsCrossFileMaybeMap(const FieldDescriptor* field) {

bool HasNonSplitOptionalString(const Descriptor* desc, const Options& options) {
for (const auto* field : FieldRange(desc)) {
if (IsString(field) && !field->is_repeated() &&
!field->real_containing_oneof() && !ShouldSplit(field, options)) {
if (!field->is_repeated() && !field->real_containing_oneof() &&
(IsString(field) || IsStringView(field)) &&
!ShouldSplit(field, options)) {
return true;
}
}
Expand Down
40 changes: 40 additions & 0 deletions src/google/protobuf/descriptor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9747,6 +9747,46 @@ bool IsLazilyInitializedFile(absl::string_view filename) {
filename == "google/protobuf/descriptor.proto";
}

StringType GetStringType(const FieldDescriptor& field, bool should_normalize) {
ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING);

const pb::CppFeatures& cpp_features =
InternalFeatureHelper::GetFeatures(field).GetExtension(::pb::cpp);
switch (cpp_features.string_type()) {
case pb::CppFeatures::CORD:
ABSL_DCHECK(!field.options().has_ctype() ||
field.options().ctype() == FieldOptions::CORD);
if (should_normalize && (field.type() == FieldDescriptor::TYPE_STRING ||
field.is_repeated() || field.is_extension())) {
return StringType::kString;
}
return StringType::kCord;
case pb::CppFeatures::STRING_PIECE:
ABSL_DCHECK(!field.options().has_ctype() ||
field.options().ctype() == FieldOptions::STRING_PIECE);
if (should_normalize) {
return StringType::kString;
}
return StringType::kStringPiece;
case pb::CppFeatures::VIEW:
return StringType::kView;
default:
return StringType::kString;
}
}

bool StringTypeIsStdString(const FieldDescriptor& field) {
return GetStringType(field) == StringType::kString;
}

bool StringTypeIsStringView(const FieldDescriptor& field) {
return GetStringType(field) == StringType::kView;
}

bool StringTypeIsCord(const FieldDescriptor& field) {
return GetStringType(field) == StringType::kCord;
}

} // namespace cpp
} // namespace internal

Expand Down
24 changes: 10 additions & 14 deletions src/google/protobuf/descriptor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2880,22 +2880,18 @@ PROTOBUF_EXPORT bool HasPreservingUnknownEnumSemantics(

PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field);

// For a string field, returns the effective ctype. If the actual ctype is
// not supported, returns the default of STRING.
template <typename FieldDesc = FieldDescriptor,
typename FieldOpts = FieldOptions>
typename FieldOpts::CType EffectiveStringCType(const FieldDesc* field) {
ABSL_DCHECK(field->cpp_type() == FieldDescriptor::CPPTYPE_STRING);
// Open-source protobuf release only supports STRING ctype and CORD for
// sinuglar bytes.
if (field->type() == FieldDescriptor::TYPE_BYTES && !field->is_repeated() &&
field->options().ctype() == FieldOpts::CORD && !field->is_extension()) {
return FieldOpts::CORD;
}
return FieldOpts::STRING;
#ifndef SWIG
enum class StringType : uint8_t;
PROTOBUF_EXPORT StringType GetStringType(const FieldDescriptor& field,
bool should_normalize = true);
PROTOBUF_EXPORT inline bool StringTypeIsSupported(
const FieldDescriptor& field) {
return GetStringType(field, true) == GetStringType(field, false);
}
PROTOBUF_EXPORT bool StringTypeIsStdString(const FieldDescriptor& field);
PROTOBUF_EXPORT bool StringTypeIsStringView(const FieldDescriptor& field);
PROTOBUF_EXPORT bool StringTypeIsCord(const FieldDescriptor& field);

#ifndef SWIG
enum class Utf8CheckMode {
kStrict = 0, // Parsing will fail if non UTF-8 data is in string fields.
kVerify = 1, // Only log an error but parsing will succeed.
Expand Down
42 changes: 30 additions & 12 deletions src/google/protobuf/dynamic_message.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,12 @@ int FieldSpaceUsed(const FieldDescriptor* field) {
}

case FD::CPPTYPE_STRING:
switch (field->options().ctype()) {
switch (internal::cpp::GetStringType(*field)) {
default: // TODO: Support other string reps.
case FieldOptions::STRING:
case internal::cpp::StringType::kView:
// Currently VIEW has the same ABI than string, so this fallback is
// fine.
case internal::cpp::StringType::kString:
return sizeof(RepeatedPtrField<std::string>);
}
break;
Expand Down Expand Up @@ -147,9 +150,12 @@ int FieldSpaceUsed(const FieldDescriptor* field) {
return sizeof(Message*);

case FD::CPPTYPE_STRING:
switch (field->options().ctype()) {
switch (internal::cpp::GetStringType(*field)) {
default: // TODO: Support other string reps.
case FieldOptions::STRING:
case internal::cpp::StringType::kView:
// Currently VIEW has the same ABI than string, so this fallback is
// fine.
case internal::cpp::StringType::kString:
return sizeof(ArenaStringPtr);
}
break;
Expand Down Expand Up @@ -401,9 +407,12 @@ void DynamicMessage::SharedCtor(bool lock_factory) {
break;

case FieldDescriptor::CPPTYPE_STRING:
switch (field->options().ctype()) {
switch (internal::cpp::GetStringType(*field)) {
default: // TODO: Support other string reps.
case FieldOptions::STRING:
case internal::cpp::StringType::kView:
// Currently VIEW has the same ABI than string, so this fallback is
// fine.
case internal::cpp::StringType::kString:
if (!field->is_repeated()) {
ArenaStringPtr* asp = new (field_ptr) ArenaStringPtr();
asp->InitDefault();
Expand Down Expand Up @@ -493,9 +502,12 @@ DynamicMessage::~DynamicMessage() {
if (*(reinterpret_cast<const int32_t*>(field_ptr)) == field->number()) {
field_ptr = MutableOneofFieldRaw(field);
if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) {
switch (field->options().ctype()) {
switch (internal::cpp::GetStringType(*field)) {
default:
case FieldOptions::STRING: {
case internal::cpp::StringType::kView:
// Currently VIEW has the same ABI than string, so this fallback is
// fine.
case internal::cpp::StringType::kString: {
reinterpret_cast<ArenaStringPtr*>(field_ptr)->Destroy();
break;
}
Expand Down Expand Up @@ -527,9 +539,12 @@ DynamicMessage::~DynamicMessage() {
#undef HANDLE_TYPE

case FieldDescriptor::CPPTYPE_STRING:
switch (field->options().ctype()) {
switch (internal::cpp::GetStringType(*field)) {
default: // TODO: Support other string reps.
case FieldOptions::STRING:
case internal::cpp::StringType::kView:
// Currently VIEW has the same ABI than string, so this fallback is
// fine.
case internal::cpp::StringType::kString:
reinterpret_cast<RepeatedPtrField<std::string>*>(field_ptr)
->~RepeatedPtrField<std::string>();
break;
Expand All @@ -547,9 +562,12 @@ DynamicMessage::~DynamicMessage() {
}

} else if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) {
switch (field->options().ctype()) {
switch (internal::cpp::GetStringType(*field)) {
default: // TODO: Support other string reps.
case FieldOptions::STRING: {
case internal::cpp::StringType::kView:
// Currently VIEW has the same ABI than string, so this fallback is
// fine.
case internal::cpp::StringType::kString: {
reinterpret_cast<ArenaStringPtr*>(field_ptr)->Destroy();
break;
}
Expand Down
Loading
Loading