Explicitly set -Ccodegen-units=1 for actions that emit an object file. (#3381)

Also, remove any other `-Ccodegen-units` that may have been set in
various ways.

The build rules expect to see a single object file, not the multiple
object files that are produced when `-Ccodegen-units` (with a setting
greater than 1) is used.

The `_add_codegen_units_flags()` function already took this into account
(`is_codegen_units_enabled()` returns false when emitting an object
file), but there are various other ways that `-Ccodegen-units` might get
introduced into the command line -- see the included test for details.

Co-authored-by: scentini <rosica@google.com>

Author: martinboehme

rust/private/rustc.bzl

@@ -800,6 +800,12 @@ def collect_inputs(
     compile_inputs = depset(build_env_files + lint_files, transitive = [build_script_compile_inputs, compile_inputs])
     return compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs, ambiguous_libs
 
+def _will_emit_object_file(emit):
+    return any([e == "obj" or e.startswith("obj=") for e in emit])
+
+def _remove_codegen_units(flag):
+    return None if flag.startswith("-Ccodegen-units") else flag
+
 def construct_arguments(
         *,
         ctx,
@@ -918,6 +924,12 @@ def construct_arguments(
     rustc_path.add("--")
     rustc_path.add(tool_path)
 
+    # If we're emitting an object file, remove any `-Ccodegen-units=` flags.
+    # The build rules expect to see a single object file, not the multiple
+    # object files that are produced when `-Ccodegen-units` (with a setting
+    # greater than 1) is used.
+    map_flag = _remove_codegen_units if _will_emit_object_file(emit) else None
+
     # Rustc arguments
     rustc_flags = ctx.actions.args()
     rustc_flags.set_param_file_format("multiline")
@@ -1002,15 +1014,15 @@ def construct_arguments(
 
     # Tell Rustc where to find the standard library (or libcore)
     rustc_flags.add_all(toolchain.rust_std_paths, before_each = "-L", format_each = "%s")
-    rustc_flags.add_all(rust_flags)
+    rustc_flags.add_all(rust_flags, map_each = map_flag)
 
     # Gather data path from crate_info since it is inherited from real crate for rust_doc and rust_test
     # Deduplicate data paths due to https://github.com/bazelbuild/bazel/issues/14681
     data_paths = depset(direct = getattr(attr, "data", []), transitive = [crate_info.compile_data_targets]).to_list()
 
     add_edition_flags(rustc_flags, crate_info)
     _add_lto_flags(ctx, toolchain, rustc_flags, crate_info)
-    _add_codegen_units_flags(toolchain, rustc_flags)
+    _add_codegen_units_flags(toolchain, emit, rustc_flags)
 
     # Link!
     if ("link" in emit and crate_info.type not in ["rlib", "lib"]) or add_flags_for_binary:
@@ -1083,29 +1095,29 @@ def construct_arguments(
         env["RULES_RUST_THIRD_PARTY_DIR"] = toolchain._third_party_dir
 
     if crate_info.type in toolchain.extra_rustc_flags_for_crate_types.keys():
-        rustc_flags.add_all(toolchain.extra_rustc_flags_for_crate_types[crate_info.type])
+        rustc_flags.add_all(toolchain.extra_rustc_flags_for_crate_types[crate_info.type], map_each = map_flag)
 
     if is_exec_configuration(ctx):
-        rustc_flags.add_all(toolchain.extra_exec_rustc_flags)
+        rustc_flags.add_all(toolchain.extra_exec_rustc_flags, map_each = map_flag)
     else:
-        rustc_flags.add_all(toolchain.extra_rustc_flags)
+        rustc_flags.add_all(toolchain.extra_rustc_flags, map_each = map_flag)
 
     # extra_rustc_flags apply to the target configuration, not the exec configuration.
     if hasattr(ctx.attr, "_extra_rustc_flags") and not is_exec_configuration(ctx):
-        rustc_flags.add_all(ctx.attr._extra_rustc_flags[ExtraRustcFlagsInfo].extra_rustc_flags)
+        rustc_flags.add_all(ctx.attr._extra_rustc_flags[ExtraRustcFlagsInfo].extra_rustc_flags, map_each = map_flag)
 
     if hasattr(ctx.attr, "_extra_rustc_flag") and not is_exec_configuration(ctx):
-        rustc_flags.add_all(ctx.attr._extra_rustc_flag[ExtraRustcFlagsInfo].extra_rustc_flags)
+        rustc_flags.add_all(ctx.attr._extra_rustc_flag[ExtraRustcFlagsInfo].extra_rustc_flags, map_each = map_flag)
 
     if hasattr(ctx.attr, "_per_crate_rustc_flag") and not is_exec_configuration(ctx):
         per_crate_rustc_flags = ctx.attr._per_crate_rustc_flag[PerCrateRustcFlagsInfo].per_crate_rustc_flags
-        _add_per_crate_rustc_flags(ctx, rustc_flags, crate_info, per_crate_rustc_flags)
+        _add_per_crate_rustc_flags(ctx, rustc_flags, map_flag, crate_info, per_crate_rustc_flags)
 
     if hasattr(ctx.attr, "_extra_exec_rustc_flags") and is_exec_configuration(ctx):
-        rustc_flags.add_all(ctx.attr._extra_exec_rustc_flags[ExtraExecRustcFlagsInfo].extra_exec_rustc_flags)
+        rustc_flags.add_all(ctx.attr._extra_exec_rustc_flags[ExtraExecRustcFlagsInfo].extra_exec_rustc_flags, map_each = map_flag)
 
     if hasattr(ctx.attr, "_extra_exec_rustc_flag") and is_exec_configuration(ctx):
-        rustc_flags.add_all(ctx.attr._extra_exec_rustc_flag[ExtraExecRustcFlagsInfo].extra_exec_rustc_flags)
+        rustc_flags.add_all(ctx.attr._extra_exec_rustc_flag[ExtraExecRustcFlagsInfo].extra_exec_rustc_flags, map_each = map_flag)
 
     if _is_no_std(ctx, toolchain, crate_info):
         rustc_flags.add('--cfg=feature="no_std"')
@@ -1118,6 +1130,7 @@ def construct_arguments(
             data_paths,
             {},
         ),
+        map_each = map_flag,
     )
 
     # Needed for bzlmod-aware runfiles resolution.
@@ -1632,15 +1645,23 @@ def _add_lto_flags(ctx, toolchain, args, crate):
     lto_args = construct_lto_arguments(ctx, toolchain, crate)
     args.add_all(lto_args)
 
-def _add_codegen_units_flags(toolchain, args):
+def _add_codegen_units_flags(toolchain, emit, args):
     """Adds flags to an Args object to configure codgen_units for 'rustc'.
 
     https://doc.rust-lang.org/rustc/codegen-options/index.html#codegen-units
 
     Args:
         toolchain (rust_toolchain): The current target's `rust_toolchain`.
+        emit (list): Values for the --emit flag to rustc.
         args (Args): A reference to an Args object
     """
+
+    # If we're emitting an object file, the build rules expect to see a single
+    # object file.
+    if _will_emit_object_file(emit):
+        args.add("-Ccodegen-units=1")
+        return
+
     if not is_codegen_units_enabled(toolchain):
         return
 
@@ -2171,12 +2192,13 @@ def _get_dirname(file):
     """
     return file.dirname
 
-def _add_per_crate_rustc_flags(ctx, args, crate_info, per_crate_rustc_flags):
-    """Adds matching per-crate rustc flags to an arguments object reference
+def _add_per_crate_rustc_flags(ctx, args, map_flag, crate_info, per_crate_rustc_flags):
+    """Adds matching per-crate rustc flags to `args`.
 
     Args:
         ctx (ctx): The source rule's context object
         args (Args): A reference to an Args object
+        map_flag (function): An optional function to use to map added flags
         crate_info (CrateInfo): A CrateInfo provider
         per_crate_rustc_flags (list): A list of per_crate_rustc_flag values
     """
@@ -2200,7 +2222,10 @@ def _add_per_crate_rustc_flags(ctx, args, crate_info, per_crate_rustc_flags):
         execution_path = crate_info.root.path
 
         if label.startswith(prefix_filter) or execution_path.startswith(prefix_filter):
-            args.add(flag)
+            if map_flag:
+                flag = map_flag(flag)
+            if flag:
+                args.add(flag)
 
 def _error_format_impl(ctx):
     """Implementation of the `error_format` rule

test/integration/cc_common_link/unit/cc_common_link_test.bzl

@@ -1,13 +1,15 @@
 """Unittests for the --experimental_use_cc_common_link build setting."""
 
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("@bazel_skylib//rules:write_file.bzl", "write_file")
 load("@rules_cc//cc:defs.bzl", "cc_library")
 load(
     "@rules_rust//rust:defs.bzl",
     "rust_binary",
     "rust_shared_library",
     "rust_test",
 )
+load("@rules_rust//rust:toolchain.bzl", "rust_stdlib_filegroup", "rust_toolchain")
 
 DepActionsInfo = provider(
     "Contains information about dependencies actions.",
@@ -46,6 +48,31 @@ use_cc_common_link_on_target = rule(
     },
 )
 
+def _with_collect_dep_actions_impl(ctx):
+    # return [ctx.attr.target[DepActionsInfo], ctx.attr.target[OutputGroupInfo]]
+    return [ctx.attr.target[DepActionsInfo]]
+
+with_collect_dep_actions = rule(
+    implementation = _with_collect_dep_actions_impl,
+    attrs = {
+        "target": attr.label(
+            aspects = [collect_dep_actions_aspect],
+        ),
+    },
+)
+
+def _with_exec_cfg_impl(ctx):
+    return [ctx.attr.target[DepActionsInfo]]
+
+with_exec_cfg = rule(
+    implementation = _with_exec_cfg_impl,
+    attrs = {
+        "target": attr.label(
+            cfg = "exec",
+        ),
+    },
+)
+
 def _outputs_object_file(action):
     object_files = [output for output in action.outputs.to_list() if output.extension in ("o", "obj")]
     return len(object_files) > 0
@@ -192,24 +219,185 @@ def _cc_common_link_test_targets():
         target_under_test = ":bin_with_cc_common_link",
     )
 
+    return [
+        "use_cc_common_link_on_binary",
+        "use_cc_common_link_on_binary_with_pdb",
+        "use_cc_common_link_on_test",
+        "use_cc_common_link_on_crate_test",
+        "use_cc_common_link_on_cdylib",
+        "custom_malloc_on_binary_test",
+    ]
+
+_RUSTC_FLAGS_CODEGEN_UNITS = 2
+_SETTINGS_CODEGEN_UNITS = 3
+_PER_CRATE_RUSTC_FLAG_CODEGEN_UNITS = 4
+_EXTRA_RUSTC_FLAG_CODEGEN_UNITS = 5
+_EXTRA_RUSTC_FLAGS_CODEGEN_UNITS = 6
+_EXTRA_EXEC_RUSTC_FLAG_CODEGEN_UNITS = 7
+_EXTRA_EXEC_RUSTC_FLAGS_CODEGEN_UNITS = 8
+_TOOLCHAIN_EXTRA_RUSTC_FLAGS_CODEGEN_UNITS = 9
+_TOOLCHAIN_EXTRA_EXEC_RUSTC_FLAGS_CODEGEN_UNITS = 10
+_TOOLCHAIN_EXTRA_RUSTC_FLAGS_FOR_CRATE_TYPES = 11
+
+def _codegen_units_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    registered_actions = tut[DepActionsInfo].actions
+
+    rustc_actions = [action for action in registered_actions if action.mnemonic == "Rustc"]
+    asserts.equals(env, 1, len(rustc_actions))
+    rustc_action = rustc_actions[0]
+
+    actual = sorted([arg for arg in rustc_action.argv if arg.startswith("-Ccodegen-units=")])
+    expected = sorted(["-Ccodegen-units=%s" % codegen_units for codegen_units in ctx.attr.expected_codegen_units])
+
+    asserts.equals(env, expected, actual)
+
+    return analysistest.end(env)
+
+codegen_units_test = analysistest.make(
+    _codegen_units_test_impl,
+    config_settings = {
+        # By default, don't use cc_common.link to link.
+        str(Label("@rules_rust//rust/settings:experimental_use_cc_common_link")): False,
+        # Set `-Ccodegen-units` in various different ways.
+        str(Label("@rules_rust//rust/settings:codegen_units")): _SETTINGS_CODEGEN_UNITS,
+        # Empty prefix (before the `@`) means the flag is applied to all crates.
+        str(Label("@rules_rust//rust/settings:experimental_per_crate_rustc_flag")): ["@-Ccodegen-units=%s" % _PER_CRATE_RUSTC_FLAG_CODEGEN_UNITS],
+        str(Label("@rules_rust//rust/settings:extra_rustc_flag")): ["-Ccodegen-units=%s" % _EXTRA_RUSTC_FLAG_CODEGEN_UNITS],
+        str(Label("@rules_rust//rust/settings:extra_rustc_flags")): ["-Ccodegen-units=%s" % _EXTRA_RUSTC_FLAGS_CODEGEN_UNITS],
+        str(Label("@rules_rust//rust/settings:extra_exec_rustc_flag")): ["-Ccodegen-units=%s" % _EXTRA_EXEC_RUSTC_FLAG_CODEGEN_UNITS],
+        str(Label("@rules_rust//rust/settings:extra_exec_rustc_flags")): ["-Ccodegen-units=%s" % _EXTRA_EXEC_RUSTC_FLAGS_CODEGEN_UNITS],
+        "//command_line_option:extra_toolchains": ["//unit:codegen_units_toolchain"],
+    },
+    attrs = {"expected_codegen_units": attr.int_list()},
+)
+
+def _codegen_units_test_targets():
+    # Targets under test.
+    rust_binary(
+        name = "codegen_units_bin",
+        srcs = ["bin.rs"],
+        edition = "2018",
+        rustc_flags = ["-Ccodegen-units=%s" % _RUSTC_FLAGS_CODEGEN_UNITS],
+    )
+    use_cc_common_link_on_target(
+        name = "codegen_units_bin_with_cc_common_link",
+        target = ":codegen_units_bin",
+    )
+    with_collect_dep_actions(
+        name = "codegen_units_bin_with_collect_dep_actions",
+        target = ":codegen_units_bin",
+    )
+    with_exec_cfg(
+        name = "codegen_units_exec_bin_with_cc_common_link",
+        target = ":codegen_units_bin_with_cc_common_link",
+    )
+    with_exec_cfg(
+        name = "codegen_units_exec_bin_with_collect_dep_actions",
+        target = ":codegen_units_bin_with_collect_dep_actions",
+    )
+
+    # Fake toolchain along with its dependencies.
+    write_file(
+        name = "mock_rustc",
+        out = "mock_rustc.exe",
+    )
+    write_file(
+        name = "stdlib_src_self_contained",
+        out = "self-contained/something.o",
+    )
+    rust_stdlib_filegroup(
+        name = "std_libs",
+        srcs = ["self-contained/something.o"],
+    )
+    write_file(
+        name = "mock_rustdoc",
+        out = "mock_rustdoc.exe",
+    )
+    rust_toolchain(
+        name = "codegen_units_toolchain_impl",
+        binary_ext = "",
+        dylib_ext = ".so",
+        exec_triple = "x86_64-unknown-none",
+        target_triple = "x86_64-unknown-none",
+        rust_doc = ":mock_rustdoc",
+        rust_std = ":std_libs",
+        rustc = ":mock_rustc",
+        staticlib_ext = ".a",
+        stdlib_linkflags = [],
+        extra_rustc_flags = ["-Ccodegen-units=%s" % _TOOLCHAIN_EXTRA_RUSTC_FLAGS_CODEGEN_UNITS],
+        extra_exec_rustc_flags = ["-Ccodegen-units=%s" % _TOOLCHAIN_EXTRA_EXEC_RUSTC_FLAGS_CODEGEN_UNITS],
+        extra_rustc_flags_for_crate_types = {"bin": ["-Ccodegen-units=%s" % _TOOLCHAIN_EXTRA_RUSTC_FLAGS_FOR_CRATE_TYPES]},
+        visibility = ["//visibility:public"],
+    )
+    native.toolchain(
+        name = "codegen_units_toolchain",
+        toolchain = ":codegen_units_toolchain_impl",
+        toolchain_type = "@rules_rust//rust:toolchain",
+    )
+
+    # Tests
+    codegen_units_test(
+        name = "codegen_units_filtered",
+        target_under_test = ":codegen_units_bin_with_cc_common_link",
+        # When using cc_common.link, we expect an explicit `-Ccodegen-units=1`.
+        expected_codegen_units = [1],
+    )
+    codegen_units_test(
+        name = "codegen_units_filtered_exec",
+        target_under_test = ":codegen_units_exec_bin_with_cc_common_link",
+        # When using cc_common.link, we expect an explicit `-Ccodegen-units=1`.
+        expected_codegen_units = [1],
+    )
+    codegen_units_test(
+        name = "codegen_units_not_filtered",
+        target_under_test = ":codegen_units_bin_with_collect_dep_actions",
+        # When not using cc_common.link, all `-Ccodegen-units` flags added in
+        # various ways should remain unchanged.
+        expected_codegen_units = [
+            _RUSTC_FLAGS_CODEGEN_UNITS,
+            _SETTINGS_CODEGEN_UNITS,
+            _PER_CRATE_RUSTC_FLAG_CODEGEN_UNITS,
+            _EXTRA_RUSTC_FLAG_CODEGEN_UNITS,
+            _EXTRA_RUSTC_FLAGS_CODEGEN_UNITS,
+            _TOOLCHAIN_EXTRA_RUSTC_FLAGS_CODEGEN_UNITS,
+            _TOOLCHAIN_EXTRA_RUSTC_FLAGS_FOR_CRATE_TYPES,
+        ],
+    )
+    codegen_units_test(
+        name = "codegen_units_not_filtered_exec",
+        target_under_test = ":codegen_units_exec_bin_with_collect_dep_actions",
+        # When not using cc_common.link, all `-Ccodegen-units` flags added in
+        # various ways should remain unchanged.
+        expected_codegen_units = [
+            _RUSTC_FLAGS_CODEGEN_UNITS,
+            _SETTINGS_CODEGEN_UNITS,
+            _EXTRA_EXEC_RUSTC_FLAG_CODEGEN_UNITS,
+            _EXTRA_EXEC_RUSTC_FLAGS_CODEGEN_UNITS,
+            _TOOLCHAIN_EXTRA_EXEC_RUSTC_FLAGS_CODEGEN_UNITS,
+            _TOOLCHAIN_EXTRA_RUSTC_FLAGS_FOR_CRATE_TYPES,
+        ],
+    )
+
+    return [
+        "codegen_units_filtered",
+        "codegen_units_filtered_exec",
+        "codegen_units_not_filtered",
+        "codegen_units_not_filtered_exec",
+    ]
+
 def cc_common_link_test_suite(name, **kwargs):
     """Entry-point macro called from the BUILD file.
 
     Args:
         name: Name of the macro.
         **kwargs: Additional keyword arguments,
     """
-    _cc_common_link_test_targets()
-
     native.test_suite(
         name = name,
-        tests = [
-            "use_cc_common_link_on_binary",
-            "use_cc_common_link_on_binary_with_pdb",
-            "use_cc_common_link_on_test",
-            "use_cc_common_link_on_crate_test",
-            "use_cc_common_link_on_cdylib",
-            "custom_malloc_on_binary_test",
-        ],
+        tests =
+            _cc_common_link_test_targets() +
+            _codegen_units_test_targets(),
         **kwargs
     )