From 3dd89a457eb5c54b0cd8ae37ade9fc9337b91acd Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 5 Jun 2024 20:47:00 -0700 Subject: [PATCH] Automated rollback of commit 638ee8f6545eae9a7e65c48c6b60a83c88db605a. PiperOrigin-RevId: 640749918 --- .../upb_proto_library_internal/aspect.bzl | 78 +------ upb_generator/BUILD | 3 - .../protoc-gen-upb_minitable-main.cc | 21 +- upb_generator/protoc-gen-upb_minitable.cc | 211 ++++++++---------- upb_generator/protoc-gen-upb_minitable.h | 10 +- 5 files changed, 105 insertions(+), 218 deletions(-) diff --git a/bazel/private/upb_proto_library_internal/aspect.bzl b/bazel/private/upb_proto_library_internal/aspect.bzl index a247229715..6f7b3178a1 100644 --- a/bazel/private/upb_proto_library_internal/aspect.bzl +++ b/bazel/private/upb_proto_library_internal/aspect.bzl @@ -1,6 +1,5 @@ """Implementation of the aspect that powers the upb_*_proto_library() rules.""" -load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") load("//bazel/common:proto_common.bzl", "proto_common") load(":upb_proto_library_internal/cc_library_func.bzl", "cc_library_func") load(":upb_proto_library_internal/copts.bzl", "UpbProtoLibraryCoptsInfo") @@ -55,51 +54,7 @@ def _merge_generated_srcs(srcs): thunks = _concat_lists([s.thunks for s in srcs]), ) -def _get_implicit_weak_field_sources(ctx, proto_info): - # Creating one .cc file for each Message in a proto allows the linker to be more aggressive - # about removing unused classes. However, since the number of outputs won't be known at Blaze - # analysis time, all of the generated source files are put in a directory and a TreeArtifact is - # used to represent them. - proto_artifacts = [] - for proto_source in proto_info.direct_sources: - # We can have slashes in the target name. For example, proto_source can be: - # dir/a.proto. However proto_source.basename will return a.proto, when in reality - # the BUILD file declares it as dir/a.proto, because target name contains a slash. - # There is no good workaround for this. - # I am using ctx.label.package to check if the name of the target contains slash or not. - # This is similar to what declare_directory does. - if not proto_source.short_path.startswith(ctx.label.package): - fail("This should never happen, proto source {} path does not start with {}.".format( - proto_source.short_path, - ctx.label.package, - )) - proto_source_name = proto_source.short_path[len(ctx.label.package) + 1:] - last_dot = proto_source_name.rfind(".") - if last_dot != -1: - proto_source_name = proto_source_name[:last_dot] - proto_artifacts.append(ctx.actions.declare_directory(proto_source_name + ".upb_weak_minitables")) - - return proto_artifacts - -def _get_feature_configuration(ctx, cc_toolchain, proto_info): - requested_features = list(ctx.features) - - # Disable the whole-archive behavior for protobuf generated code when the - # proto_one_output_per_message feature is enabled. - requested_features.append("disable_whole_archive_for_static_lib_if_proto_one_output_per_message") - unsupported_features = list(ctx.disabled_features) - if len(proto_info.direct_sources) != 0: - requested_features.append("header_modules") - else: - unsupported_features.append("header_modules") - return cc_common.configure_features( - ctx = ctx, - cc_toolchain = cc_toolchain, - requested_features = requested_features, - unsupported_features = unsupported_features, - ) - -def _generate_srcs_list(ctx, generator, proto_info): +def _generate_upb_protos(ctx, generator, proto_info): if len(proto_info.direct_sources) == 0: return GeneratedSrcsInfo(srcs = [], hdrs = [], thunks = [], includes = []) @@ -137,35 +92,19 @@ def _generate_srcs_list(ctx, generator, proto_info): mnemonic = "GenUpbProtosThunks", ) - return GeneratedSrcsInfo( - srcs = srcs, - hdrs = hdrs, - thunks = thunks, - ) - -def _generate_upb_protos(ctx, generator, proto_info, feature_configuration): - implicit_weak = generator == "upb_minitable" and cc_common.is_enabled( - feature_configuration = feature_configuration, - feature_name = "proto_one_output_per_message", - ) - - srcs = _generate_srcs_list(ctx, generator, proto_info) - additional_args = ctx.actions.args() - - if implicit_weak: - srcs.srcs.extend(_get_implicit_weak_field_sources(ctx, proto_info)) - additional_args.add("--upb_minitable_opt=one_output_per_message") - proto_common.compile( actions = ctx.actions, proto_info = proto_info, proto_lang_toolchain_info = _get_lang_toolchain(ctx, generator), - generated_files = srcs.srcs + srcs.hdrs, + generated_files = srcs + hdrs, experimental_exec_group = "proto_compiler", - additional_args = additional_args, ) - return srcs + return GeneratedSrcsInfo( + srcs = srcs, + hdrs = hdrs, + thunks = thunks, + ) def _generate_name(ctx, generator, thunks = False): if thunks: @@ -278,13 +217,10 @@ def upb_proto_aspect_impl( ) else: proto_info = target[ProtoInfo] - cc_toolchain = find_cpp_toolchain(ctx) - feature_configuration = _get_feature_configuration(ctx, cc_toolchain, proto_info) files = _generate_upb_protos( ctx, generator, proto_info, - feature_configuration, ) wrapped_cc_info = _compile_upb_protos( ctx, diff --git a/upb_generator/BUILD b/upb_generator/BUILD index 4bd1ac6d8d..757d840af5 100644 --- a/upb_generator/BUILD +++ b/upb_generator/BUILD @@ -318,7 +318,6 @@ bootstrap_cc_library( "//upb:mini_table", "//upb:port", "//upb:wire_reader", - "//upb/mini_table:internal", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/log:absl_check", @@ -346,7 +345,6 @@ bootstrap_cc_library( ":common", ":plugin", ":protoc-gen-upb_minitable_lib", - "//upb/reflection:reflection", ], copts = UPB_DEFAULT_CPPOPTS, visibility = ["//pkg:__pkg__"], @@ -355,7 +353,6 @@ bootstrap_cc_library( "//upb:port", "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/log:absl_log", - "@com_google_absl//absl/strings", ], ) diff --git a/upb_generator/protoc-gen-upb_minitable-main.cc b/upb_generator/protoc-gen-upb_minitable-main.cc index e58d3edea1..5aa6fc5668 100644 --- a/upb_generator/protoc-gen-upb_minitable-main.cc +++ b/upb_generator/protoc-gen-upb_minitable-main.cc @@ -6,13 +6,11 @@ // https://developers.google.com/open-source/licenses/bsd #include +#include #include "absl/log/absl_log.h" -#include "absl/strings/string_view.h" -#include "absl/strings/substitute.h" #include "upb/base/status.hpp" #include "upb/base/string_view.h" -#include "upb/reflection/def.hpp" #include "upb_generator/common.h" #include "upb_generator/file_layout.h" #include "upb_generator/plugin.h" @@ -33,26 +31,20 @@ absl::string_view ToStringView(upb_StringView str) { } void GenerateFile(const DefPoolPair& pools, upb::FileDefPtr file, - const MiniTableOptions& options, Plugin* plugin) { + Plugin* plugin) { Output h_output; WriteMiniTableHeader(pools, file, h_output); plugin->AddOutputFile(MiniTableHeaderFilename(file), h_output.output()); Output c_output; - WriteMiniTableSource(pools, file, options, c_output); + WriteMiniTableSource(pools, file, c_output); plugin->AddOutputFile(SourceFilename(file), c_output.output()); - - if (options.one_output_per_message) { - WriteMiniTableMultipleSources(pools, file, options, plugin); - } } -bool ParseOptions(MiniTableOptions* options, Plugin* plugin) { +bool ParseOptions(Plugin* plugin) { for (const auto& pair : ParseGeneratorParameter(plugin->parameter())) { if (pair.first == "experimental_strip_nonfunctional_codegen") { continue; - } else if (pair.first == "one_output_per_message") { - options->one_output_per_message = true; } else { plugin->SetError(absl::Substitute("Unknown parameter: $0", pair.first)); return false; @@ -64,9 +56,8 @@ bool ParseOptions(MiniTableOptions* options, Plugin* plugin) { int PluginMain(int argc, char** argv) { DefPoolPair pools; - MiniTableOptions options; Plugin plugin; - if (!ParseOptions(&options, &plugin)) return 0; + if (!ParseOptions(&plugin)) return 0; plugin.GenerateFilesRaw( [&](const UPB_DESC(FileDescriptorProto) * file_proto, bool generate) { upb::Status status; @@ -77,7 +68,7 @@ int PluginMain(int argc, char** argv) { ABSL_LOG(FATAL) << "Couldn't add file " << name << " to DefPool: " << status.error_message(); } - if (generate) GenerateFile(pools, file, options, &plugin); + if (generate) GenerateFile(pools, file, &plugin); }); return 0; } diff --git a/upb_generator/protoc-gen-upb_minitable.cc b/upb_generator/protoc-gen-upb_minitable.cc index d4a2adefbd..9e80e19583 100644 --- a/upb_generator/protoc-gen-upb_minitable.cc +++ b/upb_generator/protoc-gen-upb_minitable.cc @@ -5,8 +5,6 @@ // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd -#include "upb_generator/protoc-gen-upb_minitable.h" - #include #include @@ -26,7 +24,6 @@ #include "upb/base/descriptor_constants.h" #include "upb/mini_table/enum.h" #include "upb/mini_table/field.h" -#include "upb/mini_table/internal/field.h" #include "upb/mini_table/message.h" #include "upb/reflection/def.hpp" #include "upb/wire/types.h" @@ -35,7 +32,6 @@ // Must be last. #include "upb/port/def.inc" -#include "upb_generator/plugin.h" namespace upb { namespace generator { @@ -459,13 +455,90 @@ void WriteEnum(upb::EnumDefPtr e, Output& output) { output("\n"); } -void WriteExtension(const DefPoolPair& pools, upb::FieldDefPtr ext, +int WriteEnums(const DefPoolPair& pools, upb::FileDefPtr file, Output& output) { + std::vector this_file_enums = + SortedEnums(file, kClosedEnums); + + for (const auto e : this_file_enums) { + WriteEnum(e, output); + } + + if (!this_file_enums.empty()) { + output("static const upb_MiniTableEnum *$0[$1] = {\n", kEnumsInit, + this_file_enums.size()); + for (const auto e : this_file_enums) { + output(" &$0,\n", EnumInit(e)); + } + output("};\n"); + output("\n"); + } + + return this_file_enums.size(); +} + +int WriteMessages(const DefPoolPair& pools, upb::FileDefPtr file, + Output& output) { + std::vector file_messages = SortedMessages(file); + + if (file_messages.empty()) return 0; + + for (auto message : file_messages) { + WriteMessage(message, pools, output); + } + + output("static const upb_MiniTable *$0[$1] = {\n", kMessagesInit, + file_messages.size()); + for (auto message : file_messages) { + output(" &$0,\n", MessageInitName(message)); + } + output("};\n"); + output("\n"); + return file_messages.size(); +} + +void WriteExtension(upb::FieldDefPtr ext, const DefPoolPair& pools, Output& output) { - output("const upb_MiniTableExtension $0 = {\n ", ExtensionLayout(ext)); output("$0,\n", FieldInitializer(pools, ext)); output(" &$0,\n", MessageInitName(ext.containing_type())); output(" $0,\n", GetSub(ext, true)); - output("\n};\n"); +} + +int WriteExtensions(const DefPoolPair& pools, upb::FileDefPtr file, + Output& output) { + auto exts = SortedExtensions(file); + + if (exts.empty()) return 0; + + // Order by full name for consistent ordering. + std::map forward_messages; + + for (auto ext : exts) { + forward_messages[ext.containing_type().full_name()] = ext.containing_type(); + if (ext.message_type()) { + forward_messages[ext.message_type().full_name()] = ext.message_type(); + } + } + + for (auto ext : exts) { + output("const upb_MiniTableExtension $0 = {\n ", ExtensionLayout(ext)); + WriteExtension(ext, pools, output); + output("\n};\n"); + } + + output( + "\n" + "UPB_LINKARR_APPEND(upb_AllExts)\n" + "static const upb_MiniTableExtension *$0[$1] = {\n", + kExtensionsInit, exts.size()); + + for (auto ext : exts) { + output(" &$0,\n", ExtensionLayout(ext)); + } + + output( + "};\n" + "\n"); + return exts.size(); } } // namespace @@ -534,7 +607,8 @@ void WriteMiniTableHeader(const DefPoolPair& pools, upb::FileDefPtr file, ToPreproc(file.name())); } -void WriteMiniTableSourceIncludes(upb::FileDefPtr file, Output& output) { +void WriteMiniTableSource(const DefPoolPair& pools, upb::FileDefPtr file, + Output& output) { EmitFileWarning(file.name(), output); output( @@ -552,126 +626,23 @@ void WriteMiniTableSourceIncludes(upb::FileDefPtr file, Output& output) { "// Must be last.\n" "#include \"upb/port/def.inc\"\n" "\n"); -} - -void WriteMiniTableSource(const DefPoolPair& pools, upb::FileDefPtr file, - const MiniTableOptions& options, Output& output) { - WriteMiniTableSourceIncludes(file, output); - - std::vector messages = SortedMessages(file); - std::vector extensions = SortedExtensions(file); - std::vector enums = SortedEnums(file, kClosedEnums); - - if (options.one_output_per_message) { - for (auto message : messages) { - output("extern const upb_MiniTable* $0;\n", MessagePtrName(message)); - } - for (const auto e : enums) { - output("extern const upb_MiniTableEnum $0;\n", EnumInit(e)); - } - for (const auto ext : extensions) { - output("extern const upb_MiniTableExtension $0;\n", ExtensionLayout(ext)); - } - } else { - for (auto message : messages) { - WriteMessage(message, pools, output); - } - for (const auto e : enums) { - WriteEnum(e, output); - } - for (const auto ext : extensions) { - WriteExtension(pools, ext, output); - } - } - - // Messages. - if (!messages.empty()) { - output("static const upb_MiniTable *$0[$1] = {\n", kMessagesInit, - messages.size()); - for (auto message : messages) { - output(" &$0,\n", MessageInitName(message)); - } - output("};\n"); - output("\n"); - } - // Enums. - if (!enums.empty()) { - output("static const upb_MiniTableEnum *$0[$1] = {\n", kEnumsInit, - enums.size()); - for (const auto e : enums) { - output(" &$0,\n", EnumInit(e)); - } - output("};\n"); - output("\n"); - } - - if (!extensions.empty()) { - // Extensions. - output( - "\n" - "UPB_LINKARR_APPEND(upb_AllExts)\n" - "static const upb_MiniTableExtension *$0[$1] = {\n", - kExtensionsInit, extensions.size()); - - for (auto ext : extensions) { - output(" &$0,\n", ExtensionLayout(ext)); - } - - output( - "};\n" - "\n"); - } + int msg_count = WriteMessages(pools, file, output); + int ext_count = WriteExtensions(pools, file, output); + int enum_count = WriteEnums(pools, file, output); output("const upb_MiniTableFile $0 = {\n", FileLayoutName(file)); - output(" $0,\n", messages.empty() ? "NULL" : kMessagesInit); - output(" $0,\n", enums.empty() ? "NULL" : kEnumsInit); - output(" $0,\n", extensions.empty() ? "NULL" : kExtensionsInit); - output(" $0,\n", messages.size()); - output(" $0,\n", enums.size()); - output(" $0,\n", extensions.size()); + output(" $0,\n", msg_count ? kMessagesInit : "NULL"); + output(" $0,\n", enum_count ? kEnumsInit : "NULL"); + output(" $0,\n", ext_count ? kExtensionsInit : "NULL"); + output(" $0,\n", msg_count); + output(" $0,\n", enum_count); + output(" $0,\n", ext_count); output("};\n\n"); output("#include \"upb/port/undef.inc\"\n"); output("\n"); } -std::string MultipleSourceFilename(upb::FileDefPtr file, - absl::string_view full_name) { - return absl::StrCat(StripExtension(file.name()), ".upb_weak_minitables/", - full_name, ".upb.c"); -} - -void WriteMiniTableMultipleSources(const DefPoolPair& pools, - upb::FileDefPtr file, - const MiniTableOptions& options, - Plugin* plugin) { - std::vector messages = SortedMessages(file); - std::vector extensions = SortedExtensions(file); - std::vector enums = SortedEnums(file, kClosedEnums); - - for (auto message : messages) { - Output output; - WriteMiniTableSourceIncludes(file, output); - WriteMessage(message, pools, output); - plugin->AddOutputFile(MultipleSourceFilename(file, message.full_name()), - output.output()); - } - for (const auto e : enums) { - Output output; - WriteMiniTableSourceIncludes(file, output); - WriteEnum(e, output); - plugin->AddOutputFile(MultipleSourceFilename(file, e.full_name()), - output.output()); - } - for (const auto ext : extensions) { - Output output; - WriteMiniTableSourceIncludes(file, output); - WriteExtension(pools, ext, output); - plugin->AddOutputFile(MultipleSourceFilename(file, ext.full_name()), - output.output()); - } -} - } // namespace generator } // namespace upb diff --git a/upb_generator/protoc-gen-upb_minitable.h b/upb_generator/protoc-gen-upb_minitable.h index cbc3e8de19..4bad8d5328 100644 --- a/upb_generator/protoc-gen-upb_minitable.h +++ b/upb_generator/protoc-gen-upb_minitable.h @@ -24,16 +24,8 @@ namespace upb { namespace generator { -struct MiniTableOptions { - bool one_output_per_message = false; -}; - void WriteMiniTableSource(const DefPoolPair& pools, upb::FileDefPtr file, - const MiniTableOptions& options, Output& output); -void WriteMiniTableMultipleSources(const DefPoolPair& pools, - upb::FileDefPtr file, - const MiniTableOptions& options, - Plugin* plugin); + Output& output); void WriteMiniTableHeader(const DefPoolPair& pools, upb::FileDefPtr file, Output& output);