Skip to content

Commit

Permalink
Fix C compiler failure when there are fields names prefixed with acce…
Browse files Browse the repository at this point in the history 
…ssor prefixes such as set_ and clear_.

PiperOrigin-RevId: 488672865
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Nov 15, 2022
1 parent c4f1d1b commit d76e286
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 35 deletions.
1 change: 1 addition & 0 deletions protos_generator/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ cc_library(
"//upbc:common",
"//upbc:file_layout",
"//upbc:keywords",
"//upbc:names",
"@com_google_absl//absl/base:log_severity",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/container:flat_hash_set",
Expand Down
71 changes: 44 additions & 27 deletions protos_generator/gen_accessors.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "protos_generator/output.h"
#include "upbc/file_layout.h"
#include "upbc/keywords.h"
#include "upbc/names.h"

namespace protos_generator {

Expand All @@ -45,14 +46,17 @@ using NameToFieldDescriptorMap =
void WriteFieldAccessorHazzer(const protobuf::Descriptor* desc,
const protobuf::FieldDescriptor* field,
const absl::string_view resolved_field_name,
const absl::string_view resolved_upbc_name,
const FileLayout& layout, Output& output);
void WriteFieldAccessorClear(const protobuf::Descriptor* desc,
const protobuf::FieldDescriptor* field,
const absl::string_view resolved_field_name,
const absl::string_view resolved_upbc_name,
const FileLayout& layout, Output& output);
void WriteMapFieldAccessors(const protobuf::Descriptor* desc,
const protobuf::FieldDescriptor* field,
const absl::string_view resolved_field_name,
const absl::string_view resolved_upbc_name,
Output& output);

void WriteMapAccessorDefinitions(const protobuf::Descriptor* message,
Expand Down Expand Up @@ -101,15 +105,20 @@ void WriteFieldAccessorsInHeader(const protobuf::Descriptor* desc,
OutputIndenter i(output);

auto field_names = CreateFieldNameMap(desc);
auto upbc_field_names = upbc::CreateFieldNameMap(desc);

for (auto field : ::upbc::FieldNumberOrder(desc)) {
std::string resolved_field_name = ResolveFieldName(field, field_names);
absl::string_view upbc_name = field->name();
WriteFieldAccessorHazzer(desc, field, resolved_field_name, layout, output);
WriteFieldAccessorClear(desc, field, resolved_field_name, layout, output);
std::string resolved_upbc_name =
upbc::ResolveFieldName(field, upbc_field_names);
WriteFieldAccessorHazzer(desc, field, resolved_field_name,
resolved_upbc_name, layout, output);
WriteFieldAccessorClear(desc, field, resolved_field_name,
resolved_upbc_name, layout, output);

if (field->is_map()) {
WriteMapFieldAccessors(desc, field, resolved_field_name, output);
WriteMapFieldAccessors(desc, field, resolved_field_name,
resolved_upbc_name, output);
} else if (desc->options().map_entry()) {
// TODO(b/237399867) Implement map entry
} else if (field->is_repeated()) {
Expand All @@ -123,18 +132,18 @@ void WriteFieldAccessorsInHeader(const protobuf::Descriptor* desc,
inline void clear_$1() { $0_clear_$2(msg_); }
)cc",
MessageName(desc), resolved_field_name, upbc_name);
MessageName(desc), resolved_field_name, resolved_upbc_name);

if (field->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) {
output(
R"cc(
$1 $2(size_t index) const;
absl::StatusOr<$0> add_$2();
$0 mutable_$3(size_t index) const;
$0 mutable_$2(size_t index) const;
)cc",
MessagePtrConstType(field, /* const */ false),
MessagePtrConstType(field, /* const */ true), resolved_field_name,
upbc_name);
resolved_upbc_name);
} else {
output(
R"cc(
Expand All @@ -157,19 +166,19 @@ void WriteFieldAccessorsInHeader(const protobuf::Descriptor* desc,
protobuf::FieldDescriptor::CPPTYPE_MESSAGE) {
output(R"cc(
$1 $2() const;
$0 mutable_$3();
$0 mutable_$2();
)cc",
MessagePtrConstType(field, /* const */ false),
MessagePtrConstType(field, /* const */ true),
resolved_field_name, upbc_name);
resolved_field_name, resolved_upbc_name);
} else {
output(
R"cc(
inline $0 $1() const { return $2_$3(msg_); }
inline void set_$1($0 value) { return $2_set_$3(msg_, value); }
)cc",
CppConstType(field), resolved_field_name, MessageName(desc),
upbc_name);
resolved_upbc_name);
}
}
}
Expand All @@ -178,42 +187,43 @@ void WriteFieldAccessorsInHeader(const protobuf::Descriptor* desc,
void WriteFieldAccessorHazzer(const protobuf::Descriptor* desc,
const protobuf::FieldDescriptor* field,
const absl::string_view resolved_field_name,
const absl::string_view resolved_upbc_name,
const FileLayout& layout, Output& output) {
// Generate hazzer (if any).
if (layout.HasHasbit(field) || field->real_containing_oneof()) {
absl::string_view upbc_name = field->name();
// Has presence.
output("inline bool has_$0() const { return $1_has_$2(msg_); }\n",
resolved_field_name, MessageName(desc), upbc_name);
resolved_field_name, MessageName(desc), resolved_upbc_name);
}
}

void WriteFieldAccessorClear(const protobuf::Descriptor* desc,
const protobuf::FieldDescriptor* field,
const absl::string_view resolved_field_name,
const absl::string_view resolved_upbc_name,
const FileLayout& layout, Output& output) {
if (layout.HasHasbit(field) || field->real_containing_oneof()) {
absl::string_view upbc_name = field->name();
output("void clear_$0() { $2_clear_$1(msg_); }\n", resolved_field_name,
upbc_name, MessageName(desc));
resolved_upbc_name, MessageName(desc));
}
}

void WriteMapFieldAccessors(const protobuf::Descriptor* desc,
const protobuf::FieldDescriptor* field,
const absl::string_view resolved_field_name,
const absl::string_view resolved_upbc_name,
Output& output) {
const protobuf::Descriptor* entry = field->message_type();
const protobuf::FieldDescriptor* key = entry->FindFieldByNumber(1);
const protobuf::FieldDescriptor* val = entry->FindFieldByNumber(2);
absl::string_view upbc_name = field->name();
output(
R"cc(
inline size_t $0_size() const { return $1_$3_size(msg_); }
inline void clear_$0() { $1_clear_$3(msg_); }
void delete_$0($2 key);
)cc",
resolved_field_name, MessageName(desc), CppConstType(key), upbc_name);
resolved_field_name, MessageName(desc), CppConstType(key),
resolved_upbc_name);

if (val->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) {
output(
Expand Down Expand Up @@ -242,11 +252,14 @@ void WriteAccessorsInSource(const protobuf::Descriptor* desc,
output("namespace internal {\n");
const char arena_expression[] = "arena_";
auto field_names = CreateFieldNameMap(desc);
auto upbc_field_names = upbc::CreateFieldNameMap(desc);

// Generate const methods.
OutputIndenter i(output);
for (auto field : ::upbc::FieldNumberOrder(desc)) {
std::string resolved_field_name = ResolveFieldName(field, field_names);
std::string resolved_upbc_name =
upbc::ResolveFieldName(field, upbc_field_names);
if (field->is_map()) {
WriteMapAccessorDefinitions(desc, field, resolved_field_name, class_name,
output);
Expand All @@ -265,7 +278,6 @@ void WriteAccessorsInSource(const protobuf::Descriptor* desc,
class_name, output);
}
} else {
absl::string_view upbc_name = field->name();
// non-repeated field.
if (field->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING) {
output(
Expand All @@ -275,16 +287,16 @@ void WriteAccessorsInSource(const protobuf::Descriptor* desc,
}
)cc",
class_name, CppConstType(field), resolved_field_name,
MessageName(desc), upbc_name);
MessageName(desc), resolved_upbc_name);
// Set string.
output(
R"cc(
void $0::set_$2($1 value) {
$4_set_$3(msg_, ::protos::UpbStrFromStringView(value, $5));
}
)cc",
class_name, CppConstType(field), resolved_field_name, upbc_name,
MessageName(desc), arena_expression);
class_name, CppConstType(field), resolved_field_name,
resolved_upbc_name, MessageName(desc), arena_expression);
} else if (field->cpp_type() ==
protobuf::FieldDescriptor::CPPTYPE_MESSAGE) {
output(
Expand All @@ -298,7 +310,8 @@ void WriteAccessorsInSource(const protobuf::Descriptor* desc,
)cc",
class_name, MessagePtrConstType(field, /* is_const */ true),
resolved_field_name, MessageName(desc),
MessageBaseType(field, /* maybe_const */ false), upbc_name);
MessageBaseType(field, /* maybe_const */ false),
resolved_upbc_name);

output(
R"cc(
Expand All @@ -309,7 +322,7 @@ void WriteAccessorsInSource(const protobuf::Descriptor* desc,
)cc",
class_name, MessagePtrConstType(field, /* is_const */ false),
resolved_field_name, MessageName(desc),
MessageBaseType(field, /* maybe_const */ false), upbc_name,
MessageBaseType(field, /* maybe_const */ false), resolved_upbc_name,
arena_expression);
}
}
Expand Down Expand Up @@ -683,15 +696,15 @@ std::string ResolveFieldName(const protobuf::FieldDescriptor* field,
"arena_",
});

static constexpr absl::string_view kClearAccessor = "clear_";
static constexpr absl::string_view kSetAccessor = "set_";

// List of generated accessor prefixes to check against.
// Example:
// optional repeated string phase = 236;
// optional bool clear_phase = 237;
static constexpr absl::string_view kAccessorPrefixes[] = {
"clear_",
"delete_",
"add_",
"resize_",
kClearAccessor, "delete_", "add_", "resize_", kSetAccessor,
};

absl::string_view field_name = field->name();
Expand All @@ -710,7 +723,11 @@ std::string ResolveFieldName(const protobuf::FieldDescriptor* field,
auto match = field_names.find(field_name.substr(prefix.size()));
if (match != field_names.end()) {
const auto* candidate = match->second;
if (candidate->is_repeated() || candidate->is_map()) {
if (candidate->is_repeated() || candidate->is_map() ||
(candidate->cpp_type() ==
protobuf::FieldDescriptor::CPPTYPE_STRING &&
prefix == kClearAccessor) ||
prefix == kSetAccessor) {
return absl::StrCat(field_name, "_");
}
}
Expand Down
8 changes: 5 additions & 3 deletions protos_generator/tests/test_model.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,11 @@ message TestModel {
// Tests publicly imported enum.
optional TestEnum imported_enum = 238;

// TODO(243705098) accessor name collision with field name
// optional repeated string phase = 236;
// optional bool clear_phase = 237;
optional string phase = 239;
optional bool clear_phase = 240;

optional string doc_id = 241;
optional bool set_doc_id = 242;

extensions 10000 to max;
}
Expand Down
14 changes: 9 additions & 5 deletions upbc/names.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ namespace upbc {

namespace protobuf = ::google::protobuf;

static constexpr absl::string_view kClearAccessor = "clear_";
static constexpr absl::string_view kSetAccessor = "set_";

// List of generated accessor prefixes to check against.
// Example:
// optional repeated string phase = 236;
// optional bool clear_phase = 237;
static constexpr absl::string_view kAccessorPrefixes[] = {
"clear_",
"delete_",
"add_",
"resize_",
kClearAccessor, "delete_", "add_", "resize_", kSetAccessor,
};

std::string ResolveFieldName(const protobuf::FieldDescriptor* field,
Expand All @@ -58,7 +58,11 @@ std::string ResolveFieldName(const protobuf::FieldDescriptor* field,
auto match = field_names.find(field_name.substr(prefix.size()));
if (match != field_names.end()) {
const auto* candidate = match->second;
if (candidate->is_repeated() || candidate->is_map()) {
if (candidate->is_repeated() || candidate->is_map() ||
(candidate->cpp_type() ==
protobuf::FieldDescriptor::CPPTYPE_STRING &&
prefix == kClearAccessor) ||
prefix == kSetAccessor) {
return absl::StrCat(field_name, "_");
}
}
Expand Down

0 comments on commit d76e286

Please sign in to comment.