Skip to content

Commit

Permalink
Add inner verification code to detect accessors missing required anno…
Browse files Browse the repository at this point in the history 
…tations.

This change has no behavior change, other than CHECK-fail `protoc` when a bug is detected instead of generating the buggy code.

PiperOrigin-RevId: 595138538
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Jan 3, 2024
1 parent 807f00b commit 0e7c351
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 132 deletions.
2 changes: 2 additions & 0 deletions src/google/protobuf/compiler/cpp/field.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ std::vector<Sub> FieldVars(const FieldDescriptor* field, const Options& opts) {
// This will eventually be renamed to "field", once the existing "field"
// variable is replaced with "field_" everywhere.
{"name", FieldName(field)},
// Same as above, but represents internal use.
{"name_internal", FieldName(field)},

{"index", field->index()},
{"number", field->number()},
Expand Down
24 changes: 13 additions & 11 deletions src/google/protobuf/compiler/cpp/field_generators/cord_field.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void CordFieldGenerator::GenerateInlineAccessorDefinitions(
io::Printer* printer) const {
auto v = printer->WithVars(variables_);
printer->Emit(R"cc(
inline const ::absl::Cord& $classname$::_internal_$name$() const {
inline const ::absl::Cord& $classname$::_internal_$name_internal$() const {
return $field$;
}
)cc");
Expand All @@ -183,18 +183,20 @@ void CordFieldGenerator::GenerateInlineAccessorDefinitions(
ABSL_ATTRIBUTE_LIFETIME_BOUND {
$annotate_get$;
// @@protoc_insertion_point(field_get:$full_name$)
return _internal_$name$();
return _internal_$name_internal$();
}
)cc");
printer->Emit(R"cc(
inline void $classname$::_internal_set_$name$(const ::absl::Cord& value) {
inline void $classname$::_internal_set_$name_internal$(
const ::absl::Cord& value) {
$set_hasbit$;
$field$ = value;
}
)cc");
printer->Emit(R"cc(
inline void $classname$::set_$name$(const ::absl::Cord& value) {
$PrepareSplitMessageForWrite$ _internal_set_$name$(value);
$PrepareSplitMessageForWrite$;
_internal_set_$name_internal$(value);
$annotate_set$;
// @@protoc_insertion_point(field_set:$full_name$)
}
Expand All @@ -209,7 +211,7 @@ void CordFieldGenerator::GenerateInlineAccessorDefinitions(
}
)cc");
printer->Emit(R"cc(
inline ::absl::Cord* $classname$::_internal_mutable_$name$() {
inline ::absl::Cord* $classname$::_internal_mutable_$name_internal$() {
$set_hasbit$;
return &$field$;
}
Expand Down Expand Up @@ -333,7 +335,7 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
io::Printer* printer) const {
auto v = printer->WithVars(variables_);
printer->Emit(R"cc(
inline const ::absl::Cord& $classname$::_internal_$name$() const {
inline const ::absl::Cord& $classname$::_internal_$name_internal$() const {
if ($has_field$) {
return *$field$;
}
Expand All @@ -345,14 +347,14 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
ABSL_ATTRIBUTE_LIFETIME_BOUND {
$annotate_get$;
// @@protoc_insertion_point(field_get:$full_name$)
return _internal_$name$();
return _internal_$name_internal$();
}
)cc");
printer->Emit(R"cc(
inline void $classname$::set_$name$(const ::absl::Cord& value) {
if ($not_has_field$) {
clear_$oneof_name$();
set_has_$name$();
set_has_$name_internal$();
$field$ = new ::absl::Cord;
::$proto_ns$::Arena* arena = GetArena();
if (arena != nullptr) {
Expand All @@ -368,7 +370,7 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
inline void $classname$::set_$name$(::absl::string_view value) {
if ($not_has_field$) {
clear_$oneof_name$();
set_has_$name$();
set_has_$name_internal$();
$field$ = new ::absl::Cord;
::$proto_ns$::Arena* arena = GetArena();
if (arena != nullptr) {
Expand All @@ -381,10 +383,10 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
}
)cc");
printer->Emit(R"cc(
inline ::absl::Cord* $classname$::_internal_mutable_$name$() {
inline ::absl::Cord* $classname$::_internal_mutable_$name_internal$() {
if ($not_has_field$) {
clear_$oneof_name$();
set_has_$name$();
set_has_$name_internal$();
$field$ = new ::absl::Cord;
::$proto_ns$::Arena* arena = GetArena();
if (arena != nullptr) {
Expand Down
32 changes: 17 additions & 15 deletions src/google/protobuf/compiler/cpp/field_generators/enum_field.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ void SingularEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
inline $Enum$ $Msg$::$name$() const {
$annotate_get$;
// @@protoc_insertion_point(field_get:$pkg.Msg.field$)
return _internal_$name$();
return _internal_$name_internal$();
}
)cc");

Expand All @@ -178,13 +178,13 @@ void SingularEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
$assert_valid$;
if ($not_has_field$) {
clear_$oneof_name$();
set_has_$name$();
set_has_$name_internal$();
}
$field_$ = value;
$annotate_set$;
// @@protoc_insertion_point(field_set:$pkg.Msg.field$)
}
inline $Enum$ $Msg$::_internal_$name$() const {
inline $Enum$ $Msg$::_internal_$name_internal$() const {
if ($has_field$) {
return static_cast<$Enum$>($field_$);
}
Expand All @@ -195,16 +195,16 @@ void SingularEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
p->Emit(R"cc(
inline void $Msg$::set_$name$($Enum$ value) {
$PrepareSplitMessageForWrite$;
_internal_set_$name$(value);
_internal_set_$name_internal$(value);
$set_hasbit$;
$annotate_set$;
// @@protoc_insertion_point(field_set:$pkg.Msg.field$)
}
inline $Enum$ $Msg$::_internal_$name$() const {
inline $Enum$ $Msg$::_internal_$name_internal$() const {
$TsanDetectConcurrentRead$;
return static_cast<$Enum$>($field_$);
}
inline void $Msg$::_internal_set_$name$($Enum$ value) {
inline void $Msg$::_internal_set_$name_internal$($Enum$ value) {
$TsanDetectConcurrentMutation$;
$assert_valid$;
$field_$ = value;
Expand Down Expand Up @@ -400,13 +400,13 @@ void RepeatedEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
inline $Enum$ $Msg$::$name$(int index) const {
$annotate_get$;
// @@protoc_insertion_point(field_get:$pkg.Msg.field$)
return static_cast<$Enum$>(_internal_$name$().Get(index));
return static_cast<$Enum$>(_internal_$name_internal$().Get(index));
}
)cc");
p->Emit(R"cc(
inline void $Msg$::set_$name$(int index, $Enum$ value) {
$assert_valid$;
_internal_mutable_$name$()->Set(index, value);
_internal_mutable_$name_internal$()->Set(index, value);
$annotate_set$
// @@protoc_insertion_point(field_set:$pkg.Msg.field$)
}
Expand All @@ -415,7 +415,7 @@ void RepeatedEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
inline void $Msg$::add_$name$($Enum$ value) {
$assert_valid$;
$TsanDetectConcurrentMutation$;
_internal_mutable_$name$()->Add(value);
_internal_mutable_$name_internal$()->Add(value);
$annotate_add$
// @@protoc_insertion_point(field_add:$pkg.Msg.field$)
}
Expand All @@ -425,7 +425,7 @@ void RepeatedEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
ABSL_ATTRIBUTE_LIFETIME_BOUND {
$annotate_list$;
// @@protoc_insertion_point(field_list:$pkg.Msg.field$)
return _internal_$name$();
return _internal_$name_internal$();
}
)cc");
p->Emit(R"cc(
Expand All @@ -434,16 +434,17 @@ void RepeatedEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
$annotate_mutable_list$;
// @@protoc_insertion_point(field_mutable_list:$pkg.Msg.field$)
$TsanDetectConcurrentMutation$;
return _internal_mutable_$name$();
return _internal_mutable_$name_internal$();
}
)cc");
if (should_split()) {
p->Emit(R"cc(
inline const $pb$::RepeatedField<int>& $Msg$::_internal_$name$() const {
inline const $pb$::RepeatedField<int>& $Msg$::_internal_$name_internal$()
const {
$TsanDetectConcurrentRead$;
return *$field_$;
}
inline $pb$::RepeatedField<int>* $Msg$::_internal_mutable_$name$() {
inline $pb$::RepeatedField<int>* $Msg$::_internal_mutable_$name_internal$() {
$TsanDetectConcurrentRead$;
$PrepareSplitMessageForWrite$;
if ($field_$.IsDefault()) {
Expand All @@ -455,11 +456,12 @@ void RepeatedEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
)cc");
} else {
p->Emit(R"cc(
inline const $pb$::RepeatedField<int>& $Msg$::_internal_$name$() const {
inline const $pb$::RepeatedField<int>& $Msg$::_internal_$name_internal$()
const {
$TsanDetectConcurrentRead$;
return $field_$;
}
inline $pb$::RepeatedField<int>* $Msg$::_internal_mutable_$name$() {
inline $pb$::RepeatedField<int>* $Msg$::_internal_mutable_$name_internal$() {
$TsanDetectConcurrentRead$;
return &$field_$;
}
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/compiler/cpp/field_generators/map_field.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ void Map::GenerateAccessorDeclarations(io::Printer* p) const {

void Map::GenerateInlineAccessorDefinitions(io::Printer* p) const {
p->Emit(R"cc(
inline const $Map$& $Msg$::_internal_$name$() const {
inline const $Map$& $Msg$::_internal_$name_internal$() const {
$TsanDetectConcurrentRead$;
return $field_$.GetMap();
}
Expand All @@ -224,11 +224,11 @@ void Map::GenerateInlineAccessorDefinitions(io::Printer* p) const {
inline const $Map$& $Msg$::$name$() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
$annotate_get$;
// @@protoc_insertion_point(field_map:$pkg.Msg.field$)
return _internal_$name$();
return _internal_$name_internal$();
}
)cc");
p->Emit(R"cc(
inline $Map$* $Msg$::_internal_mutable_$name$() {
inline $Map$* $Msg$::_internal_mutable_$name_internal$() {
$PrepareSplitMessageForWrite$;
$TsanDetectConcurrentMutation$;
return $field_$.MutableMap();
Expand All @@ -238,7 +238,7 @@ void Map::GenerateInlineAccessorDefinitions(io::Printer* p) const {
inline $Map$* $Msg$::mutable_$name$() ABSL_ATTRIBUTE_LIFETIME_BOUND {
$annotate_mutable$;
// @@protoc_insertion_point(field_mutable_map:$pkg.Msg.field$)
return _internal_mutable_$name$();
return _internal_mutable_$name_internal$();
}
)cc");
}
Expand Down
Loading

0 comments on commit 0e7c351

Please sign in to comment.