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.

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

Migrate type_name/cpp_type_name to absl::string_view. #19606

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7375,7 +7375,7 @@ void DescriptorBuilder::CheckExtensionDeclarationFieldType(
const FieldDescriptor& field, const FieldDescriptorProto& proto,
absl::string_view type) {
if (had_errors_) return;
std::string actual_type = field.type_name();
std::string actual_type(field.type_name());
std::string expected_type(type);
if (field.message_type() || field.enum_type()) {
// Field message type descriptor can be in a partial state which will cause
Expand Down
24 changes: 16 additions & 8 deletions src/google/protobuf/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,13 @@ namespace internal {
// `typedef` instead of `using` for SWIG
#if defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
typedef absl::string_view DescriptorStringView;
// typedef absl::string_view DescriptorTypeStringView;
#else
typedef const std::string& DescriptorStringView;
// typedef const char* DescriptorTypeStringView;
#endif
// DO NOT SUBMIT: Just testing to see how much stuff breaks.
typedef absl::string_view DescriptorTypeStringView;

class FlatAllocator;

Expand Down Expand Up @@ -870,9 +874,11 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
internal::DescriptorStringView camelcase_name() const;

Type type() const; // Declared type of this field.
const char* type_name() const; // Name of the declared type.
// Name of the declared type.
internal::DescriptorTypeStringView type_name() const;
CppType cpp_type() const; // C++ type of this field.
const char* cpp_type_name() const; // Name of the C++ type.
// Name of the C++ type.
internal::DescriptorTypeStringView cpp_type_name() const;
Label label() const; // optional/required/repeated

#ifndef SWIG
Expand Down Expand Up @@ -1024,10 +1030,10 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
static CppType TypeToCppType(Type type);

// Helper method to get the name of a Type.
static const char* TypeName(Type type);
static internal::DescriptorTypeStringView TypeName(Type type);

// Helper method to get the name of a CppType.
static const char* CppTypeName(CppType cpp_type);
static internal::DescriptorTypeStringView CppTypeName(CppType cpp_type);

// Return true iff [packed = true] is valid for fields of this type.
static inline bool IsTypePackable(Type field_type);
Expand Down Expand Up @@ -2864,27 +2870,29 @@ inline int MethodDescriptor::index() const {
return static_cast<int>(this - service_->methods_);
}

inline const char* FieldDescriptor::type_name() const {
inline internal::DescriptorTypeStringView FieldDescriptor::type_name() const {
return kTypeToName[type()];
}

inline FieldDescriptor::CppType FieldDescriptor::cpp_type() const {
return kTypeToCppTypeMap[type()];
}

inline const char* FieldDescriptor::cpp_type_name() const {
inline internal::DescriptorTypeStringView FieldDescriptor::cpp_type_name()
const {
return kCppTypeToName[kTypeToCppTypeMap[type()]];
}

inline FieldDescriptor::CppType FieldDescriptor::TypeToCppType(Type type) {
return kTypeToCppTypeMap[type];
}

inline const char* FieldDescriptor::TypeName(Type type) {
inline internal::DescriptorTypeStringView FieldDescriptor::TypeName(Type type) {
return kTypeToName[type];
}

inline const char* FieldDescriptor::CppTypeName(CppType cpp_type) {
inline internal::DescriptorTypeStringView FieldDescriptor::CppTypeName(
CppType cpp_type) {
return kCppTypeToName[cpp_type];
}

Expand Down
166 changes: 100 additions & 66 deletions src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2636,7 +2636,7 @@ class MiscTest : public testing::Test {
}
}

const char* GetTypeNameForFieldType(FieldDescriptor::Type type) {
absl::string_view GetTypeNameForFieldType(FieldDescriptor::Type type) {
const FieldDescriptor* field = GetFieldDescriptorOfType(type);
return field != nullptr ? field->type_name() : "";
}
Expand All @@ -2647,7 +2647,7 @@ class MiscTest : public testing::Test {
: static_cast<FieldDescriptor::CppType>(0);
}

const char* GetCppTypeNameForFieldType(FieldDescriptor::Type type) {
absl::string_view GetCppTypeNameForFieldType(FieldDescriptor::Type type) {
const FieldDescriptor* field = GetFieldDescriptorOfType(type);
return field != nullptr ? field->cpp_type_name() : "";
}
Expand All @@ -2672,49 +2672,65 @@ TEST_F(MiscTest, TypeNames) {

typedef FieldDescriptor FD; // avoid ugly line wrapping

EXPECT_STREQ("double", GetTypeNameForFieldType(FD::TYPE_DOUBLE));
EXPECT_STREQ("float", GetTypeNameForFieldType(FD::TYPE_FLOAT));
EXPECT_STREQ("int64", GetTypeNameForFieldType(FD::TYPE_INT64));
EXPECT_STREQ("uint64", GetTypeNameForFieldType(FD::TYPE_UINT64));
EXPECT_STREQ("int32", GetTypeNameForFieldType(FD::TYPE_INT32));
EXPECT_STREQ("fixed64", GetTypeNameForFieldType(FD::TYPE_FIXED64));
EXPECT_STREQ("fixed32", GetTypeNameForFieldType(FD::TYPE_FIXED32));
EXPECT_STREQ("bool", GetTypeNameForFieldType(FD::TYPE_BOOL));
EXPECT_STREQ("string", GetTypeNameForFieldType(FD::TYPE_STRING));
EXPECT_STREQ("group", GetTypeNameForFieldType(FD::TYPE_GROUP));
EXPECT_STREQ("message", GetTypeNameForFieldType(FD::TYPE_MESSAGE));
EXPECT_STREQ("bytes", GetTypeNameForFieldType(FD::TYPE_BYTES));
EXPECT_STREQ("uint32", GetTypeNameForFieldType(FD::TYPE_UINT32));
EXPECT_STREQ("enum", GetTypeNameForFieldType(FD::TYPE_ENUM));
EXPECT_STREQ("sfixed32", GetTypeNameForFieldType(FD::TYPE_SFIXED32));
EXPECT_STREQ("sfixed64", GetTypeNameForFieldType(FD::TYPE_SFIXED64));
EXPECT_STREQ("sint32", GetTypeNameForFieldType(FD::TYPE_SINT32));
EXPECT_STREQ("sint64", GetTypeNameForFieldType(FD::TYPE_SINT64));
EXPECT_EQ(absl::string_view("double"),
GetTypeNameForFieldType(FD::TYPE_DOUBLE));
EXPECT_EQ(absl::string_view("float"),
GetTypeNameForFieldType(FD::TYPE_FLOAT));
EXPECT_EQ(absl::string_view("int64"),
GetTypeNameForFieldType(FD::TYPE_INT64));
EXPECT_EQ(absl::string_view("uint64"),
GetTypeNameForFieldType(FD::TYPE_UINT64));
EXPECT_EQ(absl::string_view("int32"),
GetTypeNameForFieldType(FD::TYPE_INT32));
EXPECT_EQ(absl::string_view("fixed64"),
GetTypeNameForFieldType(FD::TYPE_FIXED64));
EXPECT_EQ(absl::string_view("fixed32"),
GetTypeNameForFieldType(FD::TYPE_FIXED32));
EXPECT_EQ(absl::string_view("bool"), GetTypeNameForFieldType(FD::TYPE_BOOL));
EXPECT_EQ(absl::string_view("string"),
GetTypeNameForFieldType(FD::TYPE_STRING));
EXPECT_EQ(absl::string_view("group"),
GetTypeNameForFieldType(FD::TYPE_GROUP));
EXPECT_EQ(absl::string_view("message"),
GetTypeNameForFieldType(FD::TYPE_MESSAGE));
EXPECT_EQ(absl::string_view("bytes"),
GetTypeNameForFieldType(FD::TYPE_BYTES));
EXPECT_EQ(absl::string_view("uint32"),
GetTypeNameForFieldType(FD::TYPE_UINT32));
EXPECT_EQ(absl::string_view("enum"), GetTypeNameForFieldType(FD::TYPE_ENUM));
EXPECT_EQ(absl::string_view("sfixed32"),
GetTypeNameForFieldType(FD::TYPE_SFIXED32));
EXPECT_EQ(absl::string_view("sfixed64"),
GetTypeNameForFieldType(FD::TYPE_SFIXED64));
EXPECT_EQ(absl::string_view("sint32"),
GetTypeNameForFieldType(FD::TYPE_SINT32));
EXPECT_EQ(absl::string_view("sint64"),
GetTypeNameForFieldType(FD::TYPE_SINT64));
}

TEST_F(MiscTest, StaticTypeNames) {
// Test that correct type names are returned.

typedef FieldDescriptor FD; // avoid ugly line wrapping

EXPECT_STREQ("double", FD::TypeName(FD::TYPE_DOUBLE));
EXPECT_STREQ("float", FD::TypeName(FD::TYPE_FLOAT));
EXPECT_STREQ("int64", FD::TypeName(FD::TYPE_INT64));
EXPECT_STREQ("uint64", FD::TypeName(FD::TYPE_UINT64));
EXPECT_STREQ("int32", FD::TypeName(FD::TYPE_INT32));
EXPECT_STREQ("fixed64", FD::TypeName(FD::TYPE_FIXED64));
EXPECT_STREQ("fixed32", FD::TypeName(FD::TYPE_FIXED32));
EXPECT_STREQ("bool", FD::TypeName(FD::TYPE_BOOL));
EXPECT_STREQ("string", FD::TypeName(FD::TYPE_STRING));
EXPECT_STREQ("group", FD::TypeName(FD::TYPE_GROUP));
EXPECT_STREQ("message", FD::TypeName(FD::TYPE_MESSAGE));
EXPECT_STREQ("bytes", FD::TypeName(FD::TYPE_BYTES));
EXPECT_STREQ("uint32", FD::TypeName(FD::TYPE_UINT32));
EXPECT_STREQ("enum", FD::TypeName(FD::TYPE_ENUM));
EXPECT_STREQ("sfixed32", FD::TypeName(FD::TYPE_SFIXED32));
EXPECT_STREQ("sfixed64", FD::TypeName(FD::TYPE_SFIXED64));
EXPECT_STREQ("sint32", FD::TypeName(FD::TYPE_SINT32));
EXPECT_STREQ("sint64", FD::TypeName(FD::TYPE_SINT64));
EXPECT_EQ(absl::string_view("double"), FD::TypeName(FD::TYPE_DOUBLE));
EXPECT_EQ(absl::string_view("float"), FD::TypeName(FD::TYPE_FLOAT));
EXPECT_EQ(absl::string_view("int64"), FD::TypeName(FD::TYPE_INT64));
EXPECT_EQ(absl::string_view("uint64"), FD::TypeName(FD::TYPE_UINT64));
EXPECT_EQ(absl::string_view("int32"), FD::TypeName(FD::TYPE_INT32));
EXPECT_EQ(absl::string_view("fixed64"), FD::TypeName(FD::TYPE_FIXED64));
EXPECT_EQ(absl::string_view("fixed32"), FD::TypeName(FD::TYPE_FIXED32));
EXPECT_EQ(absl::string_view("bool"), FD::TypeName(FD::TYPE_BOOL));
EXPECT_EQ(absl::string_view("string"), FD::TypeName(FD::TYPE_STRING));
EXPECT_EQ(absl::string_view("group"), FD::TypeName(FD::TYPE_GROUP));
EXPECT_EQ(absl::string_view("message"), FD::TypeName(FD::TYPE_MESSAGE));
EXPECT_EQ(absl::string_view("bytes"), FD::TypeName(FD::TYPE_BYTES));
EXPECT_EQ(absl::string_view("uint32"), FD::TypeName(FD::TYPE_UINT32));
EXPECT_EQ(absl::string_view("enum"), FD::TypeName(FD::TYPE_ENUM));
EXPECT_EQ(absl::string_view("sfixed32"), FD::TypeName(FD::TYPE_SFIXED32));
EXPECT_EQ(absl::string_view("sfixed64"), FD::TypeName(FD::TYPE_SFIXED64));
EXPECT_EQ(absl::string_view("sint32"), FD::TypeName(FD::TYPE_SINT32));
EXPECT_EQ(absl::string_view("sint64"), FD::TypeName(FD::TYPE_SINT64));
}

TEST_F(MiscTest, CppTypes) {
Expand Down Expand Up @@ -2747,41 +2763,59 @@ TEST_F(MiscTest, CppTypeNames) {

typedef FieldDescriptor FD; // avoid ugly line wrapping

EXPECT_STREQ("double", GetCppTypeNameForFieldType(FD::TYPE_DOUBLE));
EXPECT_STREQ("float", GetCppTypeNameForFieldType(FD::TYPE_FLOAT));
EXPECT_STREQ("int64", GetCppTypeNameForFieldType(FD::TYPE_INT64));
EXPECT_STREQ("uint64", GetCppTypeNameForFieldType(FD::TYPE_UINT64));
EXPECT_STREQ("int32", GetCppTypeNameForFieldType(FD::TYPE_INT32));
EXPECT_STREQ("uint64", GetCppTypeNameForFieldType(FD::TYPE_FIXED64));
EXPECT_STREQ("uint32", GetCppTypeNameForFieldType(FD::TYPE_FIXED32));
EXPECT_STREQ("bool", GetCppTypeNameForFieldType(FD::TYPE_BOOL));
EXPECT_STREQ("string", GetCppTypeNameForFieldType(FD::TYPE_STRING));
EXPECT_STREQ("message", GetCppTypeNameForFieldType(FD::TYPE_GROUP));
EXPECT_STREQ("message", GetCppTypeNameForFieldType(FD::TYPE_MESSAGE));
EXPECT_STREQ("string", GetCppTypeNameForFieldType(FD::TYPE_BYTES));
EXPECT_STREQ("uint32", GetCppTypeNameForFieldType(FD::TYPE_UINT32));
EXPECT_STREQ("enum", GetCppTypeNameForFieldType(FD::TYPE_ENUM));
EXPECT_STREQ("int32", GetCppTypeNameForFieldType(FD::TYPE_SFIXED32));
EXPECT_STREQ("int64", GetCppTypeNameForFieldType(FD::TYPE_SFIXED64));
EXPECT_STREQ("int32", GetCppTypeNameForFieldType(FD::TYPE_SINT32));
EXPECT_STREQ("int64", GetCppTypeNameForFieldType(FD::TYPE_SINT64));
EXPECT_EQ(absl::string_view("double"),
GetCppTypeNameForFieldType(FD::TYPE_DOUBLE));
EXPECT_EQ(absl::string_view("float"),
GetCppTypeNameForFieldType(FD::TYPE_FLOAT));
EXPECT_EQ(absl::string_view("int64"),
GetCppTypeNameForFieldType(FD::TYPE_INT64));
EXPECT_EQ(absl::string_view("uint64"),
GetCppTypeNameForFieldType(FD::TYPE_UINT64));
EXPECT_EQ(absl::string_view("int32"),
GetCppTypeNameForFieldType(FD::TYPE_INT32));
EXPECT_EQ(absl::string_view("uint64"),
GetCppTypeNameForFieldType(FD::TYPE_FIXED64));
EXPECT_EQ(absl::string_view("uint32"),
GetCppTypeNameForFieldType(FD::TYPE_FIXED32));
EXPECT_EQ(absl::string_view("bool"),
GetCppTypeNameForFieldType(FD::TYPE_BOOL));
EXPECT_EQ(absl::string_view("string"),
GetCppTypeNameForFieldType(FD::TYPE_STRING));
EXPECT_EQ(absl::string_view("message"),
GetCppTypeNameForFieldType(FD::TYPE_GROUP));
EXPECT_EQ(absl::string_view("message"),
GetCppTypeNameForFieldType(FD::TYPE_MESSAGE));
EXPECT_EQ(absl::string_view("string"),
GetCppTypeNameForFieldType(FD::TYPE_BYTES));
EXPECT_EQ(absl::string_view("uint32"),
GetCppTypeNameForFieldType(FD::TYPE_UINT32));
EXPECT_EQ(absl::string_view("enum"),
GetCppTypeNameForFieldType(FD::TYPE_ENUM));
EXPECT_EQ(absl::string_view("int32"),
GetCppTypeNameForFieldType(FD::TYPE_SFIXED32));
EXPECT_EQ(absl::string_view("int64"),
GetCppTypeNameForFieldType(FD::TYPE_SFIXED64));
EXPECT_EQ(absl::string_view("int32"),
GetCppTypeNameForFieldType(FD::TYPE_SINT32));
EXPECT_EQ(absl::string_view("int64"),
GetCppTypeNameForFieldType(FD::TYPE_SINT64));
}

TEST_F(MiscTest, StaticCppTypeNames) {
// Test that correct CPP type names are returned.

typedef FieldDescriptor FD; // avoid ugly line wrapping

EXPECT_STREQ("int32", FD::CppTypeName(FD::CPPTYPE_INT32));
EXPECT_STREQ("int64", FD::CppTypeName(FD::CPPTYPE_INT64));
EXPECT_STREQ("uint32", FD::CppTypeName(FD::CPPTYPE_UINT32));
EXPECT_STREQ("uint64", FD::CppTypeName(FD::CPPTYPE_UINT64));
EXPECT_STREQ("double", FD::CppTypeName(FD::CPPTYPE_DOUBLE));
EXPECT_STREQ("float", FD::CppTypeName(FD::CPPTYPE_FLOAT));
EXPECT_STREQ("bool", FD::CppTypeName(FD::CPPTYPE_BOOL));
EXPECT_STREQ("enum", FD::CppTypeName(FD::CPPTYPE_ENUM));
EXPECT_STREQ("string", FD::CppTypeName(FD::CPPTYPE_STRING));
EXPECT_STREQ("message", FD::CppTypeName(FD::CPPTYPE_MESSAGE));
EXPECT_EQ(absl::string_view("int32"), FD::CppTypeName(FD::CPPTYPE_INT32));
EXPECT_EQ(absl::string_view("int64"), FD::CppTypeName(FD::CPPTYPE_INT64));
EXPECT_EQ(absl::string_view("uint32"), FD::CppTypeName(FD::CPPTYPE_UINT32));
EXPECT_EQ(absl::string_view("uint64"), FD::CppTypeName(FD::CPPTYPE_UINT64));
EXPECT_EQ(absl::string_view("double"), FD::CppTypeName(FD::CPPTYPE_DOUBLE));
EXPECT_EQ(absl::string_view("float"), FD::CppTypeName(FD::CPPTYPE_FLOAT));
EXPECT_EQ(absl::string_view("bool"), FD::CppTypeName(FD::CPPTYPE_BOOL));
EXPECT_EQ(absl::string_view("enum"), FD::CppTypeName(FD::CPPTYPE_ENUM));
EXPECT_EQ(absl::string_view("string"), FD::CppTypeName(FD::CPPTYPE_STRING));
EXPECT_EQ(absl::string_view("message"), FD::CppTypeName(FD::CPPTYPE_MESSAGE));
}

TEST_F(MiscTest, MessageType) {
Expand Down
6 changes: 3 additions & 3 deletions src/google/protobuf/reflection_tester.cc
Original file line number Diff line number Diff line change
Expand Up @@ -624,19 +624,19 @@ Message* MapReflectionTester::GetMapEntryViaReflection(
}

MapIterator MapReflectionTester::MapBegin(Message* message,
const std::string& field_name) {
absl::string_view field_name) {
const Reflection* reflection = message->GetReflection();
return reflection->MapBegin(message, F(field_name));
}

MapIterator MapReflectionTester::MapEnd(Message* message,
const std::string& field_name) {
absl::string_view field_name) {
const Reflection* reflection = message->GetReflection();
return reflection->MapEnd(message, F(field_name));
}

int MapReflectionTester::MapSize(const Message& message,
const std::string& field_name) {
absl::string_view field_name) {
const Reflection* reflection = message.GetReflection();
return reflection->MapSize(message, F(field_name));
}
Expand Down
6 changes: 3 additions & 3 deletions src/google/protobuf/reflection_tester.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ class MapReflectionTester {
const MapKey& map_key);
Message* GetMapEntryViaReflection(Message* message,
const std::string& field_name, int index);
MapIterator MapBegin(Message* message, const std::string& field_name);
MapIterator MapEnd(Message* message, const std::string& field_name);
int MapSize(const Message& message, const std::string& field_name);
MapIterator MapBegin(Message* message, absl::string_view field_name);
MapIterator MapEnd(Message* message, absl::string_view field_name);
int MapSize(const Message& message, absl::string_view field_name);

static std::optional<MapValueConstRef> LookupMapValue(
const Reflection& reflection, const Message& message,
Expand Down
Loading