Skip to content

Commit

Permalink
Merge two functions that calculate the effective string field type an…
Browse files Browse the repository at this point in the history
…d migrate all callers to the new one.

The one removed was not taking into account edition features.

PiperOrigin-RevId: 619577375
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Apr 18, 2024
1 parent b4bd664 commit 7c76c0c
Show file tree
Hide file tree
Showing 17 changed files with 363 additions and 243 deletions.
37 changes: 3 additions & 34 deletions src/google/protobuf/compiler/cpp/field.cc
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
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
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
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
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
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
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
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
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

0 comments on commit 7c76c0c

Please sign in to comment.