Skip to content

Commit

Permalink
Breaking change: Upgrade return type of type_name() and `cpp_type_n…
Browse files Browse the repository at this point in the history 
…ame()` from `const char*` to `absl::string_view`.

Part of the changes announced in https://protobuf.dev/news/2024-10-02/#descriptor-apis

PiperOrigin-RevId: 705895312
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Dec 13, 2024
1 parent a2ef571 commit a9ad51f
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 88 deletions.
2 changes: 1 addition & 1 deletion src/google/protobuf/descriptor.cc
View file
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
34 changes: 23 additions & 11 deletions src/google/protobuf/descriptor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@ typedef absl::string_view DescriptorStringView;
typedef const std::string& DescriptorStringView;
#endif

#if defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE_TYPENAME)
typedef absl::string_view DescriptorTypenameStringView;
#else
typedef const char* DescriptorTypenameStringView;
#endif

class FlatAllocator;

class PROTOBUF_EXPORT LazyDescriptor {
Expand Down Expand Up @@ -873,11 +879,13 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
// when parsing formats which prefer to use camel-case naming style.
internal::DescriptorStringView camelcase_name() const;

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

#ifndef SWIG
CppStringType cpp_string_type() const; // The C++ string type of this field.
Expand Down Expand Up @@ -1028,10 +1036,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::DescriptorTypenameStringView TypeName(Type type);

// Helper method to get the name of a CppType.
static const char* CppTypeName(CppType cpp_type);
static internal::DescriptorTypenameStringView 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 @@ -2897,27 +2905,31 @@ inline int MethodDescriptor::index() const {
return static_cast<int>(this - service_->methods_);
}

inline const char* FieldDescriptor::type_name() const {
inline internal::DescriptorTypenameStringView 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::DescriptorTypenameStringView 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::DescriptorTypenameStringView FieldDescriptor::TypeName(
Type type) {
return kTypeToName[type];
}

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

Expand Down
166 changes: 100 additions & 66 deletions src/google/protobuf/descriptor_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2637,7 +2637,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 @@ -2648,7 +2648,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 @@ -2673,49 +2673,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 @@ -2748,41 +2764,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
5 changes: 5 additions & 0 deletions src/google/protobuf/port_def.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3),
#define PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE 1
#endif

#ifdef PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE_TYPENAME
#error PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE_TYPENAME was previously defined
#endif
#define PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE_TYPENAME 1

#ifdef PROTOBUF_ALWAYS_INLINE
#error PROTOBUF_ALWAYS_INLINE was previously defined
#endif
Expand Down
2 changes: 2 additions & 0 deletions src/google/protobuf/port_undef.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
#undef PROTOBUF_FUTURE_REMOVE_CREATEMESSAGE
#endif

#undef PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE_TYPENAME

// Restore macros that may have been #undef'd in port_def.inc.

#ifdef PROTOBUF_DID_UNDEF_noreturn
Expand Down
10 changes: 5 additions & 5 deletions src/google/protobuf/reflection_tester.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ void MapReflectionTester::SetMapFieldsViaMapReflection(Message* message) {
}

void MapReflectionTester::GetMapValueViaMapReflection(
Message* message, const std::string& field_name, const MapKey& map_key,
Message* message, absl::string_view field_name, const MapKey& map_key,
MapValueRef* map_val) {
const Reflection* reflection = message->GetReflection();
EXPECT_FALSE(reflection->InsertOrLookupMapValue(message, F(field_name),
Expand All @@ -618,25 +618,25 @@ void MapReflectionTester::DeleteMapValueViaMapReflection(
}

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

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
11 changes: 6 additions & 5 deletions src/google/protobuf/reflection_tester.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <optional>

#include "absl/strings/string_view.h"
#include "google/protobuf/map_field.h"
#include "google/protobuf/message.h"

Expand Down Expand Up @@ -40,16 +41,16 @@ class MapReflectionTester {
void ExpectClearViaReflection(const Message& message);
void ExpectClearViaReflectionIterator(Message* message);
void GetMapValueViaMapReflection(Message* message,
const std::string& field_name,
absl::string_view field_name,
const MapKey& map_key, MapValueRef* map_val);
void DeleteMapValueViaMapReflection(Message* message,
absl::string_view field_name,
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);
absl::string_view field_name, int index);
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

0 comments on commit a9ad51f

Please sign in to comment.