Skip to content

Commit

Permalink
Change namespace scope constants from internal linkage to inline li…
Browse files Browse the repository at this point in the history
…nkage to avoid potential ODR violations and code bloat.

Also, remove the now unnecessary redundant definition of class-scope `static
constexpr` variables.

PiperOrigin-RevId: 691426487
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Oct 30, 2024
1 parent 5fe1196 commit 0c0cdb3
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 389 deletions.
47 changes: 5 additions & 42 deletions src/google/protobuf/compiler/cpp/enum.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,10 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
$dllexport_decl $bool $Msg_Enum$_IsValid(int value);
$dllexport_decl $extern const uint32_t $Msg_Enum$_internal_data_[];
constexpr $Msg_Enum$ $Msg_Enum_Enum_MIN$ = static_cast<$Msg_Enum$>($kMin$);
constexpr $Msg_Enum$ $Msg_Enum_Enum_MAX$ = static_cast<$Msg_Enum$>($kMax$);
inline constexpr $Msg_Enum$ $Msg_Enum_Enum_MIN$ =
static_cast<$Msg_Enum$>($kMin$);
inline constexpr $Msg_Enum$ $Msg_Enum_Enum_MAX$ =
static_cast<$Msg_Enum$>($kMax$);
)cc");

if (generate_array_size_) {
Expand All @@ -180,7 +182,7 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
"_ARRAYSIZE"))
.AnnotatedAs(enum_)},
R"cc(
constexpr int $Msg_Enum_Enum_ARRAYSIZE$ = $kMax$ + 1;
inline constexpr int $Msg_Enum_Enum_ARRAYSIZE$ = $kMax$ + 1;
)cc");
}

Expand Down Expand Up @@ -548,45 +550,6 @@ void EnumGenerator::GenerateMethods(int idx, io::Printer* p) {
}
)cc");
}

if (enum_->containing_type() != nullptr) {
// Before C++17, we must define the static constants which were
// declared in the header, to give the linker a place to put them.
// But MSVC++ pre-2015 and post-2017 (version 15.5+) insists that we not.
p->Emit(
{
{"Msg_", ClassName(enum_->containing_type(), false)},
{"constexpr_storage",
[&] {
for (int i = 0; i < enum_->value_count(); i++) {
p->Emit({{"VALUE", EnumValueName(enum_->value(i))}},
R"cc(
constexpr $Msg_Enum$ $Msg_$::$VALUE$;
)cc");
}
}},
{"array_size",
[&] {
if (generate_array_size_) {
p->Emit(R"cc(
constexpr int $Msg_$::$Enum$_ARRAYSIZE;
)cc");
}
}},
},
R"(
#if (__cplusplus < 201703) && \
(!defined(_MSC_VER) || (_MSC_VER >= 1900 && _MSC_VER < 1912))
$constexpr_storage$;
constexpr $Msg_Enum$ $Msg_$::$Enum$_MIN;
constexpr $Msg_Enum$ $Msg_$::$Enum$_MAX;
$array_size$;
#endif // (__cplusplus < 201703) &&
// (!defined(_MSC_VER) || (_MSC_VER >= 1900 && _MSC_VER < 1912))
)");
}
}
} // namespace cpp
} // namespace compiler
Expand Down
41 changes: 14 additions & 27 deletions src/google/protobuf/compiler/cpp/extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,13 @@ bool ExtensionGenerator::IsScoped() const {
void ExtensionGenerator::GenerateDeclaration(io::Printer* p) const {
auto var = p->WithVars(variables_);
auto annotate = p->WithAnnotations({{"name", descriptor_}});

p->Emit({{"qualifier",
p->Emit({{"constant_qualifier",
// If this is a class member, it needs to be declared
// `static constexpr`.
// Otherwise, it will be
// `inline constexpr`.
IsScoped() ? "static" : ""},
{"id_qualifier",
// If this is a class member, it needs to be declared "static".
// Otherwise, it needs to be "extern". In the latter case, it
// also needs the DLL export/import specifier.
Expand All @@ -99,8 +104,9 @@ void ExtensionGenerator::GenerateDeclaration(io::Printer* p) const {
? "extern"
: absl::StrCat(options_.dllexport_decl, " extern")}},
R"cc(
static const int $constant_name$ = $number$;
$qualifier$ ::$proto_ns$::internal::ExtensionIdentifier<
inline $constant_qualifier $constexpr int $constant_name$ =
$number$;
$id_qualifier$ ::$proto_ns$::internal::ExtensionIdentifier<
$extendee$, ::$proto_ns$::internal::$type_traits$, $field_type$,
$packed$>
$name$;
Expand Down Expand Up @@ -147,32 +153,13 @@ void ExtensionGenerator::GenerateDefinition(io::Printer* p) {
const std::string $default_str$($default_val$);
)cc");
}},
{"declare_const_var",
[&] {
if (!IsScoped()) return;
// Likewise, class members need to declare the field constant
// variable.
p->Emit(R"cc(
#if !defined(_MSC_VER) || (_MSC_VER >= 1900 && _MSC_VER < 1912)
const int $scope$$constant_name$;
#endif
)cc");
}},
{"define_extension_id",
[&] {
p->Emit(R"cc(
PROTOBUF_CONSTINIT$ dllexport_decl$
PROTOBUF_ATTRIBUTE_INIT_PRIORITY2 ::_pbi::
ExtensionIdentifier<$extendee$, ::_pbi::$type_traits$,
$field_type$, $packed$>
$scoped_name$($constant_name$, $default_str$);
)cc");
}},
},
R"cc(
$declare_default_str$;
$declare_const_var$;
$define_extension_id$;
PROTOBUF_CONSTINIT$ dllexport_decl$
PROTOBUF_ATTRIBUTE_INIT_PRIORITY2 ::_pbi::ExtensionIdentifier<
$extendee$, ::_pbi::$type_traits$, $field_type$, $packed$>
$scoped_name$($constant_name$, $default_str$);
)cc");
}

Expand Down
19 changes: 3 additions & 16 deletions src/google/protobuf/compiler/java/java_features.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 7 additions & 4 deletions src/google/protobuf/compiler/java/java_features.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 0 additions & 12 deletions src/google/protobuf/compiler/plugin.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions src/google/protobuf/compiler/plugin.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 3 additions & 17 deletions src/google/protobuf/cpp_features.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 7 additions & 4 deletions src/google/protobuf/cpp_features.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 0c0cdb3

Please sign in to comment.